Merge remote-tracking branch 'origin/stable' into next-minor

src/entity/Skin.php

@@ -24,6 +24,7 @@ declare(strict_types=1);
 namespace pocketmine\entity;
 
 use Ahc\Json\Comment as CommentedJsonDecoder;
+use pocketmine\utils\Limits;
 use function implode;
 use function in_array;
 use function json_encode;
@@ -49,7 +50,17 @@ final class Skin{
 	/** @var string */
 	private $geometryData;
 
+	private static function checkLength(string $string, string $name, int $maxLength) : void{
+		if(strlen($string) > $maxLength){
+			throw new InvalidSkinException("$name must be at most $maxLength bytes, but have " . strlen($string) . " bytes");
+		}
+	}
+
 	public function __construct(string $skinId, string $skinData, string $capeData = "", string $geometryName = "", string $geometryData = ""){
+		self::checkLength($skinId, "Skin ID", Limits::INT16_MAX);
+		self::checkLength($geometryName, "Geometry name", Limits::INT16_MAX);
+		self::checkLength($geometryData, "Geometry data", Limits::INT32_MAX);
+
 		if($skinId === ""){
 			throw new InvalidSkinException("Skin ID must not be empty");
 		}

src/item/WritableBookPage.php

@@ -23,17 +23,32 @@ declare(strict_types=1);
 
 namespace pocketmine\item;
 
+use pocketmine\utils\Limits;
 use pocketmine\utils\Utils;
+use function sprintf;
+use function strlen;
 
 class WritableBookPage{
+	public const PAGE_LENGTH_HARD_LIMIT_BYTES = Limits::INT16_MAX;
+	public const PHOTO_NAME_LENGTH_HARD_LIMIT_BYTES = Limits::INT16_MAX;
 
 	/** @var string */
 	private $text;
 	/** @var string */
 	private $photoName;
 
+	/**
+	 * @throws \InvalidArgumentException
+	 */
+	private static function checkLength(string $string, string $name, int $maxLength) : void{
+		if(strlen($string) > $maxLength){
+			throw new \InvalidArgumentException(sprintf("$name must be at most %d bytes, but have %d bytes", $maxLength, strlen($string)));
+		}
+	}
+
 	public function __construct(string $text, string $photoName = ""){
-		//TODO: data validation
+		self::checkLength($text, "Text", self::PAGE_LENGTH_HARD_LIMIT_BYTES);
+		self::checkLength($photoName, "Photo name", self::PHOTO_NAME_LENGTH_HARD_LIMIT_BYTES);
 		Utils::checkUTF8($text);
 		$this->text = $text;
 		$this->photoName = $photoName;

src/item/WrittenBook.php

@@ -24,7 +24,10 @@ declare(strict_types=1);
 namespace pocketmine\item;
 
 use pocketmine\nbt\tag\CompoundTag;
+use pocketmine\utils\Limits;
 use pocketmine\utils\Utils;
+use function sprintf;
+use function strlen;
 
 class WrittenBook extends WritableBookBase{
 
@@ -85,6 +88,9 @@ class WrittenBook extends WritableBookBase{
 	 * @return $this
 	 */
 	public function setAuthor(string $authorName) : self{
+		if(strlen($authorName) > Limits::INT16_MAX){
+			throw new \InvalidArgumentException(sprintf("Author must be at most %d bytes, but have %d bytes", Limits::INT16_MAX, strlen($authorName)));
+		}
 		Utils::checkUTF8($authorName);
 		$this->author = $authorName;
 		return $this;
@@ -103,6 +109,9 @@ class WrittenBook extends WritableBookBase{
 	 * @return $this
 	 */
 	public function setTitle(string $title) : self{
+		if(strlen($title) > Limits::INT16_MAX){
+			throw new \InvalidArgumentException(sprintf("Title must be at most %d bytes, but have %d bytes", Limits::INT16_MAX, strlen($title)));
+		}
 		Utils::checkUTF8($title);
 		$this->title = $title;
 		return $this;

src/network/mcpe/handler/InGamePacketHandler.php

@@ -37,6 +37,7 @@ use pocketmine\inventory\transaction\TransactionException;
 use pocketmine\inventory\transaction\TransactionValidationException;
 use pocketmine\item\VanillaItems;
 use pocketmine\item\WritableBook;
+use pocketmine\item\WritableBookPage;
 use pocketmine\item\WrittenBook;
 use pocketmine\math\Facing;
 use pocketmine\math\Vector3;
@@ -101,6 +102,8 @@ use pocketmine\network\mcpe\protocol\types\PlayerBlockActionWithBlockInfo;
 use pocketmine\network\PacketHandlingException;
 use pocketmine\player\Player;
 use pocketmine\utils\AssumptionFailedError;
+use pocketmine\utils\Limits;
+use pocketmine\utils\TextFormat;
 use function array_push;
 use function base64_encode;
 use function count;
@@ -112,8 +115,10 @@ use function json_decode;
 use function json_encode;
 use function json_last_error_msg;
 use function max;
+use function mb_strlen;
 use function microtime;
 use function preg_match;
+use function sprintf;
 use function strlen;
 use function strpos;
 use function substr;
@@ -741,6 +746,24 @@ class InGamePacketHandler extends PacketHandler{
 		return false; //TODO
 	}
 
+	/**
+	 * @throws PacketHandlingException
+	 */
+	private function checkBookText(string $string, string $fieldName, int $softLimit, int $hardLimit, bool &$cancel) : string{
+		if(strlen($string) > $hardLimit){
+			throw new PacketHandlingException(sprintf("Book %s must be at most %d bytes, but have %d bytes", $fieldName, $hardLimit, strlen($string)));
+		}
+
+		$result = TextFormat::clean($string, false);
+		//strlen() is O(1), mb_strlen() is O(n)
+		if(strlen($result) > $softLimit * 4 || mb_strlen($result, 'UTF-8') > $softLimit){
+			$cancel = true;
+			$this->session->getLogger()->debug("Cancelled book edit due to $fieldName exceeded soft limit of $softLimit chars");
+		}
+
+		return $result;
+	}
+
 	public function handleBookEdit(BookEditPacket $packet) : bool{
 		//TODO: break this up into book API things
 		$oldBook = $this->player->getInventory()->getItem($packet->inventorySlot);
@@ -750,10 +773,11 @@ class InGamePacketHandler extends PacketHandler{
 
 		$newBook = clone $oldBook;
 		$modifiedPages = [];
-
+		$cancel = false;
 		switch($packet->type){
 			case BookEditPacket::TYPE_REPLACE_PAGE:
-				$newBook->setPageText($packet->pageNumber, $packet->text);
+				$text = self::checkBookText($packet->text, "page text", 256, WritableBookPage::PAGE_LENGTH_HARD_LIMIT_BYTES, $cancel);
+				$newBook->setPageText($packet->pageNumber, $text);
 				$modifiedPages[] = $packet->pageNumber;
 				break;
 			case BookEditPacket::TYPE_ADD_PAGE:
@@ -762,7 +786,8 @@ class InGamePacketHandler extends PacketHandler{
 					//TODO: the client can send insert-before actions on trailing client-side pages which cause odd behaviour on the server
 					return false;
 				}
-				$newBook->insertPage($packet->pageNumber, $packet->text);
+				$text = self::checkBookText($packet->text, "page text", 256, WritableBookPage::PAGE_LENGTH_HARD_LIMIT_BYTES, $cancel);
+				$newBook->insertPage($packet->pageNumber, $text);
 				$modifiedPages[] = $packet->pageNumber;
 				break;
 			case BookEditPacket::TYPE_DELETE_PAGE:
@@ -781,11 +806,14 @@ class InGamePacketHandler extends PacketHandler{
 				$modifiedPages = [$packet->pageNumber, $packet->secondaryPageNumber];
 				break;
 			case BookEditPacket::TYPE_SIGN_BOOK:
-				/** @var WrittenBook $newBook */
+				$title = self::checkBookText($packet->title, "title", 16, Limits::INT16_MAX, $cancel);
+				//this one doesn't have a limit in vanilla, so we have to improvise
+				$author = self::checkBookText($packet->author, "author", 256, Limits::INT16_MAX, $cancel);
+
 				$newBook = VanillaItems::WRITTEN_BOOK()
 					->setPages($oldBook->getPages())
-					->setAuthor($packet->author)
-					->setTitle($packet->title)
+					->setAuthor($author)
+					->setTitle($title)
 					->setGeneration(WrittenBook::GENERATION_ORIGINAL);
 				break;
 			default:
@@ -801,7 +829,23 @@ class InGamePacketHandler extends PacketHandler{
 			BookEditPacket::TYPE_SIGN_BOOK => PlayerEditBookEvent::ACTION_SIGN_BOOK,
 			default => throw new AssumptionFailedError("We already filtered unknown types in the switch above")
 		};
+
+		/*
+		 * Plugins may have created books with more than 50 pages; we allow plugins to do this, but not players.
+		 * Don't allow the page count to grow past 50, but allow deleting, swapping or altering text of existing pages.
+		 */
+		$oldPageCount = count($oldBook->getPages());
+		$newPageCount = count($newBook->getPages());
+		if(($newPageCount > $oldPageCount && $newPageCount > 50)){
+			$this->session->getLogger()->debug("Cancelled book edit due to adding too many pages (new page count would be $newPageCount)");
+			$cancel = true;
+		}
+
 		$event = new PlayerEditBookEvent($this->player, $oldBook, $newBook, $action, $modifiedPages);
+		if($cancel){
+			$event->cancel();
+		}
+
 		$event->call();
 		if($event->isCancelled()){
 			return true;

src/world/World.php

@@ -2258,6 +2258,11 @@ class World implements ChunkManager{
 		$previousSpawn = $this->getSpawnLocation();
 		$this->provider->getWorldData()->setSpawn($pos);
 		(new SpawnChangeEvent($this, $previousSpawn))->call();
+
+		$location = Position::fromObject($pos, $this);
+		foreach($this->players as $player){
+			$player->getNetworkSession()->syncWorldSpawnPoint($location);
+		}
 	}
 
 	/**