Add reserved name handling and add user filename validation

[MichalLabuda]

Summary

Adds Windows reserved device name support to makePortableName and introduces two new predicates (isValidFileNameChar, isValidFileName) for validating user-provided filenames.

Motivation

These utilities are needed in s25client to validate user-provided filenames in save file dialogs and similar text input controls, where both real-time character filtering and save-time structural validation are required.

Note

makePortableFileName (and by extension makePortableName) is intended for programmatic filename creation - it silently and automatically replaces invalid characters without user involvement.

For user-provided filenames - such as save games and the incoming addon presets feature (s25client PR #1951) - the intended approach is different: invalid characters will be suppressed at the ctrlEdit input level using isValidFileNameChar, and isValidFileName will perform a final structural check on save. If validation fails, the user receives a general message asking them to correct the name before saving. There is no automatic character replacement and no per-reason error detail - both would complicate the implementation, and silent replacement in particular would create an inconsistent experience where the saved filename silently differs from what the user typed.

Changes

makePortableName

Windows reserved device names (CON, PRN, AUX, NUL, COM0–COM9, LPT0–LPT9) were previously passed through unchanged when they already satisfied bfs::portable_name. This caused a subtle bug where e.g. "nul" was returned as-is despite being unusable as a filename on Windows. The fix appends '_' to any reserved name.

isValidFileNameChar(char32_t)

New predicate for real-time filtering of user-typed filename characters. Rejects control characters and characters forbidden on Windows (< > : " / \ | ? *), which is the most restrictive set across all supported platforms.

isValidFileName(const std::string&)

New structural validator for complete user-provided filenames. Rejects:

• Empty names
• Windows reserved device names - On Windows 7 and earlier, the device name is determined by the part before the first dot - making "nul.ini" equivalent to NUL and therefore invalid. Windows 10/11 relaxed this so that only bare names like "nul" are restricted, while "nul.ini" is a valid regular file. For cross-version portability, fileName is checked against the segment before the first dot, so "nul.ini" is also rejected
• Names starting or ending with a dot
• Names with a trailing space (Windows silently strips it, causing a mismatch between the displayed and actual filename; trimming before calling is left to the caller)

Comment corrections

The existing doc comments on makePortableName, makePortableFileName and makePortableDirName incorrectly stated that invalid characters were removed, when they are in fact replaced with _. Comments have been corrected to accurately describe the sanitization behavior.

Tests

Added test cases covering reserved device name handling in makePortableName (bare names such as "con", "NUL", "com0") and the new isValidFileNameChar and isValidFileName functions.

[MichalLabuda]

If you decide to accept and merge this, could you bump the submodule reference in s25client? There's a follow-up PR there that depends on this. Thanks!

[Flamefire]

For user-provided filenames - such as save games and the incoming addon presets feature (Return-To-The-Roots/s25client#1951) - the intended approach is different: invalid characters will be suppressed at the ctrlEdit input level using isValidFileNameChar, and isValidFileName will perform a final structural check on save. If validation fails, the user receives a general message asking them to correct the name before saving.

I'm not sure about the suppressing part.
I think the easiest way would be to simply run the users input through makePortableName and show an error if the result is different.
The current handling seems complicated: isValidFileName allows invalid characters and isValidFileNameChar seemingly allows at least non-portable characters.

On second thought I think it does make sense to allow users more freedom in their choice of file names, so fine by me.

Just fix isValidFileName to use isValidFileNameChar and add a single test for that (or 3: invalid char at start, middle, end, but I'd say one is enough)

I'm not a big fan of verbose tests, but having the valid and invalid chars in an array and using a loop might not be much clearer either. Just mentioning it for consideration where it might make sense in the future.

[MichalLabuda]

I added isValidFileNameChar to isValidFileName, applied your other suggestions and trimmed the test cases. Will remember for the future to be a bit more conservative with them :-)
With the suppressing in ctrlEdit (similarly to numberOnly_ but for this it would be fileNameOnly_) I found it the easiest solution to implement without passing back some error codes and creating text messages for each case...
Would you like ctrlEdit modifications (literally 9 new lines including comments, and 4 removed for double-space bug fix) and save game fixes in a separate PR, single PR or added to addon-presets PR? Personally would prefer addon-presets, then I wouldn't have to wait for the ctrlEdit PR to be merged but that is impatient me ;-)

[Flow86]

its completely overengineered.

Use PathIsValidFileName on windows, and on linux/macos theoretically (since its posix...): only '/' is forbidden.

[Flamefire]

What/where is PathIsValidFileName? Other questions suggest there isn't some WinAPI function for that. Can you add a link?

[Flow86]

hmm shit the blogpost I found listed it in shlwapi, but its not in there.

But its still overengineered. maybe something simple like

#include <string>
#include <algorithm>
#include <cwctype>

bool is_valid_filename(const std::wstring& name)
{
    if (name.empty()) return false;

    // 1. length (NTFS: 255 UTF-16 Codeunits)
    if (name.size() > 255) return false;

    // 2. forbidden chars, also works for posix (linux/macos)
    static const std::wstring forbidden = L"<>:\"/\\|?*";

    for (wchar_t c : name) {
        if (forbidden.find(c) != std::wstring::npos)
            return false;
        if (c < 32) // control chars
            return false;
    }

    // 3. trailing space oder dot
    wchar_t last = name.back();
    if (last == L' ' || last == L'.')
        return false;

    // 4. reserved names
    std::wstring upper;
    upper.reserve(name.size());
    std::transform(name.begin(), name.end(), std::back_inserter(upper),
                   [](wchar_t c){ return std::towupper(c); });

    static const std::wstring reserved[] = {
        L"CON", L"PRN", L"AUX", L"NUL",
        L"COM1", L"COM2", L"COM3", L"COM4", L"COM5", L"COM6", L"COM7", L"COM8", L"COM9",
        L"LPT1", L"LPT2", L"LPT3", L"LPT4", L"LPT5", L"LPT6", L"LPT7", L"LPT8", L"LPT9"
    };

    for (const auto& r : reserved) {
        if (upper == r)
            return false;
    }

    return true;
}

[MichalLabuda]

The goal is to have consistent restrictions across platforms and to avoid potential issues when files are transferred in multiplayer from Linux to Windows, which is the most restrictive one.

isReservedName is extracted so it can be re-used in makePortableName
isValidFileNameChar is extracted because it serves a different purpose than isValidFileName - it's planned to be used for real-time character filtering in ctrlEdit:

void ctrlEdit::SetText(const std::string& text)
{
    text_ = s25util::utf8to32(text);
    if(numberOnly_)
        helpers::erase_if(text_, [](char32_t c) { return c < '0' || c > '9'; });
    if(fileNameOnly_)
        helpers::erase_if(text_, [](char32_t c) { return !isValidFileNameChar(c); });
    ...
}

As for the proposed alternative: it does the same things as the current implementation just reorganized into a single function using wstring. The logic is equivalent, wstring would be inconsistent with the rest of the codebase which uses UTF-8 + char32_t.

One subtle difference: the proposed code checks the full name against the reserved list, so NUL.txt would pass. The current code checks fileName.substr(0, fileName.find('.')) which correctly rejects it - this is the Windows 7 and earlier behavior mentioned in the comment.

One thing that is genuinely missing is a length check - I'll add that.

[Flamefire]

trimmed the test cases. Will remember for the future to be a bit more conservative with them :-)

A bit too much, I might have been not clear enough. See suggestions. For each test simply think if this could potentially catch a (class of) mistake. I put my own reasoning on the reserved-with-extension suggestion. The others are similar: CON-NUL-nul-Lpt --> 2 examples, 3rd is same as 2 in lowercase, and a 4th mixed-case one.
For the special chars I was aiming for reasonably common ones.

Would you like ctrlEdit modifications (literally 9 new lines including comments, and 4 removed for double-space bug fix) and save game fixes in a separate PR, single PR or added to addon-presets PR? Personally would prefer addon-presets, then I wouldn't have to wait for the ctrlEdit PR to be merged but that is impatient me ;-)

Fine for me to add it where it's used, ie. the existing PR

[MichalLabuda]

Added the length check - using utf8to32 to count Unicode code points rather than raw bytes, so accented characters (e.g. é = 2 UTF-8 bytes) correctly count as 1 toward the limit.

Tests cover both ASCII (255/256 'a') and a multibyte case (255/256 repetitions of é) to verify the byte-vs-code-point distinction explicitly.