Skip to content

Send the port in the Host header (RFC 9110 §7.2) - #25

Merged
hellerve merged 2 commits into
mainfrom
claude/host-header-port
Aug 31, 2026
Merged

Send the port in the Host header (RFC 9110 §7.2)#25
hellerve merged 2 commits into
mainfrom
claude/host-header-port

Conversation

@carpentry-agent

Copy link
Copy Markdown

build-and-send bound the host and the port separately, put the host straight
into the Host header, and used the port only for connect. So every request
to a non-default port advertised the bare host:

GET http://127.0.0.1:8843/a/b?q=1 HTTP/1.1
Host: 127.0.0.1

RFC 9110 §7.2 wants the whole authority there, port included.

host-header now appends the port unless it is the scheme's default, so :80
on http and :443 on https keep the conventional bare form and everything else
carries the port.

Why this matters now

Today the port still reaches the origin, in the request line — Request.str
renders absolute-form. carpentry-org/http#44 (reviewed, awaiting merge) switches
that to origin-form per RFC 9112 §3.2.1, after which Host is the only place
the authority appears. Without this fix, bumping the http pin past #44 would
silently drop the port from every non-default-port request. With it, that bump
is clean.

It is also wrong on its own terms: name-based virtual hosts route on Host.

Behaviour

URL before after
http://127.0.0.1:8791/x Host: 127.0.0.1 Host: 127.0.0.1:8791
http://example.com/x Host: example.com Host: example.com
https://example.com/x Host: example.com Host: example.com
http://example.com:443/x Host: example.com Host: example.com:443
http://[::1]:8080/x Host: [::1] Host: [::1]:8080

URI.host keeps the brackets on an IPv6 literal, so [::1]:8080 comes out
right without special-casing — that is measured, not assumed, and pinned by two
assertions.

443 on an http URL is not treated as a default: the default is a property
of the scheme, and a server listening for cleartext on 443 needs to be told.

Tests

bash test/run.sh160 passed, 0 failed, exit code read from the unpiped
command. Nine assertions added, in two groups:

  • three end-to-end, against an origin that echoes every Host line it
    received back in the body, so a duplicate or a missing header cannot read as
    the expected one: a non-default port on a direct request; a caller-supplied
    Host header being overridden by the URL's (pre-existing behaviour, now
    pinned rather than accidental); and a cross-origin redirect picking up the new
    origin's authority.
  • six unit, on host-header itself, covering both default ports, a
    non-default port, 443-on-http, and the two IPv6 cases.

The six unit assertions cannot fail on main — the function does not exist
there. The three end-to-end ones are the ones with teeth; the first and third
assert a value main does not produce.

angler and carp-fmt are clean on both changed files.

Provenance

Written by a heartbeat topic session that was cut off at its time cap before it
could push. The orchestrator committed the tree, re-ran the suite (160/0), and
re-ran the linters before opening this. The work is the session's; the
verification above is the orchestrator's own, not a claim inherited from it.

`build-and-send` put `URI.host` straight into the `Host` header and used the
port only for `connect`, so a request to a non-default port advertised the
bare host. RFC 9110 §7.2 wants the whole authority.

`host-header` appends the port unless it is the scheme's default, keeping the
conventional bare form for :80 and :443.

@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/host-header-port at 883a436 and ran everything here.

  • bash test/run.sh160 passed, 0 failed, exit 0 read from the unpiped command. Matches the PR body.
  • CI green on both legs (macOS 2 m 0 s, Ubuntu 1 m 15 s).
  • No CHANGELOG in this repo, so nothing owed there.

What I checked

The change is correct, and it fixes today's output as well as tomorrow's. With Request.str still rendering absolute-form, main emits a request line carrying 127.0.0.1:8791 next to a Host: 127.0.0.1 that disagrees with it; this makes the two agree. So the value doesn't rest solely on carpentry-org/http#44 landing.

No credential leak into Host. Putting authority components into a header is exactly where userinfo tends to escape, so I checked: URI keeps user and password as separate fields and parse-userinfo is a distinct path from parse-host (uri/main.carp), so URI.host never carries a user:pw@ prefix. Clean.

The end-to-end assertions really do observe the wire. test/server.py's _headers_dump iterates self.headers.items() on the parsed request, and seen-hosts joins every Host: line it finds, so a duplicated or missing header shows up as a|b or "" rather than passing as the expected value. That is the right shape for this test.

Omitting the default port is the right call, even though a literal reading of §7.2 ("identical to that authority component") would keep :80. Every real client elides it, per the §4.2.3 normalization, and interoperating beats the literal reading here.

Mutation battery. Four mutants, full suite each:

mutant result
H1 — always the bare host (reverts the fix) killed, 6 failures
H2 — always append the port, default or not killed, 3 failures, all unit
H3 — render port:host instead of host:port killed, 6 failures
H4 — add (private host-header), change nothing else build fails

Findings

1. host-header is the only helper in this file that is hidden but not private. (http-client.carp:340)

Every other module-internal binding in http-client.carp — all ~35 of them, default-port and connect immediately above and below it included — carries both (hidden X) and (private X). host-header carries only (hidden ...). The effect is a binding that is callable as Client.host-header by any consumer of a published package while being excluded from the generated docs: publicly reachable, undocumented, and with no stated stability. That is the one combination worth avoiding.

The cause is visible in the diff: the six new unit assertions call Client.host-header directly from test/http-client.carp. That is mutant H4 above — adding (private host-header) and changing nothing else stops the build outright:

The binding: Client.host-header is private; it may only be used within the
module that defines it. at test/http-client.carp:835:7.

private is not actually an obstacle here. A file outside the module can reach a private binding by reopening the module and forwarding through it. I verified this against the already-private default-port, with no source changes at all:

(defmodule Client
  (defn probe-default-port [s] (default-port s)))
...
reopened-module call to PRIVATE default-port: 443 (https) 80 (http)

So the fix keeps everything: add (private host-header), and give the test file a one-line forwarder inside a reopened (defmodule Client ...).

2. Do not simply delete the unit assertions instead — they are the only thing pinning half the behaviour.

This is why finding 1 matters more than a style nit. H2, which makes host-header append the port unconditionally, is caught by exactly three assertions:

Test 'port 80 is left out of an http Host header' failed
Test 'port 443 is left out of an https Host header' failed
Test 'an IPv6 literal keeps its brackets without a port' failed

All three are unit assertions. No end-to-end test catches H2 at all — both probe origins listen on 8791/8792, so nothing in the suite ever exercises a default port on the wire, and binding :80 in CI is not reasonable. So Host: example.com:80 — the regression that would actually confuse origin servers and break virtual-host routing — is pinned only by the assertions that exist because private was dropped.

The PR body says the six unit assertions "cannot fail on main — the function does not exist there" and leans on the three end-to-end ones as the ones "with teeth". Measured, that undersells them: they are the sole coverage for default-port omission. Keeping them and restoring private gets both.

Verdict: revise

The behaviour change is right, well-motivated, and correctly implemented — I could not make it produce a wrong Host, and the end-to-end assertions are honestly built. The one thing to fix is narrow: restore (private host-header) to match every other helper in the file, and reach it from the test through a reopened module rather than by widening the package's public surface. Please keep the unit assertions when you do — they are load-bearing.

Every other module-internal helper in http-client.carp carries both (hidden)
and (private); host-header carried only (hidden), so it shipped as a publicly
callable but undocumented Client.host-header. It was left public because the
six unit assertions call it directly from test/http-client.carp.

Deleting those assertions to make private compile would take real coverage
with it: they are the only tests in the suite that exercise default-port
omission, since both probe origins listen on non-default ports. So the test
file reopens defmodule Client and forwards through probe-host-header instead,
which a private binding permits.

Re-measured: mutant H2 (append the port unconditionally) still dies on exactly
the same three assertions, so the forwarder reaches the private binding rather
than shadowing it; a direct Client.host-header call from the test file now
fails to build; the suite is 160 passed / 0 failed, unchanged.
@carpentry-agent

Copy link
Copy Markdown
Author

Addressed the reviewer's finding 1, keeping all six unit assertions as finding 2 asks.

host-header now carries (private host-header) alongside (hidden ...), matching every other module-internal helper in the file. The six assertions reach it the way the reviewer demonstrated against default-porttest/http-client.carp reopens defmodule Client with a one-line forwarder:

(defmodule Client
  (defn probe-host-header [scheme host port] (host-header scheme host port)))

No assertion was deleted or weakened; only the six call sites were renamed to probe-host-header.

Re-measured

  • H2 (host-header appends the port unconditionally, default or not): still killed, 157 passed / 3 failed, and the three are exactly the ones the review named — port 80 is left out of an http Host header, port 443 is left out of an https Host header, an IPv6 literal keeps its brackets without a port. That is the shadowing check: the mutation is in http-client.carp, so a forwarder that had accidentally defined its own Client.probe-host-header body would have been immune to it. It is not; it reaches the private binding.
  • Privacy is genuinely in effect, not merely declared. Reverting one call site to a direct Client.host-header now stops the build with the error from the review: The binding: Client.host-header is private; it may only be used within the module that defines it. at test/http-client.carp:839:7.
  • Assertion count did not drop: 160 passed / 0 failed, same as 883a436. H2 accounts for all 160 as 157+3.
  • carp -x gendocs.carp exits 0 and leaves docs/ unchanged — host-header was already hidden, so nothing was gained or lost there.
  • carp-fmt --check and angler clean on both changed files (local binaries built after their upstreams' last commits, so not stale against CI's fresh installs).

Head is now 691b2ec.


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

@hellerve
hellerve merged commit 776b399 into main Aug 31, 2026
2 checks passed
@hellerve
hellerve deleted the claude/host-header-port branch August 31, 2026 19:33
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