cleaning up NBT handling on packet decode/encode

now we always decode, because it's not safe to assume that we can just grab the rest of the bytes in the packet.

src/network/mcpe/protocol/AvailableActorIdentifiersPacket.php

@@ -25,9 +25,7 @@ namespace pocketmine\network\mcpe\protocol;
 
 #include <rules/DataPacket.h>
 
-use pocketmine\nbt\NbtDataException;
 use pocketmine\network\mcpe\protocol\serializer\NetworkBinaryStream;
-use pocketmine\network\mcpe\protocol\serializer\NetworkNbtSerializer;
 use pocketmine\network\mcpe\protocol\types\CacheableNbt;
 
 class AvailableActorIdentifiersPacket extends DataPacket implements ClientboundPacket{
@@ -49,13 +47,7 @@ class AvailableActorIdentifiersPacket extends DataPacket implements ClientboundP
 	}
 
 	protected function decodePayload(NetworkBinaryStream $in) : void{
-		$offset = $in->getOffset();
+		$this->identifiers = new CacheableNbt($in->getNbtCompoundRoot());
-		try{
-			$this->identifiers = new CacheableNbt((new NetworkNbtSerializer())->read($in->getBuffer(), $offset)->mustGetCompoundTag());
-		}catch(NbtDataException $e){
-			throw PacketDecodeException::wrap($e, "Failed decoding actor identifiers");
-		}
-		$in->setOffset($offset);
 	}
 
 	protected function encodePayload(NetworkBinaryStream $out) : void{

src/network/mcpe/protocol/BiomeDefinitionListPacket.php

@@ -25,9 +25,7 @@ namespace pocketmine\network\mcpe\protocol;
 
 #include <rules/DataPacket.h>
 
-use pocketmine\nbt\NbtDataException;
 use pocketmine\network\mcpe\protocol\serializer\NetworkBinaryStream;
-use pocketmine\network\mcpe\protocol\serializer\NetworkNbtSerializer;
 use pocketmine\network\mcpe\protocol\types\CacheableNbt;
 
 class BiomeDefinitionListPacket extends DataPacket implements ClientboundPacket{
@@ -49,13 +47,7 @@ class BiomeDefinitionListPacket extends DataPacket implements ClientboundPacket{
 	}
 
 	protected function decodePayload(NetworkBinaryStream $in) : void{
-		$offset = $in->getOffset();
+		$this->defs = new CacheableNbt($in->getNbtCompoundRoot());
-		try{
-			$this->defs = new CacheableNbt((new NetworkNbtSerializer())->read($in->getBuffer(), $offset)->mustGetCompoundTag());
-		}catch(NbtDataException $e){
-			throw PacketDecodeException::wrap($e, "Failed decoding biome definitions");
-		}
-		$in->setOffset($offset);
 	}
 
 	protected function encodePayload(NetworkBinaryStream $out) : void{

src/network/mcpe/protocol/BlockActorDataPacket.php

@@ -25,9 +25,7 @@ namespace pocketmine\network\mcpe\protocol;
 
 #include <rules/DataPacket.h>
 
-use pocketmine\nbt\NbtDataException;
 use pocketmine\network\mcpe\protocol\serializer\NetworkBinaryStream;
-use pocketmine\network\mcpe\protocol\serializer\NetworkNbtSerializer;
 use pocketmine\network\mcpe\protocol\types\CacheableNbt;
 
 class BlockActorDataPacket extends DataPacket implements ClientboundPacket, ServerboundPacket{
@@ -57,15 +55,7 @@ class BlockActorDataPacket extends DataPacket implements ClientboundPacket, Serv
 
 	protected function decodePayload(NetworkBinaryStream $in) : void{
 		$in->getBlockPosition($this->x, $this->y, $this->z);
-		try{
+		$this->namedtag = new CacheableNbt($in->getNbtCompoundRoot());
-			$offset = $in->getOffset();
-			$this->namedtag = new CacheableNbt(
-				(new NetworkNbtSerializer())->read($this->getBinaryStream()->getBuffer(), $offset, 512)->mustGetCompoundTag()
-			);
-			$in->setOffset($offset);
-		}catch(NbtDataException $e){
-			throw PacketDecodeException::wrap($e, "Failed decoding block actor NBT");
-		}
 	}
 
 	protected function encodePayload(NetworkBinaryStream $out) : void{

src/network/mcpe/protocol/LevelEventGenericPacket.php

@@ -26,22 +26,24 @@ namespace pocketmine\network\mcpe\protocol;
 #include <rules/DataPacket.h>
 
 use pocketmine\nbt\tag\CompoundTag;
-use pocketmine\nbt\TreeRoot;
 use pocketmine\network\mcpe\protocol\serializer\NetworkBinaryStream;
-use pocketmine\network\mcpe\protocol\serializer\NetworkNbtSerializer;
+use pocketmine\network\mcpe\protocol\types\CacheableNbt;
 
 class LevelEventGenericPacket extends DataPacket implements ClientboundPacket{
 	public const NETWORK_ID = ProtocolInfo::LEVEL_EVENT_GENERIC_PACKET;
 
 	/** @var int */
 	private $eventId;
-	/** @var string network-format NBT */
+	/**
+	 * @var CacheableNbt
+	 * @phpstan-var CacheableNbt<\pocketmine\nbt\tag\CompoundTag>
+	 */
 	private $eventData;
 
 	public static function create(int $eventId, CompoundTag $data) : self{
 		$result = new self;
 		$result->eventId = $eventId;
-		$result->eventData = (new NetworkNbtSerializer())->write(new TreeRoot($data));
+		$result->eventData = new CacheableNbt($data);
 		return $result;
 	}
 
@@ -49,18 +51,21 @@ class LevelEventGenericPacket extends DataPacket implements ClientboundPacket{
 		return $this->eventId;
 	}
 
-	public function getEventData() : string{
+	/**
+	 * @phpstan-return CacheableNbt<\pocketmine\nbt\tag\CompoundTag>
+	 */
+	public function getEventData() : CacheableNbt{
 		return $this->eventData;
 	}
 
 	protected function decodePayload(NetworkBinaryStream $in) : void{
 		$this->eventId = $in->getVarInt();
-		$this->eventData = $in->getRemaining();
+		$this->eventData = new CacheableNbt($in->getNbtCompoundRoot());
 	}
 
 	protected function encodePayload(NetworkBinaryStream $out) : void{
 		$out->putVarInt($this->eventId);
-		$out->put($this->eventData);
+		$out->put($this->eventData->getEncodedNbt());
 	}
 
 	public function handle(PacketHandlerInterface $handler) : bool{

src/network/mcpe/protocol/StartGamePacket.php

@@ -28,7 +28,6 @@ namespace pocketmine\network\mcpe\protocol;
 use pocketmine\math\Vector3;
 use pocketmine\nbt\tag\ListTag;
 use pocketmine\network\mcpe\protocol\serializer\NetworkBinaryStream;
-use pocketmine\network\mcpe\protocol\serializer\NetworkNbtSerializer;
 use pocketmine\network\mcpe\protocol\types\CacheableNbt;
 use pocketmine\network\mcpe\protocol\types\PlayerPermissions;
 use function count;
@@ -214,9 +213,7 @@ class StartGamePacket extends DataPacket implements ClientboundPacket{
 
 		$this->enchantmentSeed = $in->getVarInt();
 
-		$offset = $in->getOffset();
+		$blockTable = $in->getNbtRoot()->getTag();
-		$blockTable = (new NetworkNbtSerializer())->read($in->getBuffer(), $offset, 512)->getTag();
-		$in->setOffset($offset);
 		if(!($blockTable instanceof ListTag)){
 			throw new \UnexpectedValueException("Wrong block table root NBT tag type");
 		}

src/network/mcpe/protocol/StructureTemplateDataResponsePacket.php

@@ -26,19 +26,23 @@ namespace pocketmine\network\mcpe\protocol;
 #include <rules/DataPacket.h>
 
 use pocketmine\network\mcpe\protocol\serializer\NetworkBinaryStream;
+use pocketmine\network\mcpe\protocol\types\CacheableNbt;
 
 class StructureTemplateDataResponsePacket extends DataPacket implements ClientboundPacket{
 	public const NETWORK_ID = ProtocolInfo::STRUCTURE_TEMPLATE_DATA_RESPONSE_PACKET;
 
 	/** @var string */
 	public $structureTemplateName;
-	/** @var string|null */
+	/**
+	 * @var CacheableNbt|null
+	 * @phpstan-var CacheableNbt<\pocketmine\nbt\tag\CompoundTag>
+	 */
 	public $namedtag;
 
 	protected function decodePayload(NetworkBinaryStream $in) : void{
 		$this->structureTemplateName = $in->getString();
 		if($in->getBool()){
-			$this->namedtag = $in->getRemaining();
+			$this->namedtag = new CacheableNbt($in->getNbtCompoundRoot());
 		}
 	}
 
@@ -46,7 +50,7 @@ class StructureTemplateDataResponsePacket extends DataPacket implements Clientbo
 		$out->putString($this->structureTemplateName);
 		$out->putBool($this->namedtag !== null);
 		if($this->namedtag !== null){
-			$out->put($this->namedtag);
+			$out->put($this->namedtag->getEncodedNbt());
 		}
 	}
 

src/network/mcpe/protocol/UpdateBlockPropertiesPacket.php

@@ -26,32 +26,37 @@ namespace pocketmine\network\mcpe\protocol;
 #include <rules/DataPacket.h>
 
 use pocketmine\nbt\tag\CompoundTag;
-use pocketmine\nbt\TreeRoot;
 use pocketmine\network\mcpe\protocol\serializer\NetworkBinaryStream;
-use pocketmine\network\mcpe\protocol\serializer\NetworkNbtSerializer;
+use pocketmine\network\mcpe\protocol\types\CacheableNbt;
 
 class UpdateBlockPropertiesPacket extends DataPacket implements ClientboundPacket{
 	public const NETWORK_ID = ProtocolInfo::UPDATE_BLOCK_PROPERTIES_PACKET;
 
-	/** @var string */
+	/**
-	private $nbt;
+	 * @var CacheableNbt
+	 * @phpstan-var CacheableNbt<\pocketmine\nbt\tag\CompoundTag>
+	 */
+	private $blockProperties;
 
 	public static function create(CompoundTag $data) : self{
 		$result = new self;
-		$result->nbt = (new NetworkNbtSerializer())->write(new TreeRoot($data));
+		$result->blockProperties = new CacheableNbt($data);
 		return $result;
 	}
 
-	public function getNbt() : string{
+	/**
-		return $this->nbt;
+	 * @phpstan-return CacheableNbt<\pocketmine\nbt\tag\CompoundTag>
+	 */
+	public function getBlockProperties() : CacheableNbt{
+		return $this->blockProperties;
 	}
 
 	protected function decodePayload(NetworkBinaryStream $in) : void{
-		$this->nbt = $in->getRemaining();
+		$this->blockProperties = new CacheableNbt($in->getNbtCompoundRoot());
 	}
 
 	protected function encodePayload(NetworkBinaryStream $out) : void{
-		$out->put($this->nbt);
+		$out->put($this->blockProperties->getEncodedNbt());
 	}
 
 	public function handle(PacketHandlerInterface $handler) : bool{

src/network/mcpe/protocol/UpdateEquipPacket.php

@@ -26,6 +26,7 @@ namespace pocketmine\network\mcpe\protocol;
 #include <rules/DataPacket.h>
 
 use pocketmine\network\mcpe\protocol\serializer\NetworkBinaryStream;
+use pocketmine\network\mcpe\protocol\types\CacheableNbt;
 
 class UpdateEquipPacket extends DataPacket implements ClientboundPacket{
 	public const NETWORK_ID = ProtocolInfo::UPDATE_EQUIP_PACKET;
@@ -38,7 +39,10 @@ class UpdateEquipPacket extends DataPacket implements ClientboundPacket{
 	public $windowSlotCount; //useless, seems to be part of a standard container header
 	/** @var int */
 	public $entityUniqueId;
-	/** @var string */
+	/**
+	 * @var CacheableNbt
+	 * @phpstan-var CacheableNbt<\pocketmine\nbt\tag\CompoundTag>
+	 */
 	public $namedtag;
 
 	protected function decodePayload(NetworkBinaryStream $in) : void{
@@ -46,7 +50,7 @@ class UpdateEquipPacket extends DataPacket implements ClientboundPacket{
 		$this->windowType = $in->getByte();
 		$this->windowSlotCount = $in->getVarInt();
 		$this->entityUniqueId = $in->getEntityUniqueId();
-		$this->namedtag = $in->getRemaining();
+		$this->namedtag = new CacheableNbt($in->getNbtCompoundRoot());
 	}
 
 	protected function encodePayload(NetworkBinaryStream $out) : void{
@@ -54,7 +58,7 @@ class UpdateEquipPacket extends DataPacket implements ClientboundPacket{
 		$out->putByte($this->windowType);
 		$out->putVarInt($this->windowSlotCount);
 		$out->putEntityUniqueId($this->entityUniqueId);
-		$out->put($this->namedtag);
+		$out->put($this->namedtag->getEncodedNbt());
 	}
 
 	public function handle(PacketHandlerInterface $handler) : bool{

src/network/mcpe/protocol/UpdateTradePacket.php

@@ -26,6 +26,7 @@ namespace pocketmine\network\mcpe\protocol;
 #include <rules/DataPacket.h>
 
 use pocketmine\network\mcpe\protocol\serializer\NetworkBinaryStream;
+use pocketmine\network\mcpe\protocol\types\CacheableNbt;
 use pocketmine\network\mcpe\protocol\types\inventory\WindowTypes;
 
 class UpdateTradePacket extends DataPacket implements ClientboundPacket{
@@ -51,7 +52,10 @@ class UpdateTradePacket extends DataPacket implements ClientboundPacket{
 	public $isV2Trading;
 	/** @var bool */
 	public $isWilling;
-	/** @var string */
+	/**
+	 * @var CacheableNbt
+	 * @phpstan-var CacheableNbt<\pocketmine\nbt\tag\CompoundTag>
+	 */
 	public $offers;
 
 	protected function decodePayload(NetworkBinaryStream $in) : void{
@@ -64,7 +68,7 @@ class UpdateTradePacket extends DataPacket implements ClientboundPacket{
 		$this->displayName = $in->getString();
 		$this->isV2Trading = $in->getBool();
 		$this->isWilling = $in->getBool();
-		$this->offers = $in->getRemaining();
+		$this->offers = new CacheableNbt($in->getNbtCompoundRoot());
 	}
 
 	protected function encodePayload(NetworkBinaryStream $out) : void{
@@ -77,7 +81,7 @@ class UpdateTradePacket extends DataPacket implements ClientboundPacket{
 		$out->putString($this->displayName);
 		$out->putBool($this->isV2Trading);
 		$out->putBool($this->isWilling);
-		$out->put($this->offers);
+		$out->put($this->offers->getEncodedNbt());
 	}
 
 	public function handle(PacketHandlerInterface $handler) : bool{

src/network/mcpe/protocol/serializer/NetworkBinaryStream.php

@@ -223,11 +223,7 @@ class NetworkBinaryStream extends BinaryStream{
 			if($c !== 1){
 				throw new PacketDecodeException("Unexpected NBT count $c");
 			}
-			try{
+			$compound = $this->getNbtCompoundRoot();
-				$compound = (new NetworkNbtSerializer())->read($this->buffer, $this->offset, 512)->mustGetCompoundTag();
-			}catch(NbtDataException $e){
-				throw new PacketDecodeException($e->getMessage(), 0, $e);
-			}
 		}elseif($nbtLen !== 0){
 			throw new PacketDecodeException("Unexpected fake NBT length $nbtLen");
 		}
@@ -707,4 +703,23 @@ class NetworkBinaryStream extends BinaryStream{
 		$this->putStructureSettings($structureEditorData->structureSettings);
 		$this->putVarInt($structureEditorData->structureRedstoneSaveMove);
 	}
+
+	public function getNbtRoot() : TreeRoot{
+		$offset = $this->getOffset();
+		try{
+			return (new NetworkNbtSerializer())->read($this->getBuffer(), $offset, 512);
+		}catch(NbtDataException $e){
+			throw PacketDecodeException::wrap($e, "Failed decoding NBT root");
+		}finally{
+			$this->setOffset($offset);
+		}
+	}
+
+	public function getNbtCompoundRoot() : CompoundTag{
+		try{
+			return $this->getNbtRoot()->mustGetCompoundTag();
+		}catch(NbtDataException $e){
+			throw PacketDecodeException::wrap($e, "Expected TAG_Compound NBT root");
+		}
+	}
 }

src/network/mcpe/protocol/types/entity/CompoundTagMetadataProperty.php

@@ -23,7 +23,6 @@ declare(strict_types=1);
 
 namespace pocketmine\network\mcpe\protocol\types\entity;
 
-use pocketmine\nbt\NbtDataException;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\nbt\TreeRoot;
 use pocketmine\network\mcpe\protocol\PacketDecodeException;
@@ -54,14 +53,7 @@ final class CompoundTagMetadataProperty implements MetadataProperty{
 	 * @throws PacketDecodeException
 	 */
 	public static function read(NetworkBinaryStream $in) : self{
-		$offset = $in->getOffset();
+		return new self($in->getNbtCompoundRoot());
-		try{
-			$tag = (new NetworkNbtSerializer())->read($in->getBuffer(), $offset, 512)->mustGetCompoundTag();
-		}catch(NbtDataException $e){
-			throw new PacketDecodeException($e->getMessage(), 0, $e);
-		}
-		$in->setOffset($offset);
-		return new self($tag);
 	}
 
 	public function write(NetworkBinaryStream $out) : void{