Skip to content

Slice headers, redirect targets and cookie attributes by byte - #16

Merged
hellerve merged 1 commit into
mainfrom
claude/byte-safe-slicing
Aug 14, 2026
Merged

Slice headers, redirect targets and cookie attributes by byte#16
hellerve merged 1 commit into
mainfrom
claude/byte-safe-slicing

Conversation

@carpentry-agent

Copy link
Copy Markdown

Every string this library slices comes off the wire. Core's String.index-of,
index-of-string, index-of-from, char-at and length are byte-based, while
String.prefix, suffix and slice count characters, and starts-with? /
ends-with? guard on byte length and then slice characters. Wherever a byte
offset met a character-indexed cut, the client either lost body bytes or died.

This is the same fix shape as http #31, uri #33, time #23 and web #52.

Measured on unmodified main

End-to-end through Client.get against a local server that writes raw response
bytes, one case per process (an abort would otherwise mask the rest):

case main this branch
control, ASCII headers body HELLO-BODY (10 bytes) unchanged
Server: caf\xc3\xa9 body ELLO-BODY (9 bytes) HELLO-BODY (10)
X-Weird: \x80\x80\x80\x80\x80 body 15 bytes of mojibake HELLO-BODY (10)
cookie Domain=\x80 SIGABRT 0 matches
cookie Domain=example.com 1 match unchanged
cookie Path=\x80 0 matches unchanged

So a single two-byte UTF-8 sequence anywhere in the headers silently ate the
first byte of every body; five continuation bytes replaced the body entirely.

Isolating the split itself (String.index-of-string + String.prefix/suffix
on the raw accumulator, no client involved):

  • 44-byte response, one 2-byte sequence in a header value: split-pos 30,
    header-end 34, header text 35 bytes, body 9 bytes — one body byte crossed
    into the header text.
  • 32-byte response, four continuation bytes: String.prefix returned 44
    bytes
    and String.suffix returned 0 — the whole body gone. The growth
    is chars/from-chars failing to round-trip bytes that are not valid UTF-8,
    not a read past the buffer.
  • 33-byte response, five continuation bytes: String.prefix aborts
    Array_unsafe_nth__Char: Assertion 'n < a.len' failed, byte offset 29 against
    28 characters.

Five hostile cases that still abort, and why

A redirect whose Location is not ASCII, and any request URL that is not ASCII,
still abort on this branch. They do not abort in this repo's code. Measured
against http@0.3.0 alone, one case per process:

  • URI.parse "http://127.0.0.1:8799/x" — fine
  • URI.parse with caf\xc3\xa9 in the path, in the host, or a lone \x80 in
    the path — SIGABRT, all three
  • Response.parse of a 302 carrying Location: /caf\xc3\xa9/x or
    Location: \x80 — fine, both parse as 302

That is uri's own byte/character mismatch, fixed upstream in "Extract URI
components with byte indices, not character indices" but not in any release this
package can pin yet (http@0.3.0 and @0.4.0 both carry the old uri, and
neither has String.byte-starts-with?, which is why the helpers below are
local). The resolver fix here is necessary but not sufficient for those cases
until the pins move; I left it in rather than shipping a half-byte-safe
resolver.

Tally over the 11-case corpus: 3 fixed, 3 unchanged and correct, 5 unchanged and
broken upstream, 0 newly differing.

What changed

Three local helpers over String.byte-slice (byte-starts-with?,
byte-ends-with?, drop-bytes), and then:

  • Client.read-headers cuts the header/body boundary with byte-slice.
  • The RFC 3986 resolversplit-query, strip-fragment,
    path-after-authority, remove-last-segment, first-segment-end,
    remove-dot-segments, merge-paths, resolve-authority-ref,
    resolve-path-ref, resolve-location — takes every span by byte offset and
    tests every prefix bytewise. last-index-of-char already returned a byte
    index by construction and keeps that contract.
  • CookieJar.domain-matches? / path-matches? / matching compare the
    server's Domain= and Path= bytewise. The . test on line 31 was a
    one-character needle, which a single continuation byte defeats.

String.byte-slice is an unchecked memcpy, so each call needs its offsets
proven in range:

  • the two helpers slice only after their own and-guarded length comparison,
    which short-circuits
  • read-headers: split-pos >= 0 means the four needle bytes are inside acc,
    so header-end = split-pos + 4 <= length acc
  • split-query, strip-fragment, path-after-authority,
    resolve-authority-ref: every index comes from index-of/index-of-from,
    and the < 0 case takes the other branch, so a used index is a real position
    in the string
  • remove-last-segment, merge-paths: last-index-of-char scans 0 ..< length s, so a non-negative result is < length s and idx + 1 <= length s
  • remove-dot-segments: each drop-bytes sits under the byte-starts-with?
    that just proved those bytes are present; first-segment-end returns either
    length input or an index-of-from hit inside it
  • resolve-authority-ref's drop-bytes r 2 is reached only through
    (byte-starts-with? location "//")
  • the cookie jar's drop-bytes cd 1 is reached only through
    (byte-starts-with? cd ".")

Tests

bash test/run.sh: 89 passed, 0 failed (82 on main, 7 new). Two new
endpoints in test/server.py write raw bytes so the exact header encoding
reaches the client.

Teeth, with the new tests in place and both source files reverted to main. Of
the seven new assertions, three fail on main and four are regression guards that
already held:

  • a multi-byte UTF-8 header value does not eat the front of the body — fails,
    got ELLO-BODY
  • a header value that is not valid UTF-8 leaves the body intact — fails, got
    15 bytes of mojibake
  • a Domain that is not valid UTF-8 does not matchaborts the whole run
    with Array_unsafe_nth__Char: Assertion 'n < a.len' failed, so the suite
    never reaches the assertions after it

Because that abort masks the rest, the five cookie assertions were also measured
one per process against verbatim copies of main's domain-matches? and
path-matches?: the plain-domain control matches, Domain=\x80 aborts, and
Domain=.\x80\x80\x80, Path=\x80 and Path=/\x80\x80\x80/ already answered
false. Those last three are guards, not evidence.

The redirect resolver has no new end-to-end test: every input that would
distinguish it has to travel through URI.parse, which aborts first on the
pinned uri. It is covered by the existing redirect tests for the ASCII case.


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

Core's index-of, index-of-string, index-of-from, char-at and length are
byte-based while prefix, suffix and slice count characters, and starts-with? /
ends-with? guard on byte length and then slice characters. Every one of those
pairings in this package sits on a string the server chose.

Measured on unmodified main, end to end through Client.get against a server
writing raw response bytes, one case per process:

  Server: caf<c3><a9>       body HELLO-BODY arrived as ELLO-BODY
  X-Weird: <80 x5>          body arrived as 15 bytes of mojibake
  Set-Cookie Domain=<80>    SIGABRT in CookieJar.domain-matches?

Isolating the header/body split: a 32-byte response with four continuation
bytes in a header value made String.prefix return 44 bytes and String.suffix
return 0, because chars/from-chars does not round-trip bytes that are not valid
UTF-8; one more continuation byte pushed the byte offset past the character
count, which aborts.

The RFC 3986 redirect resolver has the same shape throughout, on the Location
header: every index it derives comes from a byte-based primitive and every span
it cut counted characters. last-index-of-char already returned a byte index by
construction and keeps that contract.

byte-slice is an unchecked memcpy, so each new call is dominated by a guard
that proves its offsets: index-of results are tested for -1 before use,
last-index-of-char scans 0..<length, split-pos + 4 is inside the accumulator
because the needle matched there, and each drop-bytes sits under the
byte-starts-with? that just matched those bytes. The two helpers slice only
after their own short-circuiting length comparison.

Redirects whose Location is not ASCII still abort, inside URI.parse in the uri
that http@0.3.0 carries. That is fixed upstream but not in a release this
package can pin, which is also why the byte helpers here are local rather than
http's String.byte-starts-with?.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

Checked out claude/byte-safe-slicing at 4d275b1 and ran the real suite locally on armhf:
bash test/run.sh (two local Python origins) → 89 passed, 0 failed, run.sh exit 0. CI is green
on both OSes and the ubuntu raw log confirms Passed: 89 Failed: 0. Worth stating explicitly since
it has bitten this org before: this repo's Run tests step is not continue-on-error any more —
c707fd6 re-gated it — so the green check really is the tests.

Merge-base is 1a49948, which is current origin/main, so no stale-branch drift.

Two pre-existing No 'prn' function found with type (Fn [TlsStream] String) warnings still print on
every build. Unrelated to this PR; already on the standing list.

Note this is still a draft, so it cannot merge as-is regardless of verdict.

Findings

The guards hold — I checked every one, not just the summary

String.byte-slice is memcpy(ptr, *s + a, b - a) with no bounds check (core/carp_string.h:378),
so the PR body's per-call-site proofs are the load-bearing part of this change. I walked all nine
drop-bytes sites and every byte-slice site against them and they hold. byte-starts-with? and
byte-ends-with? mirror core's starts-with?/ends-with? exactly (core/String.carp:136,141) with
byte-slice swapped for prefix/suffix, so they are identical for ASCII and correct beyond it,
and their and guards short-circuit (core/Macros.carp:118). last-index-of-char does keep its
byte-index contract — it scans 0 ..< String.length with char-at, both byte-based. header-end = split-pos + 4 is in range because index-of-string is strstr. ascii-to-lower is
to-bytes/from-bytes, so the cookie jar's normalisation was already byte-safe and does not undo
the fix.

The sweep is complete: on this branch there is no remaining String.prefix, suffix, slice,
starts-with? or ends-with?
anywhere in http-client.carp, src/cookie-jar.carp or
src/multipart.carp. The only hit is a comment.

Mutation battery: both coverage claims are exactly true

A — reverted all of http-client.carp to main, keeping only the three helpers so cookie-jar
still builds:
87 passed, 2 failed. The two failures are precisely the two new header tests. Nothing
else moved.
So the entire RFC 3986 resolver — all eleven functions — can regress to character
slicing without a single test noticing. The PR says so ("The redirect resolver has no new end-to-end
test"); this confirms it is exactly, not approximately, true.

B — reverted only src/cookie-jar.carp: exit 134, Array_unsafe_nth__Char: Assertion 'n < a.len' failed, aborting partway through the earlier cookie tests and taking the rest of the run
with it. That is the abort the PR describes, and it is why the five cookie assertions had to be
measured one per process. Confirmed.

The resolver fix is real — here is the evidence the PR is missing

Since no test can distinguish it, I measured it directly instead. I reopened defmodule Client in a
probe to reach the private remove-dot-segments, put a verbatim copy of main's char-sliced
resolver (old-remove-dot-segments, plus main's first-segment-end and remove-last-segment) in
the same binary, and ran one case per process so an abort on one side could not hide the other.

path branch (byte) main (char)
/a/b/../c /a/c /a/c
/a/./b /a/b /a/b
/x/../y/./z /y/z /y/z
/caf<c3><a9>/../b /b /caf<c3><a9>/b.. silently fails to remove the segment
/a/./caf<c3><a9> /a/caf<c3><a9> SIGABRT
/<80><80><80>/../b /b SIGABRT
/a/<80>/./c /a/<80>/c SIGABRT
../caf<c3><a9>/x caf<c3><a9>/x caf<c3><a9>/x

(<c3><a9> and <80> are literal bytes in the input, written as hex so nothing here is a raw byte.)

So the resolver change turns three aborts and one silently wrong URL into correct answers, and
leaves all four ASCII cases byte-identical. The A/B is not vacuous — row 4 differs without aborting,
so it can report a difference. This belongs in the PR body: it is the strongest evidence for the
largest part of the diff, and right now that part rests on argument alone.

It does not change the end-to-end story — URI.parse still aborts first on the pinned uri, so
none of this is reachable through Client.get until that pin moves — but it does move the resolver
from "unverified" to "verified correct, untestable in CI for now".

The cookie assertions do reach the code they claim to

Worth confirming rather than assuming, since four of the five expect 0 and would pass for free if
they short-circuited earlier. CookieJar.matching reaches domain-matches? only after the expiry,
secure and Maybe.Nothing-domain guards; the fixtures pass all three, and the positive control
(jar-with-domain "example.com" → 1) proves the whole path can yield a non-zero. So the zeros are
real zeros. Their value is the absence of the abort, which mutation B independently demonstrates.

1. Three unqualified names enter the global namespace

byte-starts-with?, byte-ends-with? and drop-bytes are the only top-level non-module defns
in http-client.carp — everything else lives in Connection, RequestConfig, ResponseStream or
Client, and every module-internal helper carries both (hidden x) and (private x). These carry
only (hidden x), which hides them from docs but not from consumers, so anyone who loads
http-client now gets three new unqualified globals. Given web already silently shadows three of
http's Response functions, this is the same class of hazard.

I am not asking you to move them here — where these live is the cross-repo call already sitting
on your list, and four private copies across web, http, uri and this repo is the actual
problem. Flagging it only so the choice is deliberate.

2. drop-bytes is the one helper that does not guard itself

(defn drop-bytes [s n] (String.byte-slice s n (String.length s)))

n > String.length s gives byte-slice a negative length, i.e. CARP_MALLOC(negative + 1) followed
by a memcpy of the same. Its two siblings guard themselves; this one relies entirely on its callers.
All nine in-repo callers are correctly guarded, so there is no live bug — but it is exported
unqualified (see above), so a library user can reach it. If you want it airtight for one line:

(defn drop-bytes [s n]
  (let [l (String.length s)] (String.byte-slice s (Int.min n l) l)))

Optional, and it costs nothing at the call sites that are already proven.

Verdict: merge

The change is correct, the guards are sound, the sweep is complete, and the suite is a genuine 89/0
locally as well as in CI. I could not break it: mutation confirms both of the PR's own coverage
claims verbatim, and the resolver — the one part no test covers — I verified directly, where it
fixes three aborts and one silently wrong URL with zero change to ASCII behaviour. The PR is unusually
honest about what it could not close, and that assessment stands up.

Two caveats that are yours, not defects: it is still a draft, and the three helpers land in the
global namespace. Adding the resolver A/B above to the PR body would be worth doing before it lands.

@hellerve
hellerve marked this pull request as ready for review August 14, 2026 23:26
@hellerve
hellerve merged commit 63b0559 into main Aug 14, 2026
2 checks passed
@hellerve
hellerve deleted the claude/byte-safe-slicing branch August 14, 2026 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant