From 4c419c4020018a01e6175f94fee0c89e9fbe34f4 Mon Sep 17 00:00:00 2001
From: Lars Mueller <appgurulars@gmx.de>
Date: Sat, 12 Oct 2024 23:24:47 +0200
Subject: [PATCH] Improve `minetest.parse_json`

Let modders handle parsing errors, get rid of two unnecessary copies.
---
 doc/lua_api.md                        |  6 ++-
 games/devtest/mods/unittests/misc.lua | 23 ++++++++----
 src/script/lua_api/l_util.cpp         | 54 ++++++++++++++-------------
 3 files changed, 48 insertions(+), 35 deletions(-)

diff --git a/doc/lua_api.md b/doc/lua_api.md
index 2c827d7ad..622993b7a 100644
--- a/doc/lua_api.md
+++ b/doc/lua_api.md
@@ -7404,11 +7404,13 @@ Misc.
     * Gets the internal content ID of `name`
 * `minetest.get_name_from_content_id(content_id)`: returns a string
     * Gets the name of the content with that content ID
-* `minetest.parse_json(string[, nullvalue])`: returns something
+* `minetest.parse_json(string[, nullvalue, return_error])`: returns something
     * Convert a string containing JSON data into the Lua equivalent
     * `nullvalue`: returned in place of the JSON null; defaults to `nil`
     * On success returns a table, a string, a number, a boolean or `nullvalue`
-    * On failure outputs an error message and returns `nil`
+    * On failure: If `return_error` is not set or is `false`,
+      outputs an error message and returns `nil`.
+      Otherwise returns `nil, err` (error message).
     * Example: `parse_json("[10, {\"a\":false}]")`, returns `{10, {a = false}}`
 * `minetest.write_json(data[, styled])`: returns a string or `nil` and an error
   message.
diff --git a/games/devtest/mods/unittests/misc.lua b/games/devtest/mods/unittests/misc.lua
index a807a390f..67473b8b5 100644
--- a/games/devtest/mods/unittests/misc.lua
+++ b/games/devtest/mods/unittests/misc.lua
@@ -155,13 +155,22 @@ unittests.register("test_urlencode", test_urlencode)
 
 local function test_parse_json()
 	local raw = "{\"how\\u0000weird\":\n\"yes\\u0000really\",\"n\":-1234567891011,\"z\":null}"
-	local data = core.parse_json(raw)
-	assert(data["how\000weird"] == "yes\000really")
-	assert(data.n == -1234567891011)
-	assert(data.z == nil)
-	local null = {}
-	data = core.parse_json(raw, null)
-	assert(data.z == null)
+	do
+		local data = core.parse_json(raw)
+		assert(data["how\000weird"] == "yes\000really")
+		assert(data.n == -1234567891011)
+		assert(data.z == nil)
+	end
+	do
+		local null = {}
+		local data = core.parse_json(raw, null)
+		assert(data.z == null)
+	end
+	do
+		local data, err = core.parse_json('"ceci n\'est pas un json', nil, true)
+		assert(data == nil)
+		assert(type(err) == "string")
+	end
 end
 unittests.register("test_parse_json", test_parse_json)
 
diff --git a/src/script/lua_api/l_util.cpp b/src/script/lua_api/l_util.cpp
index c899e55f4..6ead0a067 100644
--- a/src/script/lua_api/l_util.cpp
+++ b/src/script/lua_api/l_util.cpp
@@ -93,12 +93,12 @@ int ModApiUtil::l_get_us_time(lua_State *L)
 	return 1;
 }
 
-// parse_json(str[, nullvalue])
+// parse_json(str[, nullvalue, return_error])
 int ModApiUtil::l_parse_json(lua_State *L)
 {
 	NO_MAP_LOCK_REQUIRED;
 
-	const char *jsonstr = luaL_checkstring(L, 1);
+	const auto jsonstr = readParam<std::string_view>(L, 1);
 
 	// Use passed nullvalue or default to nil
 	int nullindex = 2;
@@ -107,36 +107,38 @@ int ModApiUtil::l_parse_json(lua_State *L)
 		nullindex = lua_gettop(L);
 	}
 
+	bool return_error = lua_toboolean(L, 3);
+	const auto handle_error = [&](const char *errmsg) {
+		if (return_error) {
+			lua_pushnil(L);
+			lua_pushstring(L, errmsg);
+			return 2;
+		}
+		errorstream << "Failed to parse json data: " << errmsg << std::endl;
+		errorstream << "data: \"";
+		if (jsonstr.size() <= 100) {
+			errorstream << jsonstr << "\"";
+		} else {
+			errorstream << jsonstr.substr(0, 100) << "\"... (truncated)";
+		}
+		errorstream << std::endl;
+		lua_pushnil(L);
+		return 1;
+	};
+
 	Json::Value root;
-
 	{
-		std::istringstream stream(jsonstr);
-
 		Json::CharReaderBuilder builder;
 		builder.settings_["collectComments"] = false;
-		std::string errs;
-
-		if (!Json::parseFromStream(builder, stream, &root, &errs)) {
-			errorstream << "Failed to parse json data " << errs << std::endl;
-			size_t jlen = strlen(jsonstr);
-			if (jlen > 100) {
-				errorstream << "Data (" << jlen
-					<< " bytes) printed to warningstream." << std::endl;
-				warningstream << "data: \"" << jsonstr << "\"" << std::endl;
-			} else {
-				errorstream << "data: \"" << jsonstr << "\"" << std::endl;
-			}
-			lua_pushnil(L);
-			return 1;
-		}
+		const std::unique_ptr<Json::CharReader> reader(builder.newCharReader());
+		std::string errmsg;
+		if (!reader->parse(jsonstr.begin(), jsonstr.end(), &root, &errmsg))
+			return handle_error(errmsg.c_str());
 	}
 
-	if (!push_json_value(L, root, nullindex)) {
-		errorstream << "Failed to parse json data, "
-			<< "depth exceeds lua stack limit" << std::endl;
-		errorstream << "data: \"" << jsonstr << "\"" << std::endl;
-		lua_pushnil(L);
-	}
+	if (!push_json_value(L, root, nullindex))
+		return handle_error("depth exceeds lua stack limit");
+
 	return 1;
 }