Skip to content

Update uri, time and strbuf to their latest releases - #33

Merged
hellerve merged 3 commits into
masterfrom
claude/bump-uri-time-strbuf
Aug 15, 2026
Merged

Update uri, time and strbuf to their latest releases#33
hellerve merged 3 commits into
masterfrom
claude/bump-uri-time-strbuf

Conversation

@carpentry-agent

@carpentry-agent carpentry-agent Bot commented Aug 15, 2026

Copy link
Copy Markdown

Brings the three stale intra-org pins up to their newest released tags, checked against gh api repos/carpentry-org/<repo>/tags at the time of writing:

dep was now latest tag
uri 0.1.0 0.5.0 0.5.0
time 0.5.0 0.5.1 0.5.1
strbuf 0.1.0 0.2.1 0.2.1
base64.carp 0.2.0 0.2.0 0.2.0 (already current)

Tests: 385 passed / 0 failed → 392 passed / 0 failed, exit 0.

http has no CHANGELOG.md (verified — no changelog file exists in the repo), so this body is the release note. Everything below was measured by building master's http.carp and this branch's http.carp into the same driver, one case per process, reading each exit code directly rather than through a pipe.

The bump fixes a remotely triggerable process abort in Form.parse

uri 0.1.0's URI.unescape (main.carp:256-257) reads (unsafe-nth cs (inc i)) and (unsafe-nth cs (+ i 2)) on seeing a %, with no bounds check, so a % in the last two bytes walks off the array. uri 0.5.0 guards with (< (+ i 2) n) and hex-tests both nibbles before decoding.

Form.parse (http.carp:934) is http's documented entry point for application/x-www-form-urlencoded bodies, so this is reachable from a request body in any server built on this library — a form body ending in % kills the process.

call master this branch
Form.parse "k=b%4" Array_unsafe_nth__Char: Assertion 'n < a.len' failed, exit 134 k => "b%4"
Form.parse "k=b%" same abort, exit 134 k => "b%"
URI.unescape "a%" same abort, exit 134 "a%"

A real bug the bump exposed: empty request-target

uri 0.1.0's URI.str concatenated an unconditional @"/" before the path, so (URI.str (URI.zero)) was "/". uri 0.5.0 emits that slash only when there is a path that does not already start with one, so the same call now returns "" — and Request.str serialised GET HTTP/1.1, an empty request-target, which is not a valid request line and does not re-parse.

http had been getting its origin-form slash by accident, from a quirk of the URI serialiser. Request.target now guarantees it directly: RFC 9112 §3.2.1 says a client sending origin-form MUST send / as the path when the target URI's path component is empty, so http applies that itself rather than inheriting it.

The guard is structural (no string sniffing): the slash is prepended only when the URI has no path and no scheme, host, user or port — i.e. only for origin-form. Absolute-form targets are passed through untouched.

URI URI.str under 0.5.0 request-target
URI.zero "" /
query only (?a=1) ?a=1 /?a=1
/foo /foo /foo
http://example.com http://example.com http://example.com (see below)

Behaviour differences a consumer can see

1. The absolute-form request line loses a trailing slash

An earlier revision of this body recorded http://example.com as unchanged. That is true of URI.str in isolation; it is not true of what http puts on the wire, because http-client's build-and-send hands the whole parsed URI to Request.str (http-client.carp:273), so absolute-form is what actually goes out. Measured Request.str:

target master this branch
http://example.com GET http://example.com/ HTTP/1.1 GET http://example.com HTTP/1.1
http://example.com/ GET http://example.com/ HTTP/1.1 identical
/foo GET /foo HTTP/1.1 identical

This is deliberate, not a regression to fix: RFC 3986 §4.3 permits an empty path in an absolute-URI, RFC 9112 §3.2.2 requires servers to accept absolute-form, and the output re-parses cleanly. It is recorded here because it is wire-visible. Restoring the slash would belong inside the URI (empty path → / when an authority is present), never as a prefix on the serialised string — prepending there would produce /http://example.com.

2. Percent-encoding moved from codepoint-indexed to byte-indexed

uri 0.5.0's URI.escape does not merely switch hex case. 0.1.0 walked the string as codepoints and masked each one to its low byte ((bit-and i 240) / (bit-and i 15)), so every non-ASCII value came out wrong; 0.5.0 escapes the UTF-8 bytes. Form.encode output therefore changes for every non-ASCII key or value, and the old output was lossy — U+20AC encoded as %ac, indistinguishable from U+00AC.

input master this branch
Form.encode {k: "a+b"} k=a%2bb k=a%2Bb
Form.encode {k: "é"} k=%e9 k=%C3%A9
Form.encode {k: "€"} k=%ac k=%E2%82%AC
Form.encode {"näme": "v"} n%e4me=v n%C3%A4me=v

Decoding moved with it:

input master this branch
Form.parse "k=%C3%A9" é — bytes [195 131 194 169] é — bytes [195 169]
Form.parse "k=%zz" "", value silently truncated %zz kept verbatim
Form.parse (Form.encode {k: "€"}) ¬ — bytes [194 172] — bytes [226 130 172]

One nuance worth recording, because it makes the old breakage easy to under-test: master's Form.encode/Form.parse roundtrip does survive for U+0080–U+00FF, since %e9 decodes back to U+00E9. é therefore roundtrips on both sides and pins nothing. U+20AC is where the low-byte mask actually destroys information, which is why the new roundtrip test uses .

+ encoding as %2B rather than %2b (RFC 3986 §2.1 requires uppercase) is the smallest of these changes, not the only one. Anyone byte-comparing stored Form.encode output is affected by more than a case change; anyone storing non-ASCII form encodings was storing lossy ones.

Tests changed

Seven added, one updated. Nothing else in the suite moved.

test change reason
request-target of an empty path is / new pins the RFC 9112 §3.2.1 rule on Request.str output directly, not just via roundtrip
request-target of a query-only URI keeps its leading / new a query-only URI is the other shape where URI.str yields no leading slash
absolute-form request-target is left alone new pins that the guard does not over-trigger when there is an authority
encode a non-ASCII value one byte at a time new pins as %E2%82%AC; uri 0.1.0 gives k=%ac
encode/parse roundtrip a non-ASCII value new pins that the value survives; uri 0.1.0 gives ¬
parse a value ending in a truncated percent escape new k=b%4; aborts the process under uri 0.1.0
parse a value ending in a bare percent new k=b%; aborts the process under uri 0.1.0
encode literal + as %2b%2B updated uri 0.5.0 emits uppercase hex per RFC 3986 §2.1; only the expectation changed, Form.encode itself is untouched

request roundtrip preserves verb was the failing test that found the empty-target bug; it passes again unchanged.

The four new tests were checked for teeth, not assumed to have them. This branch's test/http.carp was built verbatim against master's http.carp (i.e. uri 0.1.0). The two encoding tests fail with exactly the values tabled above, and the run then dies at the truncated-escape test:

Test 'encode a non-ASCII value one byte at a time' failed:
	Expected value: 'k=%E2%82%AC', actual value: 'k=%ac'
Test 'encode/parse roundtrip a non-ASCII value' failed:
	Expected value: '€', actual value: '¬'
teeth: Array_unsafe_nth__Char: Assertion `n < a.len' failed.      ← exit 134

Both truncated-escape shapes abort independently (k=b%4 and k=b%, exit 134 each) — the suite run only reaches the first, so they were also measured one per process.

Verification

1. Pins are the newest released tags. Re-checked at run time via the tags API rather than trusting a written-down number. One caveat: uri 0.5.0 does not contain uri #33 (byte-consistent parsing). uri master is exactly 3 commits ahead of the 0.5.0 tag and those 3 commits are #33, so a further bump to 0.5.1 is still owed once that tag is cut.

2. The use-all leak (uri #32). uri 0.1.0 did (use-all Array Int Maybe Result) at top level, which in Carp leaks into every loading file; uri 0.5.0 scopes it inside defmodule URI. Confirmed the scoping change is in 0.5.0 and that http does not depend on the leak: outside doc strings, http.carp has no unqualified Just/Nothing/Success/Error and no unqualified Array/Int calls.

Worth recording, because it makes the leak harder to observe than expected: I probed for it and found that a Carp file with no load at all already resolves bare Just, unsafe-nth and unsafe-from-success. The reason is that core/Statistics.carp itself does top-level (use Int), (use Double), (use Array), which leaks org-wide from core. So uri's top-level use-all was largely shadowed by core's own leak, and removing it changes nothing observable for consumers of these four modules — the fix is still right as hygiene (it removes the ambiguity risk for a consumer that defines colliding names), but nobody downstream should expect a behaviour change from it.

3. strbuf 0.2.1's automatic cleanup — no double-free, no leak. 0.2.x adds (implements delete StringBuf.delete), so buffers are now freed automatically at scope exit. delete takes StringBuf by value, so http's 6 existing explicit (StringBuf.delete sb) calls consume the buffer and no second free is inserted; StringBuf.copy is a deep copy, so no aliasing either. All 6 StringBuf.create sites pair with a delete on every exit path, dechunk included.

ASan/LSan was run, not just reasoned about: carp -b test/http.carp, then the generated main.c hand-compiled with clang -fsanitize=address -g -O0 -I ~/carp-lang/core. All 388 tests of the previous revision pass under ASan with zero sanitizer errors (no double-free, no use-after-free, no overflow) and the only leak reported is an injected positive control — a deliberately orphaned 4321-byte allocation, which LSan did report, confirming leak detection was actually armed:

Direct leak of 4321 byte(s) in 1 object(s) allocated from:
    #1 0x923644 in asan_positive_control http_new.c:48242
SUMMARY: AddressSanitizer: 4321 byte(s) leaked in 1 allocation(s).

Zero leaks besides the control, so this is stronger than "no new leak" — the suite leaks nothing at all under the new strbuf. The four tests added since are plain Form string assertions and were not re-run under ASan.

4. Counts. Both sides measured, exit code read directly rather than through a pipe: master is 385 passed / 0 failed, exit 0; this branch is 392 passed / 0 failed, exit 0. carp-fmt -c and angler are clean on both changed files.

5. Blast radius. http-client pins http@0.3.0, so none of the above reaches it until a release is cut and the pin bumped.


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

uri 0.1.0 -> 0.5.0, time 0.5.0 -> 0.5.1, strbuf 0.1.0 -> 0.2.1.
base64.carp is already on its latest tag, 0.2.0.

Two behaviours change with the newer uri.

uri 0.1.0's URI.str prepended an unconditional slash, so a URI with no
path serialised as "/" and Request.str got a valid origin-form target by
accident. uri 0.5.0 emits the slash only for a path that needs one, so
the same URI serialises as "" and the request line becomes "GET  HTTP/1.1",
which is not a valid request line and does not re-parse. Request.target
now applies RFC 9112 3.2.1 -- an origin-form target whose path component
is empty is sent as "/" -- instead of relying on the URI serialiser.

uri 0.5.0's URI.escape emits uppercase percent-encoding as RFC 3986 2.1
requires, so Form.encode now yields %2B rather than %2b. The expectation
in the test is updated to match.

@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/bump-uri-time-strbuf at 4fde0c46. carp -x test/http.carp388 passed,
0 failed
, exit 0 (read unpiped). CI green on both OSes with head_sha = 4fde0c46…, so the green
is this commit. Base master, merge-base 31ca711 = current origin/master, no stale-branch drift.
No CHANGELOG.md in the repo — confirmed, so the PR body really is the only record this change gets.

The new guard is load-bearing, and the new tests are precisely aimed

I reverted http.carp:331 alone, back to &(URI.str (uri r)), keeping uri 0.5.0 — i.e. the exact
state the PR says the bump would have left:

GET  HTTP/1.1        ← wire output, empty request-target
reparse → ERROR "Malformed request: found first line 'GET  HTTP/1.1'"

and the suite goes 385 passed / 3 failed: the two new origin-form tests plus the pre-existing
request roundtrip preserves verb. The third new test (absolute-form request-target is left alone) still passes without the guard, which is right — absolute-form was never affected, so that
test pins the guard against over-triggering rather than padding the count. All five shapes I tried
re-parse cleanly on the branch.

Findings

1. The bump fixes a remotely triggerable abort in Form.parse, and the PR does not mention it

This is the most valuable thing in the change and it is missing from the body. Same driver, http
master vs this branch:

MASTER   Form.parse "k=b%4"   Array_unsafe_nth__Char: Assertion `n < a.len' failed  →  exit 134
BRANCH   Form.parse "k=b%4"   k => "b%4"
BRANCH   Form.parse "k=b%"    k => "b%"

The cause is in the dependency, at uri 0.1.0 main.carp:256-257: on seeing %, unescape reads
(unsafe-nth cs (inc i)) and (unsafe-nth cs (+ i 2)) with no bounds check at all, so a % in
the last two positions walks off the array. Isolated at the uri level, URI.unescape "a%" aborts
under 0.1.0 and returns "a%" under 0.5.0, which added the (< (+ i 2) n) check plus a hex test on
both nibbles.

Form.parse is http's documented entry point for application/x-www-form-urlencoded bodies
(http.carp:928) — that is attacker-controlled input in any server built on this library. A form
body ending in % kills the process. That turns this PR from dependency hygiene into a fix, and
it belongs in the body where a reader will see it.

2. "One user-visible output change" undercounts it substantially

URI.escape did not merely switch hex case — it moved from codepoint-indexed to byte-indexed,
so Form.encode changes for every non-ASCII value, and the old output was silently lossy:

input master branch
{k: "a+b"} k=a%2bb k=a%2Bb
{k: "é"} k=%e9 k=%C3%A9
{k: "€"} k=%ac k=%E2%82%AC
{"näme": "v"} n%e4me=v n%C3%A4me=v

Master masked each codepoint to its low byte, so U+20AC came out as %ac — the same encoding it
would produce for U+00AC, unrecoverable. Decoding moved too: Form.parse "k=%C3%A9" yielded
[195 131 194 169] (mojibake) on master and [195 169] (a correct é) here, and Form.parse "k=%zz" silently truncated the value to empty on master where the branch keeps %zz verbatim.

All of these are improvements. The point is that "a literal + now encodes as %2B" is the
smallest of them, and anyone with stored form encodings is affected by far more than a case change.

3. The http://example.com row says "unchanged", but the request line did change

The PR's table records that shape as unchanged. That is true of URI.str under uri 0.5.0; it is not
true of what http emits. Measured Request.str:

MASTER   GET http://example.com/ HTTP/1.1
BRANCH   GET http://example.com HTTP/1.1

The trailing slash is gone. This is wire-visible rather than internal, because http-client's
build-and-send hands the whole parsed URI straight to Request.str (http-client.carp:272), so
absolute-form is what actually goes out.

I do not think it is a bug: RFC 3986 §4.3 permits an empty path in an absolute-URI and RFC 9112
§3.2.2 requires servers to accept absolute-form, and the output re-parses fine. But a PR whose
stated method is "treat every behaviour difference as a finding" should record it as a difference
rather than as "unchanged". If you would rather keep the slash, the fix belongs inside the URI
(set an empty path to / when an authority is present), not as a prefix on the serialised string —
prepending would produce /http://example.com.

Worth noting alongside: http-client pins http@0.3.0, so none of §1–§3 reaches it until you cut a
release and bump.

4. The guard itself is right

Prepending only when path, scheme, host, user and port are all Nothing fires exactly on
origin-form, which is what RFC 9112 §3.2.1 is about. Query-only gives /?a=1; a relative path a
gives /a (uri supplies that slash itself, since the path is Just); absolute-form passes through.
The one degenerate shape, a host with no scheme, yields //example.com — not a valid request
target, but master produced example.com/ for the same input, equally invalid, so nothing regressed.

Pre-existing and untouched, but target is now the natural place for it: Request.str still puts a
fragment in the request-target (GET /#top HTTP/1.1), which RFC 9112 §3.2.1 forbids. Identical on
both sides, so not this PR's doing.

Verdict: revise

No code change needed — I could not break it. The bump is right, the guard is load-bearing and
correctly scoped, the tests fail for the right reasons when it is removed, 388/0 reproduces, and the
ASan reasoning holds up. What needs revising is the record: this repo has no changelog, so the PR
body is the release note, and it currently omits the crash fix (§1) and most of the encoding
change (§2) while describing as "unchanged" a request line that changed (§3). Fold those in and this
is ready.

uri 0.1.0's URI.unescape read one and two chars past the array on seeing
a %, so a form body ending in a truncated escape aborted the process:
Form.parse "k=b%4" and "k=b%" both exit 134 under the old pin. uri
0.5.0 bounds-checks and hex-tests both nibbles.

The same bump moved escaping from codepoint-indexed to byte-indexed, so
non-ASCII values now survive a Form.encode/Form.parse roundtrip; uri
0.1.0 masked each codepoint to its low byte and lost U+20AC as %ac.

All four fail under the old dependency: the two encoding tests as plain
mismatches, the two truncated-escape tests by killing the process.
@carpentry-agent

Copy link
Copy Markdown
Author

Worked all three review findings. Everything below was re-measured here rather than copied from the review — master's http.carp and this branch's http.carp built into the same driver, one case per process, exit codes read directly instead of through a pipe.

§1 — the Form.parse abort. Reproduced. Form.parse "k=b%4" and Form.parse "k=b%" both die with Array_unsafe_nth__Char: Assertion 'n < a.len' failed, exit 134, on master; they return k => "b%4" and k => "b%" here. Isolated at the dependency, URI.unescape "a%" aborts under uri 0.1.0 and returns "a%" under 0.5.0 — the missing bounds check is at uri 0.1.0 main.carp:256-257, confirmed by reading it. The body now leads with this, and cites Form.parse at http.carp:934 (the defn; :928 is form-unescape).

§2 — "one user-visible output change" undercounted. Confirmed and expanded, with one correction to how it is tested. Measured Form.encode: {k: "a+b"} k=a%2bbk=a%2Bb, {k: "é"} k=%e9k=%C3%A9, {k: "€"} k=%ack=%E2%82%AC, {"näme": "v"} n%e4me=vn%C3%A4me=v. Decoding too: Form.parse "k=%C3%A9" gives [195 131 194 169] on master and [195 169] here, and "k=%zz" truncates to empty on master where this branch keeps %zz.

The correction: master's roundtrip survives U+0080–U+00FF, because %e9 decodes back to U+00E9. So an é roundtrip test would pass on both sides and pin nothing. is where the low-byte mask actually destroys information — Form.parse (Form.encode {k: "€"}) gives ¬ ([194 172]) on master and ([226 130 172]) here — so that is what the new test uses.

§3 — the http://example.com row. Confirmed: Request.str emits GET http://example.com/ HTTP/1.1 on master and GET http://example.com HTTP/1.1 here. http://example.com/ and /foo are byte-identical on both. Recorded as a difference in its own section, not "fixed", with the RFC 3986 §4.3 / RFC 9112 §3.2.2 reasoning and the note that the fix would belong inside the URI if it were ever wanted. http-client.carp:273 is the Request.str call site (:272 is one line up).

New tests (6ed1f9e), four of them: encode a non-ASCII value one byte at a time, encode/parse roundtrip a non-ASCII value, parse a value ending in a truncated percent escape, parse a value ending in a bare percent. The existing three are untouched.

Teeth were proven, not assumed: this branch's test/http.carp built verbatim against master's http.carp fails the two encoding tests with exactly the values above and then aborts at the truncated-escape test (exit 134). Both crash shapes were also confirmed to abort independently one per process, since a suite run only ever reaches the first.

Counts, both measured, unpiped: master 385 passed / 0 failed exit 0; this branch 392 passed / 0 failed exit 0. carp-fmt -c and angler clean on test/http.carp.

No code changed — as the review said, there was nothing to fix. ASan was not re-run for the four new tests; they are plain Form string assertions and the body says so.

@hellerve hellerve left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Verified locally: 392/0 on the branch, 385/0 on master. Bumping the three pins alone on master fails exactly two tests, so Request.target is a required repair rather than scope creep. Reproduced both claimed defects on published 0.4.0 one case per process — Form.parse "k=b%4" aborts at exit 134, and encodes to k=%ac — both correct after the bump. Repinned to uri 0.5.1 and time 0.5.2, cut just now, so the release carries the byte-consistency fixes rather than trailing them; still 392/0.

@hellerve
hellerve merged commit 1d264fd into master Aug 15, 2026
2 checks passed
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