Merge branch 'feature/pcall-protection' into dev

promise_tech.lua

@@ -53,12 +53,14 @@ local f = function(val) end
 -- Table tweaks (because this is for Minetest)
 --- @class	table
 local table = table
-if not table.unpack then table.unpack = unpack end
+-- @diagnostic disable-next-line
-table.join = function(tbl, sep)
+if not table.unpack then table.unpack = unpack end --luacheck: ignore
-	local function fn_iter(tbl,sep,i)
+-- @diagnostic disable-next-line
-		if i < #tbl then
+table.join = function(tbl, sep) --luacheck: ignore
-			return (tostring(tbl[i]) or "").. sep .. fn_iter(tbl,sep,i+1)
+	local function fn_iter(tbl_inner,sep_inner,i)
-		else return (tostring(tbl[i]) or "") end
+		if i < #tbl_inner then
+			return (tostring(tbl_inner[i]) or "").. sep_inner .. fn_iter(tbl_inner,sep_inner,i+1)
+		else return (tostring(tbl_inner[i]) or "") end
 	end
 	return fn_iter(tbl,sep,1)
 end
@@ -93,7 +95,7 @@ local function_type_warn = function(called_from, position, arg_name, must_be, se
 end
 
 local type_enforce = function(called_from, args)
-	local err_str = nil
+	local err_str
 	for i, arg in ipairs(args) do
 		local is_err = true
 		for _, should_be in ipairs(arg.should_be) do
@@ -105,7 +107,7 @@ local type_enforce = function(called_from, args)
 			end
 		end
 		if is_err then
-			err_str = function_type_warn(called_from, i, arg.name, table.join(arg.should_be, " or "), arg.name == "self" and true or false)
+			err_str = function_type_warn(called_from, i, arg.name, table.join(arg.should_be, " or "), arg.name == "self" and true or false) --luacheck: ignore
 			if arg.error then error(err_str)
 			else warn(err_str) end
 		end

worldeditadditions_commands/commands/maze.lua

@@ -71,13 +71,14 @@ wea_c.register_command("maze", {
 		return success, replace_node, seed, path_length, path_width
 	end,
 	nodes_needed = function(name)
-		-- Note that we could take in additional parameters from the return value of parse (minue the success bool there), but we don't actually need them here
+		-- Note that we could take in additional parameters from the return value of parse (minus the success bool there), but we don't actually need them here
-		return worldedit.volume(worldedit.pos1[name], worldedit.pos2[name])
+		local pos1, pos2 = wea_c.pos.get12(name)
+		return worldedit.volume(pos1, pos2)
 	end,
 	func = function(name, replace_node, seed, path_length, path_width)
 		local start_time = wea_c.get_ms_time()
 		
-		local pos1, pos2 = Vector3.sort(worldedit.pos1[name], worldedit.pos2[name])
+		local pos1, pos2 = wea_c.pos.get12(name)
 		local replaced = wea.maze2d(
 			pos1, pos2,
 			replace_node,
@@ -114,7 +115,7 @@ wea_c.register_command("maze3d", {
 	end,
 	func = function(name, replace_node, seed, path_length, path_width, path_depth)
 		local start_time = wea_c.get_ms_time()
-		local pos1, pos2 = Vector3.sort(worldedit.pos1[name], worldedit.pos2[name])
+		local pos1, pos2 = Vector3.sort(wea_c.pos.get12(name))
 		local replaced = wea.maze3d(
 			pos1, pos2,
 			replace_node,

worldeditadditions_core/core/run_command.lua

@@ -2,13 +2,14 @@
 -- @module worldeditadditions_core
 
 -- WARNING: safe_region MUST NOT be imported more than once, as it defines chat commands. If you want to import it again elsewhere, check first that multiple dofile() calls don't execute a file more than once.
-local wea_c = worldeditadditions_core
+local weac = worldeditadditions_core
-local safe_region = dofile(wea_c.modpath.."/core/safe_region.lua")
+local safe_region = dofile(weac.modpath.."/core/safe_region.lua")
-local human_size = wea_c.format.human_size
+local human_size = weac.format.human_size
+local safe_function = weac.safe_function
 
 -- TODO: Reimplement worldedit.player_notify(player_name, msg_text)
 
---- Actually runs the command in question.
+--- Actually runs the command in question. [HIDDEN]
 -- Unfortunately needed to keep the codebase clena because Lua sucks.
 -- @internal
 -- @param	player_name	string		The name of the player executing the function. 
@@ -17,11 +18,17 @@ local human_size = wea_c.format.human_size
 -- @param	tbl_event	table		Internal event table used when calling `worldeditadditions_core.emit(event_name, tbl_event)`.
 -- @returns	nil
 local function run_command_stage2(player_name, func, parse_result, tbl_event)
-	wea_c:emit("pre-execute", tbl_event)
+	weac:emit("pre-execute", tbl_event)
-	local success, result_message = func(player_name, wea_c.table.unpack(parse_result))
+	local success_safefn, retvals = safe_function(func, { player_name, weac.table.unpack(parse_result) }, player_name, "The function crashed during execution.", tbl_event.cmdname)
+	if not success_safefn then return false end
+	
+	if #retvals ~= 2 then
+		worldedit.player_notify(player_name, "[//"..tostring(tbl_event.cmdname).."] The main execution function for this chat command returned "..tostring(#retvals).." arguments instead of the expected 2 (success, message), so it is unclear whether it succeeded or not. This is a bug!")
+	end
+	
+	local success, result_message = retvals[1], retvals[2]
 	print("DEBUG:run_command_stage2 SUCCESS", success, "RESULT_MESSAGE", result_message)
 	if not success then
-		
 		result_message = "[//"..tostring(tbl_event.cmdname).."] "..result_message
 	end
 	
@@ -31,9 +38,11 @@ local function run_command_stage2(player_name, func, parse_result, tbl_event)
 	end
 	tbl_event.success = success
 	tbl_event.result = result_message
-	wea_c:emit("post-execute", tbl_event)
+	weac:emit("post-execute", tbl_event)
 end
 
+
+
 --- Command execution pipeline: before `paramtext` parsing but after validation.
 -- 
 -- See `worldeditadditions_core.run_command`
@@ -89,15 +98,15 @@ end
 -- @param	player_name	string	The name of the player to execute the command for.
 -- @param	paramtext	string	The unparsed argument string to pass to the command when executing it.
 local function run_command(cmdname, options, player_name, paramtext)
-	if options.require_pos > 0 and not worldedit.pos1[player_name] and not wea_c.pos.get1(player_name) then
+	if options.require_pos > 0 and not worldedit.pos1[player_name] and not weac.pos.get1(player_name) then
 		worldedit.player_notify(player_name, "Error: pos1 must be selected to use this command.")
 		return false
 	end
-	if options.require_pos > 1 and not worldedit.pos2[player_name] and not wea_c.pos.get2(player_name) then
+	if options.require_pos > 1 and not worldedit.pos2[player_name] and not weac.pos.get2(player_name) then
 		worldedit.player_notify(player_name, "Error: Both pos1 and pos2 must be selected (together making a region) to use this command.")
 		return false
 	end
-	local pos_count = wea_c.pos.count(player_name)
+	local pos_count = weac.pos.count(player_name)
 	if options.require_pos > 2 and pos_count < options.require_pos then
 		worldedit.player_notify(player_name, "Error: At least "..options.require_pos.."positions must be defined to use this command, but you only have "..pos_count.." defined (try using the multiwand).")
 		return false
@@ -110,33 +119,52 @@ local function run_command(cmdname, options, player_name, paramtext)
 		player_name = player_name
 	}
 	
-	wea_c:emit("pre-parse", tbl_event)
+	weac:emit("pre-parse", tbl_event)
 	
-	local parse_result = { options.parse(paramtext) }
+	-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-	local success = table.remove(parse_result, 1)
+	
-	if not success then
+	-- local did_error = false
-		worldedit.player_notify(player_name, ("[//"..tostring(cmdname).."] "..tostring(parse_result[1])) or "Invalid usage (no further error message was provided by the command. This is probably a bug.)")
+	local success_safefn, parse_result = safe_function(options.parse, { paramtext }, player_name, "The command crashed when parsing the arguments.", cmdname)
+	if not success_safefn then return false end -- error already sent to the player above
+	
+	if #parse_result == 0 then 
+		worldedit.player_notify(player_name, "[//"..tostring(cmdname).."] No return values at all were returned by the parsing function - not even a success boolean. This is a bug - please report it :D")
 		return false
 	end
 	
+	local success = table.remove(parse_result, 1)
+	if not success then
+		worldedit.player_notify(player_name, "[//"..tostring(cmdname).."] "..(tostring(parse_result[1]) or "Invalid usage (no further error message was provided by the command. This is probably a bug.)"))
+		return false
+	end
+	
+	-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~	
+	
 	tbl_event.paramargs = parse_result
-	wea_c:emit("post-parse", tbl_event)
+	weac:emit("post-parse", tbl_event)
 	
 	
 	if options.nodes_needed then
-		local potential_changes = options.nodes_needed(player_name, wea_c.table.unpack(parse_result))
+		local success_xpcall_nn, retvals_nn = safe_function(options.nodes_needed, { player_name, weac.table.unpack(parse_result) }, player_name, "The nodes_needed function crashed!", cmdname)
+		if not success_xpcall_nn then return false end
+		
+		if #retvals_nn == 0 then
+			worldedit.player_notify(player_name, "[//"..tostring(cmdname).."] Error: The nodes_needed function didn't return any values. This is a bug!")
+			return false
+		end
+		local potential_changes = retvals_nn[1]
 		
 		tbl_event.potential_changes = potential_changes
-		wea_c:emit("post-nodesneeded", tbl_event)
+		weac:emit("post-nodesneeded", tbl_event)
 		
 		if type(potential_changes) ~= "number" then
 			worldedit.player_notify(player_name, "Error: The command '"..cmdname.."' returned a "..type(potential_changes).." instead of a number when asked how many nodes might be changed. Abort. This is a bug.")
 			return
 		end
 		
-		local limit = wea_c.safe_region_limit_default
+		local limit = weac.safe_region_limit_default
-		if wea_c.safe_region_limits[player_name] then
+		if weac.safe_region_limits[player_name] then
-			limit = wea_c.safe_region_limits[player_name]
+			limit = weac.safe_region_limits[player_name]
 		end
 		if type(potential_changes) == "string" then
 			worldedit.player_notify(player_name, "/"..cmdname.." "..paramtext.." "..potential_changes..". Type //y to continue, or //n to cancel (in this specific situation, your configured limit via the //saferegion command does not apply).")

worldeditadditions_core/core/safe_function.lua

@@ -0,0 +1,98 @@
+local weac = worldeditadditions_core
+---
+-- @module worldeditadditions_core
+
+-- ███████  █████  ███████ ███████         ███████ ███    ██ 
+-- ██      ██   ██ ██      ██              ██      ████   ██ 
+-- ███████ ███████ █████   █████           █████   ██ ██  ██ 
+--      ██ ██   ██ ██      ██              ██      ██  ██ ██ 
+-- ███████ ██   ██ ██      ███████ ███████ ██      ██   ████ 
+
+
+local function send_error(player_name, cmdname, msg, stack_trace)
+	print("DEBUG:HAI SEND_ERROR")
+	local msg_compiled = table.concat({
+		"[//", cmdname, "] Error: ",
+		msg,
+		"\n",
+		"Please report this by opening an issue on GitHub! Bug report link (ctrl + click):\n",
+		"https://github.com/sbrl/Minetest-WorldEditAdditions/issues/new?title=",
+		weac.format.escape(stack_trace:match("^[^\n]+")), -- extract 1st line & escape
+		"&body=",
+		weac.format.escape(table.concat({
+			[[## Describe the bug
+What's the bug? Be clear and detailed but concise in our explanation. Don't forget to include any context, error messages, logs, and screenshots required to understand the issue if applicable.
+
+## Reproduction steps
+Steps to reproduce the behaviour:
+1. Go to '...'
+2. Click on '....'
+3. Enter this command to '....'
+4. See error
+
+## System information (please complete the following information)
+- **Operating system and version:** [e.g. iOS]
+- **Minetest version:** [e.g. 5.8.0]
+- **WorldEdit version:**
+- **WorldEditAdditions version:**
+
+Please add any other additional specific system information here too if you think it would help.
+
+## Stack trace
+- **Command name:** ]],
+			cmdname,
+			"\n",
+			"```\n",
+			stack_trace,
+			"\n",
+			"```\n",
+		}, "")),
+
+		"\n",
+		"-------------------------------------\n",
+		"*** Stack trace ***\n",
+		stack_trace,
+		"\n",
+		"-------------------------------------\n"
+	}, "")
+
+	print("DEBUG:player_notify player_name", player_name, "msg_compiled", msg_compiled)
+	worldedit.player_notify(player_name, msg_compiled)
+end
+
+
+--- Calls the given function `fn` with the UNPACKED arguments from `args`, catching errors and sending the calling player a nice error message with a report link.
+-- 
+-- WARNING: Do NOT nest `safe_function()` calls!!!
+-- @param	fn			function		The function to call
+-- @param	args		table			The table of args to unpack and send to `fn` as arguments
+-- @param	string|nil	player_name		The name of the player affected. If nil then no message is sent to the player.
+-- @param	string		error_msg		The error message to send when `fn` inevitably crashes.
+-- @param	string|nil		cmdname		Optional. The name of the command being run.
+-- @returns	bool,any,...	A success bool (true == success), and then if success == true the rest of the arguments are the (unpacked) return values from the function called. If success == false, then the 2nd argument will be the stack trace.
+local function safe_function(fn, args, player_name, error_msg, cmdname)
+	local retvals
+	local success_xpcall, stack_trace = xpcall(function()
+		retvals = { fn(weac.table.unpack(args)) }
+	end, debug.traceback)
+	
+	if not success_xpcall then
+		send_error(player_name, cmdname, error_msg, stack_trace)
+		weac:emit("error", {
+			fn = fn,
+			args = args,
+			player_name = player_name,
+			cmdname = cmdname,
+			stack_trace = stack_trace,
+			error_msg = error_msg
+		})
+		minetest.log("error", "[//"..tostring(cmdname).."] Caught error from running function ", fn, "with args", weac.inspect(args), "for player ", player_name, "with provided error message", error_msg, ". Stack trace: ", stack_trace)
+		return false, stack_trace
+	end
+	
+	
+	return true, retvals
+end
+
+
+return safe_function

worldeditadditions_core/init.lua

@@ -11,14 +11,33 @@ local modpath = minetest.get_modpath("worldeditadditions_core")
 
 local EventEmitter = dofile(modpath .. "/utils/EventEmitter.lua")
 
+local directory_separator = "/"
+if package and package.config then
+	directory_separator = package.config:sub(1,1)
+end
+
 worldeditadditions_core = EventEmitter.new({
 	version = "1.15-dev",
+	--- The directory separator on the current host system
+	-- @value string
+	dirsep = directory_separator,
+	--- The full absolute filepath to the mod worldeditadditions_core
+	-- @value 
 	modpath = modpath,
+	--- The full absolute filepath to the data directory WorldEditAdditions can store miscellaneous data in.
+	-- @value
+	datapath = minetest.get_worldpath() .. directory_separator .."worldeditadditions",
+	--- A table containing the definitions for all commands registered in WorldEditAdditions.
+	-- Keys are the command name SANS any forward slashes! So //replacemix would be registered as simply replacemix.
+	-- @value table<string, table>
 	registered_commands = {},
-	-- Storage for per-player node limits before safe_region kicks in.
+	--- Storage for per-player node limits before safe_region kicks in.
 	-- TODO: Persist these to disk.
+	-- @value table<string, number>
 	safe_region_limits = {},
-	-- The default limit for new players on the number of potential nodes changed before safe_region kicks in.
+	--- The default limit for new players on the number of potential nodes changed before safe_region kicks in.
+	-- TODO make this configurable
+	-- @value number
 	safe_region_limit_default = 100000,
 })
 local wea_c = worldeditadditions_core
@@ -68,6 +87,7 @@ dofile(wea_c.modpath.."/utils/player.lua") -- Player info functions
 wea_c.setting_handler = dofile(wea_c.modpath.."/utils/setting_handler.lua") -- AFTER parser
 
 wea_c.pos = dofile(modpath.."/core/pos.lua") -- AFTER EventEmitter
+wea_c.safe_function = dofile(modpath.."/core/safe_function.lua")
 wea_c.register_command = dofile(modpath.."/core/register_command.lua")
 wea_c.command_exists = dofile(modpath.."/core/command_exists.lua")
 wea_c.fetch_command_def = dofile(modpath.."/core/fetch_command_def.lua")

worldeditadditions_core/utils/format/escape.lua

@@ -0,0 +1,25 @@
+--- 
+-- @module worldeditadditions_core
+
+-- decodeURIComponent() implementation
+-- Ref https://stackoverflow.com/a/78225561/1460422
+-- Adapted by @sbrl to:
+--	- Print leading 0 behind escape codes as it should
+--	- Also escape ' and #
+
+-- TODO this doesn't work. It replaces \n with %A instead of %0A, though we don't know if that's a problem or not
+-- it also doesn't handle quotes even though we've clearly got them in the Lua pattern
+local function _escape_char(char)
+	return string.format('%%%02X', string.byte(char))
+end
+
+--- Escape the given string for use in a url.
+-- In other words, like a space turns into %20.
+-- Similar to Javascript's `encodeURIComponent()`.
+-- @param	string	str		The string to escape.
+-- @returns	string	The escaped string.
+local function escape(str)
+	return (string.gsub(str, "[^%a%d%-_%.!~%*%(%);/%?:@&=%+%$,]", _escape_char))
+end
+
+return escape

worldeditadditions_core/utils/format/init.lua

@@ -8,5 +8,6 @@ wea_c.format = {
 	node_distribution = dofile(wea_c.modpath.."/utils/format/node_distribution.lua"),
 	make_ascii_table = dofile(wea_c.modpath.."/utils/format/make_ascii_table.lua"),
 	map = dofile(wea_c.modpath.."/utils/format/map.lua"),
+	escape = dofile(wea_c.modpath.."/utils/format/escape.lua")
 }
 

worldeditadditions_core/utils/setting_handler.lua

@@ -4,10 +4,10 @@ local wea_c = worldeditadditions_core
 wea_c.settings = {}
 
 -- Initialize wea world folder if not already existing
-local path = minetest.get_worldpath() .. "/worldeditadditions"
+local path = minetest.get_worldpath() .. wea_c.dirsep .. "worldeditadditions"
 minetest.mkdir(path)
 
---- A wrapper to simultaniously handle global and world settings.
+--- A wrapper to simultaneously handle global and world settings.
 -- @namespace worldeditadditions_core.setting_handler
 local setting_handler = {}
 

worldeditadditions_core/utils/strings/split.lua

@@ -62,7 +62,7 @@ end
 -- @param	plain	boolean	If true (or truthy), pattern is interpreted as a
 -- 							plain string, not a Lua pattern
 -- @returns	table	A sequence table containing the substrings
-local function split(str,dlm,plain)
+local function split(str, dlm, plain)
 	if not dlm then dlm = "%s+" end
 	local pos, ret = 0, {}
 	local ins, i = str:find(dlm,pos,plain)