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

src/block/tile/Tile.php

@@ -30,6 +30,7 @@ namespace pocketmine\block\tile;
 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 @@ abstract class Tile{
 
 	/**
 	 * @internal
+	 * @throws NbtDataException
 	 * Reads additional data from the CompoundTag on tile creation.
 	 */
 	abstract public function readSaveData(CompoundTag $nbt) : void;

src/block/tile/TileFactory.php

@@ -24,6 +24,7 @@ declare(strict_types=1);
 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 @@ final class TileFactory{
 
 	/**
 	 * @internal
+	 * @throws NbtDataException
 	 */
 	public function createFromData(World $world, CompoundTag $nbt) : ?Tile{
 		$type = $nbt->getString(Tile::TAG_ID, "");

src/entity/EntityFactory.php

@@ -43,6 +43,7 @@ use pocketmine\entity\projectile\SplashPotion;
 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 @@ final class EntityFactory{
 	 * Creates an entity from data stored on a chunk.
 	 *
 	 * @throws \RuntimeException
+	 * @throws NbtDataException
 	 * @internal
 	 */
 	public function createFromData(World $world, CompoundTag $nbt) : ?Entity{

src/world/World.php

@@ -56,6 +56,7 @@ use pocketmine\item\ItemUseResult;
 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,10 +2451,15 @@ class World implements ChunkManager{
 		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)){
+				}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){
@@ -2462,12 +2468,9 @@ class World implements ChunkManager{
 						$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);
-					continue;
 				}
+				//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 @@ class World implements ChunkManager{
 		if($chunk->NBTtiles !== null){
 			$this->timings->syncChunkLoadTileEntities->startTiming();
 			$tileFactory = TileFactory::getInstance();
-			foreach($chunk->NBTtiles as $nbt){
+			foreach($chunk->NBTtiles as $k => $nbt){
-				if(($tile = $tileFactory->createFromData($this, $nbt)) !== null){
+				try{
-					$this->addTile($tile);
+					$tile = $tileFactory->createFromData($this, $nbt);
-				}else{
+				}catch(NbtDataException $e){
-					$this->getLogger()->warning("Chunk $chunkX $chunkZ: Deleted unknown tile entity type " . $nbt->getString("id", "<unknown>"));
+					$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);