From 541a624d483cb5cc621b8708f22d208ab2bebd0e Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 27 Jun 2022 17:14:43 +0100 Subject: [PATCH] ItemFactory::get() now consistently throws SavedDataLoadingException on any error, including unknown items --- .../CraftingManagerFromDataHelper.php | 73 +++++++++---------- .../bedrock/item/upgrade/ItemDataUpgrader.php | 2 + src/inventory/CreativeInventory.php | 7 +- src/item/Item.php | 4 +- src/item/ItemFactory.php | 30 ++++---- src/item/LegacyStringToItemParser.php | 10 ++- src/network/mcpe/convert/TypeConverter.php | 11 ++- 7 files changed, 69 insertions(+), 68 deletions(-) diff --git a/src/crafting/CraftingManagerFromDataHelper.php b/src/crafting/CraftingManagerFromDataHelper.php index bcc829790..d4bb14b3d 100644 --- a/src/crafting/CraftingManagerFromDataHelper.php +++ b/src/crafting/CraftingManagerFromDataHelper.php @@ -24,7 +24,7 @@ declare(strict_types=1); namespace pocketmine\crafting; use pocketmine\data\bedrock\item\ItemTypeDeserializeException; -use pocketmine\item\Durable; +use pocketmine\data\SavedDataLoadingException; use pocketmine\item\Item; use pocketmine\item\ItemFactory; use pocketmine\utils\AssumptionFailedError; @@ -37,26 +37,6 @@ use function is_int; use function json_decode; final class CraftingManagerFromDataHelper{ - - /** - * @param Item[] $items - */ - private static function containsUnknownItems(array $items) : bool{ - $factory = ItemFactory::getInstance(); - foreach($items as $item){ - if($item instanceof Durable){ - //TODO: this check is imperfect and might cause problems if meta 0 isn't used for some reason - if(!$factory->isRegistered($item->getId())){ - return true; - } - }elseif(!$factory->isRegistered($item->getId(), $item->getMeta())){ - return true; - } - } - - return false; - } - /** * @param mixed[] $data */ @@ -76,9 +56,14 @@ final class CraftingManagerFromDataHelper{ } //TODO: we need to stop using jsonDeserialize for this - $item = Item::jsonDeserialize($data); + try{ + $item = Item::jsonDeserialize($data); + }catch(SavedDataLoadingException){ + //unknown item + return null; + } - return self::containsUnknownItems([$item]) ? null : new ExactRecipeIngredient($item); + return new ExactRecipeIngredient($item); } public static function make(string $filePath) : CraftingManager{ @@ -109,8 +94,10 @@ final class CraftingManagerFromDataHelper{ } $inputs[] = $input; } - $outputs = array_map($itemDeserializerFunc, $recipe["output"]); - if(self::containsUnknownItems($outputs)){ + try{ + $outputs = array_map($itemDeserializerFunc, $recipe["output"]); + }catch(SavedDataLoadingException){ + //unknown output item continue; } $result->registerShapelessRecipe(new ShapelessRecipe( @@ -131,8 +118,10 @@ final class CraftingManagerFromDataHelper{ } $inputs[$symbol] = $input; } - $outputs = array_map($itemDeserializerFunc, $recipe["output"]); - if(self::containsUnknownItems($outputs)){ + try{ + $outputs = array_map($itemDeserializerFunc, $recipe["output"]); + }catch(SavedDataLoadingException){ + //unknown output item continue; } $result->registerShapedRecipe(new ShapedRecipe( @@ -152,9 +141,13 @@ final class CraftingManagerFromDataHelper{ if($furnaceType === null){ continue; } - $output = Item::jsonDeserialize($recipe["output"]); + try{ + $output = Item::jsonDeserialize($recipe["output"]); + }catch(SavedDataLoadingException){ + continue; + } $input = self::deserializeIngredient($recipe["input"]); - if($input === null || self::containsUnknownItems([$output])){ + if($input === null){ continue; } $result->getFurnaceRecipeManager($furnaceType)->register(new FurnaceRecipe( @@ -163,11 +156,12 @@ final class CraftingManagerFromDataHelper{ )); } foreach($recipes["potion_type"] as $recipe){ - $input = Item::jsonDeserialize($recipe["input"]); - $ingredient = Item::jsonDeserialize($recipe["ingredient"]); - $output = Item::jsonDeserialize($recipe["output"]); - - if(self::containsUnknownItems([$input, $ingredient, $output])){ + try{ + $input = Item::jsonDeserialize($recipe["input"]); + $ingredient = Item::jsonDeserialize($recipe["ingredient"]); + $output = Item::jsonDeserialize($recipe["output"]); + }catch(SavedDataLoadingException){ + //unknown item continue; } $result->registerPotionTypeRecipe(new PotionTypeRecipe( @@ -177,11 +171,12 @@ final class CraftingManagerFromDataHelper{ )); } foreach($recipes["potion_container_change"] as $recipe){ - $input = ItemFactory::getInstance()->get($recipe["input_item_id"]); - $ingredient = Item::jsonDeserialize($recipe["ingredient"]); - $output = ItemFactory::getInstance()->get($recipe["output_item_id"]); - - if(self::containsUnknownItems([$input, $ingredient, $output])){ + try{ + $input = ItemFactory::getInstance()->get($recipe["input_item_id"]); + $ingredient = Item::jsonDeserialize($recipe["ingredient"]); + $output = ItemFactory::getInstance()->get($recipe["output_item_id"]); + }catch(SavedDataLoadingException){ + //unknown item continue; } $result->registerPotionContainerChangeRecipe(new PotionContainerChangeRecipe( diff --git a/src/data/bedrock/item/upgrade/ItemDataUpgrader.php b/src/data/bedrock/item/upgrade/ItemDataUpgrader.php index 6e73c02b9..e01aeb791 100644 --- a/src/data/bedrock/item/upgrade/ItemDataUpgrader.php +++ b/src/data/bedrock/item/upgrade/ItemDataUpgrader.php @@ -102,6 +102,8 @@ final class ItemDataUpgrader{ /** * This function replaces the legacy ItemFactory::get(). + * + * @throws SavedDataLoadingException if the legacy numeric ID doesn't map to a string ID */ public function upgradeItemTypeDataInt(int $legacyNumericId, int $meta, int $count, ?CompoundTag $nbt) : SavedItemStackData{ $rawNameId = $this->legacyIntToStringIdMap->legacyToString($legacyNumericId); diff --git a/src/inventory/CreativeInventory.php b/src/inventory/CreativeInventory.php index 40b3075d2..f3943907c 100644 --- a/src/inventory/CreativeInventory.php +++ b/src/inventory/CreativeInventory.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace pocketmine\inventory; +use pocketmine\data\SavedDataLoadingException; use pocketmine\item\Durable; use pocketmine\item\Item; use pocketmine\utils\SingletonTrait; @@ -40,8 +41,10 @@ final class CreativeInventory{ $creativeItems = json_decode(file_get_contents(Path::join(\pocketmine\BEDROCK_DATA_PATH, "creativeitems.json")), true); foreach($creativeItems as $data){ - $item = Item::jsonDeserialize($data); - if($item->getName() === "Unknown"){ + try{ + $item = Item::jsonDeserialize($data); + }catch(SavedDataLoadingException){ + //unknown item continue; } $this->add($item); diff --git a/src/item/Item.php b/src/item/Item.php index f618a687f..60605d7a8 100644 --- a/src/item/Item.php +++ b/src/item/Item.php @@ -38,7 +38,6 @@ use pocketmine\item\enchantment\EnchantmentInstance; use pocketmine\math\Vector3; use pocketmine\nbt\LittleEndianNbtSerializer; use pocketmine\nbt\NBT; -use pocketmine\nbt\NbtDataException; use pocketmine\nbt\NbtException; use pocketmine\nbt\tag\CompoundTag; use pocketmine\nbt\tag\ListTag; @@ -606,8 +605,7 @@ class Item implements \JsonSerializable{ * Returns an Item from properties created in an array by {@link Item#jsonSerialize} * @param mixed[] $data * - * @throws NbtDataException - * @throws \InvalidArgumentException + * @throws SavedDataLoadingException */ final public static function jsonDeserialize(array $data) : Item{ $nbt = ""; diff --git a/src/item/ItemFactory.php b/src/item/ItemFactory.php index 432913335..c9c45003b 100644 --- a/src/item/ItemFactory.php +++ b/src/item/ItemFactory.php @@ -36,6 +36,7 @@ use pocketmine\data\bedrock\CompoundTypeIds; use pocketmine\data\bedrock\DyeColorIdMap; use pocketmine\data\bedrock\EntityLegacyIds; use pocketmine\data\bedrock\PotionTypeIdMap; +use pocketmine\data\SavedDataLoadingException; use pocketmine\entity\Entity; use pocketmine\entity\Location; use pocketmine\entity\Squid; @@ -50,6 +51,7 @@ use pocketmine\nbt\tag\CompoundTag; use pocketmine\utils\SingletonTrait; use pocketmine\world\format\io\GlobalBlockStateHandlers; use pocketmine\world\World; +use function min; /** * Manages deserializing item types from their legacy ID/metadata. @@ -451,26 +453,24 @@ class ItemFactory{ * * Deserializes an item from the provided legacy ID, legacy meta, count and NBT. * - * @throws \InvalidArgumentException - * @throws NbtException + * @throws SavedDataLoadingException */ public function get(int $id, int $meta = 0, int $count = 1, ?CompoundTag $tags = null) : Item{ /** @var Item|null $item */ $item = null; + if($id < -0x8000 || $id > 0x7fff){ + throw new SavedDataLoadingException("Legacy ID must be in the range " . -0x8000 . " ... " . 0x7fff); + } if($meta < 0 || $meta > 0x7ffe){ //0x7fff would cause problems with recipe wildcards - throw new \InvalidArgumentException("Meta cannot be negative or larger than " . 0x7ffe); + throw new SavedDataLoadingException("Meta cannot be negative or larger than " . 0x7ffe); } if(isset($this->list[$offset = self::getListOffset($id, $meta)])){ $item = clone $this->list[$offset]; }elseif(isset($this->list[$zero = self::getListOffset($id, 0)]) && $this->list[$zero] instanceof Durable){ - if($meta <= $this->list[$zero]->getMaxDurability()){ - $item = clone $this->list[$zero]; - $item->setDamage($meta); - }else{ - $item = new Item(new IID($id, $meta)); - } + $item = clone $this->list[$zero]; + $item->setDamage(min($meta, $this->list[$zero]->getMaxDurability())); }elseif($id < 256){ //intentionally includes negatives, for extended block IDs //TODO: do not assume that item IDs and block IDs are the same or related $blockStateData = GlobalBlockStateHandlers::getUpgrader()->upgradeIntIdMeta(self::itemToBlockId($id), $meta & 0xf); @@ -479,20 +479,22 @@ class ItemFactory{ $blockStateId = GlobalBlockStateHandlers::getDeserializer()->deserialize($blockStateData); $item = new ItemBlock(new IID($id, $meta), BlockFactory::getInstance()->fromFullBlock($blockStateId)); }catch(BlockStateDeserializeException $e){ - \GlobalLogger::get()->logException($e); - //fallthru + throw new SavedDataLoadingException("Failed to deserialize itemblock: " . $e->getMessage(), 0, $e); } } } if($item === null){ - //negative damage values will fallthru to here, to avoid crazy shit with crafting wildcard hacks - $item = new Item(new IID($id, $meta)); + throw new SavedDataLoadingException("No registered item is associated with this ID and meta"); } $item->setCount($count); if($tags !== null){ - $item->setNamedTag($tags); + try{ + $item->setNamedTag($tags); + }catch(NbtException $e){ + throw new SavedDataLoadingException("Invalid item NBT: " . $e->getMessage(), 0, $e); + } } return $item; } diff --git a/src/item/LegacyStringToItemParser.php b/src/item/LegacyStringToItemParser.php index 6fe1f6063..5450e0bb0 100644 --- a/src/item/LegacyStringToItemParser.php +++ b/src/item/LegacyStringToItemParser.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace pocketmine\item; +use pocketmine\data\SavedDataLoadingException; use pocketmine\utils\AssumptionFailedError; use pocketmine\utils\SingletonTrait; use pocketmine\utils\Utils; @@ -104,15 +105,16 @@ final class LegacyStringToItemParser{ $meta = 0; }elseif(is_numeric($b[1])){ $meta = (int) $b[1]; - if($meta < 0 || $meta > 0x7ffe){ - throw new LegacyStringToItemParserException("Meta value $meta is outside the range 0 - " . 0x7ffe); - } }else{ throw new LegacyStringToItemParserException("Unable to parse \"" . $b[1] . "\" from \"" . $input . "\" as a valid meta value"); } if(isset($this->map[strtolower($b[0])])){ - $item = $this->itemFactory->get($this->map[strtolower($b[0])], $meta); + try{ + $item = $this->itemFactory->get($this->map[strtolower($b[0])], $meta); + }catch(SavedDataLoadingException $e){ + throw new LegacyStringToItemParserException($e->getMessage(), 0, $e); + } }else{ throw new LegacyStringToItemParserException("Unable to resolve \"" . $input . "\" to a valid item"); } diff --git a/src/network/mcpe/convert/TypeConverter.php b/src/network/mcpe/convert/TypeConverter.php index 8300fa794..ec699e62a 100644 --- a/src/network/mcpe/convert/TypeConverter.php +++ b/src/network/mcpe/convert/TypeConverter.php @@ -32,6 +32,7 @@ use pocketmine\block\VanillaBlocks; use pocketmine\crafting\ExactRecipeIngredient; use pocketmine\crafting\MetaWildcardRecipeIngredient; use pocketmine\crafting\RecipeIngredient; +use pocketmine\data\SavedDataLoadingException; use pocketmine\inventory\transaction\action\CreateItemAction; use pocketmine\inventory\transaction\action\DestroyItemAction; use pocketmine\inventory\transaction\action\DropItemAction; @@ -239,13 +240,11 @@ class TypeConverter{ $compound = null; } if($meta !== null){ - if($id !== null && ($id < -0x8000 || $id >= 0x7fff)){ - throw new TypeConversionException("Item ID must be in range " . -0x8000 . " ... " . 0x7fff . " (received $id)"); + try{ + $itemResult = ItemFactory::getInstance()->get($id ?? $itemResult->getId(), $meta); + }catch(SavedDataLoadingException $e){ + throw new TypeConversionException("Failed loading network item: " . $e->getMessage(), 0, $e); } - if($meta < 0 || $meta >= 0x7ffe){ //this meta value may have been restored from the NBT - throw new TypeConversionException("Item meta must be in range 0 ... " . 0x7ffe . " (received $meta)"); - } - $itemResult = ItemFactory::getInstance()->get($id ?? $itemResult->getId(), $meta); } }