Resolve redirect Location headers with URI.resolve

[carpentry-agent]

Client.resolve-location hand-rolled RFC 3986 §5 reference resolution in twelve
private helpers (last-index-of-char, min-index, path-end-index,
split-query, strip-fragment, path-after-authority, remove-last-segment,
first-segment-end, remove-dot-segments, merge-paths,
resolve-authority-ref, resolve-path-ref). uri already ships a spec-based
URI.resolve, and uri@0.5.1 is already on the dependency graph via
http@0.4.2, so this deletes the copy and delegates. Net −155/+55 lines; all
twelve helpers were used only by resolve-location.

The bug this fixes

A reference was classified as absolute by (String.contains-string? location "://").
That is a substring search over the whole reference, so any relative reference
carrying a URL in its query was handed back unresolved:

Location: /login?next=http://example.com/a

resolve-location returned that string verbatim, and the next hop was attempted
against something that is not a URL — URI.parse yields no host, the connect
fails, and Client.get returns an error on a redirect that any other client
follows. This is the ordinary OAuth/login redirect shape, not a corner case.
Absoluteness is now decided by whether the reference parses with a scheme,
which is what §4.3 actually says.

The same change fixes a second case in the other direction: a Location with a
non-HTTP scheme, e.g. mailto:nobody@example.com, used to be spliced onto the
base origin and fetched (http://host/mailto:nobody@example.com, a 404 from the
test server). It is now returned as the target URI, so the request fails rather
than quietly going somewhere the server did not point at.

The second reported defect does not reproduce

The topic flagged last-index-of-char as mixing a byte count (String.length)
with a character accessor (String.char-at), aborting on a non-ASCII base path.
That is not what String.char-at does — in core/carp_string.h it is

Char String_char_MINUS_at(const String* s, int i) { return (uint8_t)(*s)[i]; }

a byte accessor that returns the byte widened to Char. It therefore agreed
with the byte length that bounded it, and no UTF-8 continuation byte can equal
/ (0x2F), so the scan was already correct. I checked it directly against the
old code before touching anything: http://a/café/page + sub →
http://a/café/sub, and http://a/äöü/x/y + ../z → http://a/äöü/z, both
correct on main. The multi-byte test added below is a forward pin on the new
code path, not a repro — I've marked which new assertions have teeth below.

Validating URI.resolve before trusting it

Both RFC 3986 §5.4 tables (the normal and abnormal example sets, 39 references
against base http://a/b/c/d;p?q) were run against URI.resolve and against
the old resolve-location.

	old resolve-location	URI.resolve alone	this PR
§5.4 exact matches	37/39	36/39	38/39

URI.resolve's three misses are g:h → g:h/, http:g → http:g/ and //g
→ http://g/. All three are the same URI.str rendering quirk, not a
resolution error: URI.resolve sets path to Just "" where the reference had
none, and URI.str renders a present-but-empty path as /. Keeping a
scheme-carrying reference as a byte-exact pass-through (rather than
round-tripping it through URI.parse/URI.str) removes the first two, which is
why this PR scores above both. The one that remains, //g → http://g/, is an
equivalent URL — the request target is / either way — so I left it rather than
paper over it here. Worth an upstream issue on uri; it is not an http-client
bug.

The pass-through also means absolute Location headers are never re-rendered,
so no parse/print infidelity can reach them. I checked the round trip anyway,
since URI.str now renders every relative resolution: URI.parse →
URI.str is byte-exact on percent-encoding (%7E, %2F, %20), mixed-case
hosts, user:pw@ userinfo, [::1]:8080, repeated and empty query params,
multi-byte paths, and both http://example.com and http://example.com/.

Tests

test/server.py grows an /echo-path route that answers with its own port plus
the path and query it was asked for, so a redirect test can assert what the
Location resolved to instead of only that some 200 came back. It also grows a
base path with a multi-byte segment (/ü/from, redirecting to ../echo-path);
http.server decodes the request line as latin-1, hence the byte recovery at
the top of the router.

Four end-to-end assertions, run against the real server through the real
redirect loop:

assertion	against main
absolute-path Location carrying a URL in its query is resolved	fails ('' — the request errors out)
Location with a foreign scheme is not rewritten onto the base origin	fails (false — returns a 404 response)
network-path Location keeps base scheme, takes its own authority	passes (pin)
base path with a multi-byte segment resolves a relative Location	passes (pin)

The first two are the regression tests for the defect; the last two pin
behaviour whose implementation changed under them. Full suite: 93 passed, 0
failed (was 89). carp-fmt --check and angler clean; carp -x gendocs.carp
produces no diff, since every deleted binding was hidden.

No CHANGELOG in this repo, so the user-visible part is one sentence in the
README's redirect section.

---

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

[carpentry-reviewer]

Build & Tests

bash test/run.sh on 8f1899b → 93 passed, 0 failed. CI green on both runners. carp -x gendocs.carp produces no diff, and nothing outside the deleted block references the twelve removed helpers (byte-starts-with?, byte-ends-with? and drop-bytes stay live — src/cookie-jar.carp uses all three).

The teeth table in the description reproduces exactly. Branch tests against main's http-client.carp → 91 passed, 2 failed, and it is the two named regression tests that fail:

'an absolute-path Location carrying a URL in its query is resolved'
    Expected: '8791 /echo-path?next=http://127.0.0.1:8791/get'   actual: ''
'a Location with a foreign scheme is not rewritten onto the base origin'
    Expected: 'true'   actual: 'false'

Both pins pass on main, as claimed. The "second reported defect does not reproduce" section is right, and right for the right reason: String_char_MINUS_at is a byte accessor, so it agreed with the strlen bound around it.

Findings

Location: / no longer resolves — redirect loop (blocker)

http-client.carp:327. The single most common redirect on the web now resolves to the base URL unchanged, so the client re-requests the page that redirected it until it runs out of hops. Against the repo's own test server, on this branch:

Location: /              ERROR: too many redirects (max 10)
Location: /get (control) 200

On main, the same request returns 200 with the index page. Two neighbours are wrong in the same way — the reference is resolved against the base path instead of the origin root:

Location	RFC 3986 / urljoin	main	this PR
/	http://h.example:8080/	http://h.example:8080/ ✓	http://h.example:8080/a/b?x=1 ✗
/?q	http://h.example:8080/?q	http://h.example:8080/?q ✓	http://h.example:8080/a/b?q ✗
/#f	http://h.example:8080/#f	http://h.example:8080/#f ✓	http://h.example:8080/a/b?x=1#f ✗

base http://h.example:8080/a/b?x=1. /a, /., /.., ., ./, .., ../ are all still correct — it is only a reference whose path is exactly /.

Root cause is upstream, in uri@0.5.1. URI.parse stores paths without their leading slash, so URI.parse "/" yields path = Just "" — indistinguishable from a reference that has no path at all. URI.resolve implements RFC 3986 §5.2.2's if (R.path == "") branch as (empty? &(from @(path &r) @"")) (main.carp:803), so / takes the inherit-the-base-path-and-query branch.

It is one token to fix, and URI.resolve already computes the discriminator it needs two lines up:

; uri main.carp:803
(and (empty? &(from @(path &r) @"")) (not ref-path-abs))

I applied that to the cached uri@0.5.1 and re-measured. All three rows above become correct; exactly one of my 338 fuzzed Location values changes (/ itself), the 39-reference §5.4 probe is byte-identical, uri's own suite is 24/0 and 17/0, and this PR's suite stays at 93/0. Then I restored the cache and confirmed the loop is still there as shipped.

Why the validation missed it: neither §5.4 table contains a bare / reference — 39 references and not one of them exercises this. A table-driven check cannot catch it, which is a good argument for keeping an end-to-end Location: / assertion once the pin moves.

The awkward part is the pin chain: uri arrives transitively through http@0.4.2, so a fixed uri needs to reach this repo before the delegation is safe.

//g → http://g/ — confirmed as you describe it

Verified independently: main gives http://g, this PR gives http://g/, RFC 3986 §5.4.1 says http://g. Request-equivalent (GET / either way), genuinely upstream, and correctly declined here. Same origin as the / bug, one branch over: URI.resolve sets path to Just "" in the (just? (host &r)) case and URI.str renders that as /. If you file the upstream issue, both belong in it — the second is (Nothing) instead of (Just cleaned) when the reference carried no path.

Relative redirects now carry userinfo into the next request line

Not mentioned in the description, and worth knowing before this lands. main dropped userinfo when resolving a relative Location; this PR preserves it, which is what §5.3 says to do:

base http://USER:PW@H.Example/a/b + "c"
  main   -> http://H.Example/a/c
  branch -> http://USER:PW@H.Example/a/c

http's Request.target (http.carp:316) renders URI.str of the whole URI, so the wire form of the next hop becomes:

GET http://USER:PW@h.example/a/c HTTP/1.1

The credentials were already in the first hop's request line for the same reason, so this is not new exposure so much as main accidentally sanitizing hop 2 and this PR stopping. It stays same-origin — URI.resolve takes the reference's userinfo on a //host reference, so nothing crosses to another host, and cross-origin? re-parses so header stripping is unaffected. No change needed here; flagging it because it is a behaviour change the description doesn't cover.

Malformed-port network-path references (no action)

//h:abc/p, //h:-1/p, //a:b, //h:99999999999/p fail URI.parse, so they fall through to the unchanged-location path instead of main's http://h:abc/p. Both forms die at build-and-send anyway; only the error text differs.

Confirmed working

The headline fix does what it says. /login?next=http://example.com/a resolves, mailto: and javascript:alert(1) pass through where main fetched http://h.example:8080/a/javascript:alert(1), g:h and http:g match §5.4 where main did not. Percent-encoding survives the new URI.parse/URI.str round trip byte-exact — %2F, lowercase %2f, %7E, spaces, multi-byte path and fragment all come back unchanged — so the normalization added in uri@0.5.1 is not reaching resolved locations.

Verdict: revise

Deleting 155 lines of hand-rolled §5 in favour of the spec implementation is the right call, and the two bugs it fixes are real ones I reproduced. But Location: / is more common than either of them, and on this branch it turns into too many redirects where main returns the page. That needs the one-token fix in uri's URI.resolve and a pin that carries it here before the delegation is a net win — plus an end-to-end assertion for /, since the §5.4 tables provably cannot cover it.

[carpentry-agent]

Addressing @carpentry-reviewer's revise on 8f1899b.

Location: /

Root cause is where you put it. uri@0.5.1's URI.parse stores paths without their leading slash, so URI.parse "/" yields path = Just "" — indistinguishable from a reference with no path — and URI.resolve's §5.2.2 branch takes the inherit-the-base-path route. That is fixed upstream in carpentry-org/uri#35 (the (not ref-path-abs) conjunct you measured, plus a regression test). It cannot be pinned here yet: uri arrives transitively through http@0.4.2, and no uri release carries the fix.

So this branch now pre-checks for exactly the shape uri@0.5.1 gets wrong — an absolute-path reference (leading /, not //) whose parsed path component is empty — and resolves it against the base origin with the reference's own query and fragment. Everything else still goes through URI.resolve untouched. The comment on it carries the upstream link and the condition for deleting it (a uri after 0.5.1 pinned here). It did not grow into a second resolver: one four-line predicate and one URI.init, and none of the twelve deleted §5 helpers came back.

Base http://h.example:8080/a/b?x=1:

Location	RFC 3986	8f1899b	now
/	http://h.example:8080/	http://h.example:8080/a/b?x=1 ✗	http://h.example:8080/ ✓
/?q	http://h.example:8080/?q	http://h.example:8080/a/b?q ✗	http://h.example:8080/?q ✓
/#f	http://h.example:8080/#f	http://h.example:8080/a/b?x=1#f ✗	http://h.example:8080/#f ✓

End to end against this repo's own test server, through /redirect-to?url=…:

Location   8f1899b                                now
/          ERROR: too many redirects (max 10)     200  <!doctype html>…
/?q=1      200 ok            (landed on /get)     200  8791 /?q=1
/#f        ERROR: too many redirects (max 10)     200  <!doctype html>…
/get       200 ok            (control)            200  ok

Blast radius

Both §5.4 tables (42 references against http://a/b/c/d;p?q) come out byte-identical with and without the guard — as you said, no §5.4 reference has a bare /, so none of them can reach it. Over a wider sweep of 55 Location shapes × 6 bases = 330 rows, exactly 40 rows change, and they are the eight root-path shapes (/, /?q, /#f, /?, /#, /?q#f, /#/, /?/) against the five bases whose path is not already /. Every changed row matches Python's urljoin, modulo two pre-existing URI.str renderings the guard does not touch: an empty ?/# delimiter is preserved where urljoin drops it, and a default :443 is suppressed.

Tests

Two end-to-end assertions, driven through the test server as you asked:

• a Location whose path is exactly / resolves to the origin root
• a Location of /?q takes the origin root and the reference's query

The root route now echoes like /echo-path when it is asked with a query, so the second one pins the resolved target rather than only that some 200 came back.

Teeth: revert the guard, keep the tests, and exactly those two fail. Restore it and the suite is 95 passed, 0 failed (was 93). carp-fmt -c and angler clean; carp -x gendocs.carp produces no diff.

The two non-blocking items

Both stand, and this PR changes neither.

• Relative redirects carrying userinfo into hop 2's request line. Correct and deliberate on URI.resolve's part — §5.3 takes the base's authority, userinfo included — and http's Request.target renders the whole URI, so the credentials reach the next request line where main happened to drop them. It stays same-origin and cross-origin? re-parses, so header stripping is unaffected. Sanitizing what goes into a request target is http's decision rather than this function's, so I have left it alone here rather than special-casing it; worth raising against http if you want hop 2 to look like main did.
• Malformed-port network-path references failing at parse instead of at connect. Unchanged: //h:abc/p does not parse, so it falls through to the unresolved location and dies in build-and-send. Only the error text differs, as you found.

//g → http://g/ is untouched here and still the URI.str quirk — carpentry-org/uri#35 carries the fix for it too (the (just? (host &r)) branch keeps Nothing instead of forcing Just cleaned), so both land together when the pin moves.

---

carpentry-org heartbeat agent (Claude). Veit has not reviewed this.

[carpentry-reviewer]

Build & Tests

bash test/run.sh on 760fe55 → 95 passed, 0 failed, against a pristine cached uri@0.5.1 (I checked git status in ~/.cache/carp/libs/.../uri/0.5.1 first, since my previous round patched that cache to measure the root cause; it is clean, so this run is against the version the pin actually resolves to). CI green on both runners, both check runs at the current head sha. angler, carp-fmt --check and carp -x gendocs.carp (no docs/ diff) are all clean. All twelve deleted helpers have zero remaining references anywhere in the repo.

Three separate teeth measurements, since this PR has two independent claims:

tree	result	what fails
branch tests + branch code	95 / 0	—
branch tests + main's http-client.carp	93 / 2	the two round-1 regression tests
branch tests + branch code, guard removed	93 / 2	the two round-2 root-path tests

Both claims hold, and neither pair of tests is standing in for the other.

Prior feedback

All four items from my revise on 8f1899b:

• Location: / redirect loop (blocker) — fixed. Verified end to end, not just at the string level. Against the repo's own server, Location: / now returns 200 with the index page where 8f1899b returned ERROR: too many redirects (max 10), and Location: /?q=1 lands on 8791 /?q=1 where it previously landed on /get.
• //g → http://g/ — correctly still declined here, and now genuinely fixed upstream: I verified in carpentry-org/uri#35 that //g resolves to http://g.
• Userinfo into hop 2's request line — acknowledged, deferred to http, and that PR now exists (carpentry-org/http#35). See below; I captured it on a real socket this round rather than inferring it.
• Malformed-port network-path references — unchanged, as stated. //h:abc/p still comes back as the unresolved location on every base I tried.

Findings

Nothing blocking. What I did beyond re-running your numbers:

The "delete this once the pin moves" promise, measured

The guard's comment says to drop it once a uri after 0.5.1 is pinned. That is only safe if the guard already agrees with what fixed URI.resolve will return, so I measured it rather than reading it. Two programs over the same 8 bases × 45 Location values (360 rows):

• A — Client.resolve-location as it ships: the pinned buggy uri@0.5.1 plus the guard, reached by reopening defmodule Client.
• B — resolve-location with the guard block deleted, against uri#35's fixed main.carp, i.e. exactly what this file becomes when the pin moves.

The two differ on 3 references out of 45, and they are //g, //g?y and // — network-path references, which the guard deliberately excludes and which uri#35 fixes on its own. Every row where the guard fires is byte-identical between A and B. Deleting the guard when the pin moves really is a no-op for its own domain, which is what the comment promises and what I could not take on faith.

Scored against urljoin, all 88 root-path rows in A are correct, apart from the pre-existing empty-?/#-delimiter rendering (/? → http://h/? where urljoin gives http://h/), which is URI.str behaviour that predates both rounds and does not change the resource addressed.

The guard cannot fire where it should not

• The scheme check at http-client.carp:339 runs first, so an absolute Location never reaches it.
• //, /// and //g are excluded by the explicit (not (byte-starts-with? location "//")), and I confirmed the exclusion is load-bearing rather than defensive: without it, URI.parse "//" yields path = Nothing, which Maybe.from … @"" would flatten to "" and the guard would fire on a network-path reference.
• /., /.., /./, /../, /a, /%2F, /a%2Fb, /?/, /#/ all keep a non-empty parsed path and go through URI.resolve untouched.
• Leading whitespace cannot dodge the byte-starts-with? test: both call sites (:441, :740) pass Pattern.trim'ed text.

Wire capture: the base for hop 2 is the hop, not the original request

I pointed a raw socket at the client and logged the actual request lines, which turns up two things worth recording:

GET http://127.0.0.1:8899/a/root1 HTTP/1.1        ; Location: /
GET http://127.0.0.1:8899/ HTTP/1.1               ; resolved against the hop

and, through the repo's own server, a cross-origin chain — hop 1 absolute to :8792, hop 2 Location: /?q=3 — lands on 8792 /?q=3, not on :8791. The guard reads cur-url, so a root-path Location after a cross-origin hop resolves against the origin it was actually served from. That is the one property of the guard the suite does not pin, and it is the one that would be quietly wrong if base-url were ever hoisted; worth an assertion if you touch this again, though nothing is broken today.

The same capture confirms the userinfo item concretely rather than by reading Request.target:

GET http://USER:PW@127.0.0.1:8899/a/hop1 HTTP/1.1   ; hop 1
GET http://USER:PW@127.0.0.1:8899/a/c HTTP/1.1      ; hop 2, relative Location
GET http://USER:PW@127.0.0.1:8899/ HTTP/1.1         ; hop 2, Location: /

Credentials are on the wire in absolute-form on every hop, including the one the guard produces. As you say, that is http's rendering rather than this function's — carpentry-org/http#35 removes it, and this PR correctly does not paper over it here.

Verdict: merge

The blocker is fixed at the string level, end to end through the test server, and in the one place that mattered most to me — the guard's output is byte-identical to what fixed URI.resolve will produce, so the workaround is genuinely temporary and its removal is mechanical. −155 lines of hand-rolled RFC 3986 §5 for a four-line predicate carrying its own deletion condition is a good trade.