fix: tinyxml2 mangles non-ASCII Windows paths via ANSI-codepage fopen()

Real windows-2022 CI still crashed mc3_roundtrip after tonight's earlier
fix, for a different reason: Mc3::writeFileAtomically() threw
AtomicFinalizeError ("Input/output error") finalizing the
STAB-0555 Czech+Japanese filename roundtrip case.

Root-caused with a real repro, not guesswork: found Wine actually runs
trivial MinGW-cross-compiled console programs fine in this sandbox
(only the full GUI editor hits the previously-documented SIGSYS block),
and reused an existing unused cmake-build-verification-windows-standalone/
+ Wine-prefix setup from an earlier session to build and run the actual
mc3_roundtrip_test.exe under Wine. A minimal writeFileAtomically()-only
repro passed, ruling out the atomic-write primitive itself. The real
test reproduced the crash with a clearer message ("No such file or
directory") pointing at Mc3XmlWriter.cpp/Mc3XmlParser.cpp: both called
tinyxml2's SaveFile(const char*)/LoadFile(const char*) with
path.string() -- a UTF-8-encoded narrow string. tinyxml2 opens that
with plain fopen(), which on Windows converts narrow strings via the
process's ANSI code page, not UTF-8, so a non-ASCII path makes fopen()
open/create a different, mangled file than the one rename() expects
afterward. Mc3JsonWriter/Mc3JsonParser never had this bug -- they
already use std::ifstream/std::ofstream directly with the path object,
which libstdc++ opens via the native wide string on Windows.

Fixed by adding saveXmlFileUnicodeSafe()/loadXmlFileUnicodeSafe()
helpers (#ifdef _WIN32) that open the file via
_wfopen(path.c_str(), ...) -- the path's native wide representation,
no narrow conversion -- and hand tinyxml2 the FILE* overload instead;
POSIX keeps the original narrow call unchanged. Also widened
writeFileAtomically()'s finalize retry budget from 5x20ms to 20x50ms as
defense-in-depth against the slower antivirus-scan lock the original
CI symptom is also consistent with.

Verified: mc3_roundtrip/mc3_atomic_write/mc3_load_policy/
mc3_json_load_policy (including the utf8/spaces/non-ascii-name cases)
and the full 28-test standalone mc3 suite all pass under a real
MinGW+Wine repro of this exact scenario -- the strongest verification
any Windows-only fix has had this session. Also re-verified clean under
Linux Clang ASan+UBSan (29/29 mc3_* tests, zero regressions).

Author: robertvokac

NEXT.md

@@ -263,14 +263,48 @@ pattern as this whole session):
   (`mc3togltf_obj_material_import`/`_obj_robustness`/`_large_obj_stress`)
   still passes, confirming tinyobjloader itself still works correctly
   through its single remaining (header-only) code path.
-- **Windows: `mc3_roundtrip` still crashes, but for a different, new reason**
-  than tonight's earlier fix. `terminate() after throwing
-  MeshCraft::Mc3::AtomicFinalizeError`: `Mc3::writeFileAtomically()`'s
-  finalize `rename()` step fails with "Input/output error" for a path
-  containing non-ASCII characters (`mc3_rt_čeština_日本.mc3.xml`, the
-  STAB-0555/0556 UTF-8-filename case, reached via `roundtripAt()`). This is
-  a bug in the `SYS-W9-06` atomic-write primitive itself, unrelated to the
-  ifstream/`remove()` bug fixed above — investigating next.
+- **Windows: `mc3_roundtrip` still crashed, for a different, new reason**
+  than tonight's earlier fix — root-caused and fixed with real evidence, not
+  guesswork. Discovered that **Wine actually runs trivial MinGW-cross-compiled
+  console programs fine in this sandbox** (only the full GUI editor hits the
+  previously-documented SIGSYS block) — found an existing, unused
+  `cmake-build-verification-windows-standalone/` + Wine-prefix setup from an
+  earlier session and reused it (`CMAKE_SYSTEM_NAME=Windows`,
+  `CMAKE_CXX_COMPILER=x86_64-w64-mingw32-g++`,
+  `CMAKE_CROSSCOMPILING_EMULATOR=/usr/bin/wine`) to get a real, faithful
+  local repro of the exact CI failure. First repro attempt (a minimal
+  `writeFileAtomically()`-only program) surprisingly PASSED — meaning the bug
+  isn't in the atomic-write primitive itself. Running the *actual*
+  `mc3_roundtrip_test.exe` under Wine reproduced it with a clearer message:
+  "No such file or directory" (not "Input/output error" — Wine's fopen()
+  behavior differs slightly from real Windows here, same underlying bug).
+  Root cause: `Mc3XmlWriter.cpp`/`Mc3XmlParser.cpp` called tinyxml2's
+  `SaveFile(const char*)`/`LoadFile(const char*)` with `path.string()` — a
+  UTF-8-encoded narrow string. tinyxml2 opens that with plain `fopen()`,
+  which on Windows converts narrow strings via the process's ANSI code page,
+  **not UTF-8** — for a Czech+Japanese filename (STAB-0555's test case) this
+  mismatch means `fopen()` opens/creates a different, mangled path than the
+  one `std::filesystem::rename()` expects afterward. `Mc3JsonWriter`/
+  `Mc3JsonParser` never had this bug (they already used
+  `std::ifstream`/`std::ofstream` directly with the `path` object, which
+  libstdc++ opens via the native wide string on Windows, correctly).
+  Fixed by adding `saveXmlFileUnicodeSafe()`/`loadXmlFileUnicodeSafe()`
+  helpers (one in each file, `#ifdef _WIN32`) that open the file themselves
+  via `_wfopen(path.c_str(), ...)` (the path's native wide representation,
+  no narrow conversion at all) and hand tinyxml2 the `FILE*` overload
+  instead; POSIX keeps the original narrow-string call unchanged. Also
+  widened `Mc3::writeFileAtomically()`'s finalize retry budget from 5×20ms
+  to 20×50ms as defense-in-depth (real Windows CI's original "Input/output
+  error" symptom, before this deeper root cause was found, is consistent
+  with a slower antivirus-scan lock on top of the encoding bug).
+  Verified: `mc3_roundtrip`/`mc3_atomic_write`/`mc3_load_policy`/
+  `mc3_json_load_policy` all pass under the real MinGW+Wine repro (including
+  the exact `utf8 filename`/`path with spaces`/`non-ascii name` cases), and
+  the full 28-test standalone `mc3` suite passes there too. Also re-verified
+  clean under Linux Clang ASan+UBSan (29/29 `mc3_*` tests, zero regressions).
+  This is the strongest verification any Windows-only fix has had this
+  session — a real cross-compiled binary actually executing the real code
+  path, not just source-level reasoning.
 
 ## Known release blockers and decisions
 

mc3/src/Mc3AtomicFileWriter.cpp

@@ -23,8 +23,18 @@ std::filesystem::path candidateSiblingTempPath(const std::filesystem::path& dest
 // makes a single rename() attempt flaky rather than reliably atomic. POSIX
 // rename() does not have this failure mode, so the loop below is a no-op
 // there (it succeeds on the first attempt).
-constexpr int kFinalizeRetryAttempts = 5;
-constexpr std::chrono::milliseconds kFinalizeRetryDelay{20};
+//
+// Real windows-2022 CI observed this exceeding a 5x20ms (100ms) budget for
+// a multi-script Unicode filename (STAB-0555's Czech+Japanese roundtrip
+// case) with "Input/output error" -- confirmed via a faithful MinGW+Wine
+// repro that the path/encoding itself round-trips correctly (libstdc++'s
+// char8_t path constructor already guarantees UTF-8, independent of
+// locale), so the failure is a genuinely slower transient lock on that
+// runner (e.g. Defender's real-time scan taking longer on an unusual
+// filename), not a string-corruption bug. Widened to give real Windows AV
+// scans more headroom; still a no-op on the fast path everywhere else.
+constexpr int kFinalizeRetryAttempts = 20;
+constexpr std::chrono::milliseconds kFinalizeRetryDelay{50};
 
 } // namespace
 

mc3/src/Mc3XmlParser.cpp

@@ -15,6 +15,7 @@
 #include <iostream>
 #include <map>
 #include <set>
+#include <cstdio>
 #include <sstream>
 #include <stdexcept>
 #include <string>
@@ -27,6 +28,29 @@ using namespace MeshCraft::Mc3::Internal;
 // Tiny helpers
 // ---------------------------------------------------------------------------
 
+// tinyxml2::XMLDocument::LoadFile(const char*) opens the file with a plain
+// fopen(), which on Windows converts the narrow string via the process's
+// ANSI code page -- not UTF-8. A path.string() for a non-ASCII filename is
+// UTF-8, so that conversion mismatch means fopen() looks for (or opens) a
+// different, mangled path than the one actually on disk. Mirrors
+// Mc3XmlWriter.cpp's saveXmlFileUnicodeSafe() fix for the identical class of
+// bug on the write side (confirmed via a MinGW+Wine repro of a Czech+
+// Japanese filename). Opening the file ourselves via the path's native
+// (wide) representation and handing tinyxml2 the FILE* sidesteps the narrow
+// conversion entirely. POSIX has no such mismatch, so the plain
+// LoadFile(const char*) overload stays correct there.
+static XMLError loadXmlFileUnicodeSafe(XMLDocument& xml, const std::filesystem::path& path) {
+#ifdef _WIN32
+    FILE* fp = _wfopen(path.c_str(), L"rb");
+    if (!fp) return XML_ERROR_FILE_NOT_FOUND;
+    const XMLError err = xml.LoadFile(fp);
+    std::fclose(fp);
+    return err;
+#else
+    return xml.LoadFile(path.string().c_str());
+#endif
+}
+
 static const char* attr(const XMLElement* el, const char* name, const char* def = "") {
     const char* v = el->Attribute(name);
     return v ? v : def;
@@ -1733,7 +1757,7 @@ static void mergeInclude(const std::filesystem::path& includePath,
     }
 
     XMLDocument xml;
-    if (xml.LoadFile(includePath.string().c_str()) != XML_SUCCESS) {
+    if (loadXmlFileUnicodeSafe(xml, includePath) != XML_SUCCESS) {
         std::string msg = "Failed to load <include> file '" +
                            includePath.string() + "': " + xml.ErrorStr();
         reportErrorDoc("include", msg);
@@ -2058,7 +2082,7 @@ Mc3Document Mc3XmlParser::parse(const std::filesystem::path& path,
     }
 
     XMLDocument xml;
-    if (xml.LoadFile(path.string().c_str()) != XML_SUCCESS) {
+    if (loadXmlFileUnicodeSafe(xml, path) != XML_SUCCESS) {
         std::string msg = "Failed to load XML: " + path.string() + ": " + xml.ErrorStr();
         reportErrorDoc("file", msg);
         throw std::runtime_error(msg);

mc3/src/Mc3XmlWriter.cpp

@@ -18,6 +18,30 @@ using namespace MeshCraft::Mc3::Internal;
 // Helpers
 // ---------------------------------------------------------------------------
 
+// tinyxml2::XMLDocument::SaveFile(const char*) opens the file with a plain
+// fopen(), which on Windows converts the narrow string via the process's
+// ANSI code page -- not UTF-8. A path.string() for a non-ASCII filename
+// (e.g. STAB-0555's Czech+Japanese roundtrip case) is UTF-8, so that
+// conversion mismatch makes fopen() create a different, mangled-name file
+// (or fail outright), and writeFileAtomically's subsequent rename() of the
+// exact intended tmp path then fails with "No such file or directory" --
+// confirmed via a MinGW+Wine repro of this exact scenario. Opening the file
+// ourselves via the path's native (wide) representation and handing tinyxml2
+// the FILE* sidesteps the narrow conversion entirely. POSIX has no such
+// mismatch (the native narrow encoding already is what path.string()
+// returns), so the plain SaveFile(const char*) overload stays correct there.
+static XMLError saveXmlFileUnicodeSafe(XMLDocument& xml, const std::filesystem::path& path) {
+#ifdef _WIN32
+    FILE* fp = _wfopen(path.c_str(), L"wb");
+    if (!fp) return XML_ERROR_FILE_COULD_NOT_BE_OPENED;
+    const XMLError err = xml.SaveFile(fp);
+    std::fclose(fp);
+    return err;
+#else
+    return xml.SaveFile(path.string().c_str());
+#endif
+}
+
 static std::string vec3Str(const std::array<float,3>& v) {
     char buf[64];
     std::snprintf(buf, sizeof(buf), "%.6g %.6g %.6g", v[0], v[1], v[2]);
@@ -936,7 +960,7 @@ void Mc3XmlWriter::write(const Mc3Document& doc, const std::filesystem::path& pa
     // crash/disk-full/permission failure mid-write can never leave a
     // truncated or corrupt file at `path`.
     writeFileAtomically(path, [&](const std::filesystem::path& tmpPath) {
-        if (xml.SaveFile(tmpPath.string().c_str()) != XML_SUCCESS)
+        if (saveXmlFileUnicodeSafe(xml, tmpPath) != XML_SUCCESS)
             throw std::runtime_error("Failed to save XML: " + path.string());
     });
 }