From abf111668f4d04109b56ff6450f9c59af7d444db Mon Sep 17 00:00:00 2001 From: nico2525nn <173241093+nico2525nn@users.noreply.github.com> Date: Wed, 23 Sep 2026 13:16:00 +0900 Subject: [PATCH 01/10] fix: match Java properties parsing semantics --- src/game/ServerProperties.hpp | 290 +++++++++++++++++++++++++++++----- tests/test_core_safety.cpp | 6 +- tests/test_properties.cpp | 178 +++++++++++++++++++-- 3 files changed, 417 insertions(+), 57 deletions(-) diff --git a/src/game/ServerProperties.hpp b/src/game/ServerProperties.hpp index e9b9e365..d5b295e5 100644 --- a/src/game/ServerProperties.hpp +++ b/src/game/ServerProperties.hpp @@ -5,11 +5,11 @@ #include #include #include -#include #include #include #include #include +#include #include #include #include @@ -25,12 +25,18 @@ class ServerProperties { bool load(const std::string& path) { std::ifstream f(path, std::ios::binary); if (!f) return false; - return loadStream(f); + std::string bytes; + char buffer[8192]; + while (f) { + f.read(buffer, sizeof(buffer)); + bytes.append(buffer, static_cast(f.gcount())); + } + if (f.bad()) return false; + return loadBytes(bytes); } bool loadText(std::string_view text) { - std::istringstream stream{std::string(text)}; - return loadStream(stream); + return loadBytes(text); } const std::vector>& parsedEntries() const { @@ -38,48 +44,254 @@ class ServerProperties { } private: - bool loadStream(std::istream& f) { + bool loadBytes(std::string_view bytes) { + const std::string text = decodeInput(bytes); std::map loaded; std::vector> ordered; - std::string line; - while (std::getline(f, line)) { - const std::string trimmedLine = trim(line); - if (trimmedLine.empty() || trimmedLine.front() == '#') continue; - const std::size_t eq = trimmedLine.find('='); - if (eq == std::string::npos) continue; - const std::string key = trim(trimmedLine.substr(0, eq)); - if (key.empty()) continue; - const std::string value = trim(trimmedLine.substr(eq + 1)); - - // Java Properties treats a later duplicate as the effective - // value. Apply the same rule for case variants as well, while - // retaining the final spelling for deterministic save output. - for (auto it = loaded.begin(); it != loaded.end();) { - if (asciiLower(it->first) == asciiLower(key)) it = loaded.erase(it); - else ++it; + std::string logicalLine; + bool continuing = false; + std::size_t offset = 0; + while (offset < text.size()) { + const std::size_t lineStart = offset; + while (offset < text.size() && text[offset] != '\n' && text[offset] != '\r') + ++offset; + const std::string_view physicalLine(text.data() + lineStart, offset - lineStart); + if (offset < text.size() && text[offset] == '\r') ++offset; + if (offset < text.size() && text[offset] == '\n') ++offset; + + if (continuing) { + logicalLine.append(physicalLine.substr(skipPropertyWhitespace(physicalLine))); + } else { + const std::size_t first = skipPropertyWhitespace(physicalLine); + if (first == physicalLine.size() || physicalLine[first] == '#' || + physicalLine[first] == '!') + continue; + logicalLine.assign(physicalLine); } - loaded[key] = value; - for (auto it = ordered.begin(); it != ordered.end();) { - if (asciiLower(it->first) == asciiLower(key)) it = ordered.erase(it); - else ++it; + + if (hasContinuationMarker(logicalLine)) { + logicalLine.pop_back(); + continuing = true; + continue; } - ordered.emplace_back(key, value); + + addPropertyLine(logicalLine, loaded, ordered); + logicalLine.clear(); + continuing = false; } - if (f.bad()) return false; + + // A final continuation marker is removed just as it is for any other + // physical line; Java's reader then reaches EOF with the accumulated + // logical property still available. + if (continuing) addPropertyLine(logicalLine, loaded, ordered); + props = std::move(loaded); parsedEntries_ = std::move(ordered); return true; } - static std::string trim(const std::string& value) { - const size_t first = value.find_first_not_of(" \t\r\n"); - if (first == std::string::npos) return {}; - const size_t last = value.find_last_not_of(" \t\r\n"); - return value.substr(first, last - first + 1); + static bool isValidUtf8(std::string_view input) { + std::size_t i = 0; + while (i < input.size()) { + const auto first = static_cast(input[i]); + if (first <= 0x7f) { + ++i; + continue; + } + + std::size_t length = 0; + unsigned char secondMin = 0x80; + unsigned char secondMax = 0xbf; + if (first >= 0xc2 && first <= 0xdf) { + length = 2; + } else if (first >= 0xe0 && first <= 0xef) { + length = 3; + if (first == 0xe0) secondMin = 0xa0; + if (first == 0xed) secondMax = 0x9f; + } else if (first >= 0xf0 && first <= 0xf4) { + length = 4; + if (first == 0xf0) secondMin = 0x90; + if (first == 0xf4) secondMax = 0x8f; + } else { + return false; + } + if (i + length > input.size()) return false; + const auto second = static_cast(input[i + 1]); + if (second < secondMin || second > secondMax) return false; + for (std::size_t j = 2; j < length; ++j) { + const auto continuation = static_cast(input[i + j]); + if (continuation < 0x80 || continuation > 0xbf) return false; + } + i += length; + } + return true; + } + + static void appendUtf8(std::string& output, std::uint32_t codePoint) { + if (codePoint <= 0x7f) { + output.push_back(static_cast(codePoint)); + } else if (codePoint <= 0x7ff) { + output.push_back(static_cast(0xc0 | (codePoint >> 6))); + output.push_back(static_cast(0x80 | (codePoint & 0x3f))); + } else if (codePoint <= 0xffff) { + output.push_back(static_cast(0xe0 | (codePoint >> 12))); + output.push_back(static_cast(0x80 | ((codePoint >> 6) & 0x3f))); + output.push_back(static_cast(0x80 | (codePoint & 0x3f))); + } else { + output.push_back(static_cast(0xf0 | (codePoint >> 18))); + output.push_back(static_cast(0x80 | ((codePoint >> 12) & 0x3f))); + output.push_back(static_cast(0x80 | ((codePoint >> 6) & 0x3f))); + output.push_back(static_cast(0x80 | (codePoint & 0x3f))); + } + } + + static std::string decodeInput(std::string_view bytes) { + if (isValidUtf8(bytes)) return std::string(bytes); + + // Dedicated-server properties loading first uses a strict UTF-8 + // decoder, then retries the complete file as ISO-8859-1 on failure. + std::string decoded; + decoded.reserve(bytes.size() * 2); + for (const unsigned char byte : bytes) appendUtf8(decoded, byte); + return decoded; + } + + static bool isPropertyWhitespace(char c) { + return c == ' ' || c == '\t' || c == '\f'; + } + + static std::size_t skipPropertyWhitespace(std::string_view text) { + std::size_t i = 0; + while (i < text.size() && isPropertyWhitespace(text[i])) ++i; + return i; + } + + static bool hasContinuationMarker(std::string_view line) { + std::size_t backslashes = 0; + for (std::size_t i = line.size(); i > 0 && line[i - 1] == '\\'; --i) + ++backslashes; + return (backslashes & 1U) != 0; + } + + static int hexValue(char c) { + if (c >= '0' && c <= '9') return c - '0'; + if (c >= 'a' && c <= 'f') return c - 'a' + 10; + if (c >= 'A' && c <= 'F') return c - 'A' + 10; + return -1; + } + + static std::uint16_t parseUnicodeEscape(std::string_view text, std::size_t slash) { + if (slash + 6 > text.size() || text[slash] != '\\' || text[slash + 1] != 'u') + throw std::invalid_argument("Malformed \\uxxxx encoding in properties"); + std::uint16_t codeUnit = 0; + for (std::size_t i = slash + 2; i < slash + 6; ++i) { + const int digit = hexValue(text[i]); + if (digit < 0) + throw std::invalid_argument("Malformed \\uxxxx encoding in properties"); + codeUnit = static_cast((codeUnit << 4) | digit); + } + return codeUnit; + } + + static std::string decodeEscapes(std::string_view input) { + std::string output; + output.reserve(input.size()); + for (std::size_t i = 0; i < input.size();) { + if (input[i] != '\\') { + output.push_back(input[i++]); + continue; + } + if (i + 1 == input.size()) { + output.push_back('\\'); + ++i; + continue; + } + + const char escaped = input[i + 1]; + if (escaped == 'u') { + const std::uint16_t first = parseUnicodeEscape(input, i); + i += 6; + if (first >= 0xd800 && first <= 0xdbff) { + if (i + 6 <= input.size() && input[i] == '\\' && input[i + 1] == 'u') { + const std::uint16_t second = parseUnicodeEscape(input, i); + if (second >= 0xdc00 && second <= 0xdfff) { + const std::uint32_t codePoint = 0x10000U + + ((static_cast(first) - 0xd800U) << 10) + + (static_cast(second) - 0xdc00U); + appendUtf8(output, codePoint); + i += 6; + continue; + } + } + // Java strings can retain isolated UTF-16 surrogates; + // this API stores UTF-8, so represent them with U+FFFD. + appendUtf8(output, 0xfffd); + } else if (first >= 0xdc00 && first <= 0xdfff) { + appendUtf8(output, 0xfffd); + } else { + appendUtf8(output, first); + } + continue; + } + + switch (escaped) { + case 't': output.push_back('\t'); break; + case 'n': output.push_back('\n'); break; + case 'r': output.push_back('\r'); break; + case 'f': output.push_back('\f'); break; + default: output.push_back(escaped); break; + } + i += 2; + } + return output; + } + + static void addPropertyLine(const std::string& line, + std::map& loaded, + std::vector>& ordered) { + std::size_t keyStart = skipPropertyWhitespace(line); + if (keyStart == line.size() || line[keyStart] == '#' || line[keyStart] == '!') return; + + std::size_t keyEnd = keyStart; + bool escaped = false; + for (; keyEnd < line.size(); ++keyEnd) { + const char c = line[keyEnd]; + if (c == '\\') { + escaped = !escaped; + continue; + } + if (!escaped && (isPropertyWhitespace(c) || c == '=' || c == ':')) break; + escaped = false; + } + + std::size_t valueStart = keyEnd; + if (valueStart < line.size()) { + if (isPropertyWhitespace(line[valueStart])) { + valueStart = skipPropertyWhitespace(std::string_view(line).substr(valueStart)) + + valueStart; + if (valueStart < line.size() && (line[valueStart] == '=' || line[valueStart] == ':')) + ++valueStart; + } else { + ++valueStart; + } + valueStart += skipPropertyWhitespace(std::string_view(line).substr(valueStart)); + } + + const std::string key = decodeEscapes( + std::string_view(line).substr(keyStart, keyEnd - keyStart)); + const std::string value = decodeEscapes(std::string_view(line).substr(valueStart)); + loaded[key] = value; + for (auto it = ordered.begin(); it != ordered.end();) { + if (it->first == key) it = ordered.erase(it); + else ++it; + } + ordered.emplace_back(key, value); } public: bool save(const std::string& path) const { + // This legacy helper writes a simple key=value snapshot. It does not + // implement java.util.Properties.store escaping, comments, or encoding. std::ofstream f(path); if (!f) return false; for (auto& [k,v] : props) f << k << "=" << v << "\n"; @@ -109,9 +321,7 @@ class ServerProperties { return parseInteger(v, parsed) ? parsed : def; } else if constexpr (std::is_same_v) { const std::string low = asciiLower(v); - if (low=="true" || low=="1" || low=="yes" || low=="on") return true; - if (low=="false" || low=="0" || low=="no" || low=="off") return false; - return def; + return low == "true"; } else if constexpr (std::is_same_v) { double parsed{}; return parseFloating(v, parsed) ? parsed : def; @@ -151,19 +361,13 @@ class ServerProperties { static std::string asciiLower(std::string value) { std::transform(value.begin(), value.end(), value.begin(), [](unsigned char c) { - return static_cast(std::tolower(c)); + return static_cast(c >= 'A' && c <= 'Z' ? c + ('a' - 'A') : c); }); return value; } PropertyIterator findProperty(const std::string& key) const { - auto it = props.find(key); - if (it != props.end()) return it; - const std::string loweredKey = asciiLower(key); - for (auto candidate = props.begin(); candidate != props.end(); ++candidate) { - if (asciiLower(candidate->first) == loweredKey) return candidate; - } - return props.end(); + return props.find(key); } template diff --git a/tests/test_core_safety.cpp b/tests/test_core_safety.cpp index 07eb62c4..69d99b7c 100644 --- a/tests/test_core_safety.cpp +++ b/tests/test_core_safety.cpp @@ -134,8 +134,10 @@ void testConfigurationAndEncoding() { properties.props["bad-int"] = "12trailing"; properties.props["bad-float"] = "nan"; properties.props["bad-range"] = "1e309"; - check(properties.get("view-distance", 6) == 12, - "server properties lookup is ASCII case-insensitive"); + check(properties.get("View-Distance", 6) == 12 && + properties.get("view-distance", 6) == 6 && + !properties.has("VIEW-DISTANCE"), + "server properties lookup follows Java's case-sensitive property keys"); check(properties.get("bad-int", 7) == 7, "server properties reject trailing integer data"); check(properties.get("bad-float", 3.5f) == 3.5f, diff --git a/tests/test_properties.cpp b/tests/test_properties.cpp index ec52df6d..421eaa4d 100644 --- a/tests/test_properties.cpp +++ b/tests/test_properties.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -50,18 +51,27 @@ void testPropertiesSyntax() { " # comment with CRLF\r\n" "\tview-distance = 4\r\n" "VIEW-DISTANCE=8\r\n" + "view-distance=6\r\n" "motd = path with spaces # inline text is data\r\n" "empty =\r\n" - "ignored line without equals\r\n" + "bare-property\r\n" "final-value=kept-without-newline"), "properties accept whitespace, CRLF, comments, and final line without newline"); - check(properties.get("view-distance", -1) == 8, - "duplicate keys and case variants use the last value"); + check(properties.get("view-distance", -1) == 6 && + properties.get("VIEW-DISTANCE", -1) == 8 && + properties.props.size() == 6, + "exact duplicate keys use the last value while case variants remain distinct"); + check(properties.parsedEntries().size() == 6 && + properties.parsedEntries()[0].first == "VIEW-DISTANCE" && + properties.parsedEntries()[1].first == "view-distance", + "effective entries retain source order by each exact key's last occurrence"); check(properties.getString("motd") == "path with spaces # inline text is data", "values preserve interior spaces and hash characters"); check(properties.getString("empty", "missing").empty(), "empty values remain empty"); check(properties.getString("final-value") == "kept-without-newline", "last non-newline property is retained"); + check(properties.has("bare-property") && properties.getString("bare-property").empty(), + "a bare property name is retained with an empty value"); check(properties.loadText("replacement=one\nview-distance=9"), "a second load replaces the previous property map"); @@ -80,6 +90,110 @@ void testPropertiesSyntax() { std::filesystem::remove(savePath); } +void testJavaPropertiesSyntax() { + std::cout << "\n[Java Properties syntax and escapes]\n"; + struct Fixture { + std::string_view input; + std::string_view key; + std::string_view value; + const char* description; + }; + const std::vector fixtures = { + {" # hash comment\n ! bang comment\nplain=value", "plain", "value", + "leading whitespace and both Java comment markers are recognized"}, + {"colon:key:tail", "colon", "key:tail", "colon is a property separator"}, + {"space-key \t = \t value with spaces", "space-key", "value with spaces", + "whitespace may separate a key and value and is skipped around the separator"}, + {R"(escaped\ key\:\==value)", "escaped key:=", "value", + "escaped key spaces and delimiters are literal key data"}, + {R"(value=left\=middle\:right\q)", "value", "left=middle:rightq", + "escaped value separators and Java's unknown-escape rule are applied"}, + {R"(escapes=\t\n\r\f\\)", "escapes", "\t\n\r\f\\", + "tab, newline, carriage-return, form-feed, and backslash escapes decode"}, + {R"(joined=first\ + second\ + third)", "joined", "firstsecondthird", + "odd trailing backslashes join physical lines and drop continuation indentation"}, + {R"(continued-eof=value\)", "continued-eof", "value", + "a continuation marker at EOF is discarded after the accumulated value"}, + {"continued-blank=value\\\n\nnext=value", "continued-blank", "value", + "an empty continuation line terminates the logical property"}, + {R"(joined-key\ + suffix=value)", "joined-keysuffix", "value", + "continuation also joins key text while skipping next-line indentation"}, + {"bare-key\n=value\ntrailing=value ", "bare-key", "", + "a key without a separator has an empty value"}, + {"trailing=value ", "trailing", "value ", + "trailing value whitespace is preserved"}, + {"=empty-key-value", "", "empty-key-value", "an empty property key is retained"}, + {"inline=hash # and bang ! are data", "inline", "hash # and bang ! are data", + "comment markers inside values are ordinary data"}, + {"formfeed\f:\fvalue", "formfeed", "value", + "form-feed is Java Properties whitespace"}, + {"caf\xc3\xa9=\xe9\x9b\xaa", "caf\xc3\xa9", "\xe9\x9b\xaa", + "valid UTF-8 property text remains valid UTF-8"}, + {R"(rocket=\uD83D\uDE80)", "rocket", "\xf0\x9f\x9a\x80", + "a valid UTF-16 surrogate pair becomes one UTF-8 code point"}, + {R"(orphan=\uD800)", "orphan", "\xef\xbf\xbd", + "an isolated UTF-16 surrogate is represented by UTF-8 replacement character"}, + }; + + for (const auto& fixture : fixtures) { + ServerProperties properties; + const bool loaded = properties.loadText(fixture.input); + const auto it = properties.props.find(std::string(fixture.key)); + check(loaded && it != properties.props.end() && it->second == fixture.value, + fixture.description); + } + + ServerProperties caseSensitive; + check(caseSensitive.loadText("Name=upper\nname=lower") && + caseSensitive.getString("Name") == "upper" && + caseSensitive.getString("name") == "lower" && + !caseSensitive.has("NAME"), + "public lookup uses exact Java Properties key spelling"); + + ServerProperties preserved; + (void)preserved.loadText("stable=before"); + int malformedCount = 0; + for (const std::string_view malformed : {R"(bad=\u12G4)", R"(bad=\u123)"}) { + bool rejected = false; + try { + (void)preserved.loadText(malformed); + } catch (const std::invalid_argument&) { + rejected = true; + } + if (rejected && preserved.getString("stable") == "before") ++malformedCount; + } + check(malformedCount == 2, + "malformed Unicode escapes throw and leave the previous property map intact"); + + int trueCount = 0; + for (const std::string_view value : {"true", "TRUE", "TrUe"}) { + ServerProperties boolean; + const std::string fixture = "flag=" + std::string(value); + if (boolean.loadText(fixture) && boolean.get("flag", false)) ++trueCount; + } + int falseCount = 0; + for (const std::string_view value : {"false", "FALSE", "yes", "1", "on", "true "}) { + ServerProperties boolean; + const std::string fixture = "flag=" + std::string(value); + if (boolean.loadText(fixture) && !boolean.get("flag", true)) ++falseCount; + } + ServerProperties leadingSpaceBoolean; + const bool leadingSpaceIsFalse = leadingSpaceBoolean.loadText(R"(flag=\ true)") && + !leadingSpaceBoolean.get("flag", true); + ServerProperties missingBoolean; + check(trueCount == 3 && falseCount == 6 && leadingSpaceIsFalse && + missingBoolean.get("missing", true), + "boolean getter matches Boolean.parseBoolean; only missing keys use the API default"); + + ServerProperties ignoredCommentEscape; + check(ignoredCommentEscape.loadText("# ignored \\u12G4\nkey=value") && + ignoredCommentEscape.getString("key") == "value", + "malformed escape-looking text in a comment is not decoded"); +} + void testSupportedProperties() { std::cout << "\n[supported properties, aliases, typed fallback, and clamping]\n"; ServerConfig config; @@ -97,20 +211,20 @@ void testSupportedProperties() { "start-time=-7\n" "rcon.port=0\n" "rcon.password=secret\n" - "enable-rcon=YeS\n" - "whitelist=on\n" + "enable-rcon=TRUE\n" + "whitelist=false\n" "whitelist=true\n" "onlineMode=false\n" - "onlineMode=0\n" + "onlineMode=false\n" "enforces-secure-chat=true\n" "compression-threshold=-4\n" "maxLoadedChunks=0\n" "io-worker-threads=99\n" - "pvp=no\n" - "allowFlight=YES\n" - "hardcore=1\n" - "jvm-enabled=off\n" - "jvm-strict=ON\n" + "pvp=false\n" + "allowFlight=TRUE\n" + "hardcore=true\n" + "jvm-enabled=false\n" + "jvm-strict=TRUE\n" "jvm-mods=mods with spaces\n" "jvm-config=config with spaces\n"), "supported properties fixture parses"); @@ -134,7 +248,7 @@ void testSupportedProperties() { "level, RCON, and boolean properties apply through aliases"); check(!config.pvp && config.allowFlight && config.hardcore && !config.jvmEnabled && config.jvmStrict, - "boolean aliases accept case-insensitive true/false spellings"); + "configuration accepts canonical case-insensitive Java true/false spellings"); check(config.jvmModsDir == "mods with spaces" && config.jvmConfigDir == "config with spaces", "JVM paths with spaces are not split or normalized"); @@ -193,6 +307,45 @@ void testFileLoadingAndPrecedence() { config.motd == "file value" && config.worldDir == "file world", "file values are loaded before CLI overrides"); + const auto utf8Path = base / "utf8.properties"; + { + std::ofstream output(utf8Path, std::ios::binary); + output << "motd=caf\xc3\xa9 \xe9\x9b\xaa\n"; + } + ServerProperties utf8Properties; + check(utf8Properties.load(utf8Path.string()) && + utf8Properties.getString("motd") == "caf\xc3\xa9 \xe9\x9b\xaa", + "file loading accepts strict UTF-8 and preserves non-ASCII code points"); + + const auto latin1Path = base / "latin1.properties"; + { + std::ofstream output(latin1Path, std::ios::binary); + std::string fixture = "motd=caf"; + fixture.push_back(static_cast(0xe9)); + fixture.push_back('\n'); + output.write(fixture.data(), static_cast(fixture.size())); + } + ServerProperties latin1Properties; + check(latin1Properties.load(latin1Path.string()) && + latin1Properties.getString("motd") == "caf\xc3\xa9", + "invalid UTF-8 file bytes retry as ISO-8859-1 and become valid UTF-8"); + + const auto malformedPath = base / "malformed-unicode.properties"; + { + std::ofstream output(malformedPath, std::ios::binary); + output << R"(bad=\u12G4)" << '\n'; + } + ServerProperties malformedFile; + (void)malformedFile.loadText("stable=before"); + bool malformedFileRejected = false; + try { + (void)malformedFile.load(malformedPath.string()); + } catch (const std::invalid_argument&) { + malformedFileRejected = true; + } + check(malformedFileRejected && malformedFile.getString("stable") == "before", + "malformed Unicode escapes in a file are rejected without partial state changes"); + const CommandLineOptions options = parseCommandLine({ "--port=0", "--view-distance", "32", "--motd", "CLI value with spaces", "--world-dir=CLI world with spaces", "--max-players", "9", "--jvm", "YES"}); @@ -289,6 +442,7 @@ void testInformationalFlags(const char* binaryArgument) { int main(int argc, char** argv) { testPropertiesSyntax(); + testJavaPropertiesSyntax(); testSupportedProperties(); testFileLoadingAndPrecedence(); testInformationalFlags(argc > 1 ? argv[1] : nullptr); From 2dc083e7d4c1047b93eef20640e397b0555a49c0 Mon Sep 17 00:00:00 2001 From: nico2525nn <173241093+nico2525nn@users.noreply.github.com> Date: Wed, 23 Sep 2026 13:22:01 +0900 Subject: [PATCH 02/10] fix: use vanilla view distance fallback --- src/game/ServerProperties.hpp | 3 ++- tests/test_properties.cpp | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/game/ServerProperties.hpp b/src/game/ServerProperties.hpp index d5b295e5..2e6608a5 100644 --- a/src/game/ServerProperties.hpp +++ b/src/game/ServerProperties.hpp @@ -344,7 +344,8 @@ class ServerProperties { // Convenience typed getters matching spec: get(key), get(key) Usage: props.get("viewDistance") etc. // Apply to ServerConfig helpers - int viewDistance() const { return get("view-distance", get("viewDistance", 6)); } + // Legacy convenience accessor; ServerConfig remains the canonical startup path. + int viewDistance() const { return get("view-distance", get("viewDistance", 10)); } int simulationDistance() const { return get("simulation-distance", get("simulationDistance", 10)); } int spawnProtection() const { return get("spawn-protection", 16); } // Chebyshev-sorted + burst 16/tick with forced/spawn ticket protection — NOT a simple clear(). diff --git a/tests/test_properties.cpp b/tests/test_properties.cpp index 421eaa4d..74ac482e 100644 --- a/tests/test_properties.cpp +++ b/tests/test_properties.cpp @@ -46,6 +46,14 @@ bool hasDiagnostic(const ConfigDiagnostics& diagnostics, ConfigDiagnosticKind ki void testPropertiesSyntax() { std::cout << "\n[properties syntax and duplicate keys]\n"; + ServerProperties defaultDistance; + check(defaultDistance.viewDistance() == 10, + "legacy view-distance helper uses the vanilla 1.21.4 default of 10"); + ServerProperties camelCaseDistance; + check(camelCaseDistance.loadText("viewDistance=7") && + camelCaseDistance.viewDistance() == 7, + "legacy view-distance helper retains its explicit camel-case alias"); + ServerProperties properties; check(properties.loadText( " # comment with CRLF\r\n" From 176a665bf567b7e6f7ae408c1242e0ccc6546573 Mon Sep 17 00:00:00 2001 From: nico2525nn <173241093+nico2525nn@users.noreply.github.com> Date: Wed, 23 Sep 2026 13:06:47 +0900 Subject: [PATCH 03/10] docs: align first-run defaults with vanilla 1.21.4 --- README.md | 23 ++++++++++-- server.properties.example | 40 ++++++++++----------- src/core/RuntimeLayout.cpp | 24 +++++++++++-- tests/test_runtime_layout.cpp | 67 +++++++++++++++++++++++++++++++++-- 4 files changed, 126 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 4e9f15cf..ea2fe6c5 100644 --- a/README.md +++ b/README.md @@ -249,9 +249,26 @@ extension surface, not a copy of the official Minecraft server or a promise that every Fabric mod can run unchanged. It does not ship the official Mojang GameProvider, client, or GUI runtime. -Connect with a Minecraft 1.21.4 client in offline mode, for example by using a -launcher profile pointed at `127.0.0.1`. The production default is normal terrain; -set `level-type=flat` explicitly when a creative superflat fixture is wanted. +On its first launch, cppfm creates `server.properties` with the supported +vanilla-facing Minecraft 1.21.4 defaults, including `motd=A Minecraft Server`, +`difficulty=easy`, `view-distance=10`, an empty `level-seed`, +`online-mode=true`, and `enforce-secure-profile=true`. An empty seed selects a +random seed for a new world; a saved world's seed is retained from `level.dat`. +Existing `server.properties` files are preserved. +Normal startup therefore expects clients to authenticate and satisfy secure +profile enforcement, as they do when joining a vanilla online-mode server. + +For an intentionally offline local test, launch with both authentication +checks disabled and point an offline launcher profile at `127.0.0.1`: + +```bash +./build/cppfm --online-mode=false --enforce-secure-profile=false +``` + +Both options must be explicit for offline clients; this is a development/test +mode, not the compatibility-oriented default. The default world uses normal +terrain; set `level-type=flat` explicitly when a creative superflat fixture is +wanted. ## Running tests diff --git a/server.properties.example b/server.properties.example index e9bc2671..a1aa6639 100644 --- a/server.properties.example +++ b/server.properties.example @@ -1,21 +1,27 @@ -# server.properties (supported CppFabricMC settings) +# Supported cppfm server.properties settings. Unsupported vanilla keys are +# intentionally omitted instead of being shown as if they changed behavior. server-port=25565 max-players=20 -view-distance=6 +view-distance=10 simulation-distance=10 -level-type=normal -motd=CppFabricMC - C++ Minecraft 1.21.4 server +level-type=minecraft:normal +difficulty=easy +motd=A Minecraft Server +level-seed= spawn-protection=16 -# online-mode=false -# whitelist=false -# pvp=true -# allow-flight=false -# hardcore=false -# enforce-secure-profile=false -# network-compression-threshold=256 -# enable-rcon=false -# rcon.port=25575 -# rcon.password= +online-mode=true +white-list=false +pvp=true +allow-flight=false +hardcore=false +enforce-secure-profile=true +network-compression-threshold=256 +enable-rcon=false +rcon.port=25575 +rcon.password= +resource-pack= +resource-pack-sha1= +require-resource-pack=false # Java integration is enabled by default when a compatible JDK is available. # Set jvm=false for a native-only run, or jvm-strict=true to fail startup when # the JDK/embedded compatibility classes cannot be loaded. @@ -26,7 +32,6 @@ jvm-strict=false # Maximum loaded chunks (0=unlimited). Default is max(8192, view-distance²*4). #max-loaded-chunks=8192 -#compression-threshold=256 #io-worker-threads=4 # Optional path/JVM settings: values may contain spaces. @@ -37,8 +42,3 @@ jvm-strict=false #jvm-java-home= #jvm-library= #jvm-libraries= - -# Unsupported vanilla properties are intentionally not emulated by this server; -# for example difficulty, gamemode, generator-settings, spawn-animals, -# spawn-monsters, generate-structures, resource-pack, and query.port are -# reported as unsupported and do not change runtime configuration. diff --git a/src/core/RuntimeLayout.cpp b/src/core/RuntimeLayout.cpp index d8e75eff..ba847ff3 100644 --- a/src/core/RuntimeLayout.cpp +++ b/src/core/RuntimeLayout.cpp @@ -335,10 +335,28 @@ bool ensureDefaultProperties(const std::filesystem::path& root, std::string& err const std::string defaults = "server-port=25565\n" "max-players=20\n" - "view-distance=6\n" + "view-distance=10\n" "simulation-distance=10\n" - "level-type=normal\n" - "motd=CppFabricMC - C++ Minecraft 1.21.4 server\n"; + "level-type=minecraft:normal\n" + "difficulty=easy\n" + "motd=A Minecraft Server\n" + "level-seed=\n" + "spawn-protection=16\n" + "online-mode=true\n" + "white-list=false\n" + "pvp=true\n" + "allow-flight=false\n" + "hardcore=false\n" + "enforce-secure-profile=true\n" + "network-compression-threshold=256\n" + "enable-rcon=false\n" + "rcon.port=25575\n" + "rcon.password=\n" + "resource-pack=\n" + "resource-pack-sha1=\n" + "require-resource-pack=false\n" + "jvm=true\n" + "jvm-strict=false\n"; const std::vector bytes(defaults.begin(), defaults.end()); return writeIfMissing(root, "server.properties", bytes, error); } diff --git a/tests/test_runtime_layout.cpp b/tests/test_runtime_layout.cpp index 72cd5e7d..2e0c44a4 100644 --- a/tests/test_runtime_layout.cpp +++ b/tests/test_runtime_layout.cpp @@ -8,7 +8,9 @@ #include #include #include +#include #include +#include #include #include @@ -26,6 +28,58 @@ bool fileContains(const std::filesystem::path& path, const std::string& expected return input.good() || input.eof() ? value == expected : false; } +bool readFile(const std::filesystem::path& path, std::string& value) { + std::ifstream input(path, std::ios::binary); + if (!input) return false; + value.assign(std::istreambuf_iterator(input), + std::istreambuf_iterator()); + return !input.bad(); +} + +std::map activeProperties(std::string_view contents) { + std::map properties; + std::size_t lineStart = 0; + while (lineStart < contents.size()) { + const auto lineEnd = contents.find('\n', lineStart); + auto line = contents.substr(lineStart, + lineEnd == std::string_view::npos ? contents.size() - lineStart + : lineEnd - lineStart); + if (!line.empty() && line.back() == '\r') line.remove_suffix(1); + if (!line.empty() && line.front() != '#' && line.front() != '!') { + const auto equals = line.find('='); + if (equals != std::string_view::npos) + properties[std::string(line.substr(0, equals))] = + std::string(line.substr(equals + 1)); + } + if (lineEnd == std::string_view::npos) break; + lineStart = lineEnd + 1; + } + return properties; +} + +bool hasExpectedFirstRunProperties(const std::filesystem::path& path) { + std::string contents; + if (!readFile(path, contents)) return false; + const auto properties = activeProperties(contents); + const std::map expected = { + {"server-port", "25565"}, {"max-players", "20"}, + {"view-distance", "10"}, {"simulation-distance", "10"}, + {"level-type", "minecraft:normal"}, {"difficulty", "easy"}, + {"motd", "A Minecraft Server"}, {"level-seed", ""}, + {"spawn-protection", "16"}, {"online-mode", "true"}, + {"white-list", "false"}, + {"pvp", "true"}, {"allow-flight", "false"}, + {"hardcore", "false"}, {"enforce-secure-profile", "true"}, + {"network-compression-threshold", "256"}, {"enable-rcon", "false"}, + {"rcon.port", "25575"}, {"rcon.password", ""}, + {"resource-pack", ""}, {"resource-pack-sha1", ""}, + {"require-resource-pack", "false"}, {"jvm", "true"}, + {"jvm-strict", "false"}, + }; + // Exact equality also rejects unsupported/unknown active vanilla keys. + return properties == expected; +} + } // namespace int main() { @@ -78,10 +132,18 @@ int main() { const std::vector required = { "world", "world/region", "mods", "config", "libraries", "logs", "crash-reports", "resourcepacks", ".cppfm/jvm/classes", - "server.properties", "assets/registry/tags.bin"}; + "server.properties", "server.properties.example", + "assets/registry/tags.bin"}; bool pass = prepared; for (const auto& relative : required) pass = pass && pathExists(root / relative); pass = pass && fileContains(root / "assets/registry/tags.bin", "user-owned"); + std::string exampleContents; + std::string copiedContents; + pass = pass && readFile(root / "server.properties.example", exampleContents) && + readFile(root / "server.properties", copiedContents) && + copiedContents == exampleContents && + hasExpectedFirstRunProperties(root / "server.properties.example") && + hasExpectedFirstRunProperties(root / "server.properties"); selected = root; std::string secondError; @@ -95,7 +157,8 @@ int main() { if (!pass) { std::cerr << "runtime layout test failed: " - << (error.empty() ? secondError : error) << '\n'; + << (error.empty() ? secondError : error) + << " (including first-run properties template/default checks)\n"; std::filesystem::remove_all(root, ec); return 1; } From 9d31a47f4640e902230a2f6562b8e7993a06aa94 Mon Sep 17 00:00:00 2001 From: nico2525nn <173241093+nico2525nn@users.noreply.github.com> Date: Wed, 23 Sep 2026 13:08:55 +0900 Subject: [PATCH 04/10] test: cover no-pack runtime defaults --- tests/test_runtime_layout.cpp | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/tests/test_runtime_layout.cpp b/tests/test_runtime_layout.cpp index 2e0c44a4..e0be350d 100644 --- a/tests/test_runtime_layout.cpp +++ b/tests/test_runtime_layout.cpp @@ -83,11 +83,6 @@ bool hasExpectedFirstRunProperties(const std::filesystem::path& path) { } // namespace int main() { - if (!cppfm::embedded::kHasPack) { - std::cout << "runtime layout: embedded pack unavailable; skipped\n"; - return 0; - } - const auto suffix = std::chrono::steady_clock::now().time_since_epoch().count(); const auto root = std::filesystem::temp_directory_path() / ("cppfm-runtime-layout-" + std::to_string(suffix)); @@ -132,18 +127,24 @@ int main() { const std::vector required = { "world", "world/region", "mods", "config", "libraries", "logs", "crash-reports", "resourcepacks", ".cppfm/jvm/classes", - "server.properties", "server.properties.example", - "assets/registry/tags.bin"}; + "server.properties", "assets/registry/tags.bin"}; bool pass = prepared; for (const auto& relative : required) pass = pass && pathExists(root / relative); pass = pass && fileContains(root / "assets/registry/tags.bin", "user-owned"); - std::string exampleContents; - std::string copiedContents; - pass = pass && readFile(root / "server.properties.example", exampleContents) && - readFile(root / "server.properties", copiedContents) && - copiedContents == exampleContents && - hasExpectedFirstRunProperties(root / "server.properties.example") && - hasExpectedFirstRunProperties(root / "server.properties"); + pass = pass && hasExpectedFirstRunProperties(root / "server.properties"); + if (cppfm::embedded::kHasPack) { + std::string exampleContents; + std::string copiedContents; + pass = pass && pathExists(root / "server.properties.example") && + readFile(root / "server.properties.example", exampleContents) && + readFile(root / "server.properties", copiedContents) && + copiedContents == exampleContents && + hasExpectedFirstRunProperties(root / "server.properties.example"); + } else { + // No-pack builds cannot extract the example and must exercise the + // compiled-in first-run fallback instead of skipping this contract. + pass = pass && !pathExists(root / "server.properties.example"); + } selected = root; std::string secondError; @@ -163,6 +164,8 @@ int main() { return 1; } std::filesystem::remove_all(root, ec); - std::cout << "runtime layout: PASS\n"; + std::cout << "runtime layout: PASS" + << (cppfm::embedded::kHasPack ? " (embedded template)\n" + : " (no-pack fallback)\n"); return 0; } From b1fa42e73f61fff3ae25148a9a51b4e01e65a602 Mon Sep 17 00:00:00 2001 From: nico2525nn <173241093+nico2525nn@users.noreply.github.com> Date: Wed, 23 Sep 2026 13:22:23 +0900 Subject: [PATCH 05/10] plan56: make offline harness authentication explicit --- tests/ServerProcess.hpp | 10 ++++- tests/bot_smoke.py | 3 +- tests/multi_client_test.py | 3 +- tests/soak_test.py | 2 +- tests/stress_test.py | 3 +- tests/test_goal_live_matrix.py | 2 + tests/test_goal_security_guards.py | 68 ++++++++++++++++++++++++++++++ tests/test_lifecycle_matrix.py | 1 + tests/test_server_full.py | 3 +- tools/replay_vanilla.py | 3 +- tools/soak_bot.py | 2 + 11 files changed, 92 insertions(+), 8 deletions(-) diff --git a/tests/ServerProcess.hpp b/tests/ServerProcess.hpp index 67533b99..c4d3d5d3 100644 --- a/tests/ServerProcess.hpp +++ b/tests/ServerProcess.hpp @@ -33,7 +33,10 @@ struct ServerProcessOptions { // at y=-61). Keep that test fixture explicit now that the production // default matches vanilla normal terrain. std::string levelType = "flat"; + // These live protocol fixtures use fake/offline clients. Keep both + // authentication settings explicit rather than inheriting server defaults. bool onlineMode = false; + bool enforceSecureProfile = false; std::string motd; std::string worldPrefix = "/tmp/cppfm-test-"; int readyTimeoutMs = 30000; @@ -100,13 +103,16 @@ class ServerProcess { worldDir.c_str()); const char* onlineArg = options.onlineMode ? "--online-mode=true" : "--online-mode=false"; + const char* secureProfileArg = options.enforceSecureProfile + ? "--enforce-secure-profile=true" : "--enforce-secure-profile=false"; const std::string motdArg = "--motd=" + options.motd; if (options.motd.empty()) { execl(serverPathAbs.c_str(), serverPathAbs.c_str(), portArg, viewArg, worldArg, - levelArg.c_str(), onlineArg, static_cast(nullptr)); + levelArg.c_str(), onlineArg, secureProfileArg, + static_cast(nullptr)); } else { execl(serverPathAbs.c_str(), serverPathAbs.c_str(), portArg, viewArg, worldArg, - levelArg.c_str(), onlineArg, motdArg.c_str(), + levelArg.c_str(), onlineArg, secureProfileArg, motdArg.c_str(), static_cast(nullptr)); } _exit(127); diff --git a/tests/bot_smoke.py b/tests/bot_smoke.py index ca59ada3..6a4b17a6 100644 --- a/tests/bot_smoke.py +++ b/tests/bot_smoke.py @@ -162,7 +162,8 @@ def free_port(): environment["CPPFM_SERVER_DIR"] = world_dir try: proc = subprocess.Popen([binary, "--port", str(port), "--view-distance", "6", - "--world-dir", world_dir, "--level-type", "flat"], + "--world-dir", world_dir, "--level-type", "flat", + "--online-mode=false", "--enforce-secure-profile=false"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, cwd=cwd, env=environment, start_new_session=True) diff --git a/tests/multi_client_test.py b/tests/multi_client_test.py index dc274060..4d7e5650 100644 --- a/tests/multi_client_test.py +++ b/tests/multi_client_test.py @@ -149,7 +149,8 @@ def free_port(): environment["CPPFM_SERVER_DIR"] = world_dir try: proc = subprocess.Popen([binary, "--port", str(port), "--view-distance", "6", - "--world-dir", world_dir, "--level-type", "flat"], + "--world-dir", world_dir, "--level-type", "flat", + "--online-mode=false", "--enforce-secure-profile=false"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, cwd=cwd, env=environment, start_new_session=True) diff --git a/tests/soak_test.py b/tests/soak_test.py index 7ee2cddd..0183ecd1 100644 --- a/tests/soak_test.py +++ b/tests/soak_test.py @@ -315,7 +315,7 @@ def main(): try: s.bind(("127.0.0.1", port)); s.close(); break except OSError: s.close(); port+=1 world_dir=tempfile.mkdtemp(prefix=f"cppfm-soak-{os.getpid()}-") - cmd=[binary, f"--port={port}", f"--view-distance={args.view_distance}", f"--world-dir={world_dir}", "--level-type=flat", "--online-mode=false"] + cmd=[binary, f"--port={port}", f"--view-distance={args.view_distance}", f"--world-dir={world_dir}", "--level-type=flat", "--online-mode=false", "--enforce-secure-profile=false"] print(f"[soak] starting server {' '.join(cmd)} for {duration}s") server_log = None stop_event = threading.Event() diff --git a/tests/stress_test.py b/tests/stress_test.py index 5818e811..5d6e092b 100644 --- a/tests/stress_test.py +++ b/tests/stress_test.py @@ -247,7 +247,8 @@ def main(): world_dir = tempfile.mkdtemp(prefix=f"cppfm-stress-{os.getpid()}-") cmd = [binary, f"--port={port}", f"--view-distance={args.view_distance}", f"--max-players={max_players}", - f"--world-dir={world_dir}", "--level-type=flat", "--online-mode=false"] + f"--world-dir={world_dir}", "--level-type=flat", "--online-mode=false", + "--enforce-secure-profile=false"] print(f"[stress] starting server {' '.join(cmd)} for {n} clients") environment = os.environ.copy() environment["CPPFM_SERVER_DIR"] = world_dir diff --git a/tests/test_goal_live_matrix.py b/tests/test_goal_live_matrix.py index 78324918..2677cadd 100644 --- a/tests/test_goal_live_matrix.py +++ b/tests/test_goal_live_matrix.py @@ -282,6 +282,7 @@ def launch(name: str, port: int, property_port: int, view: int, motd: str) -> tu "view-distance=2", "simulation-distance=2", "online-mode=false", + "enforce-secure-profile=false", "jvm=false", "compression-threshold=256", "level-seed=goal-live-seed", @@ -292,6 +293,7 @@ def launch(name: str, port: int, property_port: int, view: int, motd: str) -> tu str(binary), f"--port={port}", f"--world-dir={world}", "--level-type=flat", f"--view-distance={view}", "--simulation-distance=2", f"--motd={motd}", "--jvm=false", + "--online-mode=false", "--enforce-secure-profile=false", ] server = OwnedServer(command, root, root / "server.log") active.append(server) diff --git a/tests/test_goal_security_guards.py b/tests/test_goal_security_guards.py index d1f17c44..f7cbc268 100644 --- a/tests/test_goal_security_guards.py +++ b/tests/test_goal_security_guards.py @@ -14,6 +14,12 @@ def between(text, start, end): return text.split(start, 1)[1].split(end, 1)[0] +def require_offline_auth_flags(label, command_source): + require('"--online-mode=false"' in command_source and + '"--enforce-secure-profile=false"' in command_source, + f"{label} fake-client launch explicitly disables both authentication checks") + + def main(): config = (ROOT / "src/game/ServerConfig.cpp").read_text() profile = between(config, "case ConfigKey::SecureProfile:", @@ -56,6 +62,68 @@ def main(): require("predicateRandom01(ctx)" in predicates and "lootingLevel" in predicates, "loot predicates use seeded randomness and looting context") + server_process = (ROOT / "tests/ServerProcess.hpp").read_text() + options = between(server_process, "struct ServerProcessOptions {", "class ServerProcess {") + require("bool onlineMode = false;" in options and + "bool enforceSecureProfile = false;" in options, + "shared live-server fixture defaults to explicit offline authentication") + spawn_args = between(server_process, "const char* onlineArg", "_exit(127);") + require('"--online-mode=true"' in spawn_args and + '"--enforce-secure-profile=true"' in spawn_args and + spawn_args.count("onlineArg, secureProfileArg") == 2, + "shared fixture retains independent explicit true options for online scenarios") + require_offline_auth_flags("ServerProcess", spawn_args) + + lifecycle = (ROOT / "tests/test_lifecycle_matrix.py").read_text() + require_offline_auth_flags( + "lifecycle matrix", + between(lifecycle, "def _server_command(", "\n\ndef _start_server(")) + + goal_live = (ROOT / "tests/test_goal_live_matrix.py").read_text() + require_offline_auth_flags( + "goal live matrix", + between(goal_live, " def launch(name:", " server = OwnedServer(command")) + + server_full = (ROOT / "tests/test_server_full.py").read_text() + full_launcher = between(server_full, "def launch_server(", "\ndef run_owned_command(") + require_offline_auth_flags("server_full", full_launcher) + require(full_launcher.index('"--enforce-secure-profile=false"') < + full_launcher.index("if extra_args:") < full_launcher.index("args += extra_args"), + "server_full leaves explicit caller arguments last for CLI precedence") + + bot_smoke = (ROOT / "tests/bot_smoke.py").read_text() + require_offline_auth_flags( + "bot_smoke", + between(bot_smoke, 'proc = subprocess.Popen([binary, "--port"', + "wait_for_server(proc, host, port)")) + + multi_client = (ROOT / "tests/multi_client_test.py").read_text() + require_offline_auth_flags( + "multi_client", + between(multi_client, 'proc = subprocess.Popen([binary, "--port"', + "wait_for_server(proc, host, port)")) + + stress = (ROOT / "tests/stress_test.py").read_text() + require_offline_auth_flags( + "stress_test", between(stress, "cmd = [binary", 'print(f"[stress] starting server')) + + soak = (ROOT / "tests/soak_test.py").read_text() + require_offline_auth_flags( + "soak_test", between(soak, "cmd=[binary", 'print(f"[soak] starting server')) + + replay = (ROOT / "tools/replay_vanilla.py").read_text() + require_offline_auth_flags( + "replay_vanilla", between(replay, "def launch(", "\ndef wait_ready(")) + + soak_bot = (ROOT / "tools/soak_bot.py").read_text() + require_offline_auth_flags( + "soak_bot", + between(soak_bot, "proc = subprocess.Popen(\n [", " stdout=subprocess.DEVNULL")) + + plan43 = (ROOT / "tests/run_plan43_suite.py").read_text() + require("from test_server_full import" in plan43 and "launch_server(binary" in plan43, + "plan43 suite uses server_full's explicitly-offline launch helper") + print("goal_security_guards: PASS") diff --git a/tests/test_lifecycle_matrix.py b/tests/test_lifecycle_matrix.py index 8f34b8e3..5d276216 100644 --- a/tests/test_lifecycle_matrix.py +++ b/tests/test_lifecycle_matrix.py @@ -744,6 +744,7 @@ def _server_command( "--view-distance=2", "--jvm=false", "--online-mode=false", + "--enforce-secure-profile=false", *extra, ] diff --git a/tests/test_server_full.py b/tests/test_server_full.py index 8a46c6c1..64028c5a 100644 --- a/tests/test_server_full.py +++ b/tests/test_server_full.py @@ -153,7 +153,8 @@ def wait_for_status(host: str, port: int, timeout: float = 12.0, def launch_server(binary: str, port: int, world_dir: str, extra_env: dict | None = None, extra_args: list[str] | None = None): # This suite uses the legacy y=-61 fixture coordinates. Keep that fixture # explicit now that the production default is vanilla normal terrain. - args = [binary, f"--port={port}", f"--world-dir={world_dir}", "--level-type=flat"] + args = [binary, f"--port={port}", f"--world-dir={world_dir}", "--level-type=flat", + "--online-mode=false", "--enforce-secure-profile=false"] if extra_args: args += extra_args env = os.environ.copy() diff --git a/tools/replay_vanilla.py b/tools/replay_vanilla.py index 1e35a1d4..83ef8a0c 100644 --- a/tools/replay_vanilla.py +++ b/tools/replay_vanilla.py @@ -36,7 +36,8 @@ def launch(binary: str, port: int, world_dir: str): environment = os.environ.copy() environment["CPPFM_SERVER_DIR"] = world_dir proc = subprocess.Popen([binary, f"--port={port}", f"--world-dir={world_dir}", - "--view-distance=4", "--level-type=flat", "--online-mode=false"], + "--view-distance=4", "--level-type=flat", "--online-mode=false", + "--enforce-secure-profile=false"], stdout=logf, stderr=subprocess.STDOUT, env=environment, start_new_session=True) diff --git a/tools/soak_bot.py b/tools/soak_bot.py index 66e76e22..44a4d62c 100644 --- a/tools/soak_bot.py +++ b/tools/soak_bot.py @@ -407,6 +407,8 @@ def observe_packet(pid: int, data: bytes) -> None: f"--view-distance={args.view_distance}", f"--world-dir={world_dir}", "--level-type=flat", + "--online-mode=false", + "--enforce-secure-profile=false", ], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, From b60eb4235ff589d64551c63a508aeba8d3348bbb Mon Sep 17 00:00:00 2001 From: nico2525nn <173241093+nico2525nn@users.noreply.github.com> Date: Wed, 23 Sep 2026 13:24:06 +0900 Subject: [PATCH 06/10] plan56: align vanilla config defaults and property semantics --- src/game/ServerConfig.cpp | 100 ++++++++++++++++++++----------- src/game/ServerConfig.hpp | 20 ++++--- tests/test_settings_matrix.cpp | 105 +++++++++++++++++++++++++++++++-- 3 files changed, 176 insertions(+), 49 deletions(-) diff --git a/src/game/ServerConfig.cpp b/src/game/ServerConfig.cpp index 85bd4ba4..cad243ab 100644 --- a/src/game/ServerConfig.cpp +++ b/src/game/ServerConfig.cpp @@ -67,7 +67,7 @@ struct KeySpec { // This is the only application key registry. In particular, an entry here // does not imply support for every similarly named vanilla property. Unknown // server.properties keys are reported as unsupported by applyServerProperties. -constexpr std::array kKeySpecs{{ +constexpr std::array kKeySpecs{{ {"server-port", ConfigKey::Port, true, true}, {"port", ConfigKey::Port, true, true}, {"max-players", ConfigKey::MaxPlayers, true, true}, @@ -89,7 +89,8 @@ constexpr std::array kKeySpecs{{ {"rcon.port", ConfigKey::RconPort, true, true}, {"rcon.password", ConfigKey::RconPassword, true, true}, {"enable-rcon", ConfigKey::EnableRcon, true, true}, - {"whitelist", ConfigKey::Whitelist, true, true}, + {"white-list", ConfigKey::Whitelist, true, true}, + {"whitelist", ConfigKey::Whitelist, true, true}, // explicit legacy alias {"online-mode", ConfigKey::OnlineMode, true, true}, {"onlineMode", ConfigKey::OnlineMode, true, true}, {"enforce-secure-profile", ConfigKey::SecureProfile, true, true}, @@ -134,10 +135,13 @@ std::string asciiLower(std::string value) { } const KeySpec* findKey(std::string_view name, bool propertySource) { - const std::string lowered = asciiLower(std::string(name)); + const std::string lowered = propertySource ? std::string{} : asciiLower(std::string(name)); for (const auto& spec : kKeySpecs) { - if ((propertySource ? spec.property : spec.commandLine) && - asciiLower(std::string(spec.name)) == lowered) { + if (!(propertySource ? spec.property : spec.commandLine)) continue; + // java.util.Properties keys are case-sensitive. CLI names have their + // own, deliberately more permissive matching contract. + if ((propertySource && spec.name == name) || + (!propertySource && asciiLower(std::string(spec.name)) == lowered)) { return &spec; } } @@ -151,7 +155,15 @@ bool parseInteger(std::string_view text, T& value) { return result.ec == std::errc{} && result.ptr == text.data() + text.size(); } -bool parseBool(std::string_view text, bool& value) { +bool parsePropertyBool(std::string_view text, bool& value) { + // Vanilla's boolean properties use Boolean.parseBoolean semantics: only + // the string "true" (case-insensitive) becomes true; all other strings + // become false rather than being interpreted as CLI-style synonyms. + value = asciiLower(std::string(text)) == "true"; + return true; +} + +bool parseCliBool(std::string_view text, bool& value) { const std::string lowered = asciiLower(std::string(text)); if (lowered == "true" || lowered == "1" || lowered == "yes" || lowered == "on") { value = true; @@ -164,6 +176,11 @@ bool parseBool(std::string_view text, bool& value) { return false; } +bool parseBool(std::string_view text, bool& value, bool propertySource) { + return propertySource ? parsePropertyBool(text, value) + : parseCliBool(text, value); +} + bool parsePort(std::string_view text, std::uint16_t& port) { std::uint32_t parsed = 0; if (!parseInteger(text, parsed) || parsed > 65535U) return false; @@ -190,20 +207,9 @@ bool parseDifficulty(std::string_view text, std::string& difficulty) { void applySeed(ServerConfig& config, std::string_view value, ConfigDiagnostics* diagnostics, std::string_view originalKey) { if (value.empty()) { - try { - std::random_device rd; - const auto now = static_cast( - std::chrono::steady_clock::now().time_since_epoch().count()); - const auto generated = (static_cast(rd()) << 32U) ^ - static_cast(rd()) ^ now; - config.seed = generated; - config.hashedSeed = static_cast(generated); - } catch (...) { - const auto generated = static_cast( - std::chrono::steady_clock::now().time_since_epoch().count()); - config.seed = generated; - config.hashedSeed = static_cast(generated); - } + const auto generated = server_config_detail::randomWorldSeed(); + config.seed = generated; + config.hashedSeed = static_cast(generated); return; } @@ -227,7 +233,8 @@ void invalid(ConfigDiagnostics* diagnostics, std::string_view key, std::string_v } void applyValue(ServerConfig& config, ConfigKey key, std::string_view value, - ConfigDiagnostics* diagnostics, std::string_view originalKey) { + bool propertySource, ConfigDiagnostics* diagnostics, + std::string_view originalKey) { int intValue = 0; std::int64_t longValue = 0; bool boolValue = false; @@ -275,23 +282,23 @@ void applyValue(ServerConfig& config, ConfigKey key, std::string_view value, config.rcon.password = std::string(value); break; case ConfigKey::EnableRcon: - if (!parseBool(value, boolValue)) invalid(diagnostics, originalKey, value); + if (!parseBool(value, boolValue, propertySource)) invalid(diagnostics, originalKey, value); else config.rcon.enabled = boolValue; break; case ConfigKey::Whitelist: - if (!parseBool(value, boolValue)) invalid(diagnostics, originalKey, value); + if (!parseBool(value, boolValue, propertySource)) invalid(diagnostics, originalKey, value); else config.whitelist = boolValue; break; case ConfigKey::OnlineMode: - if (!parseBool(value, boolValue)) invalid(diagnostics, originalKey, value); + if (!parseBool(value, boolValue, propertySource)) invalid(diagnostics, originalKey, value); else config.onlineMode = boolValue; break; case ConfigKey::SecureProfile: - if (!parseBool(value, boolValue)) invalid(diagnostics, originalKey, value); + if (!parseBool(value, boolValue, propertySource)) invalid(diagnostics, originalKey, value); else config.enforceSecureProfile = boolValue; break; case ConfigKey::SecureChat: - if (!parseBool(value, boolValue)) invalid(diagnostics, originalKey, value); + if (!parseBool(value, boolValue, propertySource)) invalid(diagnostics, originalKey, value); else config.enforcesSecureChat = boolValue; break; case ConfigKey::Difficulty: @@ -307,7 +314,7 @@ void applyValue(ServerConfig& config, ConfigKey key, std::string_view value, config.resourcePackSha1 = std::string(value); break; case ConfigKey::ResourcePackForced: - if (!parseBool(value, boolValue)) invalid(diagnostics, originalKey, value); + if (!parseBool(value, boolValue, propertySource)) invalid(diagnostics, originalKey, value); else config.resourcePackForced = boolValue; break; case ConfigKey::CompressionThreshold: @@ -323,23 +330,23 @@ void applyValue(ServerConfig& config, ConfigKey key, std::string_view value, else config.ioWorkerThreads = std::clamp(intValue, 1, 64); break; case ConfigKey::Pvp: - if (!parseBool(value, boolValue)) invalid(diagnostics, originalKey, value); + if (!parseBool(value, boolValue, propertySource)) invalid(diagnostics, originalKey, value); else config.pvp = boolValue; break; case ConfigKey::AllowFlight: - if (!parseBool(value, boolValue)) invalid(diagnostics, originalKey, value); + if (!parseBool(value, boolValue, propertySource)) invalid(diagnostics, originalKey, value); else config.allowFlight = boolValue; break; case ConfigKey::Hardcore: - if (!parseBool(value, boolValue)) invalid(diagnostics, originalKey, value); + if (!parseBool(value, boolValue, propertySource)) invalid(diagnostics, originalKey, value); else config.hardcore = boolValue; break; case ConfigKey::JvmEnabled: - if (!parseBool(value, boolValue)) invalid(diagnostics, originalKey, value); + if (!parseBool(value, boolValue, propertySource)) invalid(diagnostics, originalKey, value); else config.jvmEnabled = boolValue; break; case ConfigKey::JvmStrict: - if (!parseBool(value, boolValue)) invalid(diagnostics, originalKey, value); + if (!parseBool(value, boolValue, propertySource)) invalid(diagnostics, originalKey, value); else config.jvmStrict = boolValue; break; case ConfigKey::JvmClasses: @@ -370,11 +377,24 @@ void applyKnownValue(ServerConfig& config, std::string_view key, std::string_vie if (diagnostics) diagnostics->unsupported(std::string(key), std::string(value)); return; } - applyValue(config, spec->key, value, diagnostics, key); + applyValue(config, spec->key, value, propertySource, diagnostics, key); } } // namespace +std::uint64_t server_config_detail::randomWorldSeed() noexcept { + try { + std::random_device rd; + const auto now = static_cast( + std::chrono::steady_clock::now().time_since_epoch().count()); + return (static_cast(rd()) << 32U) ^ + static_cast(rd()) ^ now; + } catch (...) { + return static_cast( + std::chrono::steady_clock::now().time_since_epoch().count()); + } +} + void ConfigDiagnostics::invalid(std::string key, std::string value) { entries.push_back({ConfigDiagnosticKind::InvalidValue, std::move(key), std::move(value)}); } @@ -389,21 +409,29 @@ void ConfigDiagnostics::missing(std::string key) { void applyServerProperties(ServerConfig& config, const ServerProperties& properties, ConfigDiagnostics* diagnostics) { - // Preserve source order for aliases while still honoring direct edits to + // Preserve source order for aliases while still honoring direct edits to // the public props map. parsedEntries contains the final occurrence of // every exact key; the current map supplies the value, so a caller that // mutates props never applies stale parsed data. + const bool hasCanonicalWhitelist = properties.props.find("white-list") != + properties.props.end(); + const auto applyProperty = [&](const std::string& key, const std::string& value) { + // `whitelist` is a cppfm legacy alias. If vanilla's canonical key is + // present, it wins independent of source order. + if (hasCanonicalWhitelist && key == "whitelist") return; + applyKnownValue(config, key, value, true, diagnostics); + }; std::vector appliedKeys; appliedKeys.reserve(properties.props.size()); for (const auto& [parsedKey, ignored] : properties.parsedEntries()) { const auto current = properties.props.find(parsedKey); if (current == properties.props.end()) continue; - applyKnownValue(config, current->first, current->second, true, diagnostics); + applyProperty(current->first, current->second); appliedKeys.push_back(current->first); } for (const auto& [key, value] : properties.props) { if (std::find(appliedKeys.begin(), appliedKeys.end(), key) == appliedKeys.end()) - applyKnownValue(config, key, value, true, diagnostics); + applyProperty(key, value); } // The cap is derived only when the user did not specify it. This keeps the diff --git a/src/game/ServerConfig.hpp b/src/game/ServerConfig.hpp index 8061c25b..a9e3a815 100644 --- a/src/game/ServerConfig.hpp +++ b/src/game/ServerConfig.hpp @@ -9,17 +9,24 @@ namespace cppfm { +namespace server_config_detail { +// The vanilla level-seed default is empty; a fresh world receives a random +// seed. Kept out of ServerConfig's public aggregate API implementation. +std::uint64_t randomWorldSeed() noexcept; +} + // The default member initializers are the single source of truth for startup // defaults. Configuration tests construct this type instead of maintaining a // second table of expected defaults. struct ServerConfig { std::uint16_t port = 25565; std::int32_t maxPlayers = 20; - std::int32_t viewDistance = 6; + std::int32_t viewDistance = 10; std::int32_t simulationDistance = 10; - std::string motd = "CppFabricMC - C++ Minecraft 1.21.4 server"; + std::string motd = "A Minecraft Server"; std::string worldBiome = "minecraft:plains"; - std::int64_t hashedSeed = 1378645410614731511LL; + std::uint64_t seed = server_config_detail::randomWorldSeed(); + std::int64_t hashedSeed = static_cast(seed); std::string assetsDir = "assets/registry"; std::string worldDir = "world"; std::string recipesDir = "assets/data/recipes"; @@ -30,13 +37,12 @@ struct ServerConfig { // Flat worlds remain available through `level-type=flat` or the CLI. std::string levelType = "normal"; bool whitelist = false; - bool onlineMode = false; - bool enforceSecureProfile = false; + bool onlineMode = true; + bool enforceSecureProfile = true; bool enforcesSecureChat = false; - std::string difficulty = "normal"; + std::string difficulty = "easy"; RconConfig rcon; std::string levelTypeCli; - std::uint64_t seed = 1378645410614731511ULL; std::int64_t startTime = 1000; int compressionThreshold = 256; int spawnProtection = 16; diff --git a/tests/test_settings_matrix.cpp b/tests/test_settings_matrix.cpp index 86e52697..0ff18376 100644 --- a/tests/test_settings_matrix.cpp +++ b/tests/test_settings_matrix.cpp @@ -30,6 +30,96 @@ bool hasDiagnostic(const ConfigDiagnostics& diagnostics, ConfigDiagnosticKind ki return false; } +void testVanillaDefaultsAndSeedSemantics() { + std::cout << "\n[vanilla 1.21.4 defaults and level seed]\n"; + const ServerConfig defaults; + check(defaults.onlineMode && defaults.enforceSecureProfile, + "online authentication and secure profiles default to vanilla enabled"); + check(defaults.viewDistance == 10 && defaults.motd == "A Minecraft Server" && + defaults.difficulty == "easy", + "view distance, MOTD, and difficulty use vanilla defaults"); + check(defaults.seed != 1378645410614731511ULL && + defaults.hashedSeed == static_cast(defaults.seed), + "an unspecified level seed is randomized and matches its signed login seed"); + + ServerConfig absentSeed = defaults; + ServerProperties noSeed; + ConfigDiagnostics absentDiagnostics; + applyServerProperties(absentSeed, noSeed, &absentDiagnostics); + check(absentDiagnostics.entries.empty() && absentSeed.seed == defaults.seed && + absentSeed.hashedSeed == defaults.hashedSeed, + "an absent level-seed property preserves the generated default seed"); + + ServerProperties emptySeed; + check(emptySeed.loadText("level-seed=\n"), "empty level-seed fixture parses"); + ServerConfig emptySeedConfig; + emptySeedConfig.seed = 17; + emptySeedConfig.hashedSeed = 17; + ConfigDiagnostics emptyDiagnostics; + applyServerProperties(emptySeedConfig, emptySeed, &emptyDiagnostics); + check(emptyDiagnostics.entries.empty() && emptySeedConfig.seed != 17 && + emptySeedConfig.hashedSeed == static_cast(emptySeedConfig.seed), + "an explicitly empty level-seed generates a fresh coherent seed"); +} + +void testCanonicalWhitelistAndAliasPrecedence() { + std::cout << "\n[canonical white-list and legacy alias precedence]\n"; + ServerProperties properties; + check(properties.loadText("whitelist=true\nwhite-list=false\n"), + "legacy alias before canonical white-list parses"); + ServerConfig config; + ConfigDiagnostics diagnostics; + applyServerProperties(config, properties, &diagnostics); + check(diagnostics.entries.empty() && !config.whitelist, + "canonical white-list wins over a contradictory legacy alias"); + + check(properties.loadText("white-list=true\nwhitelist=false\n"), + "canonical white-list before legacy alias parses"); + config = ServerConfig{}; + diagnostics.entries.clear(); + applyServerProperties(config, properties, &diagnostics); + check(diagnostics.entries.empty() && config.whitelist, + "canonical white-list remains authoritative independent of source order"); + + check(properties.loadText("whitelist=true\n"), "legacy whitelist alias parses alone"); + config = ServerConfig{}; + diagnostics.entries.clear(); + applyServerProperties(config, properties, &diagnostics); + check(diagnostics.entries.empty() && config.whitelist, + "the exact legacy alias remains supported when canonical key is absent"); +} + +void testPropertyKeyCaseAndBooleanSemantics() { + std::cout << "\n[property key case and source-specific boolean semantics]\n"; + ServerProperties properties; + check(properties.loadText("View-Distance=4\n" + "Online-Mode=false\n" + "pvp=on\n" + "allow-flight=TrUe\n" + "hardcore=1\n"), + "case and boolean semantics fixture parses"); + ServerConfig config; + ConfigDiagnostics diagnostics; + applyServerProperties(config, properties, &diagnostics); + check(config.viewDistance == 10 && config.onlineMode, + "property keys with non-canonical case do not match canonical settings"); + check(!config.pvp && config.allowFlight && !config.hardcore, + "property booleans use Java true-only semantics with case-insensitive true"); + check(hasDiagnostic(diagnostics, ConfigDiagnosticKind::UnsupportedKey, "View-Distance") && + hasDiagnostic(diagnostics, ConfigDiagnosticKind::UnsupportedKey, "Online-Mode") && + !hasDiagnostic(diagnostics, ConfigDiagnosticKind::InvalidValue, "pvp") && + !hasDiagnostic(diagnostics, ConfigDiagnosticKind::InvalidValue, "hardcore"), + "case-mismatched property keys are reported and non-true boolean tokens become false"); + + const auto cli = parseCommandLine({"--VIEW-DISTANCE=5", "--ONLINE-MODE=false", + "--pvp=on", "--hardcore=yes"}); + check(cli.diagnostics.entries.empty() && cli.assignments.size() == 4, + "CLI names remain independently case-insensitive and accept CLI boolean aliases"); + applyCommandLine(config, cli, &diagnostics); + check(config.viewDistance == 5 && !config.onlineMode && config.pvp && config.hardcore, + "CLI values override properties using the separate CLI boolean contract"); +} + void testCanonicalProperties() { std::cout << "\n[canonical server.properties settings]\n"; ServerProperties properties; @@ -47,7 +137,7 @@ void testCanonicalProperties() { "rcon.port=25570\n" "rcon.password=matrix-secret\n" "enable-rcon=true\n" - "whitelist=true\n" + "white-list=true\n" "online-mode=true\n" "enforce-secure-profile=true\n" "enforces-secure-chat=true\n" @@ -204,20 +294,23 @@ void testInvalidSettings() { "invalid settings fixture parses"); ConfigDiagnostics diagnostics; applyServerProperties(config, properties, &diagnostics); - check(config.port == 4321 && config.hashedSeed != 77 && config.resourcePackForced, - "invalid settings retain prior fields while textual seed remains valid"); + check(config.port == 4321 && config.hashedSeed != 77 && !config.resourcePackForced, + "invalid numeric settings retain values, textual seed hashes, and non-true booleans become false"); check(hasDiagnostic(diagnostics, ConfigDiagnosticKind::InvalidValue, "server-port") && !hasDiagnostic(diagnostics, ConfigDiagnosticKind::InvalidValue, "level-seed") && - hasDiagnostic(diagnostics, ConfigDiagnosticKind::InvalidValue, - "require-resource-pack") && + !hasDiagnostic(diagnostics, ConfigDiagnosticKind::InvalidValue, + "require-resource-pack") && hasDiagnostic(diagnostics, ConfigDiagnosticKind::UnsupportedKey, "unknown-setting"), - "invalid, textual, and unsupported settings remain distinguishable"); + "invalid numbers, text seeds, Java-style booleans, and unsupported keys remain distinguishable"); } } // namespace int main() { + testVanillaDefaultsAndSeedSemantics(); + testCanonicalWhitelistAndAliasPrecedence(); + testPropertyKeyCaseAndBooleanSemantics(); testCanonicalProperties(); testAliasOrderAndMapMutation(); testCommandLinePrecedence(); From 5f8bf82a5abaf63637b7db0eff57581913b5aa75 Mon Sep 17 00:00:00 2001 From: nico2525nn <173241093+nico2525nn@users.noreply.github.com> Date: Wed, 23 Sep 2026 13:32:09 +0900 Subject: [PATCH 07/10] plan56: align secure chat enforcement with vanilla --- CMakeLists.txt | 5 +++ src/game/GameServer_session.cpp | 36 ++++++++--------- src/game/SecureChatPolicy.hpp | 41 ++++++++++++++++++++ tests/test_secure_chat_policy.cpp | 64 +++++++++++++++++++++++++++++++ 4 files changed, 126 insertions(+), 20 deletions(-) create mode 100644 src/game/SecureChatPolicy.hpp create mode 100644 tests/test_secure_chat_policy.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 2ae12e37..37e6f722 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -716,6 +716,11 @@ target_include_directories(test_settings_matrix PRIVATE src src/generated) add_test(NAME settings_matrix COMMAND test_settings_matrix) set_tests_properties(settings_matrix PROPERTIES TIMEOUT 30) +add_executable(test_secure_chat_policy tests/test_secure_chat_policy.cpp) +target_include_directories(test_secure_chat_policy PRIVATE src) +add_test(NAME secure_chat_policy COMMAND test_secure_chat_policy) +set_tests_properties(secure_chat_policy PROPERTIES TIMEOUT 30) + # Version-pinned Play S→C ID locks plus selected field/order vectors. This is # broad wire coverage, not an exhaustive proof of every packet payload branch. add_executable(test_wire_full tests/test_wire_full.cpp diff --git a/src/game/GameServer_session.cpp b/src/game/GameServer_session.cpp index 14d5d7cf..332ea12e 100644 --- a/src/game/GameServer_session.cpp +++ b/src/game/GameServer_session.cpp @@ -1,4 +1,5 @@ #include "GameServer.hpp" +#include "SecureChatPolicy.hpp" #include "Messages.hpp" #include "BlockEvent.hpp" #include "MetadataTypes.hpp" @@ -1289,7 +1290,8 @@ void Session::sendJoinGame() { WriteBuffer ws = makeWorldState(c, world, self_->gamemode); b.raw(ws.data.data(), ws.data.size()); } - b.boolean(c.enforcesSecureChat); // enforces secure chat + b.boolean(secure_chat_policy::isEnforced(c.enforceSecureProfile, + c.enforcesSecureChat)); // enforces secure chat conn_->sendPacket(pl::sc::Login, b); WriteBuffer difficulty; int difficultyId = 2; @@ -4263,25 +4265,20 @@ void Session::onChatMessage(ReadBuffer& in) { (static_cast(in.u8()) << 16); if ((acknowledgedMask & 0xFFF00000u) != 0) throw std::runtime_error("invalid chat acknowledged bitset"); - if (srv_.config().onlineMode && srv_.config().enforcesSecureChat) { - if (signature.empty()) { - WriteBuffer kick; - nbt::writeTextComponent(kick, "Chat message signature required (enforce-secure-profile)"); - conn_->trySendPacket(proto::pl::sc::Disconnect, kick); - conn_->close(); - return; - } - } + const bool secureChatEnforced = secure_chat_policy::isEnforced( + srv_.config().enforceSecureProfile, srv_.config().enforcesSecureChat); - // Strict N6: verify RSA-SHA256 when hasChatSession; fallback to SystemChat. - // Slash-prefixed text is dispatched only after this verification block. + // Verify signed messages whenever a player session is available. The + // profile-enforcement setting (or cppfm's explicit extension) controls + // whether an unsigned message is rejected; online-mode by itself does + // not suppress vanilla's unsigned-chat path. bool usePlayerChat = false; - if (self_->hasChatSession) { + if (self_->hasChatSession && !signature.empty()) { const bool duplicate = std::find(self_->lastSeenChatSalts.begin(), self_->lastSeenChatSalts.end(), salt) != self_->lastSeenChatSalts.end(); if (duplicate) { - if (srv_.config().enforcesSecureChat) { + if (secureChatEnforced) { WriteBuffer kick; nbt::writeTextComponent(kick, "Duplicate chat message"); conn_->trySendPacket(proto::pl::sc::Disconnect, kick); @@ -4298,7 +4295,10 @@ void Session::onChatMessage(ReadBuffer& in) { self_->lastSeenChatSalts.erase(self_->lastSeenChatSalts.begin()); } } - if (srv_.config().enforcesSecureChat && !usePlayerChat) { + const auto disposition = secure_chat_policy::classifyMessage( + srv_.config().enforceSecureProfile, srv_.config().enforcesSecureChat, + self_->hasChatSession, !signature.empty(), usePlayerChat); + if (disposition == secure_chat_policy::MessageDisposition::Reject) { WriteBuffer kick; nbt::writeTextComponent(kick, "Chat message signature invalid or missing"); conn_->trySendPacket(proto::pl::sc::Disconnect, kick); @@ -4306,11 +4306,7 @@ void Session::onChatMessage(ReadBuffer& in) { return; } - // Never expose an unverified online message to cancellable/native/JVM - // callbacks. Offline-mode sessions have no chat certificate by design; - // online-mode sessions without a valid signature fail closed here even - // when secure-chat enforcement is disabled. - if ((self_->hasChatSession || srv_.config().onlineMode) && !usePlayerChat) + if (disposition == secure_chat_policy::MessageDisposition::DropSessionUnsigned) return; // events: PlayerChat (cancellable), after authentication and replay checks diff --git a/src/game/SecureChatPolicy.hpp b/src/game/SecureChatPolicy.hpp new file mode 100644 index 00000000..b167f975 --- /dev/null +++ b/src/game/SecureChatPolicy.hpp @@ -0,0 +1,41 @@ +#pragma once + +namespace cppfm::secure_chat_policy { + +enum class MessageDisposition { + AcceptSigned, + AcceptUnsigned, + Reject, + DropSessionUnsigned, +}; + +// Vanilla's secure-profile requirement also controls the secure-chat contract +// advertised in Join Game. cppfm's explicit secure-chat option may strengthen +// that contract, but must never weaken vanilla enforcement. +constexpr bool isEnforced(bool enforceSecureProfile, + bool cppfmEnforcesSecureChat) noexcept { + return enforceSecureProfile || cppfmEnforcesSecureChat; +} + +constexpr MessageDisposition classifyMessage(bool enforceSecureProfile, + bool cppfmEnforcesSecureChat, + bool hasSession, + bool hasSignature, + bool signatureValid) noexcept { + if (hasSignature) { + // A signature without a session, or a signature that failed + // verification, is malformed/untrusted regardless of enforcement. + return hasSession && signatureValid ? MessageDisposition::AcceptSigned + : MessageDisposition::Reject; + } + if (isEnforced(enforceSecureProfile, cppfmEnforcesSecureChat)) + return MessageDisposition::Reject; + + // Verified vanilla evidence establishes unsigned acceptance before any + // session update. Preserve cppfm's existing behavior for an unsigned + // packet after a session update until that case is separately verified. + return hasSession ? MessageDisposition::DropSessionUnsigned + : MessageDisposition::AcceptUnsigned; +} + +} // namespace cppfm::secure_chat_policy diff --git a/tests/test_secure_chat_policy.cpp b/tests/test_secure_chat_policy.cpp new file mode 100644 index 00000000..5a53107c --- /dev/null +++ b/tests/test_secure_chat_policy.cpp @@ -0,0 +1,64 @@ +#include "game/SecureChatPolicy.hpp" + +#include + +using cppfm::secure_chat_policy::MessageDisposition; + +namespace { +int passed = 0; +int failed = 0; + +void check(bool condition, const char* description) { + if (condition) { + ++passed; + std::cout << "PASS " << description << '\n'; + } else { + ++failed; + std::cout << "FAIL " << description << '\n'; + } +} +} + +int main() { + using cppfm::secure_chat_policy::classifyMessage; + using cppfm::secure_chat_policy::isEnforced; + + check(!isEnforced(false, false), + "join secure-chat flag is false when both enforcement options are false"); + check(isEnforced(true, false), + "vanilla secure-profile enforcement sets the join secure-chat flag"); + check(isEnforced(false, true), + "cppfm secure-chat extension independently sets the join flag"); + check(isEnforced(true, true), + "cppfm extension cannot weaken vanilla secure-profile enforcement"); + + check(classifyMessage(false, false, false, false, false) == + MessageDisposition::AcceptUnsigned, + "no-session unsigned chat is accepted when secure chat is unsecure"); + check(classifyMessage(true, false, false, false, false) == + MessageDisposition::Reject, + "secure-profile enforcement rejects no-session unsigned chat"); + check(classifyMessage(false, true, false, false, false) == + MessageDisposition::Reject, + "explicit cppfm secure-chat enforcement rejects unsigned chat"); + + check(classifyMessage(false, false, true, true, true) == + MessageDisposition::AcceptSigned, + "a verified signed message with a session is accepted"); + check(classifyMessage(false, false, true, true, false) == + MessageDisposition::Reject, + "an invalid signed message fails closed without enforcement"); + check(classifyMessage(false, false, false, true, false) == + MessageDisposition::Reject, + "a signed message without a player session fails closed"); + + check(classifyMessage(false, false, true, false, false) == + MessageDisposition::DropSessionUnsigned, + "session-present unsigned behavior remains unchanged pending vanilla verification"); + check(classifyMessage(true, false, true, false, false) == + MessageDisposition::Reject, + "secure-profile enforcement rejects session-present unsigned chat"); + + std::cout << "secure_chat_policy: " << passed << " PASS " << failed << " FAIL\n"; + return failed == 0 ? 0 : 1; +} From 77dad08ccf19516b6ee60c356a097f7fbf12ae37 Mon Sep 17 00:00:00 2001 From: nico2525nn <173241093+nico2525nn@users.noreply.github.com> Date: Wed, 23 Sep 2026 13:45:39 +0900 Subject: [PATCH 08/10] plan56: align status secure chat advertisement --- src/game/GameServer_session.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/game/GameServer_session.cpp b/src/game/GameServer_session.cpp index 332ea12e..24c5c5e3 100644 --- a/src/game/GameServer_session.cpp +++ b/src/game/GameServer_session.cpp @@ -596,7 +596,9 @@ std::string makeStatusJson(const GameServer& server, description.set("text", json::Value::ofString(server.config().motd)); root.set("description", std::move(description)); root.set("enforcesSecureChat", - json::Value::ofBool(server.config().enforcesSecureChat)); + json::Value::ofBool(secure_chat_policy::isEnforced( + server.config().enforceSecureProfile, + server.config().enforcesSecureChat))); const std::string favicon = readServerIconBase64(); if (!favicon.empty()) root.set("favicon", json::Value::ofString(favicon)); From 5eb101d1cc31022828feae26ec8f1eec79d4da3a Mon Sep 17 00:00:00 2001 From: nico2525nn <173241093+nico2525nn@users.noreply.github.com> Date: Wed, 23 Sep 2026 14:16:17 +0900 Subject: [PATCH 09/10] Align supported settings with vanilla 1.21.4 --- README.md | 5 +-- docs/CURRENT_STATE.md | 35 ++++++++++++++++++--- docs/DEVELOPMENT.md | 2 +- docs/MISSING_FEATURES_1_21_4.md | 10 +++--- docs/README.md | 13 ++++++-- docs/SPEC_OPS.md | 20 ++++++++++++ docs/VERIFICATION.md | 45 ++++++++++++++++++++++++++- docs/audit/goal-adversarial-review.md | 21 +++++++++++-- src/game/ServerConfig.cpp | 15 +++++++++ src/game/ServerProperties.hpp | 6 +++- tests/test_goal_security_guards.py | 11 +++++-- tests/test_properties.cpp | 17 +++++----- tests/test_settings_matrix.cpp | 33 +++++++++++++++++++- 13 files changed, 201 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index ea2fe6c5..514f9e0a 100644 --- a/README.md +++ b/README.md @@ -116,10 +116,7 @@ The latest recorded runs include: limitations remain as listed above. - The explicit no-JNI configure/build and its `42/42` non-package CTest set, plus the four-binary ASan/UBSan key set - (`core_safety`, `spec_wire`, `fuzz`, and `gameplay_full`) also pass. The safe - Plan54 cleanup is partial: the protected-scope ledger is reproducible, but the - strict 18,341-line reduction target was not reached and no tests or fixtures were - removed to claim it. + (`core_safety`, `spec_wire`, `fuzz`, and `gameplay_full`) also pass. - Real-client check: the mc-pilot-managed Fabric 1.21.4 client logged in offline, entered the world, stayed connected for more than one minute, and completed chat, `say`, position, block read/break/read, status, and screenshot probes. diff --git a/docs/CURRENT_STATE.md b/docs/CURRENT_STATE.md index d9d865fe..ff025c2c 100644 --- a/docs/CURRENT_STATE.md +++ b/docs/CURRENT_STATE.md @@ -34,7 +34,7 @@ handoff notes are not part of the public documentation set. | `research_viewpoints` | `16` current viewpoints; old `13` wording is historical | | `taxonomy_snapshot` | MISSING `#1–#90`; historical matrix counts `DONE=90, PARTIAL=0, TODO=0` | | `strict_assessment_1` | `78 gaps`; `HISTORICAL` archive label, not a current aggregate | -| `next_plan` | close current PR #1 gates; the local plan55 protocol-primitive follow-up is implemented, while plan56 server.properties audit research remains to be implemented and verified | +| `next_plan` | submit the locally validated vanilla 1.21.4 server.properties compatibility follow-up as a PR stacked on PR #1, then resolve any exact-SHA Actions failures before merge; the property surface remains explicitly partial | The previous baseline was the plan50 runtime follow-up after the plan49 implementation integration and cleanup commit `db12df96093a0869e958f62b11f9a9cd68ba3ef1` and safety commit @@ -45,11 +45,12 @@ for binaries built with the required JNI inputs, without changing generated data protocol 769, or the `docs-legacy/` archive. The plan51 implementation and its no-ff integration are recorded above. -## 1A. Current working-tree verification +## 1A. Working-tree verification baseline (2026-09-19) -The following evidence is from the current uncommitted working tree after the +The following evidence is from the 2026-09-19 working-tree snapshot after the settings/security/authority hardening pass. It supersedes neither the historical -`335fca5` baseline above nor the declared release boundaries below. +`335fca5` baseline above nor the declared release boundaries below, and it is not +the result of the separate plan56 compatibility follow-up in §1C. | gate | measured result | scope / limitation | |---|---|---| @@ -88,6 +89,32 @@ MISSING #71's row-specific evidence is now `PASS` in the 90-row coverage ledger (`14 PASS`, `38 PARTIAL`, `38 UNVERIFIED`). This does not remove the independent publication boundaries below or imply universal protocol parity. +## 1C. Vanilla 1.21.4 settings compatibility follow-up (2026-09-23) + +This work is on local branch `plan56/compatibility`, based on PR #1 head +`26e399c0`; it is not merged. The result changes vanilla-facing defaults and +property interpretation while keeping unsupported properties declared as +unsupported. + +| gate | measured result | scope / limitation | +|---|---|---| +| RelWithDebInfo configure/full build | `PASS` | Full Ninja build completed in the integration worktree; all commands had explicit timeouts | +| defaults, whitelist key, difficulty IDs, and signed integers | `47 PASS / 0 FAIL` | `settings_matrix`; vanilla online/profile defaults, view distance, MOTD, difficulty, random new-world seed, canonical `white-list`, source-order alias behavior, difficulty IDs `0..3`, and leading `+` are covered | +| Java Properties parsing and configuration | `63 PASS / 0 FAIL` | `properties`; separators, escapes, continuations, key case, UTF-8/ISO-8859-1 fallback, Java boolean semantics, and CLI precedence; isolated UTF-16 surrogates map to U+FFFD in the UTF-8 API | +| effective secure-chat policy | `12 PASS / 0 FAIL` | `secure_chat_policy`; status and Join Game share the effective policy, `enforce-secure-profile || enforces-secure-chat`; unsigned-after-session behavior remains unverified | +| fake-client authentication options | `PASS` | source guard and Python syntax checks cover local fake-client launchers; each offline launcher explicitly disables both online mode and secure-profile enforcement | +| full non-nightly CTest | `55/55 PASS` | 516.24s, exact command and scope in [VERIFICATION.md](VERIFICATION.md#vanilla-server-settings-compatibility-follow-up-2026-09-23-pre-merge) | +| server child cleanup | `PASS` | post-suite `pgrep -a -f 'cppfm --por[t]'` found no remaining `cppfm` server process | +| remaining property parity | `PARTIAL` | `enforce-whitelist`, query/status/network, permission, datapack, and several world/pack/operations effects are not fully implemented; `ServerProperties::save` is not `Properties.store`-compatible | + +Additional exploratory offline Python runs reported failures identical on the +unmodified parent binary: replay `5/8` (signed `n=0` command, sign block-entity +update, survival ability flags), `run_plan43_suite.py` `10/23` (signed chat, +damage, horse IDs, join flags, and sign placement/text), and short soak-bot runs +with `0` chunks against an expected minimum of `10`. These checks have no +retained per-run logs and are not part of the passing CTest aggregate; the C++ +`plan43` target separately passes `87/87`. + ## 2. Prior plan48 and cleanup record | item | state at this tracker | evidence / scope | diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index d5bd57f2..61578dd9 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -133,7 +133,7 @@ contracts: secure-chat policy requires it. Signed command argument transcripts are not reconstructed and therefore fail closed instead of dispatching. -The final implementation evidence is `test_settings_matrix` (25/25), +The 2026-09-19 final-gate baseline recorded `test_settings_matrix` (27/27), `test_properties` (33/33), `test_recovery` (55/55), `test_core_safety` (45/45), Plan43 (87/87), smoke80 (224/224), the full live matrix (240/240), and the non-nightly CTest aggregate (46/46). The 120-client stress, 300-second soak, diff --git a/docs/MISSING_FEATURES_1_21_4.md b/docs/MISSING_FEATURES_1_21_4.md index 512144d6..4d3c4ddf 100644 --- a/docs/MISSING_FEATURES_1_21_4.md +++ b/docs/MISSING_FEATURES_1_21_4.md @@ -20,7 +20,7 @@ - `strict_assessment_1_gap_count: 78` is a separate historical audit label. Its archive result is not a current aggregate, and it must not be added to or substituted for the 90-row taxonomy count. -- Current publication state is `BLOCKED` only by the declared scope/evidence +- At the 2026-09-19 source snapshot, publication state was `BLOCKED` by the declared scope/evidence boundaries: the final focused matrices report `settings 27 PASS / 0 FAIL`, `properties 33 PASS / 0 FAIL`, `recovery 55 PASS / 0 FAIL`, and `core_safety 45 PASS / 0 FAIL`; Plan43 reports `87 PASS / 0 FAIL`; smoke80 @@ -38,16 +38,16 @@ properties `33 PASS`, and the live server matrix `240 PASS / 0 FAIL`. Older duplicate timings are `HISTORICAL` context only. -## Current working-tree hardening evidence +## Working-tree hardening baseline (2026-09-19) This section records the later uncommitted verification pass without changing the historical numbered taxonomy. | concern | current result | evidence / declared boundary | |---|---|---| -| server settings and precedence | `PASS` | `test_settings_matrix`: `27 PASS / 0 FAIL`; invalid values retain prior fields, textual/numeric seed semantics are distinguished, secure-profile and secure-chat settings remain separate, and unsupported keys are reported | +| server settings and precedence | `PASS / SUBSET` | 2026-09-19 baseline `test_settings_matrix`: `27 PASS / 0 FAIL`; invalid values retain prior fields, textual/numeric seeds are distinguished, stored secure-profile and cppfm secure-chat settings remain separate, and unsupported keys are reported. The 2026-09-23 compatibility follow-up is recorded in [CURRENT_STATE.md §1C](CURRENT_STATE.md#1c-vanilla-1214-settings-compatibility-follow-up-2026-09-23): `47` settings, `63` properties, and `12` secure-chat-policy assertions pass, with the full settings surface still partial. | | command/authority and inventory boundaries | `PASS` | `test_properties`: `33 PASS / 0 FAIL`; full live matrix: `240 PASS / 0 FAIL`; Plan43: `87 PASS / 0 FAIL`; op-gated roots, one-shot trigger enablement, creative/range/sign/beacon/pick checks, source-dimension item commands, resource-pack UUID completion, and signed-command offset parsing are covered | -| secure chat/profile | `PASS / BOUNDED` | valid signed inbound chat is verified with the protocol-769 SHA256 transcript and relayed as `SystemChat` under enforced secure chat; Mojang profile certificates use SHA1withRSA and all configured keys are tried; rejected profiles terminate; signed command argument transcripts are deliberately fail-closed and remain a declared limitation | +| secure chat/profile | `PASS / BOUNDED` | valid signed inbound chat is verified with the protocol-769 SHA256 transcript and relayed as `SystemChat` under enforced secure chat; Mojang profile certificates use SHA1withRSA and all configured keys are tried; rejected profiles terminate; signed command argument transcripts are deliberately fail-closed and remain a declared limitation. The 2026-09-23 policy/advertisement follow-up and its unsigned-after-session boundary are recorded in [CURRENT_STATE.md §1C](CURRENT_STATE.md#1c-vanilla-1214-settings-compatibility-follow-up-2026-09-23). | | event and persistence ordering | `PASS` | cancellable block events run before mutations; scoped callbacks synchronize removal; moving-piston source/destination commits are flushed under the simulation gate before synchronous/background snapshots; transient moving-piston NBT remains intentionally omitted | | stalled-client isolation | `PASS` | framed output is encoded while the simulation gate is held and sent by a per-connection writer with a 4 MiB queue cap; encrypted frames remain FIFO; graceful output drains for at most 100 ms, then teardown is bounded; no generic gate release occurs during a mutation | @@ -263,7 +263,7 @@ table rather than being hidden inside a numbered `DONE` row. | Feature | Status | Notes | |---|---|---| | RCON `whitelist` | DONE | `RconServer` dispatches Brigadier commands and persists whitelist state. | -| `server.properties` subset | DONE | Includes spawn protection, whitelist, online mode, secure profile, view/simulation distance, MOTD, seed, level type, difficulty, resource pack, PVP, flight, hardcore, and max players. | +| `server.properties` subset | PARTIAL | The implemented subset now defaults to vanilla 1.21.4 values (including online/profile enforcement, view distance 10, vanilla MOTD, easy difficulty, and a random seed for a new world); parsing follows Java Properties key case, separators, escapes, continuations, Unicode escapes, UTF-8/ISO-8859-1 fallback, and `Boolean.parseBoolean` semantics. Supported integer values accept Java's leading `+`, and difficulty IDs `0`–`3` map to vanilla names. `white-list` is canonical and legacy `whitelist` remains an alias. This is not full property parity: `enforce-whitelist`, query/status/network, permission, datapack, and several world/pack/operations properties do not yet have complete runtime effects. `ServerProperties::save` also remains a simple legacy writer rather than a `Properties.store` equivalent. Do not interpret focused parser/config passes as completion of the full 1.21.4 settings surface. | ## Test and gate mapping diff --git a/docs/README.md b/docs/README.md index faec1459..45468b3f 100644 --- a/docs/README.md +++ b/docs/README.md @@ -78,15 +78,22 @@ default-on strict JVM startup without an external classes/assets override. ## Verification at a glance -The latest recorded working-tree evidence includes: +The 2026-09-19 integrated working-tree baseline recorded: -- The final integrated rerun on 2026-09-19 is non-nightly CTest `52/52 PASS` +- The 2026-09-19 integrated rerun is non-nightly CTest `52/52 PASS` in `441.35s`; the separately run source-tree smoke matrix reports `224 PASS / 0 FAIL`, Plan43 reports `87 PASS / 0 FAIL`, and the full `test_server_full.py` matrix reports `240 PASS / 0 FAIL / 240 total`. The named focused targets also pass: settings `27`, properties `33`, recovery `55`, core safety `45`, and the specification wire vectors `417`, all with zero failures. +- A separate pre-merge server-settings follow-up based on PR #1 head + `26e399c0` reports `settings_matrix` `47/47`, `properties` `63/63`, + `secure_chat_policy` `12/12`, and non-nightly CTest `55/55` in `516.24s`. + It aligns defaults and parsing for the implemented subset only; unsupported + vanilla settings and several runtime effects remain partial. Offline fake-client + launchers explicitly disable both authentication checks. Full commands and + limitations are recorded in [Verification](VERIFICATION.md#vanilla-server-settings-compatibility-follow-up-2026-09-23-pre-merge). - The 120-client stress gate joined `120/120` in `68.5s`; the 300-second soak passed with 150 keepalives, 0 disconnects, 2,899 actions, and `0.2%` post-fill RSS growth. The strict synthetic view-distance-32 benchmark @@ -128,7 +135,7 @@ The latest recorded working-tree evidence includes: not retained release assets; the exact scope is in [Real-client verification](MC_PILOT_REAL_TEST.md). -### Current working-tree hardening run +### 2026-09-19 working-tree hardening baseline The final deterministic settings/security/authority pass reports `test_settings_matrix` `27 PASS / 0 FAIL`, `test_properties` `33 PASS / 0 FAIL`, diff --git a/docs/SPEC_OPS.md b/docs/SPEC_OPS.md index 902ff14c..950aa62f 100644 --- a/docs/SPEC_OPS.md +++ b/docs/SPEC_OPS.md @@ -41,6 +41,26 @@ the root layout for this target, 20 TPS scheduling, and Play KeepAlive `0x27` ar 1.21.4 facts. `SPEC_OPS` does not turn a chosen safety threshold into a claim about all vanilla servers. +### Vanilla-facing `server.properties` + +The implemented settings remain a **partial** 1.21.4 subset. On first launch, +the generated defaults use `view-distance=10`, `motd=A Minecraft Server`, +`difficulty=easy`, an empty `level-seed` (random for a new world), +`online-mode=true`, and `enforce-secure-profile=true`; an existing world's +`level.dat` seed remains authoritative. Property names are case-sensitive and +use Java Properties separators, escapes, continuations, and boolean conversion. +Supported integer settings accept Java's leading `+`; difficulty IDs `0`–`3` +map to `peaceful`, `easy`, `normal`, and `hard`. `white-list` is the canonical +key; `whitelist` remains a legacy cppfm alias. + +The compatibility-oriented defaults require account authentication and secure +profile enforcement. An intentionally offline fake-client test must explicitly +pass both `--online-mode=false` and `--enforce-secure-profile=false`. This does +not make the settings surface complete: for example, `enforce-whitelist`, query, +permission, datapack, and several world/pack/operations keys still lack full +vanilla runtime behavior. Passing the parser/configuration gates is not evidence +that unsupported vanilla keys take effect. + ## 3. Classes and operational data | area | implementation path/symbol | observable metric/state | evidence/status | diff --git a/docs/VERIFICATION.md b/docs/VERIFICATION.md index 8f17b007..40382ffb 100644 --- a/docs/VERIFICATION.md +++ b/docs/VERIFICATION.md @@ -40,7 +40,7 @@ boundaries. Any unverified assertion uses `DECLARED-LIMITATION` rather than an inferred pass. See [PLAN51_JVM.md](PLAN51_JVM.md) and [MC_PILOT_REAL_TEST.md](MC_PILOT_REAL_TEST.md). -### Current working-tree hardening gate +### Working-tree hardening baseline (2026-09-19) The final focused run measured `test_settings_matrix` `27 PASS / 0 FAIL`, `test_properties` `33 PASS / 0 FAIL`, `test_recovery` `55 PASS / 0 FAIL` @@ -65,6 +65,47 @@ barriers run under the same gate. Secure-profile rejection is terminal, block callbacks cancel before mutation with post-callback revalidation, and signed command argument transcripts fail closed as a declared limitation. +### Vanilla server-settings compatibility follow-up (2026-09-23; pre-merge) + +This local compatibility follow-up is based on PR #1 head +`26e399c0d1cae542d459812ef84fd5e865515e2c`. It intentionally improves the +implemented settings subset without claiming complete `server.properties` +parity. Pre-merge local evidence: + +| check | result | command / scope | +|---|---|---| +| RelWithDebInfo configure and full build | `PASS` | `timeout --foreground --kill-after=5 120 cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo`; `timeout --foreground --kill-after=5 600 cmake --build build -j4` | +| properties/parser | `63 PASS / 0 FAIL` | `timeout --foreground --kill-after=5 60 ./build/test_properties ./build/cppfm` | +| config/settings matrix | `47 PASS / 0 FAIL` | `timeout --foreground --kill-after=5 60 ./build/test_settings_matrix` | +| secure-chat policy | `12 PASS / 0 FAIL` | `timeout --foreground --kill-after=5 60 ./build/test_secure_chat_policy` | +| source/process guards | `PASS` | `goal_security_guards`, Python syntax checks for the modified launcher scripts, and the runtime-layout CTest all pass | +| full non-nightly CTest | `55/55 PASS` in `516.24s` | `timeout --foreground --kill-after=5 2400 ctest --test-dir build -LE 'nightly|package' --output-on-failure --timeout 600` | +| owned server cleanup | `PASS` | `pgrep -a -f 'cppfm --por[t]'` found no remaining server process after the suite | + +The settings tests cover vanilla-facing defaults (`online-mode=true`, +`enforce-secure-profile=true`, view distance 10, vanilla MOTD/easy difficulty, +and a random seed for a new world), canonical `white-list`, Java property +syntax/encoding/boolean behavior, difficulty ordinals, and Java's leading plus +sign on supported integers. All offline fake-client launchers explicitly pass +both authentication flags as false. `enforce-whitelist`, query/status/network, +permission, datapack, and several world/pack/operations keys remain unsupported +or without complete vanilla runtime effects; `ServerProperties::save` is not a +`Properties.store` implementation. The secure-chat policy keeps the vanilla +profile property and cppfm extension separately configured, but advertises and +enforces their logical OR; unsigned messages after a player session remain an +unverified edge case. These are subset results, not full settings parity or an +authenticated-client revalidation. + +Separate exploratory offline Python checks exposed failures which were +reproduced against the unmodified parent binary and therefore are not attributed +to this settings change: `replay_vanilla.py` 5/8 (signed `n=0` command, sign +block-entity update, and survival ability flags); `run_plan43_suite.py` 10/23 +(signed chat/Seed, damage event, horse entity IDs, creative/survival join flags, +and sign placement/text); and short `soak_bot.py` runs reporting zero chunks +instead of the expected minimum ten. The C++ `plan43` test passes 87/87, and the +full non-nightly CTest result above is green; the exploratory failures have no +retained per-run logs. + ## 1. Feature overview Verification has six layers: @@ -90,6 +131,8 @@ current assertion. | Minecraft Wiki Java protocol page | supplemental encoding explanation | `VANILLA-CONCEPT` | | Fabric 1.21.4 release note and Loader 0.16.9 docs | platform/version boundary | `VANILLA-CONCEPT` | | [Fabric developer documentation](https://docs.fabricmc.net/develop/index), Fabric API `0.119.4+1.21.4`, and Yarn `1.21.4+build.8` Javadocs | server/common API names, signatures, lifecycle concepts, and versioned ABI audit | `OFFICIAL-API-REFERENCE` / informational audit | +| [Oracle Java SE 21 Properties](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/Properties.html), [Boolean](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Boolean.html), [Integer](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Integer.html), and [Long](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Long.html) | property separators, escapes, continuations, encoding contract, integer sign behavior, and `parseBoolean` semantics | `OFFICIAL-JAVA-REFERENCE` | +| [Mojang 1.21.4 version metadata](https://piston-meta.mojang.com/v1/packages/c16bd1251bdf2cab3d7c3b30393427eeb19c6b2e/1.21.4.json) and [official server artifact](https://piston-data.mojang.com/v1/objects/4707d00eb834b446575d89a61a11b5d548d8c001/server.jar) (`SHA-1 4707d00eb834b446575d89a61a11b5d548d8c001`) | version-pinned server-properties defaults and enum conversion checked by static bytecode inspection; artifact was not run for this audit | `OFFICIAL-BINARY-REFERENCE` | | [Yarn LocalRandom](https://maven.fabricmc.net/docs/yarn-1.21.4%2Bbuild.1/net/minecraft/util/math/random/LocalRandom.html), [Xoroshiro128PlusPlusRandom](https://maven.fabricmc.net/docs/yarn-1.21.4%2Bbuild.1/net/minecraft/util/math/random/Xoroshiro128PlusPlusRandom.html), [RandomSeed](https://maven.fabricmc.net/docs/yarn-1.21.4%2Bbuild.1/net/minecraft/util/math/random/RandomSeed.html), and [RandomSplitter](https://maven.fabricmc.net/docs/yarn-1.21.4%2Bbuild.4/net/minecraft/util/math/random/RandomSplitter.html) | 1.21.4 RNG algorithms, seed expansion, primitive API, and splitter surface | `OFFICIAL-API-REFERENCE` | | [xoroshiro128++ public-domain reference](https://prng.di.unimi.it/xoroshiro128plusplus.c) and official 1.21.4 server bytecode inspection | transition/orientation cross-check for the native vector implementation | `PUBLIC-REFERENCE` / local audit | | [EntityPositionS2CPacket](https://maven.fabricmc.net/docs/yarn-1.21.4%2Bbuild.8/net/minecraft/network/packet/s2c/play/EntityPositionS2CPacket.html) and [PlayerPosition](https://maven.fabricmc.net/docs/yarn-1.21.4%2Bbuild.1/net/minecraft/entity/player/PlayerPosition.html) | official 1.21.4 `teleport_entity` field model | `OFFICIAL-API-REFERENCE` + real-client decode | diff --git a/docs/audit/goal-adversarial-review.md b/docs/audit/goal-adversarial-review.md index 6138718b..ebb1d46d 100644 --- a/docs/audit/goal-adversarial-review.md +++ b/docs/audit/goal-adversarial-review.md @@ -30,13 +30,28 @@ The goal tests are retained as regression gates; the source-order guard intentio ## Findings -### ADV-SET-001 — fixed — secure-profile setting silently enabled secure chat +### ADV-SET-001 — reclassified — secure-profile and secure-chat configuration -`src/game/ServerConfig.cpp` now assigns only `enforceSecureProfile` for the profile key. `tests/test_settings_matrix.cpp` includes an explicit profile=true/chat=false fixture and verifies both flags independently. The source-order guard also prevents the coupling from returning. +`src/game/ServerConfig.cpp` keeps `enforceSecureProfile` and cppfm's +`enforcesSecureChat` extension as separate stored settings. The effective +secure-chat advertisement and unsigned-message policy use their logical OR, +matching vanilla 1.21.4 secure-profile enforcement. The parser does not mutate +the extension setting when the vanilla profile property is applied. +`tests/test_settings_matrix.cpp` verifies the stored settings remain +independent; `test_secure_chat_policy` covers the effective policy. ### ADV-CHAT-002 — fixed — unverified chat reached callbacks -`Session::onChatMessage` now performs signature and replay checks before `PlayerChatEvent` or `JvmRuntime::onChat`; online-mode sessions fail closed when no authenticated message exists, even if secure-chat enforcement is disabled. `tests/test_goal_security_guards.py` locks the ordering and fail-closed branch. A full signed-session callback counter remains environment-dependent because the repository has no authenticated client fixture. +`Session::onChatMessage` performs signature and replay checks before +`PlayerChatEvent` or `JvmRuntime::onChat`. With effective secure-chat +enforcement enabled, unsigned or invalidly signed messages are rejected; when +it is disabled, vanilla's unsigned path without a player session is accepted. +The packet disposition is covered by `test_secure_chat_policy`, and +`tests/test_goal_security_guards.py` locks callback ordering and policy use. +Unsigned-message behavior after a player session has been established remains +an explicitly unverified vanilla edge case. A full signed-session callback +counter also remains environment-dependent because the repository has no +authenticated client fixture. ### ADV-JVM-003 — fixed — JNI world mutation accepted unregistered block states diff --git a/src/game/ServerConfig.cpp b/src/game/ServerConfig.cpp index cad243ab..caa46bb6 100644 --- a/src/game/ServerConfig.cpp +++ b/src/game/ServerConfig.cpp @@ -151,6 +151,12 @@ const KeySpec* findKey(std::string_view name, bool propertySource) { template bool parseInteger(std::string_view text, T& value) { if (text.empty()) return false; + // Java Integer/Long parsing accepts an optional leading plus sign, while + // std::from_chars intentionally does not. + if (text.front() == '+') { + text.remove_prefix(1); + if (text.empty()) return false; + } const auto result = std::from_chars(text.data(), text.data() + text.size(), value, 10); return result.ec == std::errc{} && result.ptr == text.data() + text.size(); } @@ -197,6 +203,15 @@ bool parseLevelType(std::string_view text, std::string& levelType) { } bool parseDifficulty(std::string_view text, std::string& difficulty) { + constexpr std::array kById{ + "peaceful", "easy", "normal", "hard"}; + int id = 0; + if (parseInteger(text, id)) { + if (id < 0 || static_cast(id) >= kById.size()) return false; + difficulty = kById[static_cast(id)]; + return true; + } + const std::string normalized = asciiLower(std::string(text)); if (normalized != "peaceful" && normalized != "easy" && normalized != "normal" && normalized != "hard") return false; diff --git a/src/game/ServerProperties.hpp b/src/game/ServerProperties.hpp index 2e6608a5..52a68e16 100644 --- a/src/game/ServerProperties.hpp +++ b/src/game/ServerProperties.hpp @@ -372,8 +372,12 @@ class ServerProperties { } template - static bool parseInteger(const std::string& text, T& value) { + static bool parseInteger(std::string_view text, T& value) { if (text.empty()) return false; + if (text.front() == '+') { + text.remove_prefix(1); + if (text.empty()) return false; + } const char* first = text.data(); const char* last = first + text.size(); const auto result = std::from_chars(first, last, value, 10); diff --git a/tests/test_goal_security_guards.py b/tests/test_goal_security_guards.py index f7cbc268..547d14d7 100644 --- a/tests/test_goal_security_guards.py +++ b/tests/test_goal_security_guards.py @@ -35,8 +35,15 @@ def main(): jvm = chat.index("srv_.jvmRuntime()->onChat") require(verified < event < jvm, "chat callbacks run only after signature and replay verification") - require("srv_.config().onlineMode) && !usePlayerChat" in chat, - "online unverified chat fails closed when enforcement is disabled") + require("secure_chat_policy::classifyMessage(" in chat and + "secure_chat_policy::isEnforced(" in chat and + "MessageDisposition::Reject" in chat, + "chat disposition follows vanilla secure-profile policy and verification") + status = between(session, "std::string makeStatusJson", "void Session::answerLegacyPing") + join = between(session, "void Session::sendJoinGame()", "void Session::sendAbilities()") + require("secure_chat_policy::isEnforced(" in status and + "secure_chat_policy::isEnforced(" in join, + "status and Join Game advertise the same effective secure-chat policy") jvm_source = (ROOT / "src/jvm/JvmRuntime.cpp").read_text() set_block = between(jvm_source, "bool JvmRuntime::nativeWorldSetBlock", diff --git a/tests/test_properties.cpp b/tests/test_properties.cpp index 74ac482e..fd537255 100644 --- a/tests/test_properties.cpp +++ b/tests/test_properties.cpp @@ -53,6 +53,9 @@ void testPropertiesSyntax() { check(camelCaseDistance.loadText("viewDistance=7") && camelCaseDistance.viewDistance() == 7, "legacy view-distance helper retains its explicit camel-case alias"); + ServerProperties leadingPlus; + check(leadingPlus.loadText("number=+42") && leadingPlus.get("number", 0) == 42, + "typed integer properties accept Java's leading plus sign"); ServerProperties properties; check(properties.loadText( @@ -266,7 +269,7 @@ void testSupportedProperties() { invalid.port = 43123; invalid.viewDistance = 7; invalid.levelType = "flat"; - invalid.jvmEnabled = false; + invalid.jvmEnabled = true; ServerProperties invalidProperties; check(invalidProperties.loadText( "server-port=not-a-port\n" @@ -278,15 +281,15 @@ void testSupportedProperties() { ConfigDiagnostics invalidDiagnostics; applyServerProperties(invalid, invalidProperties, &invalidDiagnostics); check(invalid.port == 43123 && invalid.viewDistance == 7 && - invalid.levelType == "flat" && !invalid.jvmEnabled, - "invalid typed values fall back without changing the prior config"); + invalid.levelType == "flat" && !invalid.jvmEnabled && + invalid.difficulty == "hard", + "invalid typed values retain prior fields while Java boolean and difficulty values apply"); check(hasDiagnostic(invalidDiagnostics, ConfigDiagnosticKind::InvalidValue, "server-port") && hasDiagnostic(invalidDiagnostics, ConfigDiagnosticKind::InvalidValue, "view-distance") && hasDiagnostic(invalidDiagnostics, ConfigDiagnosticKind::InvalidValue, "level-type") && - hasDiagnostic(invalidDiagnostics, ConfigDiagnosticKind::InvalidValue, "jvm") && - !hasDiagnostic(invalidDiagnostics, ConfigDiagnosticKind::InvalidValue, "difficulty") && - invalid.difficulty == "hard", - "invalid values and supported difficulty remain distinguishable"); + !hasDiagnostic(invalidDiagnostics, ConfigDiagnosticKind::InvalidValue, "jvm") && + !hasDiagnostic(invalidDiagnostics, ConfigDiagnosticKind::InvalidValue, "difficulty"), + "invalid numeric/type values are diagnosed but non-true Java booleans are valid false values"); } void testFileLoadingAndPrecedence() { diff --git a/tests/test_settings_matrix.cpp b/tests/test_settings_matrix.cpp index 0ff18376..67f23a15 100644 --- a/tests/test_settings_matrix.cpp +++ b/tests/test_settings_matrix.cpp @@ -1,9 +1,12 @@ #include "../src/game/ServerConfig.hpp" #include "../src/game/ServerProperties.hpp" +#include #include #include +#include #include +#include using namespace cppfm; @@ -214,7 +217,34 @@ void testCanonicalProperties() { check(independentDiagnostics.entries.empty() && independentConfig.enforceSecureProfile && !independentConfig.enforcesSecureChat, - "secure profile enforcement does not silently enforce secure chat"); + "vanilla profile and cppfm secure-chat options remain independently stored"); +} + +void testDifficultyOrdinals() { + std::cout << "\n[vanilla difficulty ordinals]\n"; + constexpr std::array, 4> cases{{ + {"0", "peaceful"}, {"1", "easy"}, {"2", "normal"}, {"3", "hard"}}}; + bool allMapped = true; + for (const auto& [ordinal, expected] : cases) { + ServerProperties properties; + const std::string fixture = "difficulty=" + std::string(ordinal) + "\n"; + ServerConfig config; + ConfigDiagnostics diagnostics; + allMapped &= properties.loadText(fixture); + applyServerProperties(config, properties, &diagnostics); + allMapped &= config.difficulty == expected && diagnostics.entries.empty(); + } + + ServerProperties plusOrdinal; + ServerConfig plusConfig; + ConfigDiagnostics plusDiagnostics; + const bool plusLoaded = plusOrdinal.loadText("server-port=+25571\ndifficulty=+3\n"); + applyServerProperties(plusConfig, plusOrdinal, &plusDiagnostics); + check(allMapped, + "difficulty ordinals 0 through 3 map to the vanilla named difficulties"); + check(plusLoaded && plusConfig.port == 25571 && plusConfig.difficulty == "hard" && + plusDiagnostics.entries.empty(), + "Java-style leading plus signs are accepted for integers and difficulty IDs"); } void testAliasOrderAndMapMutation() { @@ -312,6 +342,7 @@ int main() { testCanonicalWhitelistAndAliasPrecedence(); testPropertyKeyCaseAndBooleanSemantics(); testCanonicalProperties(); + testDifficultyOrdinals(); testAliasOrderAndMapMutation(); testCommandLinePrecedence(); testInvalidSettings(); From b76c93b6ca294c311331067a18d70a1b731adee4 Mon Sep 17 00:00:00 2001 From: nico2525nn <173241093+nico2525nn@users.noreply.github.com> Date: Wed, 23 Sep 2026 15:25:57 +0900 Subject: [PATCH 10/10] fix: reject malformed signed property integers --- AGENTS.md | 1 + docs/CURRENT_STATE.md | 14 +++++++------- docs/MISSING_FEATURES_1_21_4.md | 2 +- docs/VERIFICATION.md | 6 +++--- src/game/ServerConfig.cpp | 6 ++---- src/game/ServerProperties.hpp | 2 +- tests/test_properties.cpp | 4 ++++ tests/test_settings_matrix.cpp | 13 +++++++++++++ 8 files changed, 32 insertions(+), 16 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 989d2b9e..8d83ab4c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,6 +65,7 @@ - PR には変更理由、対象範囲、テスト手順、既知の制限、必要な証跡を記載し、レビュー可能な小さな単位に分ける。サブエージェントの変更も同じPRに統合してから提出する。 - PR作成後は `.github/workflows/` の GitHub Actions を必ず実行し、必須ジョブ（configure/build、対象テスト、lint/quality checks）が成功するまで修正と再実行を行う。Actions が未実行・失敗・キャンセルのPRはマージしない。 - マージ、rebase、release向けタグ付けは GitHub 上のPR保護ルールとActionsの成功結果を通して行う。ローカルの `git merge` は検証用worktree内に限り、`main` の統合手段にはしない。 +- **条件付きマージ方針（全PR共通）**: このリポジトリのPRは、全差分をレビューしたうえで実装品質に確信があり、重大な未解決の正確性・セキュリティ・データ保全上の懸念がなく、当該head SHAの必須Actionsとブランチ保護条件がすべて成功している場合に限り、GitHub経由でマージしてよい。レビューコメントや競合などの未解決条件がある場合、または品質に確信が持てない場合はマージせず、懸念点を報告する。積み上げPRでは対象PRの指定baseにのみ統合し、親PRや `main` への統合は別途そのPRを評価してから行う。 - ローカルのビルド・テストはPR提出前の短時間な検証として実施してよいが、GitHub Actionsの結果を置き換えない。Actionsで再現できない結果はPR本文にコマンド、ログ、環境、制限を明記する。 - Actions設定やPR保護ルール自体を変更する場合も、別PRでレビューし、変更前後のActions実行結果を保存する。秘密情報・トークン・個人環境の設定をリポジトリへ書き込まない。 diff --git a/docs/CURRENT_STATE.md b/docs/CURRENT_STATE.md index ff025c2c..129330d1 100644 --- a/docs/CURRENT_STATE.md +++ b/docs/CURRENT_STATE.md @@ -91,19 +91,19 @@ publication boundaries below or imply universal protocol parity. ## 1C. Vanilla 1.21.4 settings compatibility follow-up (2026-09-23) -This work is on local branch `plan56/compatibility`, based on PR #1 head -`26e399c0`; it is not merged. The result changes vanilla-facing defaults and -property interpretation while keeping unsupported properties declared as -unsupported. +This work is on branch `plan56/compatibility`, based on PR #1 head +`26e399c0`, and is submitted as stacked PR #2; it is not yet part of `main`. +The result changes vanilla-facing defaults and property interpretation while +keeping unsupported properties declared as unsupported. | gate | measured result | scope / limitation | |---|---|---| | RelWithDebInfo configure/full build | `PASS` | Full Ninja build completed in the integration worktree; all commands had explicit timeouts | -| defaults, whitelist key, difficulty IDs, and signed integers | `47 PASS / 0 FAIL` | `settings_matrix`; vanilla online/profile defaults, view distance, MOTD, difficulty, random new-world seed, canonical `white-list`, source-order alias behavior, difficulty IDs `0..3`, and leading `+` are covered | -| Java Properties parsing and configuration | `63 PASS / 0 FAIL` | `properties`; separators, escapes, continuations, key case, UTF-8/ISO-8859-1 fallback, Java boolean semantics, and CLI precedence; isolated UTF-16 surrogates map to U+FFFD in the UTF-8 API | +| defaults, whitelist key, difficulty IDs, and signed integers | `49 PASS / 0 FAIL` | `settings_matrix`; vanilla online/profile defaults, view distance, MOTD, difficulty, random new-world seed, canonical `white-list`, source-order alias behavior, difficulty IDs `0..3`, leading `+`, and malformed doubled signs are covered | +| Java Properties parsing and configuration | `64 PASS / 0 FAIL` | `properties`; separators, escapes, continuations, key case, UTF-8/ISO-8859-1 fallback, Java boolean semantics, CLI precedence, and rejection of malformed `+-` integers; isolated UTF-16 surrogates map to U+FFFD in the UTF-8 API | | effective secure-chat policy | `12 PASS / 0 FAIL` | `secure_chat_policy`; status and Join Game share the effective policy, `enforce-secure-profile || enforces-secure-chat`; unsigned-after-session behavior remains unverified | | fake-client authentication options | `PASS` | source guard and Python syntax checks cover local fake-client launchers; each offline launcher explicitly disables both online mode and secure-profile enforcement | -| full non-nightly CTest | `55/55 PASS` | 516.24s, exact command and scope in [VERIFICATION.md](VERIFICATION.md#vanilla-server-settings-compatibility-follow-up-2026-09-23-pre-merge) | +| full non-nightly CTest | `55/55 PASS` | 516.24s on pre-review head `5eb101d1`; exact-SHA Actions must also pass for PR #2's current head; command and scope in [VERIFICATION.md](VERIFICATION.md#vanilla-server-settings-compatibility-follow-up-2026-09-23-pre-merge) | | server child cleanup | `PASS` | post-suite `pgrep -a -f 'cppfm --por[t]'` found no remaining `cppfm` server process | | remaining property parity | `PARTIAL` | `enforce-whitelist`, query/status/network, permission, datapack, and several world/pack/operations effects are not fully implemented; `ServerProperties::save` is not `Properties.store`-compatible | diff --git a/docs/MISSING_FEATURES_1_21_4.md b/docs/MISSING_FEATURES_1_21_4.md index 4d3c4ddf..e041b737 100644 --- a/docs/MISSING_FEATURES_1_21_4.md +++ b/docs/MISSING_FEATURES_1_21_4.md @@ -45,7 +45,7 @@ historical numbered taxonomy. | concern | current result | evidence / declared boundary | |---|---|---| -| server settings and precedence | `PASS / SUBSET` | 2026-09-19 baseline `test_settings_matrix`: `27 PASS / 0 FAIL`; invalid values retain prior fields, textual/numeric seeds are distinguished, stored secure-profile and cppfm secure-chat settings remain separate, and unsupported keys are reported. The 2026-09-23 compatibility follow-up is recorded in [CURRENT_STATE.md §1C](CURRENT_STATE.md#1c-vanilla-1214-settings-compatibility-follow-up-2026-09-23): `47` settings, `63` properties, and `12` secure-chat-policy assertions pass, with the full settings surface still partial. | +| server settings and precedence | `PASS / SUBSET` | 2026-09-19 baseline `test_settings_matrix`: `27 PASS / 0 FAIL`; invalid values retain prior fields, textual/numeric seeds are distinguished, stored secure-profile and cppfm secure-chat settings remain separate, and unsupported keys are reported. The 2026-09-23 compatibility follow-up is recorded in [CURRENT_STATE.md §1C](CURRENT_STATE.md#1c-vanilla-1214-settings-compatibility-follow-up-2026-09-23): `49` settings, `64` properties, and `12` secure-chat-policy assertions pass, with the full settings surface still partial. | | command/authority and inventory boundaries | `PASS` | `test_properties`: `33 PASS / 0 FAIL`; full live matrix: `240 PASS / 0 FAIL`; Plan43: `87 PASS / 0 FAIL`; op-gated roots, one-shot trigger enablement, creative/range/sign/beacon/pick checks, source-dimension item commands, resource-pack UUID completion, and signed-command offset parsing are covered | | secure chat/profile | `PASS / BOUNDED` | valid signed inbound chat is verified with the protocol-769 SHA256 transcript and relayed as `SystemChat` under enforced secure chat; Mojang profile certificates use SHA1withRSA and all configured keys are tried; rejected profiles terminate; signed command argument transcripts are deliberately fail-closed and remain a declared limitation. The 2026-09-23 policy/advertisement follow-up and its unsigned-after-session boundary are recorded in [CURRENT_STATE.md §1C](CURRENT_STATE.md#1c-vanilla-1214-settings-compatibility-follow-up-2026-09-23). | | event and persistence ordering | `PASS` | cancellable block events run before mutations; scoped callbacks synchronize removal; moving-piston source/destination commits are flushed under the simulation gate before synchronous/background snapshots; transient moving-piston NBT remains intentionally omitted | diff --git a/docs/VERIFICATION.md b/docs/VERIFICATION.md index 40382ffb..7a49064f 100644 --- a/docs/VERIFICATION.md +++ b/docs/VERIFICATION.md @@ -75,11 +75,11 @@ parity. Pre-merge local evidence: | check | result | command / scope | |---|---|---| | RelWithDebInfo configure and full build | `PASS` | `timeout --foreground --kill-after=5 120 cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo`; `timeout --foreground --kill-after=5 600 cmake --build build -j4` | -| properties/parser | `63 PASS / 0 FAIL` | `timeout --foreground --kill-after=5 60 ./build/test_properties ./build/cppfm` | -| config/settings matrix | `47 PASS / 0 FAIL` | `timeout --foreground --kill-after=5 60 ./build/test_settings_matrix` | +| properties/parser | `64 PASS / 0 FAIL` | `timeout --foreground --kill-after=5 60 ./build/test_properties ./build/cppfm`; includes malformed `+-` rejection | +| config/settings matrix | `49 PASS / 0 FAIL` | `timeout --foreground --kill-after=5 60 ./build/test_settings_matrix`; includes malformed integer rejection and `level-seed` string-hash fallback | | secure-chat policy | `12 PASS / 0 FAIL` | `timeout --foreground --kill-after=5 60 ./build/test_secure_chat_policy` | | source/process guards | `PASS` | `goal_security_guards`, Python syntax checks for the modified launcher scripts, and the runtime-layout CTest all pass | -| full non-nightly CTest | `55/55 PASS` in `516.24s` | `timeout --foreground --kill-after=5 2400 ctest --test-dir build -LE 'nightly|package' --output-on-failure --timeout 600` | +| full non-nightly CTest baseline | `55/55 PASS` in `516.24s` on pre-review head `5eb101d1` | `timeout --foreground --kill-after=5 2400 ctest --test-dir build -LE 'nightly|package' --output-on-failure --timeout 600`; the revised PR head is separately gated by exact-SHA Actions | | owned server cleanup | `PASS` | `pgrep -a -f 'cppfm --por[t]'` found no remaining server process after the suite | The settings tests cover vanilla-facing defaults (`online-mode=true`, diff --git a/src/game/ServerConfig.cpp b/src/game/ServerConfig.cpp index caa46bb6..98a63b12 100644 --- a/src/game/ServerConfig.cpp +++ b/src/game/ServerConfig.cpp @@ -155,7 +155,7 @@ bool parseInteger(std::string_view text, T& value) { // std::from_chars intentionally does not. if (text.front() == '+') { text.remove_prefix(1); - if (text.empty()) return false; + if (text.empty() || text.front() == '-') return false; } const auto result = std::from_chars(text.data(), text.data() + text.size(), value, 10); return result.ec == std::errc{} && result.ptr == text.data() + text.size(); @@ -229,9 +229,7 @@ void applySeed(ServerConfig& config, std::string_view value, } std::int64_t numeric = 0; - const auto numericText = !value.empty() && value.front() == '+' - ? value.substr(1) : value; - if (!numericText.empty() && parseInteger(numericText, numeric)) { + if (parseInteger(value, numeric)) { config.hashedSeed = numeric; config.seed = static_cast(numeric); return; diff --git a/src/game/ServerProperties.hpp b/src/game/ServerProperties.hpp index 52a68e16..ced7fa10 100644 --- a/src/game/ServerProperties.hpp +++ b/src/game/ServerProperties.hpp @@ -376,7 +376,7 @@ class ServerProperties { if (text.empty()) return false; if (text.front() == '+') { text.remove_prefix(1); - if (text.empty()) return false; + if (text.empty() || text.front() == '-') return false; } const char* first = text.data(); const char* last = first + text.size(); diff --git a/tests/test_properties.cpp b/tests/test_properties.cpp index fd537255..0223f251 100644 --- a/tests/test_properties.cpp +++ b/tests/test_properties.cpp @@ -56,6 +56,10 @@ void testPropertiesSyntax() { ServerProperties leadingPlus; check(leadingPlus.loadText("number=+42") && leadingPlus.get("number", 0) == 42, "typed integer properties accept Java's leading plus sign"); + ServerProperties malformedSignedInteger; + check(malformedSignedInteger.loadText("number=+-42") && + malformedSignedInteger.get("number", 7) == 7, + "a plus sign cannot be followed by a second, negative sign"); ServerProperties properties; check(properties.loadText( diff --git a/tests/test_settings_matrix.cpp b/tests/test_settings_matrix.cpp index 67f23a15..8fb6e90b 100644 --- a/tests/test_settings_matrix.cpp +++ b/tests/test_settings_matrix.cpp @@ -245,6 +245,19 @@ void testDifficultyOrdinals() { check(plusLoaded && plusConfig.port == 25571 && plusConfig.difficulty == "hard" && plusDiagnostics.entries.empty(), "Java-style leading plus signs are accepted for integers and difficulty IDs"); + + ServerProperties malformedSigns; + const bool malformedLoaded = malformedSigns.loadText( + "view-distance=+-4\nlevel-seed=+-1\n"); + ServerConfig malformedConfig; + ConfigDiagnostics malformedDiagnostics; + applyServerProperties(malformedConfig, malformedSigns, &malformedDiagnostics); + check(malformedLoaded && malformedConfig.viewDistance == 10 && + hasDiagnostic(malformedDiagnostics, ConfigDiagnosticKind::InvalidValue, + "view-distance"), + "malformed signed integer properties are rejected instead of being clamped"); + check(malformedConfig.hashedSeed == 42767 && malformedConfig.seed == 42767, + "a non-numeric level-seed with a doubled sign uses Java string hashing"); } void testAliasOrderAndMapMutation() {