connector tool: do not use :metadata() to avoid serialization bug in MT

The way MT serializes this data is fundamentally unsafe now since 5.1 or
so, and will randomly break due to random box positions. One day it
works fine and the next day the server crashes on the same apparent
link operation. All because the itemstack data can likely not handle
\000 data during either serialization or deserialization. The outcome
is 5, or even 4 bytes returned from deserialization, and thus a server
crash.

This is BW compatible. Old connector tools will be functional and appear
as linked in the tooltip, but there will be an error to the user telling
them to "start" the link again first. No box is broken - this is only
affecting itemstacks.

mods/mech/init.lua

@@ -278,15 +278,32 @@ local function mech_connect(itemstack, placer, pointed_thing, rightclick)
 		return
 	end
 
-	local tmeta = itemstack:get_metadata()
+	local meta = itemstack:get_meta()
+	-- the old code relied on itemstack:metadata. However, we were putting in binary data
+	-- blobs of 6 bytes in there and somewhere after 5.1 or so the serialization of this
+	-- data broke, and it no longer properly handles 0 (\000) data in the array. This would
+	-- result in the data being less than 6 bytes after deserialization, and crashing the server.
+	-- This is uniquely an itemstack problem. The serialization of box data and nodes are not
+	-- affected by this bug. Once connections are made, they are stable.
+	-- The actual real position of the boxes mattered here. It turns out that if you save a box
+	-- where making connections work one day, then if you reload the box in edit mode it could
+	-- possibly crash.
+	-- The only solution is to no longer rely on the itemstack:metadata() method, as it is fundamentally
+	-- unsafe for binary data.
+	--
+	-- With get/set_meta, and minetest.(de)serialize() instead, we have a safe, backwards compatible
+	-- solution. If players happen to have an old itemstack in their edit inventory, it will just
+	-- refuse to make connections with existing link data in it. Even if they have old link data
+	-- in the itemstack, it will use the new link data only.
 	if rightclick then
-		if tmeta == "" then
+		local link = meta:get_string("link")
+		if link == "" then
 			minetest.chat_send_player(placer:get_player_name(),
 				"Left-click a node first.")
 			return itemstack
 		end
 
-		local pos1 = dehash_vector(tmeta)
+		local pos1 = minetest.deserialize(link)
 		local pos2 = pointed_thing.under
 
 		if placer:get_player_control().sneak then
@@ -320,12 +337,10 @@ local function mech_connect(itemstack, placer, pointed_thing, rightclick)
 			end
 		end
 	else -- left click
-		itemstack:set_metadata(hash_vector(pointed_thing.under))
 		minetest.chat_send_player(placer:get_player_name(), S("Connection started with \"@1\" at ",
 				minetest.get_node(pointed_thing.under).name) ..
 				minetest.pos_to_string(pointed_thing.under) .. ".")
 			minetest.sound_play("button_trigger", {pos = pointed_thing.under})
-		local meta = itemstack:get_meta()
 		meta:set_string("description", S("Connector tool").."\n" ..
 			S("Right-click creates a connection from \"@1\" at ",
 				minetest.get_node(pointed_thing.under).name) ..
@@ -333,6 +348,7 @@ local function mech_connect(itemstack, placer, pointed_thing, rightclick)
 			S("Right click + Shift to remove the connection from \"@1\" at ",
 			minetest.get_node(pointed_thing.under).name) ..
 			minetest.pos_to_string(pointed_thing.under) .. ".")
+		meta:set_string("link", minetest.serialize(pointed_thing.under))
 	end
 	return itemstack
 end