diff --git a/src/pocketmine/Player.php b/src/pocketmine/Player.php index 82b4cb221..27495eaad 100644 --- a/src/pocketmine/Player.php +++ b/src/pocketmine/Player.php @@ -820,10 +820,6 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener, * @return bool */ public function changeSkin(Skin $skin, string $newSkinName, string $oldSkinName) : bool{ - if(!$skin->isValid()){ - return false; - } - $ev = new PlayerChangeSkinEvent($this, $this->getSkin(), $skin); $ev->call(); diff --git a/src/pocketmine/entity/Human.php b/src/pocketmine/entity/Human.php index c17fc1338..bc744956f 100644 --- a/src/pocketmine/entity/Human.php +++ b/src/pocketmine/entity/Human.php @@ -105,30 +105,18 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{ if($skinTag === null){ throw new \InvalidStateException((new \ReflectionClass($this))->getShortName() . " must have a valid skin set"); } - $this->skin = self::deserializeSkinNBT($skinTag); //this throws if the skin is invalid + $this->skin = new Skin( //this throws if the skin is invalid + $skinTag->getString("Name"), + $skinTag->hasTag("Data", StringTag::class) ? $skinTag->getString("Data") : $skinTag->getByteArray("Data"), //old data (this used to be saved as a StringTag in older versions of PM) + $skinTag->getByteArray("CapeData", ""), + $skinTag->getString("GeometryName", ""), + $skinTag->getByteArray("GeometryData", "") + ); } parent::__construct($world, $nbt); } - /** - * @param CompoundTag $skinTag - * - * @return Skin - * @throws \InvalidArgumentException - */ - protected static function deserializeSkinNBT(CompoundTag $skinTag) : Skin{ - $skin = new Skin( - $skinTag->getString("Name"), - $skinTag->hasTag("Data", StringTag::class) ? $skinTag->getString("Data") : $skinTag->getByteArray("Data"), //old data (this used to be saved as a StringTag in older versions of PM) - $skinTag->getByteArray("CapeData", ""), - $skinTag->getString("GeometryName", ""), - $skinTag->getByteArray("GeometryData", "") - ); - $skin->validate(); - return $skin; - } - /** * @deprecated * @@ -171,7 +159,6 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{ * @param Skin $skin */ public function setSkin(Skin $skin) : void{ - $skin->validate(); $this->skin = $skin; $this->skin->debloatGeometryData(); } @@ -841,8 +828,6 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{ } protected function sendSpawnPacket(Player $player) : void{ - $this->skin->validate(); - if(!($this instanceof Player)){ /* we don't use Server->updatePlayerListData() because that uses batches, which could cause race conditions in async compression mode */ $pk = new PlayerListPacket(); diff --git a/src/pocketmine/entity/Skin.php b/src/pocketmine/entity/Skin.php index 2c66dfaee..719274862 100644 --- a/src/pocketmine/entity/Skin.php +++ b/src/pocketmine/entity/Skin.php @@ -48,6 +48,18 @@ class Skin{ private $geometryData; public function __construct(string $skinId, string $skinData, string $capeData = "", string $geometryName = "", string $geometryData = ""){ + if($skinId === ""){ + throw new \InvalidArgumentException("Skin ID must not be empty"); + } + $len = strlen($skinData); + if(!in_array($len, self::ACCEPTED_SKIN_SIZES, true)){ + throw new \InvalidArgumentException("Invalid skin data size $len bytes (allowed sizes: " . implode(", ", self::ACCEPTED_SKIN_SIZES) . ")"); + } + if($capeData !== "" and strlen($capeData) !== 8192){ + throw new \InvalidArgumentException("Invalid cape data size " . strlen($capeData) . " bytes (must be exactly 8192 bytes)"); + } + //TODO: validate geometry + $this->skinId = $skinId; $this->skinData = $skinData; $this->capeData = $capeData; @@ -55,36 +67,6 @@ class Skin{ $this->geometryData = $geometryData; } - /** - * @deprecated - * @return bool - */ - public function isValid() : bool{ - try{ - $this->validate(); - return true; - }catch(\InvalidArgumentException $e){ - return false; - } - } - - /** - * @throws \InvalidArgumentException - */ - public function validate() : void{ - if($this->skinId === ""){ - throw new \InvalidArgumentException("Skin ID must not be empty"); - } - $len = strlen($this->skinData); - if(!in_array($len, self::ACCEPTED_SKIN_SIZES, true)){ - throw new \InvalidArgumentException("Invalid skin data size $len bytes (allowed sizes: " . implode(", ", self::ACCEPTED_SKIN_SIZES) . ")"); - } - if($this->capeData !== "" and strlen($this->capeData) !== 8192){ - throw new \InvalidArgumentException("Invalid cape data size " . strlen($this->capeData) . " bytes (must be exactly 8192 bytes)"); - } - //TODO: validate geometry - } - /** * @return string */ diff --git a/src/pocketmine/event/player/PlayerChangeSkinEvent.php b/src/pocketmine/event/player/PlayerChangeSkinEvent.php index 912f06b47..5202ff974 100644 --- a/src/pocketmine/event/player/PlayerChangeSkinEvent.php +++ b/src/pocketmine/event/player/PlayerChangeSkinEvent.php @@ -70,7 +70,6 @@ class PlayerChangeSkinEvent extends PlayerEvent implements Cancellable{ * @throws \InvalidArgumentException if the specified skin is not valid */ public function setNewSkin(Skin $skin) : void{ - $skin->validate(); $this->newSkin = $skin; } } diff --git a/src/pocketmine/network/mcpe/handler/LoginSessionHandler.php b/src/pocketmine/network/mcpe/handler/LoginSessionHandler.php index d75d0d518..e9416f68b 100644 --- a/src/pocketmine/network/mcpe/handler/LoginSessionHandler.php +++ b/src/pocketmine/network/mcpe/handler/LoginSessionHandler.php @@ -75,14 +75,16 @@ class LoginSessionHandler extends SessionHandler{ return true; } - $skin = new Skin( - $packet->clientData[LoginPacket::I_SKIN_ID], - base64_decode($packet->clientData[LoginPacket::I_SKIN_DATA]), - base64_decode($packet->clientData[LoginPacket::I_CAPE_DATA]), - $packet->clientData[LoginPacket::I_GEOMETRY_NAME], - base64_decode($packet->clientData[LoginPacket::I_GEOMETRY_DATA]) - ); - if(!$skin->isValid()){ + try{ + $skin = new Skin( + $packet->clientData[LoginPacket::I_SKIN_ID], + base64_decode($packet->clientData[LoginPacket::I_SKIN_DATA]), + base64_decode($packet->clientData[LoginPacket::I_CAPE_DATA]), + $packet->clientData[LoginPacket::I_GEOMETRY_NAME], + base64_decode($packet->clientData[LoginPacket::I_GEOMETRY_DATA]) + ); + }catch(\InvalidArgumentException $e){ + $this->session->getLogger()->debug("Invalid skin: " . $e->getMessage()); $this->session->disconnect("disconnectionScreen.invalidSkin"); return true; diff --git a/src/pocketmine/network/mcpe/protocol/PlayerListPacket.php b/src/pocketmine/network/mcpe/protocol/PlayerListPacket.php index 2d0eeb4e1..ec4f1af24 100644 --- a/src/pocketmine/network/mcpe/protocol/PlayerListPacket.php +++ b/src/pocketmine/network/mcpe/protocol/PlayerListPacket.php @@ -27,6 +27,7 @@ namespace pocketmine\network\mcpe\protocol; use pocketmine\entity\Skin; +use pocketmine\network\BadPacketException; use pocketmine\network\mcpe\handler\SessionHandler; use pocketmine\network\mcpe\protocol\types\PlayerListEntry; use function count; @@ -59,13 +60,17 @@ class PlayerListPacket extends DataPacket implements ClientboundPacket{ $geometryName = $this->getString(); $geometryData = $this->getString(); - $entry->skin = new Skin( - $skinId, - $skinData, - $capeData, - $geometryName, - $geometryData - ); + try{ + $entry->skin = new Skin( + $skinId, + $skinData, + $capeData, + $geometryName, + $geometryData + ); + }catch(\InvalidArgumentException $e){ + throw new BadPacketException($e->getMessage(), 0, $e); + } $entry->xboxUserId = $this->getString(); $entry->platformChatId = $this->getString(); }else{ diff --git a/src/pocketmine/network/mcpe/protocol/PlayerSkinPacket.php b/src/pocketmine/network/mcpe/protocol/PlayerSkinPacket.php index 066ae5828..e6c56bf20 100644 --- a/src/pocketmine/network/mcpe/protocol/PlayerSkinPacket.php +++ b/src/pocketmine/network/mcpe/protocol/PlayerSkinPacket.php @@ -26,6 +26,7 @@ namespace pocketmine\network\mcpe\protocol; #include use pocketmine\entity\Skin; +use pocketmine\network\BadPacketException; use pocketmine\network\mcpe\handler\SessionHandler; use pocketmine\utils\UUID; @@ -54,7 +55,11 @@ class PlayerSkinPacket extends DataPacket implements ClientboundPacket, Serverbo $geometryModel = $this->getString(); $geometryData = $this->getString(); - $this->skin = new Skin($skinId, $skinData, $capeData, $geometryModel, $geometryData); + try{ + $this->skin = new Skin($skinId, $skinData, $capeData, $geometryModel, $geometryData); + }catch(\InvalidArgumentException $e){ + throw new BadPacketException($e->getMessage(), 0, $e); + } $this->premiumSkin = $this->getBool(); }