From 0f941410f65a43adbaec200ebf8811c3320b624f Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 3 Jan 2019 17:57:06 +0000 Subject: [PATCH 1/2] Use more appropriate exceptions in the protocol layer --- src/pocketmine/network/mcpe/protocol/BatchPacket.php | 2 +- .../network/mcpe/protocol/BookEditPacket.php | 2 +- .../mcpe/protocol/ClientboundMapItemDataPacket.php | 4 ++-- src/pocketmine/network/mcpe/protocol/DataPacket.php | 11 +++++++++++ .../mcpe/protocol/InventoryTransactionPacket.php | 2 +- .../network/mcpe/protocol/SetScorePacket.php | 2 +- .../mcpe/protocol/types/NetworkInventoryAction.php | 6 ++++-- 7 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/pocketmine/network/mcpe/protocol/BatchPacket.php b/src/pocketmine/network/mcpe/protocol/BatchPacket.php index 48ee3e600..1c68f6865 100644 --- a/src/pocketmine/network/mcpe/protocol/BatchPacket.php +++ b/src/pocketmine/network/mcpe/protocol/BatchPacket.php @@ -111,7 +111,7 @@ class BatchPacket extends DataPacket{ $pk = PacketPool::getPacket($buf); if(!$pk->canBeBatched()){ - throw new \InvalidArgumentException("Received invalid " . get_class($pk) . " inside BatchPacket"); + throw new \UnexpectedValueException("Received invalid " . get_class($pk) . " inside BatchPacket"); } $session->handleDataPacket($pk); diff --git a/src/pocketmine/network/mcpe/protocol/BookEditPacket.php b/src/pocketmine/network/mcpe/protocol/BookEditPacket.php index d211b7f15..72c8dc032 100644 --- a/src/pocketmine/network/mcpe/protocol/BookEditPacket.php +++ b/src/pocketmine/network/mcpe/protocol/BookEditPacket.php @@ -109,7 +109,7 @@ class BookEditPacket extends DataPacket{ $this->putString($this->xuid); break; default: - throw new \UnexpectedValueException("Unknown book edit type $this->type!"); + throw new \InvalidArgumentException("Unknown book edit type $this->type!"); } } diff --git a/src/pocketmine/network/mcpe/protocol/ClientboundMapItemDataPacket.php b/src/pocketmine/network/mcpe/protocol/ClientboundMapItemDataPacket.php index 64321432d..61caa3018 100644 --- a/src/pocketmine/network/mcpe/protocol/ClientboundMapItemDataPacket.php +++ b/src/pocketmine/network/mcpe/protocol/ClientboundMapItemDataPacket.php @@ -91,7 +91,7 @@ class ClientboundMapItemDataPacket extends DataPacket{ }elseif($object->type === MapTrackedObject::TYPE_ENTITY){ $object->entityUniqueId = $this->getEntityUniqueId(); }else{ - throw new \UnexpectedValueException("Unknown map object type"); + throw new \UnexpectedValueException("Unknown map object type $object->type"); } $this->trackedEntities[] = $object; } @@ -163,7 +163,7 @@ class ClientboundMapItemDataPacket extends DataPacket{ }elseif($object->type === MapTrackedObject::TYPE_ENTITY){ $this->putEntityUniqueId($object->entityUniqueId); }else{ - throw new \UnexpectedValueException("Unknown map object type"); + throw new \InvalidArgumentException("Unknown map object type $object->type"); } } diff --git a/src/pocketmine/network/mcpe/protocol/DataPacket.php b/src/pocketmine/network/mcpe/protocol/DataPacket.php index 44bafbaad..f5f20bf3d 100644 --- a/src/pocketmine/network/mcpe/protocol/DataPacket.php +++ b/src/pocketmine/network/mcpe/protocol/DataPacket.php @@ -68,12 +68,20 @@ abstract class DataPacket extends NetworkBinaryStream{ return false; } + /** + * @throws \OutOfBoundsException + * @throws \UnexpectedValueException + */ public function decode(){ $this->offset = 0; $this->decodeHeader(); $this->decodePayload(); } + /** + * @throws \OutOfBoundsException + * @throws \UnexpectedValueException + */ protected function decodeHeader(){ $pid = $this->getUnsignedVarInt(); if($pid !== static::NETWORK_ID){ @@ -83,6 +91,9 @@ abstract class DataPacket extends NetworkBinaryStream{ /** * Note for plugin developers: If you're adding your own packets, you should perform decoding in here. + * + * @throws \OutOfBoundsException + * @throws \UnexpectedValueException */ protected function decodePayload(){ diff --git a/src/pocketmine/network/mcpe/protocol/InventoryTransactionPacket.php b/src/pocketmine/network/mcpe/protocol/InventoryTransactionPacket.php index 7a3556b70..e84f0d845 100644 --- a/src/pocketmine/network/mcpe/protocol/InventoryTransactionPacket.php +++ b/src/pocketmine/network/mcpe/protocol/InventoryTransactionPacket.php @@ -147,7 +147,7 @@ class InventoryTransactionPacket extends DataPacket{ $this->putVector3($this->trData->headPos); break; default: - throw new \UnexpectedValueException("Unknown transaction type $this->transactionType"); + throw new \InvalidArgumentException("Unknown transaction type $this->transactionType"); } } diff --git a/src/pocketmine/network/mcpe/protocol/SetScorePacket.php b/src/pocketmine/network/mcpe/protocol/SetScorePacket.php index f6fba05b0..dca31a534 100644 --- a/src/pocketmine/network/mcpe/protocol/SetScorePacket.php +++ b/src/pocketmine/network/mcpe/protocol/SetScorePacket.php @@ -81,7 +81,7 @@ class SetScorePacket extends DataPacket{ $this->putString($entry->customName); break; default: - throw new \UnexpectedValueException("Unknown entry type $entry->type"); + throw new \InvalidArgumentException("Unknown entry type $entry->type"); } } } diff --git a/src/pocketmine/network/mcpe/protocol/types/NetworkInventoryAction.php b/src/pocketmine/network/mcpe/protocol/types/NetworkInventoryAction.php index dd83a99ad..abe15d8b8 100644 --- a/src/pocketmine/network/mcpe/protocol/types/NetworkInventoryAction.php +++ b/src/pocketmine/network/mcpe/protocol/types/NetworkInventoryAction.php @@ -151,7 +151,7 @@ class NetworkInventoryAction{ $packet->putVarInt($this->windowId); break; default: - throw new \UnexpectedValueException("Unknown inventory action source type $this->sourceType"); + throw new \InvalidArgumentException("Unknown inventory action source type $this->sourceType"); } $packet->putUnsignedVarInt($this->inventorySlot); @@ -163,6 +163,8 @@ class NetworkInventoryAction{ * @param Player $player * * @return InventoryAction|null + * + * @throws \UnexpectedValueException */ public function createInventoryAction(Player $player){ switch($this->sourceType){ @@ -172,7 +174,7 @@ class NetworkInventoryAction{ return new SlotChangeAction($window, $this->inventorySlot, $this->oldItem, $this->newItem); } - throw new \InvalidStateException("Player " . $player->getName() . " has no open container with window ID $this->windowId"); + throw new \UnexpectedValueException("Player " . $player->getName() . " has no open container with window ID $this->windowId"); case self::SOURCE_WORLD: if($this->inventorySlot !== self::ACTION_MAGIC_SLOT_DROP_ITEM){ throw new \UnexpectedValueException("Only expecting drop-item world actions from the client!"); From 5e0c3333cfb06b4972afa46e2f732104160da216 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 3 Jan 2019 17:57:37 +0000 Subject: [PATCH 2/2] Player: catch more specific exceptions for transactions --- src/pocketmine/Player.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pocketmine/Player.php b/src/pocketmine/Player.php index a8379bfcc..ebe1c5044 100644 --- a/src/pocketmine/Player.php +++ b/src/pocketmine/Player.php @@ -2268,7 +2268,7 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{ if($action !== null){ $actions[] = $action; } - }catch(\Exception $e){ + }catch(\UnexpectedValueException $e){ $this->server->getLogger()->debug("Unhandled inventory action from " . $this->getName() . ": " . $e->getMessage()); $this->sendAllInventories(); return false;