Skip to content

Add reserved name handling and add user filename validation #42

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
MichalLabuda wants to merge 7 commits into
base: master
Choose a base branch
from
+165 −10
Open

Add reserved name handling and add user filename validation #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dd36255
Add reserved name handling to makePortableName and add user filename …
MichalLabuda Jun 27, 2026
6383cef
Use isValidFileNameChar in isValidFileName and trim tests to one per …
MichalLabuda Jun 28, 2026
c1773ad
Apply suggestions from code review
MichalLabuda Jun 28, 2026
2fc22bd
Add missing test case
MichalLabuda Jun 28, 2026
d37c01e
Merge remote changes into add-filename-validation
MichalLabuda Jun 28, 2026
015b15e
Test cases restored according to suggestions from code review
MichalLabuda Jun 28, 2026
70d869c
Add length check (255 code points) to isValidFileName
MichalLabuda Jun 28, 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
21 changes: 14 additions & 7 deletions libs/common/include/s25util/fileFuncs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org)
// Copyright (C) 2005 - 2026 Settlers Freaks (sf-team at siedler25.org)
//
// SPDX-License-Identifier: GPL-2.0-or-later

#pragma once

#include <string>

/// Remove all invalid chars of a file or directory name. Result may be empty!
/// --> bfs::portable_name will return true
/// Sanitizes to a portable name. Appends '_' to Windows reserved device names. Result may be empty.
/// --> bfs::portable_name will return true for non-empty results
std::string makePortableName(const std::string& fileName);
/// Remove all invalid chars so the name can be used for a file. Result may be empty!
/// --> bfs::portable_file_name will return true
/// Sanitizes to a portable filename. Result may be empty.
/// --> bfs::portable_file_name will return true for non-empty results
std::string makePortableFileName(const std::string& fileName);
/// Remove all invalid chars so the name can be used for a directory. Result may be empty!
/// --> bfs::portable_directory_name will return true
/// Sanitizes to a portable directory name. Result may be empty.
/// --> bfs::portable_directory_name will return true for non-empty results
std::string makePortableDirName(const std::string& fileName);

/// Returns true if c is valid in a user-provided filename.
/// Rejects control characters and chars forbidden on Windows (< > : " / \ | ? *).
bool isValidFileNameChar(char32_t c);
/// Returns true if fileName is a valid user-provided filename.
/// Rejects reserved device names, empty names, leading/trailing dots, and trailing spaces.
bool isValidFileName(const std::string& fileName);
54 changes: 52 additions & 2 deletions libs/common/src/fileFuncs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,31 @@
// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org)
// Copyright (C) 2005 - 2026 Settlers Freaks (sf-team at siedler25.org)
//
// SPDX-License-Identifier: GPL-2.0-or-later

#include "fileFuncs.h"
#include "s25util/strAlgos.h"
#include "s25util/utf8.h"
#include <boost/filesystem/path.hpp>
#include <algorithm>
#include <array>

namespace bfs = boost::filesystem;

// Windows reserved device names
static constexpr std::array reservedNames{"con", "prn", "aux", "nul", "com0", "com1", "com2", "com3",
"com4", "com5", "com6", "com7", "com8", "com9", "lpt0", "lpt1",
"lpt2", "lpt3", "lpt4", "lpt5", "lpt6", "lpt7", "lpt8", "lpt9"};

static bool isReservedName(const std::string& name)
{
const std::string lower = s25util::toLower(name);
return std::find(reservedNames.begin(), reservedNames.end(), lower) != reservedNames.end();
}

std::string makePortableName(const std::string& fileName)
{
if(fileName.empty() || bfs::portable_name(fileName))
return fileName;
return isReservedName(fileName) ? fileName + '_' : fileName;
std::string result;
result.reserve(fileName.size());
for(char c : fileName)
Expand All @@ -30,6 +45,8 @@ std::string makePortableName(const std::string& fileName)
while(!result.empty() && result.back() == '.')
result.erase(result.end() - 1);
}
if(isReservedName(result))
result += '_';
assert(result.empty() || bfs::portable_name(result));
return result;
}
Expand Down Expand Up @@ -74,3 +91,36 @@ std::string makePortableDirName(const std::string& fileName)
assert(result.empty() || bfs::portable_directory_name(result));
return result;
}

bool isValidFileNameChar(char32_t c)
{
// Reject control characters
if(c <= 0x1F || c == 0x7F)
return false;
// Reject characters forbidden on Windows (the most restrictive platform),
// which covers all restrictions on Linux, macOS, and Android as well.
if(c == '<' || c == '>' || c == ':' || c == '"' || c == '/' || c == '\\' || c == '|' || c == '?' || c == '*')
return false;
return true;
}

bool isValidFileName(const std::string& fileName)

@Flow86 Flow86 Jun 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

its completely overengineered.

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

@Flamefire Flamefire Jun 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@Flow86 Flow86 Jun 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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 MichalLabuda Jun 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

{
if(fileName.empty())
return false;
if(s25util::utf8to32(fileName).size() > 255)
return false;

@MichalLabuda MichalLabuda Jun 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

if(fileName.front() == '.' || fileName.back() == '.')
return false;
// Windows silently strips trailing spaces, which would create a mismatch between
// the name the user typed and the file actually created on disk.
if(fileName.back() == ' ')
return false;
for(char c : fileName)
{
if(!isValidFileNameChar(static_cast<unsigned char>(c)))
return false;
}
// On Windows 7 and earlier the device name is the part before the first dot — "nul.ini" is NUL thus forbidden.
return !isReservedName(fileName.substr(0, fileName.find('.')));
}
100 changes: 99 additions & 1 deletion tests/testFilefuncs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org)
// Copyright (C) 2005 - 2026 Settlers Freaks (sf-team at siedler25.org)
//
// SPDX-License-Identifier: GPL-2.0-or-later

Expand All @@ -20,6 +20,21 @@ BOOST_AUTO_TEST_CASE(PortableName)
BOOST_TEST(makePortableName("~abc") == "_abc");
BOOST_TEST(makePortableName("abc ") == "abc_");
BOOST_TEST(makePortableName("abc.") == "abc");

// Reserved names get _ appended
BOOST_TEST(makePortableName("con") == "con_");
BOOST_TEST(makePortableName("NUL") == "NUL_");
BOOST_TEST(makePortableName("com1") == "com1_");
BOOST_TEST(makePortableName("lpt9") == "lpt9_");
BOOST_TEST(makePortableName("com0") == "com0_");
BOOST_TEST(makePortableName("lpt0") == "lpt0_");
BOOST_TEST(makePortableName("prn") == "prn_");
BOOST_TEST(makePortableName("aux") == "aux_");
// Non-reserved names are unchanged
BOOST_TEST(makePortableName("null") == "null");
BOOST_TEST(makePortableName("console") == "console");
BOOST_TEST(makePortableName("com10") == "com10");
BOOST_TEST(makePortableName("lpt10") == "lpt10");
}

BOOST_AUTO_TEST_CASE(PortableFileName)
Expand All @@ -44,3 +59,86 @@ BOOST_AUTO_TEST_CASE(PortableDirName)
BOOST_TEST(makePortableDirName("file.extLONG") == "fileextLONG");
BOOST_TEST(makePortableDirName("file....") == "file");
}

BOOST_AUTO_TEST_CASE(ValidFileNameChar)
{
// Allowed
BOOST_TEST(isValidFileNameChar('a'));
BOOST_TEST(isValidFileNameChar('Z'));
BOOST_TEST(isValidFileNameChar('5'));
BOOST_TEST(isValidFileNameChar(' '));
BOOST_TEST(isValidFileNameChar('.'));
BOOST_TEST(isValidFileNameChar('_'));
BOOST_TEST(isValidFileNameChar('-'));
BOOST_TEST(isValidFileNameChar('('));
BOOST_TEST(isValidFileNameChar(']'));
BOOST_TEST(isValidFileNameChar(U'\u00E9')); // non-ASCII Unicode

// Rejected — Windows-forbidden
BOOST_TEST(!isValidFileNameChar('<'));
BOOST_TEST(!isValidFileNameChar('>'));
BOOST_TEST(!isValidFileNameChar(':'));
BOOST_TEST(!isValidFileNameChar('"'));
BOOST_TEST(!isValidFileNameChar('/'));
BOOST_TEST(!isValidFileNameChar('\\'));
BOOST_TEST(!isValidFileNameChar('|'));
BOOST_TEST(!isValidFileNameChar('?'));
BOOST_TEST(!isValidFileNameChar('*'));
// Rejected — control characters
BOOST_TEST(!isValidFileNameChar('\0'));
Comment thread
MichalLabuda marked this conversation as resolved.
BOOST_TEST(!isValidFileNameChar('\n'));
BOOST_TEST(!isValidFileNameChar(0x1F));
}

BOOST_AUTO_TEST_CASE(ValidFileName)
{
BOOST_TEST(isValidFileName("abc"));
BOOST_TEST(isValidFileName("Brick economy test"));
BOOST_TEST(isValidFileName("DevMap (Auto-Save)"));
BOOST_TEST(isValidFileName("save_01"));
BOOST_TEST(isValidFileName("my save.sav"));

// Empty
BOOST_TEST(!isValidFileName(""));

// Reserved name (case-insensitive)
BOOST_TEST(!isValidFileName("con"));
BOOST_TEST(!isValidFileName("CON"));
Comment thread
MichalLabuda marked this conversation as resolved.
BOOST_TEST(!isValidFileName("nul"));
BOOST_TEST(!isValidFileName("NUL"));
BOOST_TEST(!isValidFileName("Lpt0"));

// Non-reserved look-alike
BOOST_TEST(isValidFileName("null"));
BOOST_TEST(isValidFileName("null.ini"));

// Leading/trailing dots
BOOST_TEST(!isValidFileName(".hidden"));
BOOST_TEST(!isValidFileName("trail."));
Comment thread
MichalLabuda marked this conversation as resolved.
BOOST_TEST(!isValidFileName("."));
BOOST_TEST(!isValidFileName(".."));

// Trailing space
BOOST_TEST(!isValidFileName("trail "));

// Reserved base name with extension (Windows 7 compat)
BOOST_TEST(!isValidFileName("nul.ini"));
Comment thread
MichalLabuda marked this conversation as resolved.
BOOST_TEST(!isValidFileName("NUL.ini"));
BOOST_TEST(!isValidFileName("nul.txt"));
BOOST_TEST(!isValidFileName("com0.txt"));

// Invalid character
BOOST_TEST(!isValidFileName("save:game"));

// Length limit (255 Unicode code points)
BOOST_TEST(isValidFileName(std::string(255, 'a')));
BOOST_TEST(!isValidFileName(std::string(256, 'a')));

// é (U+00E9): 2 UTF-8 bytes but 1 code point - verify length is counted in code points, not bytes.
const std::string eacute = "\xC3\xA9";
std::string e256;
for(int i = 0; i < 256; ++i)
e256 += eacute;
BOOST_TEST(isValidFileName(e256.substr(0, e256.size() - eacute.size()))); // 255 code points, 510 bytes
BOOST_TEST(!isValidFileName(e256)); // 256 code points, 512 bytes
}