diff --git a/.superpowers/sdd/task-4-report.md b/.superpowers/sdd/task-4-report.md index 86b6b2e..196f55b 100644 --- a/.superpowers/sdd/task-4-report.md +++ b/.superpowers/sdd/task-4-report.md @@ -132,3 +132,60 @@ Step 9 (source file with whitespace-padded source/signal and `[0]` suffix): [StreamHub][Information]: StreamHub: initialised with 1 session(s), WSPort=8099, MaxPoints=20000, PushRate=30 Hz. ``` Entry loaded correctly (trimmed to `wave`/`Sine`, `[0]` stripped). + +## Fix round 2 + +### Problem + +The fix round 1 walk-back in `StreamHub::SetCalibrationEntry` had two bugs: + +1. It ran unconditionally, not only after truncation. A valid short unit ending in a multi-byte character (e.g. `"Ω"` = CE A9, 2 bytes) was corrupted: the trailing continuation byte A9 was stripped, leaving the lone lead CE — invalid UTF-8. +2. It only stripped continuation bytes, never an orphaned lead byte. If truncation left a lead byte at the last position with fewer continuation bytes than its sequence requires, the lead was left behind. + +### Root cause of the prior implementation + +The `strncpy` into a `char u[kMaxUnitLen+1]` buffer (size 17) caps the copy at 16 bytes, so `strlen(u) > kMaxUnitLen` was never true — meaning the old condition never fired and the walk-back ran on every call, corrupting short strings. + +### Fix + +Changed `SetCalibrationEntry` (`Source/Applications/StreamHub/StreamHub.cpp`) to: + +1. Copy the unit into a 256-byte temporary buffer (large enough to detect whether the original exceeds `kMaxUnitLen`), then trim whitespace. +2. If the trimmed length is `<= kMaxUnitLen`: copy verbatim, no repair. This matches Go's semantics where the walk-back is inside the truncation branch. +3. If trimmed length `> kMaxUnitLen`: copy first 16 bytes into `u`, then scan backwards over at most 3 continuation bytes (`(b & 0xC0) == 0x80`) to find the candidate lead byte. Derive the expected sequence length from the lead byte (`0xxxxxxx`→1, `110xxxxx`→2, `1110xxxx`→3, `11110xxx`→4). If bytes present (`cont + 1`) is fewer than expected, cut at the lead byte. If no lead is found (all scanned bytes were continuation bytes), discard the whole buffer. + +This handles all cases: orphaned continuation byte, orphaned lead byte, and a cut that lands exactly on a lead byte. + +### Build output + +``` +make -f Makefile.gcc apps +``` +Compiled cleanly with `-std=c++98 -Wall -Werror`, no warnings. + +### GTest output + +``` +[==========] 132 tests from 12 test cases ran. +[ PASSED ] 128 tests. +[ FAILED ] 4 tests (pre-existing multicast failures, unrelated to this fix) +``` + +### Behavioural check output + +Verified with a throwaway C++ program (not committed) compiled with `-std=c++98 -Wall -Werror`: + +``` +[PASS] Omega U+03A9 (CE A9): input=CE A9 (len=2) -> output=CE A9 (len=2) +[PASS] µs (C2 B5 73): input=C2 B5 73 (len=3) -> output=C2 B5 73 (len=3) +[PASS] 20-byte ASCII truncate to 16: output='1234567890123456' len=16 +[PASS] lead byte only at cut: len=15 (expected 15) +[PASS] 16-byte string ending on complete 2-byte rune: len=16 (expected 16) +[PASS] orphaned lead byte after truncation: len=15 (expected 15) +[PASS] 3-byte rune with 2 bytes after cut: len=14 (expected 14) +[PASS] degree U+00B0 (C2 B0): input=C2 B0 (len=2) -> output=C2 B0 (len=2) + +Overall: ALL PASS +``` + +All required cases verified: `"Ω"` survives unchanged, `"µs"` survives unchanged, 20-byte ASCII truncates to 16, a cut mid-rune truncates to the last complete rune, and a 16-byte string ending exactly on a complete multi-byte rune is untouched. diff --git a/Source/Applications/StreamHub/StreamHub.cpp b/Source/Applications/StreamHub/StreamHub.cpp index a8fea0d..ba0251c 100644 --- a/Source/Applications/StreamHub/StreamHub.cpp +++ b/Source/Applications/StreamHub/StreamHub.cpp @@ -755,33 +755,74 @@ bool StreamHub::SetCalibrationEntry(const char *source, const char *signal, if (!JsonIsFinite(scale) || (scale == 0.0)) { return false; } if (!JsonIsFinite(offset)) { return false; } - /* Trim unit, then truncate to kMaxUnitLen bytes, walking back any - * partial UTF-8 rune to keep the stored bytes valid UTF-8 (Go parity). */ + /* Trim unit, then — if and only if the trimmed string exceeds kMaxUnitLen + * bytes — truncate to kMaxUnitLen and repair the tail so the stored bytes + * are valid UTF-8. This exactly mirrors Go's CalConfig.Normalise(): the + * walk-back runs only inside the truncation branch, so a short valid string + * (e.g. "Ω" = CE A9, 2 bytes) is never touched. + * + * Repair algorithm (matching Go's utf8.DecodeLastRuneInString loop): + * Scan backwards over at most 3 continuation bytes (10xxxxxx, (b&0xC0)==0x80) + * to locate the lead byte of the last UTF-8 sequence. Derive the expected + * sequence length from that lead byte (0xxxxxxx→1, 110xxxxx→2, 1110xxxx→3, + * 11110xxx→4). If the bytes present are fewer than expected, cut the string + * at the lead byte. This handles an orphaned continuation byte, an orphaned + * lead byte, and the case where the cut lands exactly on the lead byte. */ char u[kMaxUnitLen + 1u]; u[0] = '\0'; if (unit != static_cast(0)) { - strncpy(u, unit, sizeof(u) - 1u); - u[sizeof(u) - 1u] = '\0'; - TrimInPlace(u); - /* Truncate to kMaxUnitLen bytes */ - if (strlen(u) > kMaxUnitLen) { + /* Use a temporary over-sized buffer so we can detect when the trimmed + * input is actually longer than kMaxUnitLen (strncpy into u[kMaxUnitLen+1] + * would silently cap the copy, making the length check always false). */ + const uint32 kTmpLen = 256u; + char tmp[256u]; + strncpy(tmp, unit, kTmpLen - 1u); + tmp[kTmpLen - 1u] = '\0'; + TrimInPlace(tmp); + + uint32 tlen = static_cast(strlen(tmp)); + if (tlen <= kMaxUnitLen) { + /* Short enough: copy verbatim, no repair needed. */ + strncpy(u, tmp, kMaxUnitLen); u[kMaxUnitLen] = '\0'; - } - /* Walk back any trailing partial UTF-8 rune. A byte b is a - * continuation byte (10xxxxxx) iff (b & 0xC0) == 0x80. A truncation - * may leave a sequence starter with fewer continuation bytes than it - * expects; drop bytes from the end while the last byte is a lone - * continuation byte that decodes as an invalid (RuneError, 1) pair. - * Concrete: if the last byte is 0x80-0xBF (continuation), remove it, - * then repeat — this matches Go's utf8.DecodeLastRuneInString loop. */ - uint32 ulen = static_cast(strlen(u)); - while (ulen > 0u) { - const unsigned char last = static_cast(u[ulen - 1u]); - /* Is it a UTF-8 continuation byte (10xxxxxx)? */ - if ((last & 0xC0u) == 0x80u) { - u[--ulen] = '\0'; + } else { + /* Truncate at kMaxUnitLen bytes, then repair any split rune. */ + strncpy(u, tmp, kMaxUnitLen); + u[kMaxUnitLen] = '\0'; + uint32 ulen = kMaxUnitLen; + + /* Scan back over continuation bytes (up to 3). */ + uint32 cont = 0u; + while ((cont < 3u) && (cont < ulen)) { + const unsigned char b = + static_cast(u[ulen - 1u - cont]); + if ((b & 0xC0u) == 0x80u) { + cont++; + } else { + break; + } + } + + /* The byte at index ulen-1-cont is the candidate lead byte. */ + if (cont < ulen) { + const unsigned char lead = + static_cast(u[ulen - 1u - cont]); + uint32 expected = 0u; + if ((lead & 0x80u) == 0x00u) { expected = 1u; } + else if ((lead & 0xE0u) == 0xC0u) { expected = 2u; } + else if ((lead & 0xF0u) == 0xE0u) { expected = 3u; } + else if ((lead & 0xF8u) == 0xF0u) { expected = 4u; } + /* bytes present in the sequence = cont + 1 (the lead itself) */ + if ((expected > 1u) && ((cont + 1u) < expected)) { + /* Incomplete multi-byte sequence: drop from the lead byte. */ + ulen = ulen - 1u - cont; + u[ulen] = '\0'; + } + /* else: complete sequence (expected==1 ASCII, or cont+1==expected) + * — nothing to do. */ } else { - break; + /* Every byte was a continuation byte with no lead: discard all. */ + u[0] = '\0'; } } }