Fix crash logging non-UTF-8 narrow strings

[marcusfreisleben]

Problem

Logging a message containing non-UTF-8 narrow bytes (e.g. a std::exception::what() built from a Windows ACP/CP1252 string, or a __FILE__ path with an accented character) crashes the app.

utility::s2us requires valid UTF-8 and throws std::range_error otherwise. nmos::experimental::details::json_from_message calls it directly on message.str(), message.file(), message.function() and categories, on the async logging worker thread. slog::message_service::run only catches its own stopped_exception, so the throw escapes the thread function and terminates the process.

Confirmed on a german Windows 11 installation.

Fix

• Add utility::s2us_lenient (cpprest/basic_utils.h/.cpp): UTF-8 fast path, Windows ANSI code page fallback, Latin-1 fallback otherwise. Never throws, never discards the message. s2us's contract and existing call sites are unchanged.
  • Validates with conversions::to_utf16string, the same decoder s2us uses on a wide build, so the fast path accepts exactly what s2us accepts. Note this decoder is not a strict UTF-8 validator (tolerates surrogates, overlongs, code points above U+10FFFF) - a hand-rolled check would have diverged from it and reopened the bug.
  • Converts the original string rather than the successfully-decoded value, so on the fast path the result is byte-identical to what s2us would have returned, on narrow builds too.
  • Skips the ANSI code page when it is itself UTF-8 (Windows 10+), where MultiByteToWideChar would just repeat the failed decode and substitute U+FFFD, losing the characters at issue.
• Use it in json_from_message (message, file, function, categories) and at the what()/error-message boundaries in the API error responses (api_utils, connection_api, channelmapping_api, authorization_redirect_api, control_protocol_ws_api).
• Guard log_gate::service as a last-resort safety net, so no future logging failure can terminate the app.

Testing

• basic_utils_test.cpp: valid UTF-8 fast path, invalid-byte fallback (CP1252 umlaut and others), decoder-tolerated sequences pass through unchanged, empty input.
• log_gate_test.cpp: testInsertLogEventNonUtf8 builds a log message with CP1252 umlaut bytes through the same path that crashed and asserts the emitted JSON is valid UTF-8. This fails against the pre-fix code (confirmed) - on non-Windows too, since s2us is a pass-through there and the invalid bytes previously reached the JSON unmodified.
• Built and tested on macOS (narrow build) and confirmed on Windows.

Open question for maintainers

There are ~15 other utility::s2us(...) call sites taking narrow strings of uncertain origin (mDNS host names, JWT claims, OpenSSL PEM output, etc). Left out of this PR to keep it focused on the crash. Happy to follow up with s2us_lenient at those sites too if that's wanted, or to fold s2us_lenient behavior into s2us itself if you'd rather not carry two functions - whichever you prefer.

[garethsb]

How does this relate to #109?

[garethsb]

I wonder what the minimum blast radius is... is it better to wrap every external API call that can return/throw a non-UTF8 string or to handle such strings as close as possible to outputting them?

[marcusfreisleben]

How does this relate to #109?

It does sound like it could be related, yeah. Fixing it defensively at the consumer side has the benefit of also covering other non-Boost sources and we don't have to diverge from the official conan boost recipe.

[garethsb]

Hi @marcusfreisleben, thanks for this!

Initial (🤖 assisted) review:

s2us_lenient call sites

Call	Notes
json_from_message: message, __FILE__, __FUNCTION__	Right. This is the logging-thread crash.
set_error_reply(response, code, exception)	Right. what() has no UTF-8 guarantee, and range_error is a runtime_error, so a throw here makes the API exception handler call this function again.
Connection, channel-mapping, and authorization-redirect helpers that copy debug.what() into an error body	Right. Same pattern, local copies of that helper.
Control-protocol websocket e.what() error messages	Right. Same bytes, sent as an error message rather than an HTTP body.
Authorization error.message in api_utils.cpp	Right for the JSON body. That std::string is often e.what() (authorization.cpp, authorization_handlers.cpp).
Log categories in json_from_message	Maybe unnecessary? 🤔

Agree slog << e.what() at the individual log statements does not need its own conversion. The narrow bytes sit in the message until json_from_message converts them once.

Where to keep strict s2us

Strict s2us should stay for data that doesn't come from the local system or a spec says is UTF-8 or ASCII, because a replacement character or a code-page guess would be published as if it were the real value:

• LLDP chassis and port IDs written onto the Node resource. MAC IDs are already formatted as hex; other subtypes may contain opaque octets, which should not be interpreted using a platform code page.
• mDNS instance names, domains, and hostnames (UTF-8 or ASCII). NMOS TXT values used for discovery (api_proto, api_ver, pri, api_auth) are ASCII. A binary TXT value in the experimental mDNS API is opaque bytes; RFC 6763 §6.5 says to show those as a hex dump, not to invent a character. Hmm. 🤔
• SDP fields. RFC 4566 requires UTF-8 except for s= and i= when a=charset says otherwise, and this code does not implement a=charset. A bad byte in c= or a=fmtp must fail the parse. Probably? 🤔
• JWT claims, PEM, JSON schemas, settings from argv, numeric ids passed through s2us(std::to_string(...)).

Suggestions

1. Drop lenient conversion for categories.
2. Decode once. On a wide build, return the to_utf16string result instead of decoding again with to_string_t. On a narrow build, that call is only the validity check.
3. Keep the Windows CP_ACP fallback for a string that is not valid UTF-8. Drop the Latin-1 fallback. Where the Windows fallback is not used, replace each malformed sequence with U+FFFD and leave the valid UTF-8 around it alone. What do you think?
4. Move the try in log_gate::service above model.write_lock(). Maybe? The catch is right; the lock acquisition is part of the worker and should be inside it, I think?
5. Tests. One Windows case for 0xFC under CP1252 becoming U+00FC. One case for a malformed sequence surrounded by valid UTF-8, asserting the valid part survives and only the bad sequence becomes U+FFFD. The current invalid-byte test only checks that the result is non-empty and well-formed.
6. Authorization header. The JSON body can carry the lenient string. error_description on WWW-Authenticate is ASCII (RFC 6749); a replacement character does not belong in that parameter. The error code (invalid_token, insufficient_scope) is already there.