From 62d5b547a0ffa3361ab4460d09f22403c3facf87 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Wed, 28 Jul 2021 20:44:48 +0200 Subject: [PATCH 1/3] Fix non-serializable item entity unload crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some items, like shulkers or books, can have so much metadata that the corresponding item entity can not be serialized by the Minetest engine. Without this patch, dropping such an item and then moving away crashes Minetest, as it can not serialize the entity with serializeString16() when unloading a map block. The patch resets the overlong metadata of non-serializable item entities. This avoids a crash and makes it possible to retrieve a “sanitized” item without metadata when the mapblock containing the item entity is reloaded. Originally sfan5 guessed the maximum possible item entity serialization size that would not lead to a crash as 65530 bytes, but anon5 calculated it to be actually 65487 bytes. This has been experimentally verified by erlehmann. --- mods/ENTITIES/mcl_item_entity/init.lua | 28 +++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/mods/ENTITIES/mcl_item_entity/init.lua b/mods/ENTITIES/mcl_item_entity/init.lua index 0ce2a537..6ed196d4 100644 --- a/mods/ENTITIES/mcl_item_entity/init.lua +++ b/mods/ENTITIES/mcl_item_entity/init.lua @@ -448,7 +448,7 @@ minetest.register_entity(":__builtin:item", { end, get_staticdata = function(self) - return minetest.serialize({ + local data = minetest.serialize({ itemstring = self.itemstring, always_collect = self.always_collect, age = self.age, @@ -456,6 +456,32 @@ minetest.register_entity(":__builtin:item", { _flowing = self._flowing, _removed = self._removed, }) + -- sfan5 guessed that the biggest serializable item + -- entity would have a size of 65530 bytes. This has + -- been experimentally verified to be still too large. + -- + -- anon5 has calculated that the biggest serializable + -- item entity has a size of exactly 65487 bytes: + -- + -- 1. serializeString16 can handle max. 65535 bytes. + -- 2. The following engine metadata is always saved: + -- • 1 byte (version) + -- • 2 byte (length prefix) + -- • 14 byte “__builtin:item” + -- • 4 byte (length prefix) + -- • 2 byte (health) + -- • 3 × 4 byte = 12 byte (position) + -- • 4 byte (yaw) + -- • 1 byte (version 2) + -- • 2 × 4 byte = 8 byte (pitch and roll) + -- 3. This leaves 65487 bytes for the serialization. + if #data > 65487 then -- would crash the engine + local stack = ItemStack(self.itemstring) + stack:get_meta():from_table(nil) + self.itemstring = stack:to_string() + return self:get_staticdata() + end + return data end, on_activate = function(self, staticdata, dtime_s) From a0c9f11af6804ef01f2d9f505ec326afb7a684cf Mon Sep 17 00:00:00 2001 From: Nils Dagsson Moskopp Date: Thu, 29 Jul 2021 15:46:50 +0200 Subject: [PATCH 2/3] Log warning for non-serializable item entity fix --- mods/ENTITIES/mcl_item_entity/init.lua | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/mods/ENTITIES/mcl_item_entity/init.lua b/mods/ENTITIES/mcl_item_entity/init.lua index 6ed196d4..ba90848d 100644 --- a/mods/ENTITIES/mcl_item_entity/init.lua +++ b/mods/ENTITIES/mcl_item_entity/init.lua @@ -479,6 +479,13 @@ minetest.register_entity(":__builtin:item", { local stack = ItemStack(self.itemstring) stack:get_meta():from_table(nil) self.itemstring = stack:to_string() + minetest.log( + "warning", + "Overlong item entity metadata removed: “" .. + self.itemstring .. + "” had serialized length of " .. + #data + ) return self:get_staticdata() end return data From ce6d6c26cc00639876bdccf6421f5a8f1e00fa94 Mon Sep 17 00:00:00 2001 From: Nils Dagsson Moskopp Date: Fri, 30 Jul 2021 01:11:58 +0200 Subject: [PATCH 3/3] Add debug command to acquire a written book MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The “getwrittenbook” command gives a player that has the “debug” privilege a book with a configurable amount of characters. This was added as a debug aid, to help reproducing situations in which items with lots of metadata trigger issues like heavy lag or server crashes. --- mods/ENTITIES/mcl_item_entity/init.lua | 27 ++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/mods/ENTITIES/mcl_item_entity/init.lua b/mods/ENTITIES/mcl_item_entity/init.lua index ba90848d..75ed19ef 100644 --- a/mods/ENTITIES/mcl_item_entity/init.lua +++ b/mods/ENTITIES/mcl_item_entity/init.lua @@ -1,3 +1,4 @@ +local S = minetest.get_translator("mcl_item_entity") --basic settings local item_drop_settings = {} --settings table item_drop_settings.age = 1.0 --how old a dropped item (_insta_collect==false) has to be before collecting @@ -781,3 +782,29 @@ minetest.register_entity(":__builtin:item", { -- Note: on_punch intentionally left out. The player should *not* be able to collect items by punching }) + +-- The “getwrittenbook” command was added as a debug aid. It can help +-- reproducing situations in which items with lots of metadata trigger +-- issues like heavy lag or server crashes. Do not remove this command +-- unless another easy way of getting items with overlong meta exists! +-- +-- “/getwrittenbook 65323” creates an item that creates the largest +-- possible serializable written book item entity when dropped. +-- +-- “/getwrittenbook 65324” creates an item that creates the smallest +-- possible non-serializable written book item entity when dropped. +minetest.register_chatcommand("getwrittenbook", { + params = S(""), + description = S("Get a written book with a configurable amount of characters."), + privs = {debug=true}, + func = function(name, param) + local count = tonumber(param) + local itemstack = ItemStack("mcl_books:written_book") + local meta = itemstack:get_meta() + meta:set_string("description", "") + meta:set_string("text", string.rep("x", count)) + local player = minetest.get_player_by_name(name) + local inv = player:get_inventory() + inv:add_item("main", itemstack) + end +})