fix(logger): write stdout/stderr via utf8_str instead of wxConvLibc (#40) - #42
Merged
Merged
Conversation
…mule-project#40) amuled never calls `setlocale(LC_ALL, "")`, so its C locale stays at the default `C`. The on-disk log path writes UTF-8 explicitly via `wxConvUTF8` in `FlushApplog` and is fine. The stdout/stderr path goes through `unicode2char()` which uses `wxConvLibc` -- the C library locale converter -- which collapses non-ASCII bytes to `?` / U+FFFD when the process locale isn't UTF-8 capable. Result: log lines containing accented filenames, server messages, fancy quotes, etc. come out mangled when `amuled -o` runs in a minimal container (`LANG`/`LC_ALL` unset), while the same lines in `~/.aMule/logfile` are perfectly UTF-8. (amule-project#40 from @ngosang with a full root-cause analysis.) Switch the stdout/stderr log sinks to `wxString::utf8_str()`, which always returns UTF-8 regardless of the process locale. The two sinks (logfile, stdout) now both write UTF-8 by construction. Also apply the same change to the four other locale-sensitive print sites that share the bug shape in the connectors: * `CaMuleExternalConnector::Show()` -- command output / prompts from amulecmd and amuleweb * `--version` banner * "FATAL ERROR: File does not exist" config-init message * `CLogger::EmergencyLog` stderr path (same pattern as DoLine) Connectors do call `setlocale(LC_ALL, "")` at startup, so they would work in a normal interactive shell -- but they hit the same "C locale fallback" trap in minimal containers where `LANG` / `LC_ALL` aren't exported. ngosang flagged this in a follow-up comment ("Review also src/ExternalConnector.cpp, I don't trust Linux locales"). Not addressed in this PR (separate concerns): * `ExternalConnector.cpp:243, 382` -- libreadline integration; readline expects locale-encoded bytes, not UTF-8. * `ExternalConnector.cpp:681, 682` -- version/OS strings cached for HTTP User-Agent. Should also be UTF-8 but the conversion is one-shot at startup so the immediate user-visible bug is elsewhere. Verified on macOS with a standalone wxString test that calls `setlocale(LC_ALL, "C")` to mimic amuled's runtime: `wxConvLibc` returns NULL (conversion failed); `utf8_str()` returns clean UTF-8 bytes for the accented chars, smart quotes, arrow, and CJK char.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #40 —
amuled -ocorrupts non-ASCII log lines on stdout when the process locale isn't UTF-8 capable (common in minimal containers withoutLANG/LC_ALLexported). The on-disk logfile path is unaffected because it writes UTF-8 explicitly viawxConvUTF8.@ngosang did the full root-cause analysis in the issue body —
unicode2char()useswxConvLibc(C-library locale converter), which returns NULL in aClocale, after which the fallback path collapses each non-ASCII char to?. amuled never callssetlocale(LC_ALL, "")(onlyCaMuleExternalConnectordoes, for amulecmd / amuleweb).Fix
Switch the stdout/stderr log sinks to
wxString::utf8_str(), which always returns UTF-8 regardless of locale. The two sinks (logfile, stdout) now both write UTF-8 by construction.Five call sites:
CLogger::DoLinestdout path (Logger.cpp:274) — the main reported bugCLogger::EmergencyLogstderr path (Logger.cpp:288) — same patternCaMuleExternalConnector::Show()(ExternalConnector.cpp:302) — amulecmd / amuleweb interactive output. Connectors do callsetlocale(LC_ALL, "")at startup, but that doesn't help in minimal containers whereLANG/LC_ALLaren't exported (setlocale("")then falls back toC). Per @ngosang's follow-up comment ("Review alsosrc/ExternalConnector.cpp, I don't trust Linux locales").--versionbanner (ExternalConnector.cpp:532)ExternalConnector.cpp:546)Not addressed in this PR
ExternalConnector.cpp:243, 382— libreadline integration; readline expects locale-encoded bytes, not UTF-8.ExternalConnector.cpp:681, 682— version / OS-description strings cached for HTTP User-Agent. Worth a follow-up but doesn't affect the immediate user-visible bug.Verification
Standalone wxString conversion test compiled against the same wxWidgets the build uses, with
setlocale(LC_ALL, "C")to mimic amuled's runtime locale:wxConvLibc.cWX2MB()returns NULL → runtime fallback inunicode2char()collapses to?(exactly the symptom @ngosang reported).utf8_str()returns clean UTF-8 bytes for accented chars, smart quotes, arrow, and CJK char.Backward compat
wxString::utf8_str()is locale-independent and produces the same UTF-8 bytes on every platform. No protocol change, no behavior change for users whose locale was already UTF-8 (they were getting UTF-8 either way — the bug was masked whensetlocale("")happened to produce a UTF-8 locale).