Address more error cases, some minor cleanup

src/pocketmine/network/mcpe/NetworkBinaryStream.php

@@ -31,6 +31,7 @@ use pocketmine\item\Item;
 use pocketmine\item\ItemFactory;
 use pocketmine\math\Vector3;
 use pocketmine\nbt\LittleEndianNbtSerializer;
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\protocol\types\CommandOriginData;
 use pocketmine\network\mcpe\protocol\types\EntityLink;
 use pocketmine\utils\BinaryDataException;
@@ -43,6 +44,10 @@ class NetworkBinaryStream extends BinaryStream{
 	/** @var LittleEndianNbtSerializer */
 	private static $itemNbtSerializer = null;
 
+	/**
+	 * @return string
+	 * @throws BinaryDataException
+	 */
 	public function getString() : string{
 		return $this->get($this->getUnsignedVarInt());
 	}
@@ -52,6 +57,10 @@ class NetworkBinaryStream extends BinaryStream{
 		$this->put($v);
 	}
 
+	/**
+	 * @return UUID
+	 * @throws BinaryDataException
+	 */
 	public function getUUID() : UUID{
 		//This is actually two little-endian longs: UUID Most followed by UUID Least
 		$part1 = $this->getLInt();
@@ -69,6 +78,12 @@ class NetworkBinaryStream extends BinaryStream{
 		$this->putLInt($uuid->getPart(2));
 	}
 
+	/**
+	 * @return Item
+	 *
+	 * @throws BadPacketException
+	 * @throws BinaryDataException
+	 */
 	public function getSlot() : Item{
 		$id = $this->getVarInt();
 		if($id === 0){
@@ -88,7 +103,11 @@ class NetworkBinaryStream extends BinaryStream{
 			if(self::$itemNbtSerializer === null){
 				self::$itemNbtSerializer = new LittleEndianNbtSerializer();
 			}
-			$compound = self::$itemNbtSerializer->read($this->get($nbtLen));
+			try{
+				$compound = self::$itemNbtSerializer->read($this->get($nbtLen));
+			}catch(\UnexpectedValueException $e){
+				throw new BadPacketException($e->getMessage(), 0, $e);
+			}
 		}
 
 		//TODO
@@ -104,7 +123,7 @@ class NetworkBinaryStream extends BinaryStream{
 		try{
 			return ItemFactory::get($id, $data, $cnt, $compound);
 		}catch(\InvalidArgumentException $e){
-			throw new BinaryDataException($e->getMessage(), 0, $e);
+			throw new BadPacketException($e->getMessage(), 0, $e);
 		}
 	}
 
@@ -146,6 +165,9 @@ class NetworkBinaryStream extends BinaryStream{
 	 * @param bool $types Whether to include metadata types along with values in the returned array
 	 *
 	 * @return array
+	 *
+	 * @throws BadPacketException
+	 * @throws BinaryDataException
 	 */
 	public function getEntityMetadata(bool $types = true) : array{
 		$count = $this->getUnsignedVarInt();
@@ -184,7 +206,7 @@ class NetworkBinaryStream extends BinaryStream{
 					$value = $this->getVector3();
 					break;
 				default:
-					throw new BinaryDataException("Invalid data type " . $type);
+					throw new BadPacketException("Unknown entity metadata type " . $type);
 			}
 			if($types){
 				$data[$key] = [$type, $value];
@@ -249,7 +271,8 @@ class NetworkBinaryStream extends BinaryStream{
 	 * Reads a list of Attributes from the stream.
 	 * @return Attribute[]
 	 *
-	 * @throws BinaryDataException if reading an attribute with an unrecognized name
+	 * @throws BadPacketException if reading an attribute with an unrecognized name
+	 * @throws BinaryDataException
 	 */
 	public function getAttributeList() : array{
 		$list = [];
@@ -271,7 +294,7 @@ class NetworkBinaryStream extends BinaryStream{
 
 				$list[] = $attr;
 			}else{
-				throw new BinaryDataException("Unknown attribute type \"$id\"");
+				throw new BadPacketException("Unknown attribute type \"$id\"");
 			}
 		}
 
@@ -297,6 +320,8 @@ class NetworkBinaryStream extends BinaryStream{
 	/**
 	 * Reads and returns an EntityUniqueID
 	 * @return int
+	 *
+	 * @throws BinaryDataException
 	 */
 	public function getEntityUniqueId() : int{
 		return $this->getVarLong();
@@ -314,6 +339,8 @@ class NetworkBinaryStream extends BinaryStream{
 	/**
 	 * Reads and returns an EntityRuntimeID
 	 * @return int
+	 *
+	 * @throws BinaryDataException
 	 */
 	public function getEntityRuntimeId() : int{
 		return $this->getUnsignedVarLong();
@@ -334,6 +361,8 @@ class NetworkBinaryStream extends BinaryStream{
 	 * @param int &$x
 	 * @param int &$y
 	 * @param int &$z
+	 *
+	 * @throws BinaryDataException
 	 */
 	public function getBlockPosition(&$x, &$y, &$z) : void{
 		$x = $this->getVarInt();
@@ -360,6 +389,8 @@ class NetworkBinaryStream extends BinaryStream{
 	 * @param int &$x
 	 * @param int &$y
 	 * @param int &$z
+	 *
+	 * @throws BinaryDataException
 	 */
 	public function getSignedBlockPosition(&$x, &$y, &$z) : void{
 		$x = $this->getVarInt();
@@ -384,6 +415,8 @@ class NetworkBinaryStream extends BinaryStream{
 	 * Reads a floating-point Vector3 object with coordinates rounded to 4 decimal places.
 	 *
 	 * @return Vector3
+	 *
+	 * @throws BinaryDataException
 	 */
 	public function getVector3() : Vector3{
 		return new Vector3(
@@ -424,6 +457,10 @@ class NetworkBinaryStream extends BinaryStream{
 		$this->putLFloat($vector->z);
 	}
 
+	/**
+	 * @return float
+	 * @throws BinaryDataException
+	 */
 	public function getByteRotation() : float{
 		return (float) ($this->getByte() * (360 / 256));
 	}
@@ -437,6 +474,9 @@ class NetworkBinaryStream extends BinaryStream{
 	 * TODO: implement this properly
 	 *
 	 * @return array, members are in the structure [name => [type, value]]
+	 *
+	 * @throws BadPacketException
+	 * @throws BinaryDataException
 	 */
 	public function getGameRules() : array{
 		$count = $this->getUnsignedVarInt();
@@ -456,7 +496,7 @@ class NetworkBinaryStream extends BinaryStream{
 					$value = $this->getLFloat();
 					break;
 				default:
-					throw new BinaryDataException("Unknown gamerule type $type");
+					throw new BadPacketException("Unknown gamerule type $type");
 			}
 
 			$rules[$name] = [$type, $value];
@@ -494,6 +534,8 @@ class NetworkBinaryStream extends BinaryStream{
 
 	/**
 	 * @return EntityLink
+	 *
+	 * @throws BinaryDataException
 	 */
 	protected function getEntityLink() : EntityLink{
 		$link = new EntityLink();
@@ -516,6 +558,10 @@ class NetworkBinaryStream extends BinaryStream{
 		$this->putBool($link->immediate);
 	}
 
+	/**
+	 * @return CommandOriginData
+	 * @throws BinaryDataException
+	 */
 	protected function getCommandOriginData() : CommandOriginData{
 		$result = new CommandOriginData();
 

src/pocketmine/network/mcpe/protocol/AddEntityPacket.php

@@ -191,9 +191,13 @@ class AddEntityPacket extends DataPacket{
 			$attr = Attribute::getAttribute($id);
 
 			if($attr !== null){
-				$attr->setMinValue($min);
+				try{
-				$attr->setMaxValue($max);
+					$attr->setMinValue($min);
-				$attr->setValue($current);
+					$attr->setMaxValue($max);
+					$attr->setValue($current);
+				}catch(\InvalidArgumentException $e){
+					throw new BadPacketException($e->getMessage(), 0, $e); //TODO: address this properly
+				}
 				$this->attributes[] = $attr;
 			}else{
 				throw new BadPacketException("Unknown attribute type \"$id\"");

src/pocketmine/network/mcpe/protocol/AvailableCommandsPacket.php

@@ -30,6 +30,7 @@ use pocketmine\network\mcpe\handler\SessionHandler;
 use pocketmine\network\mcpe\protocol\types\CommandData;
 use pocketmine\network\mcpe\protocol\types\CommandEnum;
 use pocketmine\network\mcpe\protocol\types\CommandParameter;
+use pocketmine\utils\BinaryDataException;
 use function array_flip;
 use function array_keys;
 use function array_map;
@@ -140,6 +141,11 @@ class AvailableCommandsPacket extends DataPacket{
 		}
 	}
 
+	/**
+	 * @return CommandEnum
+	 * @throws BadPacketException
+	 * @throws BinaryDataException
+	 */
 	protected function getEnum() : CommandEnum{
 		$retval = new CommandEnum();
 		$retval->enumName = $this->getString();
@@ -156,6 +162,10 @@ class AvailableCommandsPacket extends DataPacket{
 		return $retval;
 	}
 
+	/**
+	 * @return CommandEnum
+	 * @throws BinaryDataException
+	 */
 	protected function getSoftEnum() : CommandEnum{
 		$retval = new CommandEnum();
 		$retval->enumName = $this->getString();
@@ -191,6 +201,10 @@ class AvailableCommandsPacket extends DataPacket{
 		}
 	}
 
+	/**
+	 * @return int
+	 * @throws BinaryDataException
+	 */
 	protected function getEnumValueIndex() : int{
 		if($this->enumValuesCount < 256){
 			return $this->getByte();
@@ -211,6 +225,11 @@ class AvailableCommandsPacket extends DataPacket{
 		}
 	}
 
+	/**
+	 * @return CommandData
+	 * @throws BadPacketException
+	 * @throws BinaryDataException
+	 */
 	protected function getCommandData() : CommandData{
 		$retval = new CommandData();
 		$retval->commandName = $this->getString();

src/pocketmine/network/mcpe/protocol/CommandOutputPacket.php

@@ -28,6 +28,7 @@ namespace pocketmine\network\mcpe\protocol;
 use pocketmine\network\mcpe\handler\SessionHandler;
 use pocketmine\network\mcpe\protocol\types\CommandOriginData;
 use pocketmine\network\mcpe\protocol\types\CommandOutputMessage;
+use pocketmine\utils\BinaryDataException;
 use function count;
 
 class CommandOutputPacket extends DataPacket{
@@ -58,6 +59,10 @@ class CommandOutputPacket extends DataPacket{
 		}
 	}
 
+	/**
+	 * @return CommandOutputMessage
+	 * @throws BinaryDataException
+	 */
 	protected function getCommandMessage() : CommandOutputMessage{
 		$message = new CommandOutputMessage();
 

src/pocketmine/network/mcpe/protocol/MoveEntityDeltaPacket.php

@@ -26,6 +26,7 @@ namespace pocketmine\network\mcpe\protocol;
 #include <rules/DataPacket.h>
 
 use pocketmine\network\mcpe\handler\SessionHandler;
+use pocketmine\utils\BinaryDataException;
 
 class MoveEntityDeltaPacket extends DataPacket{
 	public const NETWORK_ID = ProtocolInfo::MOVE_ENTITY_DELTA_PACKET;
@@ -54,6 +55,12 @@ class MoveEntityDeltaPacket extends DataPacket{
 	/** @var float */
 	public $zRot = 0.0;
 
+	/**
+	 * @param int $flag
+	 *
+	 * @return int
+	 * @throws BinaryDataException
+	 */
 	private function maybeReadCoord(int $flag) : int{
 		if($this->flags & $flag){
 			return $this->getVarInt();
@@ -61,6 +68,12 @@ class MoveEntityDeltaPacket extends DataPacket{
 		return 0;
 	}
 
+	/**
+	 * @param int $flag
+	 *
+	 * @return float
+	 * @throws BinaryDataException
+	 */
 	private function maybeReadRotation(int $flag) : float{
 		if($this->flags & $flag){
 			return $this->getByteRotation();

src/pocketmine/network/mcpe/protocol/types/NetworkInventoryAction.php

@@ -31,6 +31,7 @@ use pocketmine\item\Item;
 use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\NetworkBinaryStream;
 use pocketmine\Player;
+use pocketmine\utils\BinaryDataException;
 
 class NetworkInventoryAction{
 	public const SOURCE_CONTAINER = 0;
@@ -96,7 +97,8 @@ class NetworkInventoryAction{
 	 * @param NetworkBinaryStream $packet
 	 *
 	 * @return $this
-	 * @throws \UnexpectedValueException
+	 *
+	 * @throws BinaryDataException
 	 * @throws BadPacketException
 	 */
 	public function read(NetworkBinaryStream $packet) : NetworkInventoryAction{

src/pocketmine/network/mcpe/protocol/types/TransactionData.php

@@ -23,7 +23,9 @@ declare(strict_types=1);
 
 namespace pocketmine\network\mcpe\protocol\types;
 
+use pocketmine\network\BadPacketException;
 use pocketmine\network\mcpe\NetworkBinaryStream;
+use pocketmine\utils\BinaryDataException;
 use function count;
 
 abstract class TransactionData{
@@ -45,8 +47,8 @@ abstract class TransactionData{
 	/**
 	 * @param NetworkBinaryStream $stream
 	 *
-	 * @throws \OutOfBoundsException
+	 * @throws BinaryDataException
-	 * @throws \UnexpectedValueException
+	 * @throws BadPacketException
 	 */
 	final public function decode(NetworkBinaryStream $stream) : void{
 		$actionCount = $stream->getUnsignedVarInt();
@@ -59,8 +61,8 @@ abstract class TransactionData{
 	/**
 	 * @param NetworkBinaryStream $stream
 	 *
-	 * @throws \OutOfBoundsException
+	 * @throws BinaryDataException
-	 * @throws \UnexpectedValueException
+	 * @throws BadPacketException
 	 */
 	abstract protected function decodeData(NetworkBinaryStream $stream) : void;