From 61726bae6dd542ebc4f8a2de1a478d35288a8d4e Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Sun, 6 Sep 2026 13:49:16 -0700 Subject: [PATCH 1/9] server: do not abort a completion when the model emits invalid UTF-8 The generated text is a raw byte stream and is not guaranteed to be valid UTF-8. A byte fallback token, or a prompt that ends in the middle of a multi-byte character (the model then continues with the remaining continuation bytes), makes the generated text start with, or contain, bytes that do not decode. task_result_state::update_chat_msg() hands that text to common_chat_parse() on every token. The PEG parsers reject malformed UTF-8 by design, and the std::runtime_error thrown for it propagates out of the streaming loop, closes the connection and cancels the task, so the request ends with no tokens at all. On a non-streaming request it turns into a 500. The endpoint does not matter: /completion parses the text for the chat message as well. Normalise the text before parsing: hold back the trailing bytes of an incomplete sequence until the next chunk completes it, and replace bytes that can never form a codepoint with U+FFFD. That is what the JSON serialiser already substitutes on the way to the client, so the parser now sees exactly the text the client receives. --- tools/server/server-task.cpp | 36 +++++++++++++++++++++++++++++++++++- tools/server/server-task.h | 3 ++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/tools/server/server-task.cpp b/tools/server/server-task.cpp index 0d3beb313cea..8dbe0bbbca49 100644 --- a/tools/server/server-task.cpp +++ b/tools/server/server-task.cpp @@ -9,6 +9,7 @@ #include "sampling.h" #include "speculative.h" #include "server-common.h" +#include "unicode.h" #include @@ -159,12 +160,45 @@ task_result_state::task_result_state(const common_chat_parser_params & chat_pars } } +// Appends `text_added` to `text`, keeping in `pending` the trailing bytes of an incomplete UTF-8 +// sequence so that the next chunk can complete it, and replacing bytes that can never form a valid +// codepoint with U+FFFD. +// +// The generated text is a raw byte stream and is not guaranteed to be valid UTF-8: a byte fallback +// token, or a prompt that ends in the middle of a multi-byte character (the model then continues +// with the remaining continuation bytes), makes it start with, or contain, bytes that do not decode. +// The chat parsers reject malformed UTF-8 by design, and the exception thrown for it propagates out +// of the streaming loop and cancels the task, so the request ends with no tokens at all. The JSON +// serialiser already substitutes U+FFFD for those bytes on the way to the client, so doing the same +// substitution before parsing keeps the parser input identical to what the client receives. +static void append_utf8_sanitized(std::string & text, std::string & pending, const std::string & text_added) { + pending += text_added; + + size_t pos = 0; + while (pos < pending.size()) { + const auto res = common_parse_utf8_codepoint(pending, pos); + if (res.status == utf8_parse_result::INCOMPLETE) { + // wait for the rest of the sequence + break; + } + if (res.status == utf8_parse_result::INVALID) { + text += "\xEF\xBF\xBD"; // U+FFFD REPLACEMENT CHARACTER + pos += 1; + continue; + } + text.append(pending, pos, res.bytes_consumed); + pos += res.bytes_consumed; + } + + pending.erase(0, pos); +} + common_chat_msg task_result_state::update_chat_msg( const std::string & text_added, bool is_partial, std::vector & diffs, bool filter_tool_calls) { - generated_text += text_added; + append_utf8_sanitized(generated_text, generated_text_pending, text_added); auto msg_prv_copy = chat_msg; //SRV_DBG("Parsing chat message: %s\n", generated_text.c_str()); auto new_msg = common_chat_parse( diff --git a/tools/server/server-task.h b/tools/server/server-task.h index 9c99143f8e19..eda5fe20e4ce 100644 --- a/tools/server/server-task.h +++ b/tools/server/server-task.h @@ -107,7 +107,8 @@ struct task_result_state { std::vector diffs; common_chat_parser_params chat_parser_params; common_chat_msg chat_msg; - std::string generated_text; // append new chunks of generated text here + std::string generated_text; // append new chunks of generated text here, see update_chat_msg() + std::string generated_text_pending; // trailing bytes of an incomplete UTF-8 sequence std::vector generated_tool_call_ids; std::unordered_set sent_tool_call_names; From 25fcc22aa292970083e20c420f7636f95a8c2671 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Tue, 8 Sep 2026 18:53:55 -0700 Subject: [PATCH 2/9] server: flush a trailing incomplete UTF-8 sequence on the final parse append_utf8_sanitized holds the tail of an incomplete sequence back so the next chunk can complete it, but on the last update nothing more is coming. A generation that stopped mid character therefore left those bytes in generated_text_pending forever, and because the parsed message is preferred over the raw content whenever it is nonempty, the response silently dropped them: 'hello\xC3' was returned as 'hello' rather than the 'hello' plus U+FFFD that dump_safe produces. One U+FFFD, not one per byte. nlohmann's replace handler emits a single replacement character per incomplete sequence, checked against the vendored header: '\xC3', '\xE2' and '\xE2\x82' each render as one, while '\xC3\xC3' renders as two because the first is terminated by the second lead byte. What stays pending is by construction exactly one incomplete sequence, since the loop consumes everything decidable before it breaks. Gated on the final update, so the streaming path is untouched. --- tools/server/server-task.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/server/server-task.cpp b/tools/server/server-task.cpp index 8dbe0bbbca49..a4183e716be1 100644 --- a/tools/server/server-task.cpp +++ b/tools/server/server-task.cpp @@ -199,6 +199,14 @@ common_chat_msg task_result_state::update_chat_msg( std::vector & diffs, bool filter_tool_calls) { append_utf8_sanitized(generated_text, generated_text_pending, text_added); + if (!is_partial && !generated_text_pending.empty()) { + // Nothing can complete this sequence now (the generation hit its limit, or stopped, mid + // character), so it gets the one U+FFFD the JSON serialiser would show for it rather than + // being dropped: a nonempty parsed message is preferred over the raw content downstream, + // so leaving it pending silently loses the byte from the response. + generated_text += "\xEF\xBF\xBD"; + generated_text_pending.clear(); + } auto msg_prv_copy = chat_msg; //SRV_DBG("Parsing chat message: %s\n", generated_text.c_str()); auto new_msg = common_chat_parse( From be6431c66217e21d21426e11443e8a79eea7786b Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Tue, 8 Sep 2026 19:15:12 -0700 Subject: [PATCH 3/9] server: match the JSON serialiser's UTF-8 replacement boundaries Two defects in the previous commit's own flush, both raised in review and both real. A trailing lead byte followed by too few bytes, such as "\xE2A", is reported INCOMPLETE by common_parse_utf8_codepoint, which returns before checking that what followed the lead was even a continuation. The flush emitted one replacement character and cleared the buffer, so the valid A was discarded. A malformed multibyte prefix was replaced a byte at a time: "\xE2\x80A" gave two replacement characters where the serialiser gives one, because consuming only the lead byte left the continuation to be re-read as a second invalid sequence. Both now go through utf8_malformed_prefix, which consumes one failed sequence the way nlohmann's replace handler does: the lead byte plus the continuation bytes that actually followed it, never past the length the lead announced, with a bare continuation counting as a failed sequence on its own. Checked against the vendored header by dumping with error_handler_t::replace rather than from memory: hello \xE2 A -> hello U+FFFD A hello \xE2\x80 A -> hello U+FFFD A hello \xC3\xC3 -> hello U+FFFD U+FFFD a \xF0\x9F\x98 -> a U+FFFD \x80\x80 -> U+FFFD U+FFFD This implementation reproduces all five. The finality now reaches the helper as a flag rather than being handled by a separate block after it, so INCOMPLETE on the last call resolves through the same path. --- tools/server/server-task.cpp | 58 ++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/tools/server/server-task.cpp b/tools/server/server-task.cpp index a4183e716be1..d31a382335e5 100644 --- a/tools/server/server-task.cpp +++ b/tools/server/server-task.cpp @@ -171,23 +171,55 @@ task_result_state::task_result_state(const common_chat_parser_params & chat_pars // of the streaming loop and cancels the task, so the request ends with no tokens at all. The JSON // serialiser already substitutes U+FFFD for those bytes on the way to the client, so doing the same // substitution before parsing keeps the parser input identical to what the client receives. -static void append_utf8_sanitized(std::string & text, std::string & pending, const std::string & text_added) { +// How many bytes the JSON serialiser's replace handler consumes for one failed sequence: the +// lead byte plus the continuation bytes that actually followed it, never past the length the +// lead announced. A byte that is itself a continuation is a failed sequence on its own, which +// is why "\x80\x80" is two replacements rather than one. +static size_t utf8_malformed_prefix(const std::string & s, size_t pos) { + const unsigned char lead = static_cast(s[pos]); + size_t want = 1; + if ((lead & 0xe0) == 0xc0) { + want = 2; + } else if ((lead & 0xf0) == 0xe0) { + want = 3; + } else if ((lead & 0xf8) == 0xf0) { + want = 4; + } else { + return 1; // a bare continuation, or a lead no encoding can use + } + size_t have = 1; + while (have < want && pos + have < s.size() && + (static_cast(s[pos + have]) & 0xc0) == 0x80) { + have++; + } + return have; +} + +static void append_utf8_sanitized(std::string & text, std::string & pending, const std::string & text_added, bool is_final = false) { pending += text_added; size_t pos = 0; while (pos < pending.size()) { const auto res = common_parse_utf8_codepoint(pending, pos); - if (res.status == utf8_parse_result::INCOMPLETE) { + if (res.status == utf8_parse_result::INCOMPLETE && !is_final) { // wait for the rest of the sequence break; } - if (res.status == utf8_parse_result::INVALID) { - text += "\xEF\xBF\xBD"; // U+FFFD REPLACEMENT CHARACTER - pos += 1; + if (res.status == utf8_parse_result::SUCCESS) { + text.append(pending, pos, res.bytes_consumed); + pos += res.bytes_consumed; continue; } - text.append(pending, pos, res.bytes_consumed); - pos += res.bytes_consumed; + // One U+FFFD for the whole malformed prefix, which is where the JSON serialiser puts its + // boundaries: with error_handler_t::replace it renders "\xE2\x80A" as one replacement + // followed by A, not two replacements, and "\xC3\xC3" as two, because the second lead + // byte ends the first sequence rather than continuing it. Byte-at-a-time would disagree + // with what the client is shown, and the parsed message is preferred over the raw content. + // On the final call INCOMPLETE lands here too: nothing more is coming, and the parser + // reports INCOMPLETE for a short sequence without checking that what followed the lead + // was even a continuation, so "\xE2A" must give one replacement and keep the A. + text += "\xEF\xBF\xBD"; // U+FFFD REPLACEMENT CHARACTER + pos += utf8_malformed_prefix(pending, pos); } pending.erase(0, pos); @@ -198,15 +230,9 @@ common_chat_msg task_result_state::update_chat_msg( bool is_partial, std::vector & diffs, bool filter_tool_calls) { - append_utf8_sanitized(generated_text, generated_text_pending, text_added); - if (!is_partial && !generated_text_pending.empty()) { - // Nothing can complete this sequence now (the generation hit its limit, or stopped, mid - // character), so it gets the one U+FFFD the JSON serialiser would show for it rather than - // being dropped: a nonempty parsed message is preferred over the raw content downstream, - // so leaving it pending silently loses the byte from the response. - generated_text += "\xEF\xBF\xBD"; - generated_text_pending.clear(); - } + // On the last update nothing can complete a held-back sequence, so it is resolved here rather + // than left pending forever, which dropped it from the response entirely. + append_utf8_sanitized(generated_text, generated_text_pending, text_added, !is_partial); auto msg_prv_copy = chat_msg; //SRV_DBG("Parsing chat message: %s\n", generated_text.c_str()); auto new_msg = common_chat_parse( From 4470648e7b4df96d8b3b87f9b941511a1a4516f8 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Tue, 8 Sep 2026 19:41:56 -0700 Subject: [PATCH 4/9] server: apply the lead-specific first-continuation bounds E0, ED, F0 and F4 restrict the first continuation byte. The serialiser's decoder rejects the sequence at that byte and reprocesses it, so E0 80, ED A0, F0 80 and F4 90 each render as two replacement characters, not one. Treating every 10xxxxxx byte as part of the malformed prefix emitted one and disagreed with the client. --- tools/server/server-task.cpp | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/tools/server/server-task.cpp b/tools/server/server-task.cpp index d31a382335e5..3ac3a3f15c64 100644 --- a/tools/server/server-task.cpp +++ b/tools/server/server-task.cpp @@ -178,18 +178,32 @@ task_result_state::task_result_state(const common_chat_parser_params & chat_pars static size_t utf8_malformed_prefix(const std::string & s, size_t pos) { const unsigned char lead = static_cast(s[pos]); size_t want = 1; - if ((lead & 0xe0) == 0xc0) { + // The FIRST continuation is range restricted for four leads, and the serialiser's decoder + // rejects the sequence at that byte rather than absorbing it: E0 80 is an overlong form, + // ED A0 a surrogate, F0 80 overlong again and F4 90 past U+10FFFF. Each of those renders as + // TWO replacement characters, the lead alone and then the stray continuation, so treating + // every 10xxxxxx byte as part of the prefix would emit one and disagree with the client. + unsigned char lo = 0x80, hi = 0xbf; + if (lead >= 0xc2 && lead <= 0xdf) { want = 2; - } else if ((lead & 0xf0) == 0xe0) { + } else if (lead >= 0xe0 && lead <= 0xef) { want = 3; - } else if ((lead & 0xf8) == 0xf0) { + if (lead == 0xe0) { lo = 0xa0; } + if (lead == 0xed) { hi = 0x9f; } + } else if (lead >= 0xf0 && lead <= 0xf4) { want = 4; + if (lead == 0xf0) { lo = 0x90; } + if (lead == 0xf4) { hi = 0x8f; } } else { - return 1; // a bare continuation, or a lead no encoding can use + return 1; // a bare continuation, C0/C1 overlong, or F5..FF: never a usable lead } size_t have = 1; - while (have < want && pos + have < s.size() && - (static_cast(s[pos + have]) & 0xc0) == 0x80) { + while (have < want && pos + have < s.size()) { + const unsigned char c = static_cast(s[pos + have]); + const bool ok = (have == 1) ? (c >= lo && c <= hi) : ((c & 0xc0) == 0x80); + if (!ok) { + break; + } have++; } return have; From 9b2955e3e69b29130d8a93d23e3363a20376e189 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Tue, 8 Sep 2026 21:03:06 -0700 Subject: [PATCH 5/9] server: reject complete but illegal UTF-8 before copying it through common_parse_utf8_codepoint checks continuation SHAPE only, never the lead's range, so it reports SUCCESS for C0 80, ED A0 80 and F4 90 80 80. Copying those through was not harmless. The JSON serialiser replaces them, so a client was shown something different from what a parser received, and the AST dump under params.debug is a plain dump() with no error handler, which throws on exactly these bytes and aborts the completion this change exists to keep alive. The check goes where the sanitising happens, not in the shared parser, which has other callers in common/trie.cpp and common/common.cpp. The lead table that utf8_malformed_prefix already carried is now shared with it, so the accept and the replace boundaries cannot drift apart. Differential tested against the serialiser over every 1- and 2-byte string plus 3-byte strings across 16 leads, 788766 inputs after excluding the ones JSON escapes rather than replaces: 0 mismatches, using the repo parser. The same test reported 34688 mismatches before this commit. --- tools/server/server-task.cpp | 73 +++++++++++++++++++++++++++++------- 1 file changed, 59 insertions(+), 14 deletions(-) diff --git a/tools/server/server-task.cpp b/tools/server/server-task.cpp index 3ac3a3f15c64..b7fa1f1df2b2 100644 --- a/tools/server/server-task.cpp +++ b/tools/server/server-task.cpp @@ -175,27 +175,72 @@ task_result_state::task_result_state(const common_chat_parser_params & chat_pars // lead byte plus the continuation bytes that actually followed it, never past the length the // lead announced. A byte that is itself a continuation is a failed sequence on its own, which // is why "\x80\x80" is two replacements rather than one. -static size_t utf8_malformed_prefix(const std::string & s, size_t pos) { - const unsigned char lead = static_cast(s[pos]); - size_t want = 1; - // The FIRST continuation is range restricted for four leads, and the serialiser's decoder - // rejects the sequence at that byte rather than absorbing it: E0 80 is an overlong form, - // ED A0 a surrogate, F0 80 overlong again and F4 90 past U+10FFFF. Each of those renders as - // TWO replacement characters, the lead alone and then the stray continuation, so treating - // every 10xxxxxx byte as part of the prefix would emit one and disagree with the client. - unsigned char lo = 0x80, hi = 0xbf; +// The lead byte's own rules: how many bytes the sequence claims, and the range the FIRST +// continuation must fall in. Four leads restrict that range, and they are the whole reason +// overlong forms, surrogates and anything past U+10FFFF can be told apart from valid text: +// E0 80 is an overlong encoding, ED A0 a surrogate, F0 80 overlong again, F4 90 out of range. +// Returns false for a byte that can never begin a sequence: a bare continuation, C0 or C1 +// (only ever overlong), or F5..FF. +static bool utf8_lead_bounds(unsigned char lead, size_t & want, unsigned char & lo, unsigned char & hi) { + lo = 0x80; + hi = 0xbf; + if (lead < 0x80) { + want = 1; + return true; + } if (lead >= 0xc2 && lead <= 0xdf) { want = 2; - } else if (lead >= 0xe0 && lead <= 0xef) { + return true; + } + if (lead >= 0xe0 && lead <= 0xef) { want = 3; if (lead == 0xe0) { lo = 0xa0; } if (lead == 0xed) { hi = 0x9f; } - } else if (lead >= 0xf0 && lead <= 0xf4) { + return true; + } + if (lead >= 0xf0 && lead <= 0xf4) { want = 4; if (lead == 0xf0) { lo = 0x90; } if (lead == 0xf4) { hi = 0x8f; } - } else { - return 1; // a bare continuation, C0/C1 overlong, or F5..FF: never a usable lead + return true; + } + return false; +} + +// Whether `len` bytes at `pos` are a legal Unicode scalar. `common_parse_utf8_codepoint` checks +// continuation SHAPE only -- `(c & 0xc0) == 0x80` -- and never the lead's range, so it reports +// SUCCESS for C0 80, ED A0 80 and F4 90 80 80. Copying those through unchanged is not harmless: +// the JSON serialiser replaces them, so the client is shown something different from what a +// parser receives, and the AST dump under `params.debug` uses a plain dump() with no error +// handler, which throws on exactly these bytes and aborts the completion this change exists to +// keep alive. The shared parser is left alone: it has other callers, and the check belongs where +// the sanitising happens. +static bool utf8_is_scalar(const std::string & s, size_t pos, size_t len) { + if (len == 0 || pos + len > s.size()) { + return false; + } + size_t want = 0; + unsigned char lo = 0, hi = 0; + if (!utf8_lead_bounds(static_cast(s[pos]), want, lo, hi) || want != len) { + return false; + } + for (size_t i = 1; i < want; i++) { + const unsigned char c = static_cast(s[pos + i]); + const bool ok = (i == 1) ? (c >= lo && c <= hi) : ((c & 0xc0) == 0x80); + if (!ok) { + return false; + } + } + return true; +} + +// How many bytes the JSON serialiser treats as ONE failed sequence, so that one U+FFFD replaces +// exactly the same span the client is shown. +static size_t utf8_malformed_prefix(const std::string & s, size_t pos) { + size_t want = 0; + unsigned char lo = 0, hi = 0; + if (!utf8_lead_bounds(static_cast(s[pos]), want, lo, hi) || want == 1) { + return 1; } size_t have = 1; while (have < want && pos + have < s.size()) { @@ -219,7 +264,7 @@ static void append_utf8_sanitized(std::string & text, std::string & pending, con // wait for the rest of the sequence break; } - if (res.status == utf8_parse_result::SUCCESS) { + if (res.status == utf8_parse_result::SUCCESS && utf8_is_scalar(pending, pos, res.bytes_consumed)) { text.append(pending, pos, res.bytes_consumed); pos += res.bytes_consumed; continue; From 6dc4a969d0f10a614ab6a52c3073e851e20ead03 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Tue, 8 Sep 2026 21:36:18 -0700 Subject: [PATCH 6/9] server: trim the UTF-8 sanitising comments Post-convergence pass, 42 comment lines to 12. Two sweeps: the first removed what the code already says, the second kept only what a reader cannot recover from the code, which is why the serialiser's replacement boundaries and the debug dump are still spelled out. Also puts the append_utf8_sanitized description back above that function. An earlier edit here left it stranded above utf8_lead_bounds with a duplicate of the prefix helper's description below it. Comments only, verified with comment_tools check; llama-server rebuilt. --- tools/server/server-task.cpp | 54 ++++++++---------------------------- 1 file changed, 12 insertions(+), 42 deletions(-) diff --git a/tools/server/server-task.cpp b/tools/server/server-task.cpp index b7fa1f1df2b2..96f5ffa66403 100644 --- a/tools/server/server-task.cpp +++ b/tools/server/server-task.cpp @@ -160,27 +160,8 @@ task_result_state::task_result_state(const common_chat_parser_params & chat_pars } } -// Appends `text_added` to `text`, keeping in `pending` the trailing bytes of an incomplete UTF-8 -// sequence so that the next chunk can complete it, and replacing bytes that can never form a valid -// codepoint with U+FFFD. -// -// The generated text is a raw byte stream and is not guaranteed to be valid UTF-8: a byte fallback -// token, or a prompt that ends in the middle of a multi-byte character (the model then continues -// with the remaining continuation bytes), makes it start with, or contain, bytes that do not decode. -// The chat parsers reject malformed UTF-8 by design, and the exception thrown for it propagates out -// of the streaming loop and cancels the task, so the request ends with no tokens at all. The JSON -// serialiser already substitutes U+FFFD for those bytes on the way to the client, so doing the same -// substitution before parsing keeps the parser input identical to what the client receives. -// How many bytes the JSON serialiser's replace handler consumes for one failed sequence: the -// lead byte plus the continuation bytes that actually followed it, never past the length the -// lead announced. A byte that is itself a continuation is a failed sequence on its own, which -// is why "\x80\x80" is two replacements rather than one. -// The lead byte's own rules: how many bytes the sequence claims, and the range the FIRST -// continuation must fall in. Four leads restrict that range, and they are the whole reason -// overlong forms, surrogates and anything past U+10FFFF can be told apart from valid text: -// E0 80 is an overlong encoding, ED A0 a surrogate, F0 80 overlong again, F4 90 out of range. -// Returns false for a byte that can never begin a sequence: a bare continuation, C0 or C1 -// (only ever overlong), or F5..FF. +// How many bytes a lead announces, and the range its FIRST continuation must fall in. Four leads +// restrict that range, which is what separates overlong forms and surrogates from valid text. static bool utf8_lead_bounds(unsigned char lead, size_t & want, unsigned char & lo, unsigned char & hi) { lo = 0x80; hi = 0xbf; @@ -207,14 +188,9 @@ static bool utf8_lead_bounds(unsigned char lead, size_t & want, unsigned char & return false; } -// Whether `len` bytes at `pos` are a legal Unicode scalar. `common_parse_utf8_codepoint` checks -// continuation SHAPE only -- `(c & 0xc0) == 0x80` -- and never the lead's range, so it reports -// SUCCESS for C0 80, ED A0 80 and F4 90 80 80. Copying those through unchanged is not harmless: -// the JSON serialiser replaces them, so the client is shown something different from what a -// parser receives, and the AST dump under `params.debug` uses a plain dump() with no error -// handler, which throws on exactly these bytes and aborts the completion this change exists to -// keep alive. The shared parser is left alone: it has other callers, and the check belongs where -// the sanitising happens. +// A legal scalar, which `common_parse_utf8_codepoint` does not check: it tests continuation shape +// only and accepts C0 80. Passing that through diverges from what the client is shown, and the AST +// dump under `params.debug` throws on it. Checked here: the shared parser has other callers. static bool utf8_is_scalar(const std::string & s, size_t pos, size_t len) { if (len == 0 || pos + len > s.size()) { return false; @@ -234,8 +210,7 @@ static bool utf8_is_scalar(const std::string & s, size_t pos, size_t len) { return true; } -// How many bytes the JSON serialiser treats as ONE failed sequence, so that one U+FFFD replaces -// exactly the same span the client is shown. +// One failed sequence as the serialiser counts it: "\xE2\x80A" is one replacement, "\xC3\xC3" two. static size_t utf8_malformed_prefix(const std::string & s, size_t pos) { size_t want = 0; unsigned char lo = 0, hi = 0; @@ -254,6 +229,9 @@ static size_t utf8_malformed_prefix(const std::string & s, size_t pos) { return have; } +// Generated text is a raw byte stream: a byte-fallback token, or a prompt cut mid-character, puts +// undecodable bytes in it, the chat parsers throw on those, and the exception cancels the task. +// Substituting as the serialiser already does keeps the parser and the client seeing one thing. static void append_utf8_sanitized(std::string & text, std::string & pending, const std::string & text_added, bool is_final = false) { pending += text_added; @@ -261,7 +239,6 @@ static void append_utf8_sanitized(std::string & text, std::string & pending, con while (pos < pending.size()) { const auto res = common_parse_utf8_codepoint(pending, pos); if (res.status == utf8_parse_result::INCOMPLETE && !is_final) { - // wait for the rest of the sequence break; } if (res.status == utf8_parse_result::SUCCESS && utf8_is_scalar(pending, pos, res.bytes_consumed)) { @@ -269,14 +246,8 @@ static void append_utf8_sanitized(std::string & text, std::string & pending, con pos += res.bytes_consumed; continue; } - // One U+FFFD for the whole malformed prefix, which is where the JSON serialiser puts its - // boundaries: with error_handler_t::replace it renders "\xE2\x80A" as one replacement - // followed by A, not two replacements, and "\xC3\xC3" as two, because the second lead - // byte ends the first sequence rather than continuing it. Byte-at-a-time would disagree - // with what the client is shown, and the parsed message is preferred over the raw content. - // On the final call INCOMPLETE lands here too: nothing more is coming, and the parser - // reports INCOMPLETE for a short sequence without checking that what followed the lead - // was even a continuation, so "\xE2A" must give one replacement and keep the A. + // INCOMPLETE lands here on the final call: the parser does not check that what followed + // the lead was a continuation, so "\xE2A" must give one replacement and keep the A. text += "\xEF\xBF\xBD"; // U+FFFD REPLACEMENT CHARACTER pos += utf8_malformed_prefix(pending, pos); } @@ -289,8 +260,7 @@ common_chat_msg task_result_state::update_chat_msg( bool is_partial, std::vector & diffs, bool filter_tool_calls) { - // On the last update nothing can complete a held-back sequence, so it is resolved here rather - // than left pending forever, which dropped it from the response entirely. + // Nothing can complete a held-back sequence after the last update; unresolved, it was dropped. append_utf8_sanitized(generated_text, generated_text_pending, text_added, !is_partial); auto msg_prv_copy = chat_msg; //SRV_DBG("Parsing chat message: %s\n", generated_text.c_str()); From b27c0b4427885106301989a270f6d72406198267 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Tue, 8 Sep 2026 23:16:04 -0700 Subject: [PATCH 7/9] tighten the comments added by this change No code change: every remaining line carries a fact the code does not state. --- tools/server/server-task.cpp | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/tools/server/server-task.cpp b/tools/server/server-task.cpp index 96f5ffa66403..fb89d13f2fde 100644 --- a/tools/server/server-task.cpp +++ b/tools/server/server-task.cpp @@ -160,8 +160,8 @@ task_result_state::task_result_state(const common_chat_parser_params & chat_pars } } -// How many bytes a lead announces, and the range its FIRST continuation must fall in. Four leads -// restrict that range, which is what separates overlong forms and surrogates from valid text. +// Bytes a lead announces, and the range its FIRST continuation must fall in: that range is what +// rejects overlong forms and surrogates. static bool utf8_lead_bounds(unsigned char lead, size_t & want, unsigned char & lo, unsigned char & hi) { lo = 0x80; hi = 0xbf; @@ -188,9 +188,8 @@ static bool utf8_lead_bounds(unsigned char lead, size_t & want, unsigned char & return false; } -// A legal scalar, which `common_parse_utf8_codepoint` does not check: it tests continuation shape -// only and accepts C0 80. Passing that through diverges from what the client is shown, and the AST -// dump under `params.debug` throws on it. Checked here: the shared parser has other callers. +// `common_parse_utf8_codepoint` tests continuation shape only and accepts C0 80, which the client +// is never shown and the `params.debug` AST dump throws on. Checked here: that parser has other callers. static bool utf8_is_scalar(const std::string & s, size_t pos, size_t len) { if (len == 0 || pos + len > s.size()) { return false; @@ -229,9 +228,8 @@ static size_t utf8_malformed_prefix(const std::string & s, size_t pos) { return have; } -// Generated text is a raw byte stream: a byte-fallback token, or a prompt cut mid-character, puts -// undecodable bytes in it, the chat parsers throw on those, and the exception cancels the task. -// Substituting as the serialiser already does keeps the parser and the client seeing one thing. +// A byte-fallback token, or a prompt cut mid-character, puts undecodable bytes in the stream and the +// chat parsers throw on them, cancelling the task. Substitute as the serialiser already does. static void append_utf8_sanitized(std::string & text, std::string & pending, const std::string & text_added, bool is_final = false) { pending += text_added; @@ -246,8 +244,7 @@ static void append_utf8_sanitized(std::string & text, std::string & pending, con pos += res.bytes_consumed; continue; } - // INCOMPLETE lands here on the final call: the parser does not check that what followed - // the lead was a continuation, so "\xE2A" must give one replacement and keep the A. + // INCOMPLETE lands here on the final call, and "\xE2A" must give one replacement and keep the A. text += "\xEF\xBF\xBD"; // U+FFFD REPLACEMENT CHARACTER pos += utf8_malformed_prefix(pending, pos); } From a5ac51d890a68023d1202eceb79104b346958c12 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Wed, 9 Sep 2026 01:41:47 -0700 Subject: [PATCH 8/9] server: unit test for the UTF-8 sanitising, against an independent WHATWG decoder --- tools/server/CMakeLists.txt | 13 + tools/server/tests/test-server-utf8.cpp | 446 ++++++++++++++++++++++++ 2 files changed, 459 insertions(+) create mode 100644 tools/server/tests/test-server-utf8.cpp diff --git a/tools/server/CMakeLists.txt b/tools/server/CMakeLists.txt index 280bd9e19dca..956114a3eef6 100644 --- a/tools/server/CMakeLists.txt +++ b/tools/server/CMakeLists.txt @@ -63,3 +63,16 @@ install(TARGETS ${TARGET} RUNTIME) target_link_libraries(${TARGET} PRIVATE llama-server-impl) target_compile_features(${TARGET} PRIVATE cxx_std_17) + +# UTF-8 sanitising unit test: needs no model, so it stays out of the default build + +if (LLAMA_BUILD_TESTS AND NOT CMAKE_CROSSCOMPILING) + set(TARGET test-server-utf8) + + add_executable(${TARGET} tests/test-server-utf8.cpp) + target_link_libraries(${TARGET} PRIVATE server-context ${CMAKE_THREAD_LIBS_INIT}) + target_include_directories(${TARGET} PRIVATE ${CMAKE_SOURCE_DIR}) + target_compile_features(${TARGET} PRIVATE cxx_std_17) + + add_test(NAME ${TARGET} COMMAND ${TARGET}) +endif() diff --git a/tools/server/tests/test-server-utf8.cpp b/tools/server/tests/test-server-utf8.cpp new file mode 100644 index 000000000000..d97ce2b86511 --- /dev/null +++ b/tools/server/tests/test-server-utf8.cpp @@ -0,0 +1,446 @@ +// Unit test for the UTF-8 sanitising in task_result_state::update_chat_msg(). +// +// The generated text is a raw byte stream: a byte fallback token, or a prompt cut mid +// character, puts undecodable bytes in it. Those bytes are substituted with U+FFFD before the +// chat parsers see them, following the Unicode "maximal subpart of an ill-formed subsequence" +// rule (Unicode 15 core spec, section 3.9, D93b) which is also what the JSON serialiser's +// error_handler_t::replace does on the way to the client. +// +// The oracle here is an independently written implementation of the WHATWG Encoding Standard +// UTF-8 decoder, https://encoding.spec.whatwg.org/#utf-8-decoder, rather than a copy of the +// code under test. Every named case, all 256 single bytes, all 65536 two byte sequences, a +// three and four byte lead sweep and a biased random fuzz are compared against it, and every +// valid Unicode scalar value must round trip byte identically. +// +// Run with no arguments for everything; a single mode name (named, split, roundtrip, +// exhaustive1, exhaustive2, exhaustive3, fuzz) runs just that one. + +#include "server-task.h" +#include "chat.h" +#include + +#include +#include +#include +#include +#include + +// --------------------------------------------------------------------------- +// independent reference: WHATWG Encoding Standard "UTF-8 decoder" (maximal subpart) +// https://encoding.spec.whatwg.org/#utf-8-decoder +// --------------------------------------------------------------------------- +static std::string ref_decode_replace(const std::string & in) { + std::string out; + size_t i = 0; + const size_t n = in.size(); + uint32_t cp = 0; + int seen = 0, needed = 0; + uint8_t lo = 0x80, hi = 0xBF; + + auto emit = [&](uint32_t c) { + if (c < 0x80) { out += (char) c; } + else if (c < 0x800) { out += (char) (0xC0 | (c >> 6)); out += (char) (0x80 | (c & 0x3F)); } + else if (c < 0x10000) { out += (char) (0xE0 | (c >> 12)); out += (char) (0x80 | ((c >> 6) & 0x3F)); out += (char) (0x80 | (c & 0x3F)); } + else { out += (char) (0xF0 | (c >> 18)); out += (char) (0x80 | ((c >> 12) & 0x3F)); out += (char) (0x80 | ((c >> 6) & 0x3F)); out += (char) (0x80 | (c & 0x3F)); } + }; + auto err = [&]() { out += "\xEF\xBF\xBD"; }; + + while (i <= n) { + if (i == n) { + if (needed != 0) { err(); } + break; + } + uint8_t b = (uint8_t) in[i]; + if (needed == 0) { + if (b <= 0x7F) { emit(b); i++; continue; } + if (b >= 0xC2 && b <= 0xDF) { needed = 1; cp = b & 0x1F; } + else if (b >= 0xE0 && b <= 0xEF) { + if (b == 0xE0) { lo = 0xA0; } + if (b == 0xED) { hi = 0x9F; } + needed = 2; cp = b & 0x0F; + } else if (b >= 0xF0 && b <= 0xF4) { + if (b == 0xF0) { lo = 0x90; } + if (b == 0xF4) { hi = 0x8F; } + needed = 3; cp = b & 0x07; + } else { err(); i++; continue; } + i++; seen = 0; continue; + } + if (b < lo || b > hi) { + // reset, do NOT consume: prefix is the maximal subpart + cp = 0; seen = 0; needed = 0; lo = 0x80; hi = 0xBF; + err(); + continue; + } + lo = 0x80; hi = 0xBF; + cp = (cp << 6) | (b & 0x3F); + seen++; i++; + if (seen == needed) { emit(cp); cp = 0; seen = 0; needed = 0; } + } + return out; +} + +// --------------------------------------------------------------------------- + +struct outcome { + bool threw = false; + std::string what; + std::string content; +}; + +// non-streaming: one call, is_partial = false +static outcome run_final(const std::string & text) { + outcome o; + common_chat_parser_params p; // defaults to COMMON_CHAT_FORMAT_CONTENT_ONLY + task_result_state st(p); + std::vector diffs; + try { + auto msg = st.update_chat_msg(text, false, diffs); + o.content = msg.content; + } catch (const std::exception & e) { + o.threw = true; + o.what = e.what(); + } + return o; +} + +// streaming: each chunk with is_partial = true, then an empty final flush +static outcome run_stream(const std::vector & chunks) { + outcome o; + common_chat_parser_params p; + task_result_state st(p); + std::vector diffs; + try { + common_chat_msg msg; + for (const auto & c : chunks) { + msg = st.update_chat_msg(c, true, diffs); + } + msg = st.update_chat_msg("", false, diffs); + o.content = msg.content; + } catch (const std::exception & e) { + o.threw = true; + o.what = e.what(); + } + return o; +} + +static std::string hex(const std::string & s) { + static const char * d = "0123456789ABCDEF"; + std::string r; + for (unsigned char c : s) { r += d[c >> 4]; r += d[c & 15]; r += ' '; } + if (!r.empty()) { r.pop_back(); } + return r; +} + +static std::string enc(uint32_t c) { + std::string out; + if (c < 0x80) { out += (char) c; } + else if (c < 0x800) { out += (char) (0xC0 | (c >> 6)); out += (char) (0x80 | (c & 0x3F)); } + else if (c < 0x10000) { out += (char) (0xE0 | (c >> 12)); out += (char) (0x80 | ((c >> 6) & 0x3F)); out += (char) (0x80 | (c & 0x3F)); } + else { out += (char) (0xF0 | (c >> 18)); out += (char) (0x80 | ((c >> 12) & 0x3F)); out += (char) (0x80 | ((c >> 6) & 0x3F)); out += (char) (0x80 | (c & 0x3F)); } + return out; +} + +// what the client actually receives: the server serialises every response through +// safe_json_to_str(), which is dump(..., error_handler_t::replace) +static std::string as_client_sees(const std::string & content) { + nlohmann::ordered_json j = nlohmann::ordered_json{{"content", content}}; + return j.dump(-1, ' ', false, nlohmann::ordered_json::error_handler_t::replace); +} + +static int n_threw_final = 0, n_threw_stream = 0, n_cases = 0; +static int n_mismatch_ref = 0; + +// named case: print everything, both drive modes +static void named(const char * id, const char * desc, const std::string & text) { + n_cases++; + outcome f = run_final(text); + // stream it one byte at a time: worst case for the hold-back buffer + std::vector bytes; + for (char c : text) { bytes.push_back(std::string(1, c)); } + outcome s = run_stream(bytes); + const std::string ref = ref_decode_replace(text); + + if (f.threw) { n_threw_final++; } + if (s.threw) { n_threw_stream++; } + if (!f.threw && f.content != ref) { n_mismatch_ref++; } + + printf("CASE %-10s in=[%-26s] final=%-6s stream=%-6s out=[%-30s] ref=[%-30s] %s%s\n", + id, + hex(text).c_str(), + f.threw ? "THROW" : "ok", + s.threw ? "THROW" : "ok", + f.threw ? f.what.substr(0, 30).c_str() : hex(f.content).c_str(), + hex(ref).c_str(), + (!f.threw && f.content == ref) ? "REF-MATCH" : (f.threw ? "" : "REF-DIFF"), + (!f.threw && !s.threw && f.content != s.content) ? " STREAM-DIFF" : ""); + if (!f.threw && !s.threw && f.content != s.content) { + printf(" stream-out=[%s]\n", hex(s.content).c_str()); + } + // the bytes the HTTP client ends up with, after the JSON serialiser's own replacement + printf("WIRE %-10s %s\n", id, f.threw ? "" : as_client_sees(f.content).c_str()); + (void) desc; +} + +static int g_fail = 0; + +int main(int argc, char ** argv) { + const std::string mode = argc > 1 ? argv[1] : "all"; + const bool all = mode == "all"; + + if (all || mode == "named") { + printf("== named enumeration ==\n"); + // valid controls, must be byte identical between base and head + named("V-ascii", "plain ascii", "Hello, world!"); + named("V-latin", "2-byte latin", "caf\xC3\xA9"); + named("V-cjk", "3-byte CJK", "\xE4\xB8\xAD\xE6\x96\x87"); + named("V-emoji", "4-byte emoji", "\xF0\x9F\x98\x80"); + named("V-combine", "combining marks", "e\xCC\x81"); + named("V-min2", "U+0080 min 2-byte", "\xC2\x80"); + named("V-max3", "U+FFFF max BMP", "\xEF\xBF\xBF"); + named("V-min4", "U+10000 min astral", "\xF0\x90\x80\x80"); + named("V-max4", "U+10FFFF max scalar", "\xF4\x8F\xBF\xBF"); + named("V-e0a0", "U+0800 E0 A0 80", "\xE0\xA0\x80"); + named("V-ed9f", "U+D7FF ED 9F BF", "\xED\x9F\xBF"); + named("V-bom", "U+FEFF BOM", "\xEF\xBB\xBF"); + named("V-fffd", "literal U+FFFD in text", "a\xEF\xBF\xBDz"); + // truncated multi-byte at the very end, no following token + named("T-c3", "trailing lead of 2-byte", "abc\xC3"); + named("T-e4", "trailing lead of 3-byte", "abc\xE4"); + named("T-e4b8", "trailing 2 of 3 bytes", "abc\xE4\xB8"); + named("T-f0", "trailing lead of 4-byte", "abc\xF0"); + named("T-f09f", "trailing 2 of 4", "abc\xF0\x9F"); + named("T-f09f98", "trailing 3 of 4", "abc\xF0\x9F\x98"); + named("T-only-c3", "lead byte is whole output", "\xC3"); + // continuation byte with no lead + named("C-80", "lone 80", "abc\x80"); + named("C-bf", "lone BF", "abc\xBF"); + named("C-a1", "lone A1 (byte fallback)","\xA1"); + named("C-lead", "continuation first", "\x80\x41"); + named("C-run", "run of continuations", "\x80\x80\x80\x80"); + // overlong + named("O-c080", "C0 80 overlong NUL", "\xC0\x80"); + named("O-c1bf", "C1 BF overlong", "\xC1\xBF"); + named("O-e080af", "E0 80 AF overlong /", "\xE0\x80\xAF"); + named("O-f08080af","F0 80 80 AF overlong", "\xF0\x80\x80\xAF"); + named("O-e09fbf", "E0 9F BF overlong", "\xE0\x9F\xBF"); + named("O-f08fbfbf","F0 8F BF BF overlong", "\xF0\x8F\xBF\xBF"); + // surrogates + named("S-d800", "ED A0 80 = U+D800", "\xED\xA0\x80"); + named("S-dfff", "ED BF BF = U+DFFF", "\xED\xBF\xBF"); + named("S-pair", "CESU-8 surrogate pair", "\xED\xA0\xBD\xED\xB8\x80"); + // above U+10FFFF + named("X-f4908080","F4 90 80 80 = U+110000", "\xF4\x90\x80\x80"); + named("X-f5", "F5 lead", "\xF5\x80\x80\x80"); + named("X-f7bfbfbf","F7 BF BF BF", "\xF7\xBF\xBF\xBF"); + // invalid leads + named("L-f8", "F8 lead (5-byte form)", "\xF8\x88\x80\x80\x80"); + named("L-fc", "FC lead (6-byte form)", "\xFC\x84\x80\x80\x80\x80"); + named("L-fe", "FE never valid", "\xFE"); + named("L-ff", "FF never valid", "\xFF"); + named("L-fefe", "FE FF", "\xFE\xFF"); + // replacement counting, the maximal-subpart cases + named("M-e280-41", "E2 80 41 -> 1 FFFD + A", "\xE2\x80\x41"); + named("M-c3c3", "C3 C3 -> 2 FFFD", "\xC3\xC3"); + named("M-f08080-41","F0 80 80 41", "\xF0\x80\x80\x41"); + named("M-uni-ex", "UTS worked example", "\x61\xF1\x80\x80\xE1\x80\xC2\x62"); + named("M-e1-80-e2","E1 80 E2 F0 91 92 F1 BF 41", "\xE1\x80\xE2\xF0\x91\x92\xF1\xBF\x41"); + // NUL + named("N-nul", "embedded NUL", std::string("a\0b", 3)); + + // mixed valid + invalid, the realistic byte-fallback stream + named("R-mix1", "valid then invalid", "hello \xC3\xA9 \x80 world"); + named("R-mix2", "invalid then valid", "\xA1 caf\xC3\xA9"); + named("R-long", "long mixed", std::string("x") + "\xE4\xB8\xAD" + "\xFF" + "\xF0\x9F\x98\x80" + "\xED\xA0\x80" + "y"); + + printf("\nnamed: cases=%d threw_final=%d threw_stream=%d ref_diff=%d\n", + n_cases, n_threw_final, n_threw_stream, n_mismatch_ref); + printf("RESULT named threw_final=%d threw_stream=%d ref_diff=%d\n", + n_threw_final, n_threw_stream, n_mismatch_ref); + g_fail += n_threw_final + n_threw_stream + n_mismatch_ref; + if (!all) { return g_fail == 0 ? 0 : 1; } + } + + if (all || mode == "split") { + // a valid character split across token / SSE chunk boundaries. + // Every split point of every valid sample, in 2 and 3 chunk form. + printf("== split enumeration ==\n"); + const std::vector samples = { + "caf\xC3\xA9 au lait", + "\xE4\xB8\xAD\xE6\x96\x87\xE3\x83\x86", + "a\xF0\x9F\x98\x80" "b\xF0\x9F\x91\x8D" "c", + "\xC2\x80\xDF\xBF\xE0\xA0\x80\xEF\xBF\xBF\xF0\x90\x80\x80\xF4\x8F\xBF\xBF", + }; + int bad = 0, threw = 0, total = 0; + for (const auto & s : samples) { + for (size_t i = 0; i <= s.size(); i++) { + total++; + outcome o = run_stream({s.substr(0, i), s.substr(i)}); + if (o.threw) { threw++; printf(" SPLIT2 THROW at %zu of [%s]: %s\n", i, hex(s).c_str(), o.what.c_str()); } + else if (o.content != s) { bad++; printf(" SPLIT2 CORRUPT at %zu: got [%s] want [%s]\n", i, hex(o.content).c_str(), hex(s).c_str()); } + for (size_t j = i; j <= s.size(); j++) { + total++; + outcome o3 = run_stream({s.substr(0, i), s.substr(i, j - i), s.substr(j)}); + if (o3.threw) { threw++; } + else if (o3.content != s) { bad++; } + } + } + // one byte at a time, the pathological SSE chunking + total++; + std::vector bytes; + for (char c : s) { bytes.push_back(std::string(1, c)); } + outcome ob = run_stream(bytes); + if (ob.threw) { threw++; printf(" SPLIT1 THROW on [%s]\n", hex(s).c_str()); } + else if (ob.content != s) { bad++; printf(" SPLIT1 CORRUPT: got [%s] want [%s]\n", hex(ob.content).c_str(), hex(s).c_str()); } + } + printf("RESULT split total=%d threw=%d corrupted=%d\n", total, threw, bad); + g_fail += bad + threw; + if (!all) { return g_fail == 0 ? 0 : 1; } + } + + if (all || mode == "roundtrip") { + // every valid Unicode scalar value must survive byte identical. + printf("== valid scalar round trip ==\n"); + int bad = 0, threw = 0; long total = 0; + std::string batch; + for (uint32_t cp = 0; cp <= 0x10FFFF; cp++) { + if (cp >= 0xD800 && cp <= 0xDFFF) { continue; } + if (cp == 0) { continue; } // NUL is tested separately, chat content trims nothing but keep it simple + batch += enc(cp); + if (batch.size() >= 4096 || cp == 0x10FFFF) { + total++; + outcome o = run_final(batch); + if (o.threw) { threw++; printf(" THROW on batch ending U+%04X: %s\n", cp, o.what.c_str()); } + else if (o.content != batch) { + bad++; + printf(" CORRUPT in batch ending U+%04X\n", cp); + } + batch.clear(); + } + } + printf("RESULT roundtrip batches=%ld threw=%d corrupted=%d\n", total, threw, bad); + g_fail += bad + threw; + if (!all) { return g_fail == 0 ? 0 : 1; } + } + + if (all || mode == "exhaustive1") { + printf("== all 256 single bytes ==\n"); + int threw = 0, refdiff = 0; + for (int b = 0; b < 256; b++) { + std::string s(1, (char) b); + outcome o = run_final(s); + if (o.threw) { threw++; printf(" THROW %02X: %s\n", b, o.what.substr(0, 60).c_str()); } + else if (o.content != ref_decode_replace(s)) { refdiff++; printf(" REFDIFF %02X got[%s] ref[%s]\n", b, hex(o.content).c_str(), hex(ref_decode_replace(s)).c_str()); } + } + printf("RESULT single threw=%d refdiff=%d\n", threw, refdiff); + g_fail += threw + refdiff; + if (!all) { return g_fail == 0 ? 0 : 1; } + } + + if (all || mode == "exhaustive2") { + printf("== all 65536 two-byte sequences ==\n"); + int threw = 0, refdiff = 0; + std::map refdiff_examples; + for (int a = 0; a < 256; a++) { + for (int b = 0; b < 256; b++) { + std::string s; + s += (char) a; s += (char) b; + outcome o = run_final(s); + if (o.threw) { + if (threw < 5) { printf(" THROW %02X %02X: %s\n", a, b, o.what.substr(0, 60).c_str()); } + threw++; + } else { + const std::string r = ref_decode_replace(s); + if (o.content != r) { + if (refdiff < 20) { printf(" REFDIFF %02X %02X got[%s] ref[%s]\n", a, b, hex(o.content).c_str(), hex(r).c_str()); } + refdiff++; + } + } + } + } + printf("RESULT two threw=%d refdiff=%d of 65536\n", threw, refdiff); + g_fail += threw + refdiff; + if (!all) { return g_fail == 0 ? 0 : 1; } + } + + if (all || mode == "exhaustive3") { + printf("== 3 and 4 byte lead sweeps ==\n"); + int threw = 0, refdiff = 0; long total = 0; + const int tail[] = {0x00, 0x41, 0x7F, 0x80, 0x9F, 0xA0, 0xBF, 0xC0, 0xE0, 0xFF}; + for (int a = 0xC0; a <= 0xFF; a++) { + for (int b = 0; b < 256; b++) { + for (int t : tail) { + std::string s; s += (char) a; s += (char) b; s += (char) t; + total++; + outcome o = run_final(s); + if (o.threw) { if (threw < 5) { printf(" THROW %02X %02X %02X\n", a, b, t); } threw++; } + else if (o.content != ref_decode_replace(s)) { + if (refdiff < 20) { printf(" REFDIFF %02X %02X %02X got[%s] ref[%s]\n", a, b, t, hex(o.content).c_str(), hex(ref_decode_replace(s)).c_str()); } + refdiff++; + } + } + } + } + for (int a = 0xF0; a <= 0xFF; a++) { + for (int b = 0; b < 256; b++) { + for (int t : tail) { + std::string s; s += (char) a; s += (char) b; s += (char) 0x80; s += (char) t; + total++; + outcome o = run_final(s); + if (o.threw) { threw++; } + else if (o.content != ref_decode_replace(s)) { refdiff++; } + } + } + } + printf("RESULT three_four total=%ld threw=%d refdiff=%d\n", total, threw, refdiff); + g_fail += threw + refdiff; + if (!all) { return g_fail == 0 ? 0 : 1; } + } + + if (all || mode == "fuzz") { + // random byte soup, streamed in random chunk sizes: the realistic byte-fallback stream + printf("== random byte fuzz ==\n"); + uint64_t seed = 0x9E3779B97F4A7C15ull; + auto rnd = [&]() { seed ^= seed << 13; seed ^= seed >> 7; seed ^= seed << 17; return seed; }; + int threw = 0, streamdiff = 0, refdiff = 0; + const int N = 20000; + for (int i = 0; i < N; i++) { + size_t len = 1 + (rnd() % 24); + std::string s; + for (size_t k = 0; k < len; k++) { + // bias towards lead and continuation bytes so the interesting cases dominate + uint64_t r = rnd() % 100; + if (r < 30) { s += (char) (0x80 + (rnd() % 0x40)); } + else if (r < 60) { s += (char) (0xC0 + (rnd() % 0x40)); } + else if (r < 80) { s += (char) (0x20 + (rnd() % 0x5F)); } + else { s += (char) (rnd() % 256); } + } + outcome f = run_final(s); + if (f.threw) { if (threw < 5) { printf(" THROW [%s]: %s\n", hex(s).c_str(), f.what.substr(0, 50).c_str()); } threw++; continue; } + if (f.content != ref_decode_replace(s)) { + if (refdiff < 10) { printf(" REFDIFF [%s]\n got[%s]\n ref[%s]\n", hex(s).c_str(), hex(f.content).c_str(), hex(ref_decode_replace(s)).c_str()); } + refdiff++; + } + // same bytes, random chunking + std::vector chunks; + size_t p = 0; + while (p < s.size()) { size_t take = 1 + (rnd() % 3); chunks.push_back(s.substr(p, take)); p += take; } + outcome st = run_stream(chunks); + if (st.threw) { threw++; continue; } + if (st.content != f.content) { + if (streamdiff < 5) { printf(" STREAMDIFF [%s] final[%s] stream[%s]\n", hex(s).c_str(), hex(f.content).c_str(), hex(st.content).c_str()); } + streamdiff++; + } + } + printf("RESULT fuzz n=%d threw=%d refdiff=%d streamdiff=%d\n", N, threw, refdiff, streamdiff); + g_fail += threw + refdiff + streamdiff; + if (!all) { return g_fail == 0 ? 0 : 1; } + } + + if (!all) { + fprintf(stderr, "unknown mode %s\n", mode.c_str()); + return 2; + } + + printf("\n%s: %d failure(s)\n", g_fail == 0 ? "OK" : "FAILED", g_fail); + return g_fail == 0 ? 0 : 1; +} From ff1d5901e7dabd5532c7026e9e43ead7867a95a1 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Wed, 9 Sep 2026 01:53:51 -0700 Subject: [PATCH 9/9] server: pin the token pipeline case, streaming against non streaming --- tools/server/tests/test-server-utf8.cpp | 99 +++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/tools/server/tests/test-server-utf8.cpp b/tools/server/tests/test-server-utf8.cpp index d97ce2b86511..370b2b236ad5 100644 --- a/tools/server/tests/test-server-utf8.cpp +++ b/tools/server/tests/test-server-utf8.cpp @@ -16,6 +16,7 @@ // exhaustive1, exhaustive2, exhaustive3, fuzz) runs just that one. #include "server-task.h" +#include "server-common.h" #include "chat.h" #include @@ -181,6 +182,74 @@ static void named(const char * id, const char * desc, const std::string & text) (void) desc; } + +// --------------------------------------------------------------------------- +// The server token pipeline, not just update_chat_msg(). +// +// tools/server/server-context.cpp process_token() appends the token text to the slot's own +// generated_text and then, if validate_utf8() says the tail is a cut-off multi-byte sequence, +// sends nothing at all for that token. So a trailing incomplete sequence is never delivered as +// a partial, and send_final_response() supplies an empty content in stream mode, which means +// update_chat_msg() never gets a chance to substitute for it. Non-streaming hands over the whole +// text and does get substituted. +// +// The data loss in streaming predates this change: the held back bytes were dropped in both +// modes before it. The divergence is new, because the non-streaming side is now substituted and +// the streaming side still is not. Fixing the streaming side means changing what +// process_token() sends, which is a different file and changes what streaming clients receive, +// so it is pinned here rather than folded in. The hard assertion is the one that matters: +// no decodable content may be lost in either mode. +static int n_pipeline_asym = 0, n_pipe_stream_bad = 0, n_pipe_nostream_bad = 0; + +static void pipeline_case(const char * id, const std::vector & tokens) { + // ---- streaming, as process_token() drives it ---- + std::string slot_text; + size_t n_sent = 0; + common_chat_parser_params p; + task_result_state st_stream(p); + std::vector diffs; + std::string streamed; + bool threw = false; + try { + for (const auto & tok : tokens) { + slot_text += tok; + if (validate_utf8(slot_text) < slot_text.size()) { + continue; // incomplete tail: process_token() sends nothing for this token + } + const std::string to_send = slot_text.substr(n_sent); + n_sent = slot_text.size(); + st_stream.update_chat_msg(to_send, true, diffs); + } + // send_final_response() sets content to "" in stream mode + streamed = st_stream.update_chat_msg("", false, diffs).content; + } catch (const std::exception &) { + threw = true; + } + + // ---- non-streaming: the whole text in one final call ---- + outcome nostream = run_final(slot_text); + + const std::string held_back = slot_text.substr(n_sent); + const std::string ref_sent = ref_decode_replace(slot_text.substr(0, n_sent)); + const std::string ref_all = ref_decode_replace(slot_text); + + const bool stream_ok = !threw && streamed == ref_sent; + const bool nostream_ok = !nostream.threw && nostream.content == ref_all; + const bool agree = !threw && !nostream.threw && streamed == nostream.content; + + if (!agree) { n_pipeline_asym++; } + if (!stream_ok) { n_pipe_stream_bad++; } + if (!nostream_ok) { n_pipe_nostream_bad++; } + + printf("PIPE %-14s text=[%-20s] held_back=[%-8s] stream=[%-14s] nostream=[%-14s] %s%s%s\n", + id, hex(slot_text).c_str(), hex(held_back).c_str(), + threw ? "THROW" : hex(streamed).c_str(), + nostream.threw ? "THROW" : hex(nostream.content).c_str(), + stream_ok ? "stream-ok" : "STREAM-LOST-CONTENT", + nostream_ok ? " nostream-ok" : " NOSTREAM-BAD", + agree ? " agree" : " ASYMMETRIC"); +} + static int g_fail = 0; int main(int argc, char ** argv) { @@ -260,6 +329,36 @@ int main(int argc, char ** argv) { if (!all) { return g_fail == 0 ? 0 : 1; } } + if (all || mode == "pipeline") { + printf("== server token pipeline, streaming against non streaming ==\n"); + // generation stops on a lead byte with nothing after it: the classic byte fallback tail + pipeline_case("tail-c3", {"abc", "\xC3"}); + pipeline_case("tail-e4", {"abc", "\xE4"}); + pipeline_case("tail-e4b8", {"abc", "\xE4", "\xB8"}); + pipeline_case("tail-f0", {"abc", "\xF0"}); + pipeline_case("only-c3", {"\xC3"}); + // a run of lead bytes: validate_utf8() holds the last one back every time + pipeline_case("run-c3", {"\xC3", "\xC3", "\xC3", "\xC3"}); + // a complete but ill formed sequence at the end: not held back, so both modes see it + pipeline_case("surrogate", {"abc", "\xED\xA0\x80"}); + pipeline_case("overlong", {"abc", "\xC0\x80"}); + pipeline_case("above-max", {"abc", "\xF4\x90\x80\x80"}); + // a stray continuation byte: also not held back + pipeline_case("stray-80", {"abc", "\x80"}); + // valid characters split across tokens: nothing may be lost or duplicated + pipeline_case("split-cjk", {"a", "\xE4", "\xB8", "\xAD", "b"}); + pipeline_case("split-emo", {"a", "\xF0\x9F", "\x98\x80", "b"}); + pipeline_case("valid-mix", {"caf", "\xC3\xA9", " ", "\xE4\xB8\xAD"}); + printf("RESULT pipeline stream_lost_content=%d nostream_bad=%d asymmetric=%d\n", + n_pipe_stream_bad, n_pipe_nostream_bad, n_pipeline_asym); + g_fail += n_pipe_stream_bad + n_pipe_nostream_bad; + printf("note: an asymmetric row means the held back bytes never reached update_chat_msg()\n" + " in stream mode, so only the non-streaming side could substitute for them.\n" + " Streaming dropped them before this change too, so nothing is lost that was\n" + " not lost already; it is not counted as a failure.\n"); + if (!all) { return g_fail == 0 ? 0 : 1; } + } + if (all || mode == "split") { // a valid character split across token / SSE chunk boundaries. // Every split point of every valid sample, in 2 and 3 chunk form.