Skip to content

Frame a 304 the way it is defined, without a Content-Length - #63

Merged
hellerve merged 1 commit into
mainfrom
claude/no-content-length-on-bodyless
Aug 24, 2026
Merged

Frame a 304 the way it is defined, without a Content-Length#63
hellerve merged 1 commit into
mainfrom
claude/no-content-length-on-bodyless

Conversation

@carpentry-agent

Copy link
Copy Markdown
Contributor

The bug

web-bodyless deliberately drops content-length when it builds a 304 or a
412, but web-finalize-response runs afterwards, sees no Transfer-Encoding
and no Content-Length, and puts one back — computed from the body the same
function just emptied. So every conditional GET/HEAD that hits the cache
goes out with Content-Length: 0, and a client that trusts it learns the
representation it already holds is zero bytes long.

RFC 9110 §15.4.5 says a 304 is terminated by the first empty line after the
header fields and cannot carry content; §6.4.1 permits a Content-Length in a
304 only when it is the length the 200 would have had, which 0 is not
for any non-empty resource. §8.6 forbids it outright on 1xx and 204.

Verified against a real server on this branch's parent, and again on afa412c
(before the recent precondition work), so it is not a regression from that:

$ printf 'HEAD / HTTP/1.1\r\nHost: x\r\nIf-None-Match: "abc"\r\n\r\n' | ...
HTTP/1.1 304 Not Modified
Date: Mon, 24 Aug 2026 04:15:02 GMT
Connection: keep-alive
ETag: "abc"
Content-Length: 0        <- wrong; the resource is 11 bytes

The fix

A web-bodyless-status? predicate (1xx, 204, 304) that the framing logic
consults, rather than a special case at the 304 call site. Two places
synthesise a Content-Length from the body and both now skip it for those
statuses:

  • web-finalize-response, the general case.
  • web-strip-head-body, which runs after the 304 conversion for a HEAD
    and was re-adding Content-Length: 0 on its own. Fixing only the first would
    have left HEAD broken.

A 412 is left alone on purpose: it can carry content, so Content-Length: 0
on its empty body is accurate framing, not a lie.

Keep-alive

A 304 with neither Content-Length nor Transfer-Encoding is not read as
"body until close" — §15.4.5 is exactly the guarantee that it ends at the header
block. Confirmed on a live server: three requests (HEAD 304, GET 304, GET
200) answered on one socket, and curl --keepalive reuses the connection across
a 304 and a following 200 without hanging.

Tests

Two existing assertions pinned the old behaviour and had to change — they are
the bug, written down:

  • a finalized 304 sends Content-Length: 0 → now asserts there is none.
  • a finalized 304 sends one Content-Length even when the handler set another
    → now asserts zero, which still says what it meant: a stale
    content-length the handler set does not survive, and nothing replaces it.

New assertions cover a 304 from a GET, a 304 from a HEAD, a 204, a
1xx, and the two boundaries of the rule — a HEAD 200 still reports the
length its GET would have had, and a 412 still frames its empty body.

Every new assertion was mutation-checked against a full suite run:

mutation assertions killed
both call-site changes reverted the five no Content-Length assertions
predicate widened to every >= 300 a 412 frames the empty body it does carry
predicate forced true a HEAD 200 keeps the length its GET would have had (and the two pre-existing framing guards)

Checks

carp -x test/web.carp 367/0, carp -x test/websocket.carp 155/0,
carp -x gendocs.carp clean — the tree stays clean. angler reports nothing
new over main (the diff is line-number shifts only). test/smoke.sh was not
run locally: it shells out to carp -b and so cannot take an isolated output
directory on this machine, but nothing it asserts depends on a 304, a 204
or a 1xx carrying a length — its one Content-Length assertion is that an
SSE stream head has none.

carp-fmt -c was not applied: the committed web.carp and test/web.carp
already fail it here in ~180 places each, so the local binary disagrees with
whatever formatted the repo. The new code follows the surrounding committed
style instead.


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

web-bodyless drops content-length when it builds a 304 or a 412, but
web-finalize-response runs afterwards, sees neither Transfer-Encoding nor
Content-Length, and puts one back computed from the body it just emptied.
Every conditional GET/HEAD that hit the cache went out with
Content-Length: 0, telling the client the representation it already held
was zero bytes long. This predates the recent precondition work; it
reproduces identically on afa412c.

RFC 9110 §15.4.5 terminates a 304 at the first empty line after the header
fields, and §6.4.1 permits a Content-Length there only when it is the
length the 200 would have had. §8.6 forbids one outright on 1xx and 204.

The rule now lives with the framing logic as web-bodyless-status?, rather
than as a special case at the 304 call site. Both places that synthesise a
Content-Length from the body consult it: web-finalize-response, and
web-strip-head-body, which runs after the 304 conversion for a HEAD and
was re-adding the header on its own.

A 412 keeps its Content-Length: it can carry content, so 0 for an empty
body is accurate framing rather than a lie.

Two existing assertions pinned the old behaviour and now assert the
absence instead.

@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

At cbfb7bf on this armhf Pi, exit codes read from the unpiped commands:

  • carp -x test/web.carp367 passed, 0 failed
  • carp -x test/websocket.carp155 passed, 0 failed
  • carp -x gendocs.carp — leaves the working tree clean

CI green on the macOS leg, including test/smoke.sh. One bot commit on 9a65a35, origin/main's head, no merge commits. The CHANGELOG entry is filed under ## Unreleased### Fixed, and git tag --contains 9a65a35 is empty, so it is not sitting on the wrong side of a release. angler and carp-fmt --check report the same finding set on web.carp and test/web.carp as main does — line numbers shifted by the five added lines, nothing new — and this repo's CI runs neither.

Findings

I could not break the change. What follows is the verification, then one wording nit.

Verified on the wire, not from the test suite. I built a server off this branch and off 9a65a35, drove both with raw sockets, and read the exact bytes. Six requests, in order, down one keep-alive connection:

                        9a65a35 (main)              cbfb7bf (this branch)
GET  /e  If-None-Match  304  Content-Length: 0      304  no Content-Length
GET  /e  plain          200  Content-Length: 2      200  Content-Length: 2
HEAD /e  If-None-Match  304  Content-Length: 0      304  no Content-Length
GET  /nc (204)          204  Content-Length: 0      204  no Content-Length
GET  /e  If-None-Match  304  Content-Length: 0      304  no Content-Length
GET  /e  plain          200  Content-Length: 2      200  Content-Length: 2

All six answered on the same connection on both trees, so the CHANGELOG's "the connection is still reused" is measured rather than argued from the RFC. The 304 goes out as headers and a single terminating CRLF, no body. The HEAD 200 still reports the length its GET would have had; the 412 from a PUT with a failed precondition still carries Content-Length: 0, which is the boundary the body says is deliberate.

The third call site I went looking for is already closed, by accident of an older decision. web-serialize-response (web.carp:2114) overrides Content-Length from the file size after web-finalize-response runs, unconditionally and with no bodyless check — which would have put the header straight back on a sendfile 304 and armed a body transfer behind it. It cannot fire: web-bodyless's drop list includes x-sendfile and x-sendfile-range, so a converted 304 reaches web-serialize-response with file-size = -1. Confirmed live rather than by reading — a conditional GET against a real Response.sendfile route (ETag: "6ab" from mtime-size) answers 304 with ETag, Last-Modified, Date, Connection and no Content-Length, and no file bytes follow.

The 1xx arm is correct but currently inert. web-bodyless-status?'s (< code 200) cannot be reached from web's own paths: the WebSocket handshake at web.carp:2554 writes HTTP/1.1 101 Switching Protocols as a raw formatted string and never passes through web-finalize-response. It only applies to a handler that returns a 1xx itself. Worth knowing that the websocket suite staying at 155/0 is not evidence about this arm either way.

Putting the rule next to the framing logic is the right shape. web-strip-head-body was the case that proves it — it runs after the 304 conversion for a HEAD and re-added the header on its own, so a fix at the web-not-modified call site would have left HEAD, the case the bug shows up on first, still broken. Both synthesis sites now ask the same predicate.

Nit: "no Content-Length at all" is slightly stronger than the code

The CHANGELOG says a 304, a 204 and a 1xx "now carry no Content-Length at all". has-cl still short-circuits ahead of the new check, so web-finalize-response declines to add the header but does not remove one a handler set itself. Measured against the live server, a 204 handler that sets Content-Length: 5:

HTTP/1.1 204 No Content
Content-Type: text/plain; charset=utf-8
Date: ...
Connection: close
Content-Length: 5

For 304 and 412 this cannot happen — web-bodyless strips content-length case-insensitively first, which is exactly what the reworked assertion "a finalized 304 sends no Content-Length even when the handler set one" pins. So the residue is only a handler-authored 204 or 1xx, where RFC 9110 §8.6 is a MUST NOT.

I would just soften the CHANGELOG line, since the statuses web itself generates are all covered. If you would rather close it in code, note that a plain Map.remove of "Content-Length" would not be enough — the header map is case-sensitive, which is why web-bodyless has to lowercase-compare, and I have not measured that variant.

Not blocking: it is a pre-existing gap this PR narrows rather than one it opens.

Verdict: merge

The header the code had already tried to remove is now actually gone, on both paths that were putting it back, and the fix is verified at the byte level on a live server against main rather than only through the suite. The 412 boundary is deliberate and the mutation table makes it load-bearing. Only the CHANGELOG's "at all" overreaches, by one adjective, on statuses web does not generate itself.

@hellerve
hellerve merged commit 8c33035 into main Aug 24, 2026
1 check passed
@hellerve
hellerve deleted the claude/no-content-length-on-bodyless branch August 24, 2026 16:43
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