From 76e5ea385b0b39670017a26347d190968a5adf1d Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Fri, 17 May 2019 17:40:27 +0100 Subject: [PATCH] Skin: improved error checking plugin devs should find this less of a pain in the ass now. --- src/pocketmine/entity/Human.php | 49 ++++++++++--------- src/pocketmine/entity/Skin.php | 39 +++++++++++++-- .../event/player/PlayerChangeSkinEvent.php | 5 +- 3 files changed, 61 insertions(+), 32 deletions(-) diff --git a/src/pocketmine/entity/Human.php b/src/pocketmine/entity/Human.php index a058fc23e..85b86071c 100644 --- a/src/pocketmine/entity/Human.php +++ b/src/pocketmine/entity/Human.php @@ -60,6 +60,7 @@ use function array_merge; use function array_rand; use function array_values; use function ceil; +use function in_array; use function max; use function min; use function mt_rand; @@ -98,18 +99,36 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{ public function __construct(Level $level, CompoundTag $nbt){ if($this->skin === null){ $skinTag = $nbt->getCompoundTag("Skin"); - if($skinTag === null or !self::isValidSkin($skinTag->hasTag("Data", ByteArrayTag::class) ? - $skinTag->getByteArray("Data") : - $skinTag->getString("Data", "") - )){ + 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 } parent::__construct($level, $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 + * * Checks the length of a supplied skin bitmap and returns whether the length is valid. * * @param string $skin @@ -117,7 +136,7 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{ * @return bool */ public static function isValidSkin(string $skin) : bool{ - return strlen($skin) === 64 * 64 * 4 or strlen($skin) === 64 * 32 * 4 or strlen($skin) === 128 * 128 * 4; + return in_array(strlen($skin), Skin::ACCEPTED_SKIN_SIZES, true); } /** @@ -149,10 +168,7 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{ * @param Skin $skin */ public function setSkin(Skin $skin) : void{ - if(!$skin->isValid()){ - throw new \InvalidStateException("Specified skin is not valid, must be 8KiB or 16KiB"); - } - + $skin->validate(); $this->skin = $skin; $this->skin->debloatGeometryData(); } @@ -587,17 +603,6 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{ $this->setNameTag($this->namedtag->getString("NameTag")); } - $skin = $this->namedtag->getCompoundTag("Skin"); - if($skin !== null){ - $this->setSkin(new Skin( - $skin->getString("Name"), - $skin->hasTag("Data", StringTag::class) ? $skin->getString("Data") : $skin->getByteArray("Data"), //old data (this used to be saved as a StringTag in older versions of PM) - $skin->getByteArray("CapeData", ""), - $skin->getString("GeometryName", ""), - $skin->getByteArray("GeometryData", "") - )); - } - $this->uuid = UUID::fromData((string) $this->getId(), $this->skin->getSkinData(), $this->getNameTag()); } @@ -836,9 +841,7 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{ } protected function sendSpawnPacket(Player $player) : void{ - if(!$this->skin->isValid()){ - throw new \InvalidStateException((new \ReflectionClass($this))->getShortName() . " must have a valid skin set"); - } + $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 */ diff --git a/src/pocketmine/entity/Skin.php b/src/pocketmine/entity/Skin.php index a6829add9..2c66dfaee 100644 --- a/src/pocketmine/entity/Skin.php +++ b/src/pocketmine/entity/Skin.php @@ -23,11 +23,18 @@ declare(strict_types=1); namespace pocketmine\entity; +use function implode; +use function in_array; use function json_decode; use function json_encode; use function strlen; class Skin{ + public const ACCEPTED_SKIN_SIZES = [ + 64 * 32 * 4, + 64 * 64 * 4, + 128 * 128 * 4 + ]; /** @var string */ private $skinId; @@ -48,12 +55,34 @@ class Skin{ $this->geometryData = $geometryData; } + /** + * @deprecated + * @return bool + */ public function isValid() : bool{ - return ( - $this->skinId !== "" and - (($s = strlen($this->skinData)) === 16384 or $s === 8192 or $s === 65536) and - ($this->capeData === "" or strlen($this->capeData) === 8192) - ); + 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 } /** diff --git a/src/pocketmine/event/player/PlayerChangeSkinEvent.php b/src/pocketmine/event/player/PlayerChangeSkinEvent.php index bfb854b72..8ebab4c16 100644 --- a/src/pocketmine/event/player/PlayerChangeSkinEvent.php +++ b/src/pocketmine/event/player/PlayerChangeSkinEvent.php @@ -67,10 +67,7 @@ class PlayerChangeSkinEvent extends PlayerEvent implements Cancellable{ * @throws \InvalidArgumentException if the specified skin is not valid */ public function setNewSkin(Skin $skin) : void{ - if(!$skin->isValid()){ - throw new \InvalidArgumentException("Skin format is invalid"); - } - + $skin->validate(); $this->newSkin = $skin; } }