World: Harden chunk loading against bad data in entity/tile NBT

pmmp · May 12, 2021 · b2e806e · b2e806e
1 parent 80e4da8
commit b2e806e
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 19 deletions.
diff --git a/src/block/tile/Tile.php b/src/block/tile/Tile.php
@@ -30,6 +30,7 @@
 use pocketmine\block\Block;
 use pocketmine\item\Item;
 use pocketmine\math\Vector3;
+use pocketmine\nbt\NbtDataException;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\timings\Timings;
 use pocketmine\timings\TimingsHandler;
@@ -58,6 +59,7 @@ public function __construct(World $world, Vector3 $pos){
 
 	/**
 	 * @internal
+	 * @throws NbtDataException
 	 * Reads additional data from the CompoundTag on tile creation.
 	 */
 	abstract public function readSaveData(CompoundTag $nbt) : void;

diff --git a/src/block/tile/TileFactory.php b/src/block/tile/TileFactory.php
@@ -24,6 +24,7 @@
 namespace pocketmine\block\tile;
 
 use pocketmine\math\Vector3;
+use pocketmine\nbt\NbtDataException;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\utils\SingletonTrait;
 use pocketmine\utils\Utils;
@@ -111,6 +112,7 @@ public function register(string $className, array $saveNames = []) : void{
 
 	/**
 	 * @internal
+	 * @throws NbtDataException
 	 */
 	public function createFromData(World $world, CompoundTag $nbt) : ?Tile{
 		$type = $nbt->getString(Tile::TAG_ID, "");

diff --git a/src/entity/EntityFactory.php b/src/entity/EntityFactory.php
@@ -43,6 +43,7 @@
 use pocketmine\item\Item;
 use pocketmine\math\Facing;
 use pocketmine\math\Vector3;
+use pocketmine\nbt\NbtDataException;
 use pocketmine\nbt\tag\ByteTag;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\nbt\tag\IntTag;
@@ -208,6 +209,7 @@ public function register(string $className, \Closure $creationFunc, array $saveN
 	 * Creates an entity from data stored on a chunk.
 	 *
 	 * @throws \RuntimeException
+	 * @throws NbtDataException
 	 * @internal
 	 */
 	public function createFromData(World $world, CompoundTag $nbt) : ?Entity{

diff --git a/src/world/World.php b/src/world/World.php
@@ -56,6 +56,7 @@
 use pocketmine\item\LegacyStringToItemParser;
 use pocketmine\math\AxisAlignedBB;
 use pocketmine\math\Vector3;
+use pocketmine\nbt\NbtDataException;
 use pocketmine\nbt\tag\IntTag;
 use pocketmine\nbt\tag\StringTag;
 use pocketmine\network\mcpe\convert\RuntimeBlockMapping;
@@ -2450,24 +2451,26 @@ private function initChunk(int $chunkX, int $chunkZ, Chunk $chunk) : void{
 		if($chunk->NBTentities !== null){
 			$this->timings->syncChunkLoadEntities->startTiming();
 			$entityFactory = EntityFactory::getInstance();
-			foreach($chunk->NBTentities as $nbt){
+			foreach($chunk->NBTentities as $k => $nbt){
 				try{
 					$entity = $entityFactory->createFromData($this, $nbt);
-					if(!($entity instanceof Entity)){
-						$saveIdTag = $nbt->getTag("id") ?? $nbt->getTag("identifier");
-						$saveId = "<unknown>";
-						if($saveIdTag instanceof StringTag){
-							$saveId = $saveIdTag->getValue();
-						}elseif($saveIdTag instanceof IntTag){ //legacy MCPE format
-							$saveId = "legacy(" . $saveIdTag->getValue() . ")";
-						}
-						$this->getLogger()->warning("Chunk $chunkX $chunkZ: Deleted unknown entity type $saveId");
-						continue;
-					}
-				}catch(\Exception $t){ //TODO: this shouldn't be here
-					$this->getLogger()->logException($t);
+				}catch(NbtDataException $e){
+					$this->getLogger()->error("Chunk $chunkX $chunkZ: Bad entity data at list position $k: " . $e->getMessage());
+					$this->getLogger()->logException($e);
 					continue;
 				}
+				if($entity === null){
+					$saveIdTag = $nbt->getTag("id") ?? $nbt->getTag("identifier");
+					$saveId = "<unknown>";
+					if($saveIdTag instanceof StringTag){
+						$saveId = $saveIdTag->getValue();
+					}elseif($saveIdTag instanceof IntTag){ //legacy MCPE format
+						$saveId = "legacy(" . $saveIdTag->getValue() . ")";
+					}
+					$this->getLogger()->warning("Chunk $chunkX $chunkZ: Deleted unknown entity type $saveId");
+				}
+				//TODO: we can't prevent entities getting added to unloaded chunks if they were saved in the wrong place
+				//here, because entities currently add themselves to the world
 			}
 
 			$chunk->setDirtyFlag(Chunk::DIRTY_FLAG_ENTITIES, true);
@@ -2477,13 +2480,21 @@ private function initChunk(int $chunkX, int $chunkZ, Chunk $chunk) : void{
 		if($chunk->NBTtiles !== null){
 			$this->timings->syncChunkLoadTileEntities->startTiming();
 			$tileFactory = TileFactory::getInstance();
-			foreach($chunk->NBTtiles as $nbt){
-				if(($tile = $tileFactory->createFromData($this, $nbt)) !== null){
-					$this->addTile($tile);
-				}else{
-					$this->getLogger()->warning("Chunk $chunkX $chunkZ: Deleted unknown tile entity type " . $nbt->getString("id", "<unknown>"));
+			foreach($chunk->NBTtiles as $k => $nbt){
+				try{
+					$tile = $tileFactory->createFromData($this, $nbt);
+				}catch(NbtDataException $e){
+					$this->getLogger()->error("Chunk $chunkX $chunkZ: Bad tile entity data at list position $k: " . $e->getMessage());
+					$this->getLogger()->logException($e);
 					continue;
 				}
+				if($tile === null){
+					$this->getLogger()->warning("Chunk $chunkX $chunkZ: Deleted unknown tile entity type " . $nbt->getString("id", "<unknown>"));
+				}elseif(!$this->isChunkLoaded($tile->getPos()->getFloorX() >> 4, $tile->getPos()->getFloorZ() >> 4)){
+					$this->logger->error("Chunk $chunkX $chunkZ: Found tile saved on wrong chunk - unable to fix due to correct chunk not loaded");
+				}else{
+					$this->addTile($tile);
+				}
 			}
 
 			$chunk->setDirtyFlag(Chunk::DIRTY_FLAG_TILES, true);