Byte-index the strptime scanner so non-ASCII input cannot abort

[carpentry-agent]

Datetime.strptime drives its scanner from byte offsets — String.char-at reads a single byte and String.length is strlen — but then sliced with String.suffix/String.prefix, which count characters. Wherever those two disagree the character slice runs off the end of the decoded array and Array.unsafe-nth aborts the process.

On unmodified master:

(Datetime.strptime <six 0x80 bytes> "%d %b %Y")
  → Array_unsafe_nth__Char: Assertion `n < a.len' failed,  exit 134

This is reachable from server-controlled input: http-client feeds a Set-Cookie: ...; Expires=<value> through a date parse.

This is the parse-side half of the byte/character split. #22 is the sibling PR on the format side (strftime/format/format-for); this branch is cut from origin/master and touches only the parse side, so the two are independent. git merge-tree against origin/claude/strftime-scanner reports no conflict.

What changed

Every confused site slices with String.byte-slice, so a byte offset meets a byte-indexed slice:

site	was
parse-digits	suffix+prefix → byte-slice s pos (+ pos n)
match-in-array (%B, %A)	suffix+prefix → byte-slice s pos (+ pos clen)
match-short-in-array (%b, %a)	suffix+prefix → byte-slice s pos (+ pos 3)
%p	suffix+prefix → byte-slice input ipos (+ ipos 2)
%Z	suffix+prefix → byte-slice input ipos (+ ipos n)
expand-compound-formats	(str c) / (str spec) → byte-slice fmt i …
unknown-specifier error	(str spec) → byte-slice &efmt …

parse-ranged needed no change — it delegates to parse-digits.

Char.str was the second half of the problem: it UTF-8 re-encodes the byte value as a codepoint, so expand-compound-formats turned a 0xC3 byte back into two bytes. Copying the format bytes straight through with byte-slice avoids that without a String.from-bytes round-trip.

Why each byte-slice is in range

String.byte-slice is an unchecked memcpy(ptr, *s + a, b - a), so every call has to be provably within [0, strlen]. and short-circuits in Carp, so guard-then-slice in one condition is sound.

• parse-digits — inside (<= (+ pos n) (String.length s)). pos starts at 0 and only ever increases.
• match-in-array — second conjunct of an and whose first is (<= (+ pos clen) slen).
• match-short-in-array — the early return covers (> (+ pos 3) (String.length s)); the candidate slice is the second conjunct after (>= (String.length candidate) 3).
• %p — the if returns an error when (> (+ ipos 2) ilen).
• %Z — the scan loop only advances n while (< (+ ipos n) ilen), so ipos + n <= ilen on exit.
• expand-compound-formats — the two-byte slice sits under (< (+ i 1) len); the one-byte slice under the loop's (< i len).
• unknown specifier — under (< (+ fpos 1) flen).

Audit

Audited strptime end to end (:763 → :1113) plus expand-compound-formats (:655). Found clean, no change needed:

• %z — guarded by (> (+ ipos 5) ilen), and its only raw read is char-at at a proven-valid index; the rest goes through parse-digits.
• The literal-matching path (:1064) — compares a byte from efmt against a byte from input and advances one byte. Guarded by (< ipos ilen). Byte-consistent already.
• %% (:1056) and the two char-at reads on efmt (:790, :793) — all guarded.
• %B/%A advance by String.length of the matched candidate — a byte count. That is correct because match-in-array now byte-compares, so the match consumed exactly clen bytes. Same for the %b/%a advance of 3.
• unexpected character at position N reports a byte position, which is now consistent with the rest of the scanner.

Two upstream Carp core bugs are in the blast radius; this PR stops triggering them and does not touch core: String.starts-with?'s byte/char inconsistency, and Array.range n n step overrunning its allocation.

time has no CHANGELOG, so there is no entry to add.

Verification

1. Aborts on master, one case per process (master aborts on some inputs and would otherwise mask the rest). 48 formats × 20 hostile inputs = 960 cases:

master:  187 aborts (SIGABRT, exit 134) across 40 of the 48 formats
branch:    0 aborts

Every hostile class landed: 20×0x80 (40 aborts), 6×0x80 (39), a 4-byte emoji (35), C3 A9 (18), a truncated 3-byte lead E2 82 (18), "A"+0x80 (18), and trailing/embedded/leading continuation bytes.

2. Differential over valid input. 31 valid (format, input) pairs covering every supported specifier — %b %B %a %A %p %z %Z %j %U %W %V %G %u %n %y %I and the compound %c %x %X %F %T %R %D %r %h %% — with and without timezone, plus a 10-case strftime → strptime round-trip. Results dumped as decimal bytes, not rendered text. Master and branch agree cell for cell (31/31 and 10/10). The corpus is not vacuous: all 31 are successful parses with substantive field values (e.g. %Y-%m-%dT%H:%M:%S%z on 2024-03-15T14:30:00+0530 → Timezone "" 19800 false).

Of the 773 hostile cases where master survived, 748 are byte-identical. All 25 differences are non-ASCII format strings, and each is a strict improvement:

• "%<0xC3><0xA9>" — master reports unknown format specifier: %<0xC3><0x83> (the Char.str re-encoding); branch reports the actual byte, %<0xC3>.
• "<0xC3><0xA9>%d" — master says unexpected character at position 1, because it had mangled the literal into four bytes so it could never match. The branch matches the literal and then fails on %d for the real reason.

No ASCII format changed behaviour on any input.

Worth flagging: fixing expand-compound-formats is what makes non-ASCII literals traversable at all, which newly puts %Z and %p at ipos > 0 with multibyte bytes behind them. Those two slice fixes are load-bearing for this PR, not hypothetical — without them the fix would introduce an abort where master merely errored.

3. New assertions have teeth. 13 new assertions; with time.carp alone reverted to master, 13/13 fail — 9 by SIGABRT and 4 by wrong result (measured one assertion per process, since an abort would otherwise mask the rest). No pre-existing assertion changes: 269/0 on master, 282/0 here.

4. ASan. Built with carp -b, hand-compiled from ~/.carp/out/main.c with clang -fsanitize=address -I …/carp-lang/core:

• valid corpus + round-trip + all 960 hostile cases in one process (1001 rows): 0 findings
• the full test suite: 282/0, 0 findings
• positive control: a deliberate 64-byte over-read injected into parse-digits fires heap-buffer-overflow … READ of size 68, so the sanitizer is provably live.

LSan not attempted — not usable on this 32-bit ARM box.

5. CI checks. carp -x test/time.carp → 282/0. carp-fmt -c clean. angler clean on both files (verified against a bait file that it does report findings — the ~/.carp/out/angler build silently lints nothing, ~/bin/angler is the working one). carp -x gendocs.carp leaves git status empty.

---

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

[carpentry-reviewer]

Build & Tests

Checked out claude/strptime-byte-indexed at f15591c and built it here.

• carp -x test/time.carp — 282 passed, 0 failed, exit 0. Matches the claim.
• CI green on both ubuntu-latest and macos-latest; the run's head_sha is f15591c, so it covers this HEAD and not an older push. ci.yml has no continue-on-error and every step runs under set -euo pipefail, so the green is real for the test, lint, format-check and gendocs steps. Note the test step builds with -Werror.
• Merge-base is the current origin/master (bd30971) — no drift. One commit, bot-authored; no human has pushed to this branch.
• angler and carp-fmt --check over CI's file set — both clean, rc=0. I checked angler against a bait file first ((let [x 1] 2) → unused-let-binding, rc=1), so the clean result is evidence rather than a null run.
• carp -x gendocs.carp leaves git status empty — committed docs are current. (CI runs gendocs but doesn't diff it, so that one is worth checking by hand; it passes.)

The abort reproduces on master, and it is not narrow. I wrote my own probe dispatching on argv[1] so each case gets a fresh process, and ran 15 hostile inputs on both sides:

master: 13 aborts (SIGABRT, rc=134)     branch: 0

Including the PR's stated repro (%d %b %Y with six 0x80 bytes), and also %c, %r, %z, %j, %I:%M %p and a 4-byte emoji against %A %d %B %Y.

The improvement is larger than the PR body claims. The body frames the non-crash half as better error text. It is more than that: on master a format string containing any non-ASCII literal could not parse at all. Same fixture, one case per process, master vs branch:

format	input	master	branch
%Y年%m月%d日	2024年03月15日	unexpected character at position 4	2024-03-15
Größe: %Y	Größe: 2024	unexpected character at position 3	2024
%d.%m.%Y — %H:%M	15.03.2024 — 14:30	unexpected character at position 11	2024-03-15 14:30
😀%Y	😀2024	unexpected character at position 0	2024
€%B %d %Y	€March 15 2024	unexpected character at position 0	2024-03-15

14 of 14 such formats went from total failure to a correct parse. A German or Japanese date format is not an exotic input, so this is a capability that did not previously exist rather than a cosmetic fix.

The "load-bearing" claim about %Z and %p holds, and I checked it the way it could have failed. Every specifier that consumes input at ipos > 0 behind multibyte bytes now works: %p (é %p …), %Z (é %Z … → Timezone "UTC", 日%Z → "CEST"), %b/%B via match-short-in-array/match-in-array (€%b, €%B), and %z (ü%z → 19800). ASCII controls for each (x %Z …, x%b …) are byte-identical on both sides, so the change is isolated to the multibyte case.

Nothing regressed. Across 91 cases — 20 valid ASCII formats covering every specifier and all nine compound expansions, 7 non-ASCII literal formats, 7 non-ASCII inputs, 16 degenerate formats ("", "%", "%%", "%Y%", "%Q", "hello"), 21 hostile byte sequences and 3 round-trips — master and branch differ on exactly two surviving cases, both the unknown-specifier error text, and both strictly better:

format	master	branch
"%<0xC3><0xA9>"	% + [195 131] (Char.str re-encoding)	% + [195] — the actual byte
"%<0x80>"	% + [195 130]	% + [128]

Teeth: 13/13, confirmed independently. Reverting time.carp alone to master and running the branch's tests aborts at Array_unsafe_nth__Char: Assertion 'n < a.len' failed, exit 134 — so the 9 Result.error? assertions are genuine crash guards, and the abort masks the rest exactly as the PR says. I scored the 4 positive assertions against my own master measurements rather than re-running them one process at a time: «%Y-%m-%d», é %Z, é%I%p all fail on master with unexpected character, and %é yields [195 131] instead of the asserted [195]. All four fail on master by wrong result.

Every byte-slice call checked against the guard that dominates it. String.byte-slice is memcpy(ptr, *s + a, b - a) with no bounds check (core/carp_string.h:378), so this matters. All seven hold, and and short-circuiting makes the guard-then-slice pairs sound: parse-digits under (<= (+ pos n) (String.length s)); match-in-array as the second conjunct after (<= (+ pos clen) slen); match-short-in-array under the (> (+ pos 3) …) early return, with byte-slice candidate 0 3 behind (>= (String.length candidate) 3); %p under (> (+ ipos 2) ilen); %Z after a loop that only advances while (< (+ ipos n) ilen); both expand-compound-formats slices under (< (+ i 1) len) / (< i len); and the unknown-specifier slice under the (< (+ fpos 1) flen) that gates the whole % arm. ipos and fpos are monotonic and never exceed their lengths, so no negative-length memcpy is reachable.

I also checked the advance arithmetic, since a byte/char mismatch there would reintroduce the bug one level up: %B/%A advance by String.length of the matched candidate, which is a byte count and is exactly what match-in-array compared; %b/%a advance by 3 under a guard that already proved ipos + 3 <= ilen. Consistent.

ASan. carp -b, then hand-compiled ~/.carp/out/main.c with clang -fsanitize=address -I ~/carp-lang/core, all 91 cases in one process: 0 findings, rc=0. Positive control: a 64-byte over-read injected into parse-digits' new byte-slice fires

ERROR: AddressSanitizer: heap-buffer-overflow ... READ of size 68
  #1 String_byte_MINUS_slice /home/hellerve/carp-lang/core/carp_string.h:381

so the sanitizer is provably live on this exact code path and the clean run means something.

Independence from #22 verified by building the merge, not just by git merge-tree. merge-tree --write-tree returns rc=0, but a clean textual merge of two PRs that both touch time.carp isn't evidence that the merged code works — #23 changes expand-compound-formats, which strftime also drives. So I materialized the merged tree with git archive and ran it:

merged (#22 + #23): 299 passed, 0 failed, exit 0

299 = 269 (master) + 17 (#22) + 13 (#23), so no assertion was lost or double-counted, and both fixes are present in the merged source. The two can merge in either order.

Findings

None. I looked specifically for a wrong success — an input master rejected that the branch now accepts with a bad value — and found none; every one of the 14 newly-parsing formats yields the correct fields. I also found no problem with: the byte-by-byte literal matcher (fc and char-at input ipos both advance one byte, so a multibyte literal matches correctly and mismatches at the right offset); %% and trailing-% handling through expand-compound-formats (a trailing lone % survives expansion and then matches a literal %, consistent with #22's decision on the format side); %Z only ever capturing [A-Za-z], so tz-nm stays ASCII; and Int.from-string, which is strtol with *err == 0, so a slice with any trailing non-digit is rejected rather than silently truncated.

Two things I turned up are pre-existing on master and not this PR's — recording them so they read as seen, not missed, and so nobody scores them against this branch:

• %p without %H/%I invents an hour. hr uses -1 as its "unset" sentinel, but line 1093 applies the PM adjustment before that is checked, so (strptime "PM 2024-03-15" "%p %Y-%m-%d") returns hours = (Just 11) — -1 + 12. Identical on master and branch; %I %p is correct.
• parse-digits inherits strtol's leniency, so %d on -1/03/2024 yields day -1 and a leading space is accepted. Identical on both sides.

Neither is touched by this change and neither should hold it.

Verdict: merge

The premise is real and I reproduced it independently — 13 aborts on master, 0 here — and the fix is the right one: every byte offset now meets a byte-indexed slice, every unchecked memcpy sits under a guard I verified, and ASan is clean over 91 cases with a live positive control firing inside String_byte_slice itself. Behaviour is unchanged everywhere except two error strings that both get more accurate, the 13 new assertions all genuinely fail on master, and the merge with #22 builds and runs 299/0. The one thing I'd change is the framing rather than the code: this PR fixes more than it claims — a format string with a non-ASCII literal was previously unusable, and now works.