Skip to content

Prevent loading of mods with invalid names #16904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
compmstr wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Prevent loading of mods with invalid names #16904

Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e1efcfa
Stop loading mod if mod name is invalid
compmstr Feb 2, 2026
d8f08f3
Fix indents (should be tabs)
compmstr Feb 3, 2026
9ab1852
Update docs for module `name` field
compmstr Feb 3, 2026
817cd45
Add module load error handling
compmstr Feb 3, 2026
8c60ce8
whitespace fixes
compmstr Feb 3, 2026
41badce
Move mod error handling into new ModErrors class
compmstr Feb 15, 2026
2e240e2
Move imports to the file where they are used
compmstr Feb 17, 2026
61f1f06
Update copyright for new files
compmstr Feb 17, 2026
2af97e1
Switch to unordered_(set|map) for errors
compmstr Feb 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/content/mods.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,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)) {
Copy link
Member

@SmallJoker SmallJoker Feb 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When starting Luanti in the "Start game" tab, the following warnings are logged. When I click "Configure mods", the same bunch of warnings are logged - again.

Error loading mod "xxx/games/VoxeLibre/mods/PLAYER": Mod does not follow naming conventions: Only characters [a-z0-9_] are allowed.
Error loading mod "xxx/mods/worldedit-modpack/bonkers": Mod does not follow naming conventions: Only characters [a-z0-9_] are allowed.
Error loading mod "xxx/mods/worldedit-modpack": Mod does not follow naming conventions: Only characters [a-z0-9_] are allowed.
Error loading mod "xxx/mods/skyblock": Mod does not fo	llow naming conventions: Only characters [a-z0-9_] are allowed.
Error loading mod "xxx/mods/nested_modpack/modpack_a1": Mod does not follow naming conventions: Only characters [a-z0-9_] are allowed.
Error loading mod "xxx/mods/nested_modpack/modpack_b2": Mod does not follow naming conventions: Only characters [a-z0-9_] are allowed.
Error loading mod "xxx/mods/nested_modpack": Mod does not follow naming conventions: Only characters [a-z0-9_] are allowed.
  1. The warnings should only logged once per mod(pack) to avoid spam.
  2. From what I could(n't find), modpacks seem to have no naming rules defined in lua_api.md
    • This needs documenting.
    • A-z_- or a-z_- might be the best retro-fit.
  3. Perhaps misleading PR title. Prevent loading of mods is done by mod.checkAndLog(); in ServerModManager::loadMods, where the server will not load the mods that do not obey the naming conventions.

EDIT: Whereas the naming is enforced upon server start-up, there is also lack of documentation on mod names in lua_api.md.

Copy link
Contributor

@rubenwardy rubenwardy Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll notice that nothing else prints to the log in this function. This is by design - it would be too spammy in the main menu. You should just return false here.

Copy link
Author

@compmstr compmstr Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, should I just drop the logging altogether, or try to find somewhere to log once for mod debugging purposes?

Copy link
Author

@compmstr compmstr Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I could add a message to deprecation_msgs, so that it gets logged out when other mod issues are.

I could also set up a new error_msgs that contains this message, as well as things like "Mod ... missing init.lua" (the other case where we skip loading a log). I could log it from within checkAndLog the same way deprecation messages are.

Copy link
Contributor

@rubenwardy rubenwardy Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look at how the missing init.lua is validated because it's the same style of issue

Copy link
Author

@compmstr compmstr Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got bit by the silent failure of the mod from a missing init.lua when I first tried to test this PR. Ideally we could log at least once whenever there's an error loading the mod.

I'll take another pass at this, see if I can get these to log out inside checkAndLog, since that's less spammy.

warningstream << "Error loading mod \"" + spec.path + "\": Mod does not follow naming conventions: " "Only characters [a-z0-9_] are allowed." << std::endl;
return false;
}
} else if (!spec.is_modpack) {
spec.deprecation_msgs.push_back("Mods not having a mod.conf file with the name is deprecated.");
}
Expand Down
Loading