From 39ee6040253255ca5e1bda6f0f15784366d00783 Mon Sep 17 00:00:00 2001 From: jasagiri <172246+jasagiri@users.noreply.github.com> Date: Mon, 7 Sep 2026 14:14:22 +0900 Subject: [PATCH] fixes #26173; don't resume iconv after a short write (#26174) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #26173. ## What breaks today `encodings.convert` allocates its output buffer as `newString(s.len)`, i.e. the same size as the input. Any conversion that expands — which is most non-ASCII to UTF-8 — therefore hits `E2BIG` and then **resumes** the same `iconv_t` where it stopped. That is fine for stateless encodings, but the shift state of a stateful encoding is not guaranteed to survive a short write, and on macOS it does not. After the `E2BIG` the converter behaves as if it were back in the initial state, so the tail of an ISO-2022-JP string is emitted as raw bytes: ```nim import std/[encodings, unicode] proc repeatedA(n: int): string = result = "\x1B\x24\x42" for _ in 0 ..< n: result.add "\x24\x22" # あ result.add "\x1B\x28\x42" let c = open("UTF-8", "ISO-2022-JP") for n in 1 .. 10: echo n, " chars in -> ", c.convert(repeatedA(n)).runeLen, " chars out" ``` ``` 6 chars in -> 6 chars out 7 chars in -> 8 chars out <-- wrong 10 chars in -> 12 chars out <-- wrong ``` Nothing is raised. The text is just wrong, and only in the tail, so short test strings pass and real data does not. The failure correlates exactly with whether the output buffer has to grow: | chars | input bytes | expected output bytes | growth needed | result | |---|---|---|---|---| | 6 | 18 | 18 | no | correct | | 7 | 20 | 21 | **yes** | corrupted | EUC-JP, which has the same 2-to-3 byte expansion but no shift state, is correct at every length — so it is the statefulness, not the growth ratio, that matters. The issue has a raw-C-API reproducer showing the state loss happens inside `iconv` and is not an artifact of the Nim string handling. ## What this PR does Two commits, because they are two separate defects: **1. `fixes #26173; don't resume iconv after a short write`** Reset the converter with `iconv(c, nil, nil, nil, nil)` and redo the whole conversion into a larger buffer instead of resuming. Adds a regression test that fails on `devel` and passes with the fix. **2. `fix out-of-bounds write in encodings.convert on a full output buffer`** The `EILSEQ`/`EINVAL` branch does `dst[0] = src[0]` and `dec(outLen)` without checking there is room, so a full output buffer writes one byte past the end and underflows `outLen` (a `csize_t`). Guarded with `outLen > 0`, letting the buffer-growth path handle the full-buffer case. This is a latent bug independent of #26173, found while working on it — happy to split it into its own PR if that is preferred. ## Testing `tests/stdlib/tencodings.nim` gains coverage for ISO-2022-JP across the buffer growth boundary (1..64 chars, plus a string with several ASCII/JIS state switches) and a stateless EUC-JP case that also crosses the boundary. - Fails on `devel` at `tencodings.nim(124)` without the fix - Passes with the fix under both `--mm:refc` and `--mm:orc` - Existing assertions in the file are unaffected Verified on macOS 15 / arm64. The Windows path (`convertWin`) does not use `iconv` and is untouched; ISO-2022-JP is already in `nameToCodePage` as 50220, so the new test exercises that path there too. --- lib/pure/encodings.nim | 21 ++++++++++++++------- tests/stdlib/tencodings.nim | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/lib/pure/encodings.nim b/lib/pure/encodings.nim index 2fc2d4c871..e74e30fe66 100644 --- a/lib/pure/encodings.nim +++ b/lib/pure/encodings.nim @@ -469,19 +469,26 @@ else: iconvres = iconv(c, addr src, addr inLen, addr dst, addr outLen) if iconvres == high(csize_t): var lerr = errno - if lerr == EILSEQ or lerr == EINVAL: + if (lerr == EILSEQ or lerr == EINVAL) and outLen > 0: # unknown char, skip dst[0] = src[0] src = cast[cstring](cast[int](src) + 1) dst = cast[cstring](cast[int](dst) + 1) dec(inLen) dec(outLen) - elif lerr == E2BIG: - var offset = cast[int](dst) - cast[int](cstring(result)) - setLen(result, len(result) + inLen.int * 2 + 5) - # 5 is minimally one utf-8 char - dst = cast[cstring](cast[int](cstring(result)) + offset) - outLen = csize_t(len(result) - offset) + elif lerr == E2BIG or lerr == EILSEQ or lerr == EINVAL: + # Either the output buffer is too small, or it is full and an unknown + # char cannot be copied over. Do not resume the conversion where it + # stopped: stateful encodings (ISO-2022-JP/KR/CN) can lose their shift + # state across a short write, which silently corrupts the tail of the + # output. Reset the converter and redo the whole conversion into a + # larger buffer instead. + discard iconv(c, nil, nil, nil, nil) + result = newString(len(result) * 2 + 16) + inLen = csize_t len(s) + outLen = csize_t len(result) + src = cstring(s) + dst = cstring(result) else: raiseOSError(lerr.OSErrorCode) # iconv has a buffer that needs flushing, specially if the last char is diff --git a/tests/stdlib/tencodings.nim b/tests/stdlib/tencodings.nim index 2f4daaba3e..7d115c58a8 100644 --- a/tests/stdlib/tencodings.nim +++ b/tests/stdlib/tencodings.nim @@ -105,3 +105,36 @@ block: block: # fixes about #23481 doAssertRaises EncodingError: discard open(destEncoding="this is a invalid enc") + +block: # bug #26173 - stateful encodings must survive output buffer growth + # ISO-2022-JP is stateful: `ESC $ B` switches to two byte JIS X 0208 mode and + # `ESC ( B` switches back to ASCII. `convert` sized its output buffer from the + # input length, so any input whose UTF-8 form is longer hit `E2BIG` and resumed + # the conversion after a short write. The shift state does not necessarily + # survive that, so the tail of the text came out as raw bytes and the result was + # silently wrong - and longer than the input. + proc repeatedA(n: int): string = + result = "\x1B\x24\x42" + for _ in 0 ..< n: result.add "\x24\x22" # あ + result.add "\x1B\x28\x42" + + var expected = "" + for n in 1 .. 64: + expected.add "あ" + doAssert convert(repeatedA(n), "UTF-8", "ISO-2022-JP") == expected + + # mixed ASCII and JIS runs, i.e. several state switches in one string + const mixed = "\x1B\x24\x42\x21\x5A\x3F\x37\x35\x2C\x21\x5B\x39\x41\x36\x68" & + "\x46\x6E\x40\x44\x3B\x33\x1B\x28\x42\x20\x1B\x24\x42\x43\x66" & + "\x38\x45\x38\x4D\x37\x7A\x24\x4E\x24\x34\x3E\x52\x32\x70\x1B\x28\x42" + doAssert convert(mixed, "UTF-8", "ISO-2022-JP") == + "【新規】港区南青山 " & + "中古戸建のご紹介" + +block: # stateless encodings keep working when the output buffer grows + var euc = "" + var expected = "" + for _ in 0 ..< 2000: + euc.add "\xA4\xA2" + expected.add "あ" + doAssert convert(euc, "UTF-8", "EUC-JP") == expected