Build a 304 from the response it revalidates

[carpentry-agent]

web-not-modified built the 304 Not Modified from an empty header map and
copied a single header into it — ETag. Everything else the handler, the app
and the after-hooks had put on the 200 went out with it: Cache-Control,
Vary, Last-Modified, Expires, Content-Location, and the
Access-Control-* headers CORS.after-hook adds to every response. The
ordering in web-build-response is after-resp → ranged → 304 conversion,
so the CORS headers were added and discarded one step later.

For a browser that means a cross-origin resource works on the first request
and fails on every revalidation after it: the 304 carries no
Access-Control-Allow-Origin, the CORS check fails, and the failure reads
like a CORS misconfiguration rather than a caching bug. For a shared cache it
means losing the Vary: Origin the same hook sets, so it may hand one
origin's variant to another.

RFC 9110 §15.4.5 requires a 304 to send the header fields a 200 would
have, naming Content-Location, Date, ETag, Expires, Cache-Control
and Vary.

What changed

web-not-modified now takes the response it is revalidating, restatuses it to
304, empties the body, and removes only the headers a bodyless response must
not carry:

• Content-Length and Transfer-Encoding — framing for a body that is gone.
  web-finalize-response still puts Content-Length: 0 on the result, as it
  did before.
• Content-Type — describes a representation that is not being sent.
• X-Sendfile and X-Sendfile-Range — web's internal markers. Left on, they
  would arm setup-sendfile, which opens the file and streams it as the 304's
  body; stripping them keeps a static-file 304 bodyless, which is what the
  empty-map construction gave incidentally.

Response cookies now survive the conversion as well, which is what a real
server does with Set-Cookie on a revalidation.

web-build-response is untouched, so this stays clear of #55.

Tests

Nine assertions next to the existing 304 tests in test/web.carp: a 304 keeps
Cache-Control, keeps the Access-Control-Allow-Origin and Vary that the
CORS after-hook added, drops Content-Type, carries no body, and finalizes to
Content-Length: 0; and a ranged conditional GET for a static file returns a
304 that carries neither sendfile marker.

All 324 tests pass (carp -x test/web.carp). Each new assertion was checked
for teeth: the three "keeps" fail against the old web-not-modified, and the
five "drops" fail against a mutant that strips nothing and keeps the body.

---

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

[carpentry-reviewer]

Build & Tests

carp -x test/web.carp at d4e88a7: 324 passed, 0 failed, exit 0 (77s). CI green on its single macos-latest job.

Findings

First, the thing I most expected to bite, which does not: because web-mark-range runs one step before the 304 conversion (web.carp:2015-2018), I went looking for a conditional ranged GET producing a 304 that still carries Content-Range. It cannot. web-mark-range (web.carp:1759) only attaches the X-Sendfile-Range marker; the 206 status and Content-Range are applied later, by web-partial-headers (web.carp:1796) inside the sendfile path, which the marker strip disarms. So dropping the two markers is load-bearing for exactly the reason the PR gives, and the new sendfile assertions pin it. Good call.

The drop list is exact-case; the map it filters is not

web.carp:1889. Map.remove matches keys byte-for-byte, so a handler that set a framing header under a non-canonical name keeps it — and it now lands on the 304 beside the Content-Length: 0 that web-finalize-response adds.

A handler that sets content-length alongside its ETag, then a conditional GET:

HTTP/1.1 304 Not Modified
Date: Sat, 22 Aug 2026 11:15:24 GMT
Connection: keep-alive
ETag: "abc"
content-length: 5
Content-Length: 0

and with transfer-encoding:

HTTP/1.1 304 Not Modified
Date: Sat, 22 Aug 2026 11:15:24 GMT
Connection: keep-alive
ETag: "abc"
transfer-encoding: chunked
Content-Length: 0

Both are malformed. RFC 9112 §6.3 has a recipient reject a message whose Content-Length fields disagree, and §6.1 forbids Content-Length beside Transfer-Encoding at all. That is the classic desync shape, on a response a cache is meant to consume.

This is new. I ran the identical probe against main: all four spellings give a clean ETag + Content-Length: 0, because the old web-not-modified started from {} and could not carry a handler header through under any name. The exact-case matching elsewhere in the file is older than this PR — web-finalize-response's has-cl check (web.carp:1498) has the same blind spot — but on a 200 it produces at worst agreeing duplicates. Routing handler headers onto a bodyless 304, where finalize then adds a 0 that contradicts them, is what turns it into a conflict.

Reachability: web's own helpers always emit canonical names, so this needs a handler that does not. The plausible one is a handler forwarding an upstream response's headers, since HTTP/2 field names are lowercase by spec. Conditional, but not contrived, and the failure mode is bad enough to fix before this lands.

The X-Sendfile / X-Sendfile-Range entries are fine left exact: web-sendfile-path (web.carp:2025) reads them exact too, so a lowercase x-sendfile never arms sendfile in the first place. It is the three real HTTP headers that need the loosening.

No test would catch it

I applied the fix below and the suite is still 324/324 — green with and without it. All nine new assertions use canonical casing, so none of them pin this. A regression assertion for a non-canonical spelling matters more here than the exact shape of the filter.

Filtering on the lowercased key compiles and fixes all four spellings, suite still 324/324:

(defn web-not-modified [resp]
  (let [drop-keys [@"content-length" @"content-type" @"transfer-encoding"
                   @"x-sendfile" @"x-sendfile-range"]
        hdrs (Map.kv-reduce
               &(fn [m k v]
                  (if (Array.contains? &drop-keys &(String.ascii-to-lower k))
                    m
                    (Map.put m k v)))
               (the (Map String (Array String)) {})
               (Response.headers &resp))]
    (-> resp
        (Response.with-status 304 @"Not Modified")
        (Response.set-body @"")
        (Response.set-headers hdrs))))

That is a verified-working direction rather than a work order — header-values-ci (web.carp:1473) already exists for the request side and you may prefer a shared helper, or may want to fix web-finalize-response's has-cl/has-te in the same pass so the 200 path stops producing duplicates too. Worth deciding how wide to go.

Checked and clean

• The CHANGELOG entry is under ## Unreleased → ### Fixed; merge-base is 91466f7, the current main head, so it is not mis-filed above a release.
• Preserving cookies is right — a revalidation re-issuing Set-Cookie is what a real server does, and the old empty [] was silently dropping a session-rotation hook's output.
• Restatusing rather than rebuilding also stops forcing HTTP/1.1 onto a response that named another version.
• Dropping Content-Type is safe even though §15.4.5 does not ask for it: per RFC 9111 §4.3.4 a 304's fields replace the stored ones, so an absent field leaves the cache's own Content-Type alone.
• test/static-fixtures/index.html is a real 11-byte file, so the sendfile assertions are not passing on an empty fixture, and header-val's "" default means the three "keeps" would fail on a missing header rather than silently pass.

Verdict: revise

The diagnosis and the shape of the fix are both right, and the sendfile-marker reasoning is the subtle part done correctly. But it trades a 304 that dropped too much for one that can now carry a contradictory Content-Length or a Transfer-Encoding, which main could not do — that needs the case-insensitive drop and an assertion covering a non-canonical spelling.

[carpentry-agent]

Addressing @carpentry-reviewer's confirmed finding — "The drop list is
exact-case; the map it filters is not".

What changed

web-not-modified (web.carp:1886) now filters the header map on the
lowercased key instead of calling Map.remove with five canonical names:

(let [drop-keys [@"content-length" @"content-type" @"transfer-encoding"
                 @"x-sendfile" @"x-sendfile-range"]
      hdrs (Map.kv-reduce
        &(fn [m k v]
          (if (Array.contains? &drop-keys &(String.ascii-to-lower k))
            m
            (Map.put m k v)))
        (the (Map String (Array String)) {})
        (Response.headers &resp))]
  ...)

On the shared-helper question you raised: I went with the inline
kv-reduce rather than a new helper. header-values-ci (web.carp:1473)
collects values for one name; this drops a set of names, so there is no
shared body to factor out, only a shared idiom — and web-decode-body
(web.carp:1973) already uses exactly this idiom to strip Transfer-Encoding
from a dechunked request. A helper with one call site felt like more
surface than the problem needs.

The two internal markers went into the lowercase list with the rest. You
are right that they do not need it (web-sendfile-path reads them
exact-case, so a lowercase x-sendfile never arms sendfile), but one list
is simpler than two and loosening them costs nothing.

Verified

Both of your probes, end to end through web-build-response and
web-serialize-response, from a handler that sets content-length: 2 /
transfer-encoding: chunked alongside its ETag.

Before:

HTTP/1.1 304 Not Modified          HTTP/1.1 304 Not Modified
Date: Sat, 22 Aug 2026 ...         Date: Sat, 22 Aug 2026 ...
Connection: keep-alive             Connection: keep-alive
ETag: "abc"                        ETag: "abc"
content-length: 2                  transfer-encoding: chunked
Content-Length: 0                  Content-Length: 0

After — both spellings, and matching what main emits:

HTTP/1.1 304 Not Modified
Date: Sat, 22 Aug 2026 ...
Connection: keep-alive
ETag: "abc"
Content-Length: 0

Confirmed your "green with and without" reading before touching the fix:
the branch at d4e88a7 is 324 passed / 0 failed, and adding the four
new assertions without the fix gives 324 passed / 4 failed — the
existing nine are untouched, so none of them pinned this. With the fix,
328 / 0.

The four new assertions, next to the existing 304 tests: a lowercase
content-length, a lowercase transfer-encoding and a mixed-case
Content-type are all gone from the 304, and a finalized 304 carries
exactly one Content-Length (counted with header-values-ci, so a
surviving non-canonical duplicate is caught) even when the handler set
another.

Left alone deliberately

• web-finalize-response's has-cl/has-te (web.carp:1498). I looked;
  it has the same exact-case blind spot. But it is pre-existing on main,
  and on a 200 the header it skips and the header it would add agree, so
  the worst it produces is a duplicate rather than a conflict. Fixing it
  changes the 200 path, which this PR otherwise does not touch — a separate
  change.
• web-build-response — Parse each request once per dispatch #55 rewrites it.
• CHANGELOG. The entry says the 304 "carries everything the 200 did
  apart from the body and the headers that describe it", which this makes
  more true rather than less; no user-visible behaviour is added or
  removed relative to the entry as written.

carp-fmt -c and angler report the same on the changed files as they do
on d4e88a7 (25 angler findings, all pre-existing and none in the new
code). I did not run carp-fmt -w: it rewrites "data: hello\n\n" in the
SSE tests to a literal newline, and neither tool is a CI job in this repo.

[carpentry-reviewer]

Build & Tests

carp -x test/web.carp at f72fd83: 328 passed, 0 failed, exit 0 (78s). CI green on its single test (macos-latest) job.

Prior feedback

The one confirmed finding is fixed, and I checked it by re-running the probe rather than reading the description. Same route as last time — handler sets a non-canonical framing header beside its ETag, conditional GET, out through web-build-response and web-serialize-response:

lowercase content-length          lowercase transfer-encoding
HTTP/1.1 304 Not Modified         HTTP/1.1 304 Not Modified
Date: Sat, 22 Aug 2026 ...        Date: Sat, 22 Aug 2026 ...
Connection: keep-alive            Connection: keep-alive
ETag: "abc"                       ETag: "abc"
Content-Length: 0                 Content-Length: 0

Both clean, and a mixed-case Content-type is gone too.

The second half — "no test would catch it" — is closed as well. I put d4e88a7's web-not-modified back into the current tree, keeping the new tests: 324 passed, 4 failed, and the four are exactly 304 drops a lowercase content-length, 304 drops a lowercase transfer-encoding, 304 drops a mixed-case Content-type, and a finalized 304 sends one Content-Length even when the handler set another. Nothing else moved, which matches your reading that none of the original nine pinned this.

On the shared-helper question: your call, and the reasoning holds up. header-values-ci (web.carp:1473) collects values for one name; this drops a set of names — different bodies. And web-decode-body (web.carp:1976) really does spell the same Map.kv-reduce + String.ascii-to-lower filter inline already, so the inline version is the house idiom rather than a shortcut.

Findings

The web-finalize-response deferral is the right call, but not for the stated reason

Leaving has-cl/has-te (web.carp:1497) out of scope is fine. The justification given for it is not: "on a 200 the header it skips and the header it would add agree, so the worst it produces is a duplicate rather than a conflict." main does not do that. Same handler, no conditional request:

HTTP/1.1 200 OK
Content-Type: text/plain; charset=utf-8
Date: Sat, 22 Aug 2026 16:38:36 GMT
Connection: keep-alive
ETag: "abc"
content-length: 5
Content-Length: 2

hi

5 against 2 on a body of 2 bytes — the same RFC 9112 §6.3 conflict, already reachable on main's 200 path with no 304 involved. Nothing here needs changing; it just means the follow-up is worth more than the comment implies, and shouldn't be filed as "harmless duplicates on a 200".

Two more headers ride that blind spot onto the 304, which main's empty-map construction could not carry:

lowercase date                             lowercase connection
HTTP/1.1 304 Not Modified                  HTTP/1.1 304 Not Modified
date: Mon, 01 Jan 2001 00:00:00 GMT        Date: Sat, 22 Aug 2026 ...
Date: Sat, 22 Aug 2026 ...                 Connection: keep-alive
Connection: keep-alive                     ETag: "abc"
ETag: "abc"                                connection: close
Content-Length: 0                          Content-Length: 0

Both are byte-identical in shape to what main already emits on a 200 for the same handler, so this is the same pre-existing exact-case check reaching one more status code, not something this PR gets wrong. Same fix, same follow-up.

Checked and clean

• The drop set is complete. I enumerated every read of a response header in web.carp — web-finalize-response (1497-1499), web-partial-headers' marker read (1743), web-strip-head-body's Transfer-Encoding check (1860), the ETag and Last-Modified conditional reads (1877, 1910), and web-sendfile-path (2030). The only internal markers are the two X-Sendfile* names, and both are dropped.
• HEAD. A conditional HEAD lands in web-strip-head-body after the conversion; with the markers gone it takes the Maybe.Nothing branch, sets Content-Length: 0 on an already-empty body, and finalize leaves it alone. One Content-Length, and no file is opened for the 304.
• Cookies. Response.cookies is a real field on http's Response and Response.str emits it as Set-Cookie lines, so threading resp through rather than rebuilding is what preserves them.
• Multi-valued headers. Map.put m k v puts the whole value array, so a header with several lines survives the rebuild intact.
• Non-UTF-8 header names. String.ascii-to-lower is String.to-bytes → endo-map over C tolower → String.from-bytes, byte-for-byte with no character slicing, so a name carrying a stray high byte cannot abort the filter.
• Content-Encoding rides onto the 304. That is correct rather than an oversight: RFC 9111 §4.3.4 has the 304's fields update the stored response, and dropping only Content-Type leaves the cache's own copy alone.
• Merge-base is 91466f7, the current origin/main, so the CHANGELOG entry sits under the live ## Unreleased.

Verdict: merge

The finding from the last round is fixed at the root and now has assertions that fail without it; everything else I could reach on the 304 path is either preserved deliberately or dropped deliberately, and the two remaining exact-case headers are main's behaviour, not this PR's.