From 1370930ea9941b657554eba99a437bcc61a9eb8d Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 16:14:30 +0000 Subject: [PATCH 01/12] Entity: remove redundant defaults from lastX lastY lastZ, remove nullability these fields are never null because they are initialized in the constructor, and they are never written to with null nor ever expected to be null. --- src/pocketmine/entity/Entity.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/pocketmine/entity/Entity.php b/src/pocketmine/entity/Entity.php index a2b3468e9..22d1e79fc 100644 --- a/src/pocketmine/entity/Entity.php +++ b/src/pocketmine/entity/Entity.php @@ -463,12 +463,12 @@ abstract class Entity extends Location implements Metadatable, EntityIds{ /** @var Block[]|null */ protected $blocksAround = null; - /** @var float|null */ - public $lastX = null; - /** @var float|null */ - public $lastY = null; - /** @var float|null */ - public $lastZ = null; + /** @var float */ + public $lastX; + /** @var float */ + public $lastY; + /** @var float */ + public $lastZ; /** @var Vector3 */ protected $motion; From e8d3a2502880db87b2d459373cd82ea8d68479c5 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 16:15:49 +0000 Subject: [PATCH 02/12] Position::__construct() accepts floats for x,y,z --- src/pocketmine/level/Position.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pocketmine/level/Position.php b/src/pocketmine/level/Position.php index 608c412c5..cb8ea44b7 100644 --- a/src/pocketmine/level/Position.php +++ b/src/pocketmine/level/Position.php @@ -33,10 +33,10 @@ class Position extends Vector3{ public $level = null; /** - * @param int $x - * @param int $y - * @param int $z - * @param Level $level + * @param float|int $x + * @param float|int $y + * @param float|int $z + * @param Level $level */ public function __construct($x = 0, $y = 0, $z = 0, Level $level = null){ parent::__construct($x, $y, $z); From 77f7595e0e5d82be4f22dc3a3682593e9d1bb87a Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 16:16:16 +0000 Subject: [PATCH 03/12] Location::__construct() accepts floats for x,y,z --- src/pocketmine/level/Location.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/pocketmine/level/Location.php b/src/pocketmine/level/Location.php index d29f8b26c..435e82575 100644 --- a/src/pocketmine/level/Location.php +++ b/src/pocketmine/level/Location.php @@ -33,12 +33,12 @@ class Location extends Position{ public $pitch; /** - * @param int $x - * @param int $y - * @param int $z - * @param float $yaw - * @param float $pitch - * @param Level $level + * @param float|int $x + * @param float|int $y + * @param float|int $z + * @param float $yaw + * @param float $pitch + * @param Level $level */ public function __construct($x = 0, $y = 0, $z = 0, $yaw = 0.0, $pitch = 0.0, Level $level = null){ $this->yaw = $yaw; From f39fc7e525b23a274e1163bf837a03f250dfce9b Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 16:16:48 +0000 Subject: [PATCH 04/12] CompressBatchedTask::__construct() accepts Player[] not string[] --- src/pocketmine/network/CompressBatchedTask.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pocketmine/network/CompressBatchedTask.php b/src/pocketmine/network/CompressBatchedTask.php index 86b784d31..d0f6757c2 100644 --- a/src/pocketmine/network/CompressBatchedTask.php +++ b/src/pocketmine/network/CompressBatchedTask.php @@ -35,7 +35,7 @@ class CompressBatchedTask extends AsyncTask{ /** * @param BatchPacket $batch - * @param string[] $targets + * @param Player[] $targets */ public function __construct(BatchPacket $batch, array $targets){ $this->data = $batch->payload; From 77795ae3bc58146fcd10ae6d363eff06f5656d09 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 16:18:11 +0000 Subject: [PATCH 05/12] BaseLang::translateString() accepts float and int too (they can be casted to string) --- src/pocketmine/lang/BaseLang.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pocketmine/lang/BaseLang.php b/src/pocketmine/lang/BaseLang.php index aece04221..0ec3a68f2 100644 --- a/src/pocketmine/lang/BaseLang.php +++ b/src/pocketmine/lang/BaseLang.php @@ -115,9 +115,9 @@ class BaseLang{ } /** - * @param string $str - * @param string[] $params - * @param string|null $onlyPrefix + * @param string $str + * @param (float|int|string)[] $params + * @param string|null $onlyPrefix * * @return string */ From 00888fdc553a45146d2811ca6a186e4f6186611b Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 16:19:04 +0000 Subject: [PATCH 06/12] TranslationContainer::__construct() accepts float and int too (they can be casted to string) --- src/pocketmine/lang/TranslationContainer.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pocketmine/lang/TranslationContainer.php b/src/pocketmine/lang/TranslationContainer.php index aa9bf3d01..69461b281 100644 --- a/src/pocketmine/lang/TranslationContainer.php +++ b/src/pocketmine/lang/TranslationContainer.php @@ -31,8 +31,8 @@ class TranslationContainer extends TextContainer{ protected $params = []; /** - * @param string $text - * @param string[] $params + * @param string $text + * @param (float|int|string)[] $params */ public function __construct(string $text, array $params = []){ parent::__construct($text); From cb598155a467366b5058b59462c19dfa8f5397fa Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 16:19:57 +0000 Subject: [PATCH 07/12] Server: add @return annotation to crashDump() to make phpstan happy this is technically a bug in PHPStan, but it's easier to do this than report a bug. --- src/pocketmine/Server.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pocketmine/Server.php b/src/pocketmine/Server.php index a1e2e6821..da4e56d61 100644 --- a/src/pocketmine/Server.php +++ b/src/pocketmine/Server.php @@ -2246,6 +2246,9 @@ class Server{ $this->crashDump(); } + /** + * @return void + */ public function crashDump(){ while(@ob_end_flush()){} if(!$this->isRunning){ From 919534d978f16d1b099bbd74c0a87743cb2341da Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 16:28:30 +0000 Subject: [PATCH 08/12] EnderChest: fixed crash when plugins overwrite tile classes with incompatible ones relates to 47a959dace95eafe79212a444e3f865475c6ee8e --- src/pocketmine/block/EnderChest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pocketmine/block/EnderChest.php b/src/pocketmine/block/EnderChest.php index ea5afb257..1f1f5e6dd 100644 --- a/src/pocketmine/block/EnderChest.php +++ b/src/pocketmine/block/EnderChest.php @@ -84,6 +84,9 @@ class EnderChest extends Chest{ $enderChest = $t; }else{ $enderChest = Tile::createTile(Tile::ENDER_CHEST, $this->getLevel(), TileEnderChest::createNBT($this)); + if(!($enderChest instanceof TileEnderChest)){ + return true; + } } if(!$this->getSide(Vector3::SIDE_UP)->isTransparent()){ From 9a4b72add5e3cc332dee324a0c6cc2f746c3f11e Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 16:31:22 +0000 Subject: [PATCH 09/12] PlayerInventory: fix type violation when calling equipItem() for non-Player holder --- src/pocketmine/inventory/PlayerInventory.php | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/pocketmine/inventory/PlayerInventory.php b/src/pocketmine/inventory/PlayerInventory.php index 067eccb41..6ffca62df 100644 --- a/src/pocketmine/inventory/PlayerInventory.php +++ b/src/pocketmine/inventory/PlayerInventory.php @@ -66,19 +66,23 @@ class PlayerInventory extends BaseInventory{ * @return bool if the equipment change was successful, false if not. */ public function equipItem(int $hotbarSlot) : bool{ + $holder = $this->getHolder(); if(!$this->isHotbarSlot($hotbarSlot)){ - $this->sendContents($this->getHolder()); + if($holder instanceof Player){ + $this->sendContents($holder); + } return false; } - $ev = new PlayerItemHeldEvent($this->getHolder(), $this->getItem($hotbarSlot), $hotbarSlot); - $ev->call(); + if($holder instanceof Player){ + $ev = new PlayerItemHeldEvent($holder, $this->getItem($hotbarSlot), $hotbarSlot); + $ev->call(); - if($ev->isCancelled()){ - $this->sendHeldItem($this->getHolder()); - return false; + if($ev->isCancelled()){ + $this->sendHeldItem($holder); + return false; + } } - $this->setHeldItemIndex($hotbarSlot, false); return true; From 37a8d95464facc71248455c0dc80e789bd6d9ce9 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 18:21:23 +0000 Subject: [PATCH 10/12] world IO: fixed crashes when garbage data found in tile/entity NBT data --- .../level/format/io/leveldb/LevelDB.php | 30 +++++++++++-------- .../level/format/io/region/Anvil.php | 4 +-- .../level/format/io/region/McRegion.php | 29 ++++++++++++++++-- 3 files changed, 46 insertions(+), 17 deletions(-) diff --git a/src/pocketmine/level/format/io/leveldb/LevelDB.php b/src/pocketmine/level/format/io/leveldb/LevelDB.php index 10229f49c..a6aef3060 100644 --- a/src/pocketmine/level/format/io/leveldb/LevelDB.php +++ b/src/pocketmine/level/format/io/leveldb/LevelDB.php @@ -26,6 +26,7 @@ namespace pocketmine\level\format\io\leveldb; use pocketmine\level\format\Chunk; use pocketmine\level\format\io\BaseLevelProvider; use pocketmine\level\format\io\ChunkUtils; +use pocketmine\level\format\io\exception\CorruptedChunkException; use pocketmine\level\format\io\exception\UnsupportedChunkFormatException; use pocketmine\level\format\SubChunk; use pocketmine\level\generator\Flat; @@ -414,24 +415,27 @@ class LevelDB extends BaseLevelProvider{ /** @var CompoundTag[] $entities */ $entities = []; if(($entityData = $this->db->get($index . self::TAG_ENTITY)) !== false and $entityData !== ""){ - $entities = $nbt->read($entityData, true); - if(!is_array($entities)){ - $entities = [$entities]; - } - } - - /** @var CompoundTag $entityNBT */ - foreach($entities as $entityNBT){ - if($entityNBT->hasTag("id", IntTag::class)){ - $entityNBT->setInt("id", $entityNBT->getInt("id") & 0xff); //remove type flags - TODO: use these instead of removing them) + $entityTags = $nbt->read($entityData, true); + foreach((is_array($entityTags) ? $entityTags : [$entityTags]) as $entityTag){ + if(!($entityTag instanceof CompoundTag)){ + throw new CorruptedChunkException("Entity root tag should be TAG_Compound"); + } + if($entityTag->hasTag("id", IntTag::class)){ + $entityTag->setInt("id", $entityTag->getInt("id") & 0xff); //remove type flags - TODO: use these instead of removing them) + } + $entities[] = $entityTag; } } + /** @var CompoundTag[] $tiles */ $tiles = []; if(($tileData = $this->db->get($index . self::TAG_BLOCK_ENTITY)) !== false and $tileData !== ""){ - $tiles = $nbt->read($tileData, true); - if(!is_array($tiles)){ - $tiles = [$tiles]; + $tileTags = $nbt->read($tileData, true); + foreach((is_array($tileTags) ? $tileTags : [$tileTags]) as $tileTag){ + if(!($tileTag instanceof CompoundTag)){ + throw new CorruptedChunkException("Tile root tag should be TAG_Compound"); + } + $tiles[] = $tileTag; } } diff --git a/src/pocketmine/level/format/io/region/Anvil.php b/src/pocketmine/level/format/io/region/Anvil.php index 12b1cc965..907b423e8 100644 --- a/src/pocketmine/level/format/io/region/Anvil.php +++ b/src/pocketmine/level/format/io/region/Anvil.php @@ -122,8 +122,8 @@ class Anvil extends McRegion{ $chunk->getInt("xPos"), $chunk->getInt("zPos"), $subChunks, - $chunk->hasTag("Entities", ListTag::class) ? $chunk->getListTag("Entities")->getValue() : [], - $chunk->hasTag("TileEntities", ListTag::class) ? $chunk->getListTag("TileEntities")->getValue() : [], + $chunk->hasTag("Entities", ListTag::class) ? self::getCompoundList("Entities", $chunk->getListTag("Entities")) : [], + $chunk->hasTag("TileEntities", ListTag::class) ? self::getCompoundList("TileEntities", $chunk->getListTag("TileEntities")) : [], $biomeIds, $chunk->getIntArray("HeightMap", []) ); diff --git a/src/pocketmine/level/format/io/region/McRegion.php b/src/pocketmine/level/format/io/region/McRegion.php index 9f2871635..51cad2438 100644 --- a/src/pocketmine/level/format/io/region/McRegion.php +++ b/src/pocketmine/level/format/io/region/McRegion.php @@ -194,8 +194,8 @@ class McRegion extends BaseLevelProvider{ $chunk->getInt("xPos"), $chunk->getInt("zPos"), $subChunks, - $chunk->hasTag("Entities", ListTag::class) ? $chunk->getListTag("Entities")->getValue() : [], - $chunk->hasTag("TileEntities", ListTag::class) ? $chunk->getListTag("TileEntities")->getValue() : [], + $chunk->hasTag("Entities", ListTag::class) ? self::getCompoundList("Entities", $chunk->getListTag("Entities")) : [], + $chunk->hasTag("TileEntities", ListTag::class) ? self::getCompoundList("TileEntities", $chunk->getListTag("TileEntities")) : [], $biomeIds, $heightMap ); @@ -205,6 +205,31 @@ class McRegion extends BaseLevelProvider{ return $result; } + /** + * @param string $context + * @param ListTag $list + * + * @return CompoundTag[] + * @throws CorruptedChunkException + */ + protected static function getCompoundList(string $context, ListTag $list) : array{ + if($list->count() === 0){ //empty lists might have wrong types, we don't care + return []; + } + if($list->getTagType() !== NBT::TAG_Compound){ + throw new CorruptedChunkException("Expected TAG_List for '$context'"); + } + $result = []; + foreach($list as $tag){ + if(!($tag instanceof CompoundTag)){ + //this should never happen, but it's still possible due to lack of native type safety + throw new CorruptedChunkException("Expected TAG_List for '$context'"); + } + $result[] = $tag; + } + return $result; + } + public static function getProviderName() : string{ return "mcregion"; } From cf73d74bd0178d3dced1a46f90545f33e8ae840c Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 18:24:21 +0000 Subject: [PATCH 11/12] format/anvil: fixed possible type violation on saving chunk --- src/pocketmine/level/format/io/region/Anvil.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pocketmine/level/format/io/region/Anvil.php b/src/pocketmine/level/format/io/region/Anvil.php index 907b423e8..d602fd8a2 100644 --- a/src/pocketmine/level/format/io/region/Anvil.php +++ b/src/pocketmine/level/format/io/region/Anvil.php @@ -51,7 +51,7 @@ class Anvil extends McRegion{ $subChunks = []; foreach($chunk->getSubChunks() as $y => $subChunk){ - if($subChunk->isEmpty()){ + if(!($subChunk instanceof SubChunk) or $subChunk->isEmpty()){ continue; } From 76994f15acb0122ca43fef10e873c703f3b9adcc Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 18:27:26 +0000 Subject: [PATCH 12/12] phpstan: green on level 5 --- phpstan.neon.dist | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan.neon.dist b/phpstan.neon.dist index c8556a7fd..3880ad102 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -8,7 +8,7 @@ includes: - tests/phpstan/configs/runtime-type-checks.neon parameters: - level: 4 + level: 5 autoload_files: - tests/phpstan/bootstrap.php - src/pocketmine/PocketMine.php