Prevent loading of mods with invalid names

builtin/settingtypes.txt

@@ -1941,6 +1941,12 @@ chat_log_level (Chat log level) [client] enum error ,none,error,warning,action,i
 #    -    error: abort on usage of deprecated call (suggested for mod developers).
 deprecated_lua_api_handling (Deprecated Lua API handling) [common] enum log none,log,error
 
+#    Handling for errors when loading mods:
+#    -    none: Do not log mod load errors
+#    -    log: log errors (default).
+#    -    error: abort on failed load of mod (suggested for mod developers).
+mod_error_handling (Mod load error handling) [common] enum log none,log,error
+
 #    Enable random user input (only used for testing).
 random_input (Random input) [client] bool false
 

doc/lua_api.md

@@ -214,7 +214,8 @@ The location of this directory can be fetched by using
 A `Settings` file that provides meta information about the mod.
 
 * `name`: The mod name. Allows Luanti to determine the mod name even if the
-          folder is wrongly named.
+          folder is wrongly named. Valid names include only the characters
+          `a-zA-Z0-9_`.
 * `title`: A human-readable title to address the mod. See [Translating content meta](#translating-content-meta).
 * `description`: Description of mod to be shown in the Mods tab of the main
                  menu. See [Translating content meta](#translating-content-meta).

minetest.conf.example

@@ -3012,6 +3012,12 @@
 #    type: enum values: none, log, error
 # deprecated_lua_api_handling = log
 
+#    Handling for errors when loading mods:
+#    -    none: Do not log mod load errors
+#    -    log: log errors (default).
+#    -    error: abort on failed load of mod (suggested for mod developers).
+# mod_error_handling = log
+
 #    Enable random user input (only used for testing).
 #    type: bool
 # random_input = false

src/content/CMakeLists.txt

@@ -5,6 +5,7 @@ set(content_SRCS
 	${CMAKE_CURRENT_SOURCE_DIR}/content.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/mod_configuration.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/mods.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/mod_errors.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/subgames.cpp
 	PARENT_SCOPE
 )

src/content/mod_errors.cpp

@@ -0,0 +1,53 @@
+// Luanti
+// SPDX-License-Identifier: LGPL-2.1-or-later
+// Copyright (C) 2013 celeron55, Perttu Ahola <celeron55@gmail.com>
+
+#include <set>
+#include <map>
+#include <sstream>
+#include "mod_errors.h"
+#include "common/c_internal.h"
+#include "log.h"
+#include "exceptions.h"
+
+struct SingleModErrors {
+	std::set<std::string> messages;
+	bool was_handled;
+};
+
+static std::map<std::string, SingleModErrors> mod_errors;
+
+void ModErrors::logErrors() {
+	auto error_handling_mode = get_mod_error_handling_mode();
+	if (error_handling_mode == ModErrorHandlingMode::IgnoreModError) {
+		return;
+	}
+
+	for (auto &pair : mod_errors) {
+		const auto path = pair.first;
+		SingleModErrors& err = pair.second;
+
+		if (!err.was_handled && !err.messages.empty()) {
+			err.was_handled = true;
+			std::ostringstream os;
+			os << "Mod at " << path << ":" << std::endl;
+			for (const auto& msg : err.messages) {
+				os << "\t" << msg << std::endl;
+			}
+			if (error_handling_mode == ModErrorHandlingMode::ThrowModError)
+				throw ModError(os.str());
+			else
+				warningstream << os.str();
+		}
+	}
+}
+
+
+void ModErrors::setModError(const std::string &path, std::string error_message) {
+	SingleModErrors &cur = mod_errors[path];
+	auto result = cur.messages.emplace(error_message);
+	if (result.second) {
+		// New message was added, reset the was_handled flag
+		cur.was_handled = false;
+	}
+}

src/content/mod_errors.h

@@ -0,0 +1,13 @@
+// Luanti
+// SPDX-License-Identifier: LGPL-2.1-or-later
+// Copyright (C) 2013 celeron55, Perttu Ahola <celeron55@gmail.com>
+
+#pragma once
+
+#include <string>
+
+class ModErrors {
+public:
+	static void logErrors();
+	static void setModError(const std::string &path, std::string error_message);
+};

src/content/mods.cpp

@@ -6,6 +6,7 @@
 #include <fstream>
 #include <json/json.h>
 #include <algorithm>
+#include "content/mod_errors.h"
 #include "content/mods.h"
 #include "database/database.h"
 #include "filesys.h"
@@ -16,21 +17,17 @@
 
 void ModSpec::checkAndLog() const
 {
-	if (!string_allowed(name, MODNAME_ALLOWED_CHARS)) {
-		throw ModError("Error loading mod \"" + name +
-			"\": Mod name does not follow naming conventions: "
-				"Only characters [a-z0-9_] are allowed.");
-	}
+	ModErrors::logErrors();
 
 	// Log deprecation messages
-	auto handling_mode = get_deprecated_handling_mode();
-	if (!deprecation_msgs.empty() && handling_mode != DeprecatedHandlingMode::Ignore) {
+	auto deprecation_handling_mode = get_deprecated_handling_mode();
+	if (!deprecation_msgs.empty() && deprecation_handling_mode != DeprecatedHandlingMode::Ignore) {
 		std::ostringstream os;
 		os << "Mod " << name << " at " << path << ":" << std::endl;
 		for (auto msg : deprecation_msgs)
 			os << "\t" << msg << std::endl;
 
-		if (handling_mode == DeprecatedHandlingMode::Error)
+		if (deprecation_handling_mode == DeprecatedHandlingMode::Error)
 			throw ModError(os.str());
 		else
 			warningstream << os.str();
@@ -70,6 +67,7 @@ bool parseModContents(ModSpec &spec)
 	} else if (fs::IsFile(spec.path + DIR_DELIM + "modpack.txt")) {
 		spec.is_modpack = true;
 	} else if (!fs::IsFile(spec.path + DIR_DELIM + "init.lua")) {
+		ModErrors::setModError(spec.path, "Mod does not contain 'init.lua");
 		return false;
 	} else {
 		// Is a mod
@@ -86,6 +84,10 @@ bool parseModContents(ModSpec &spec)
 	if (info.exists("name")) {
 		spec.name = info.get("name");
 		spec.is_name_explicit = true;
+		if (!string_allowed(spec.name, MODNAME_ALLOWED_CHARS)) {
+			ModErrors::setModError(spec.path, "Mod does not follow naming conventions" "Only characters [a-zA-Z0-9_] are allowed.");
+			return false;
+		}
 	} else if (!spec.is_modpack) {
 		spec.deprecation_msgs.push_back("Mods not having a mod.conf file with the name is deprecated.");
 	}
@@ -192,6 +194,8 @@ std::map<std::string, ModSpec> getModsInPath(
 			result[modname] = std::move(spec);
 		}
 	}
+	// Log mod errors after we finish loading them all
+	ModErrors::logErrors();
 	return result;
 }
 

src/content/mods.h

@@ -5,14 +5,15 @@
 #pragma once
 
 #include <vector>
+#include <set>
 #include <string>
 #include <map>
 #include <unordered_set>
 #include "metadata.h"
 
 class ModStorageDatabase;
 
-#define MODNAME_ALLOWED_CHARS "abcdefghijklmnopqrstuvwxyz0123456789_"
+#define MODNAME_ALLOWED_CHARS "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_"
 
 struct ModSpec
 {
@@ -65,6 +66,8 @@ struct ModSpec
 	}
 
 	void checkAndLog() const;
+
+	static void logModLoadErrors();
 };
 
 /**

src/defaultsettings.cpp

@@ -460,6 +460,7 @@ void set_default_settings()
 	settings->setDefault("anticheat_movement_tolerance", "1.0");
 	settings->setDefault("enable_rollback_recording", "false");
 	settings->setDefault("deprecated_lua_api_handling", "log");
+	settings->setDefault("mod_error_handling", "log");
 
 	settings->setDefault("kick_msg_shutdown", "Server shutting down.");
 	settings->setDefault("kick_msg_crash", "This server has experienced an internal error. You will now be disconnected.");

src/script/common/c_internal.cpp

@@ -197,3 +197,22 @@ void call_string_dump(lua_State *L, int idx)
 	lua_pushvalue(L, idx);
 	lua_call(L, 1, 1);
 }
+
+ModErrorHandlingMode get_mod_error_handling_mode()
+{
+	static thread_local bool configured = false;
+	static thread_local ModErrorHandlingMode ret = ModErrorHandlingMode::IgnoreModError;
+
+	// Only read settings on first call
+	if (!configured) {
+		std::string value = g_settings->get("mod_error_handling");
+		if (value == "log") {
+			ret = ModErrorHandlingMode::LogModError;
+		} else if (value == "error") {
+			ret = ModErrorHandlingMode::ThrowModError;
+		}
+		configured = true;
+	}
+
+	return ret;
+}

src/script/common/c_internal.h

@@ -145,3 +145,16 @@ void log_deprecated(lua_State *L, std::string_view message,
 // Safely call string.dump on a function value
 // (does not pop, leaves one value on stack)
 void call_string_dump(lua_State *L, int idx);
+
+enum ModErrorHandlingMode {
+	IgnoreModError,
+	LogModError,
+	ThrowModError,
+};
+
+/**
+ * Reads `mod_error_handling` in settings, returns cached value.
+ *
+ * @return ModErrorHandlingMode
+ */
+ModErrorHandlingMode get_mod_error_handling_mode();