From d4e88a729f1a120f6bead16d5494b9bbaf863216 Mon Sep 17 00:00:00 2001 From: "carpentry-heartbeat[bot]" Date: Sat, 22 Aug 2026 12:52:09 +0200 Subject: [PATCH 1/2] Build a 304 from the response it revalidates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit web-not-modified built the 304 from an empty header map and copied only ETag into it, so everything the handler, the app and the after-hooks had put on the 200 was discarded: Cache-Control, Vary, Last-Modified, Expires, Content-Location and the Access-Control-* headers CORS.after-hook adds. web-build-response runs after-resp, then ranged, then the 304 conversion, so the CORS headers were added and thrown away one step later. A browser revalidating a cross-origin resource got a 304 with no Access-Control-Allow-Origin and failed the CORS check; a shared cache lost the Vary: Origin telling it to key the entry by origin. 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. The 304 is now built from the response: restatused, body emptied, and stripped of Content-Length, Transfer-Encoding, Content-Type and web's internal X-Sendfile / X-Sendfile-Range markers. Leaving the markers on would arm setup-sendfile, which opens the file and streams it as the 304's body. web-finalize-response still sets Content-Length: 0. web-build-response is untouched, so this stays clear of #55. --- CHANGELOG.md | 9 +++++++ test/web.carp | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++ web.carp | 17 +++++++----- 3 files changed, 94 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55450b7..ed61147 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,15 @@ connection with 1002 or 1007. A well-formed close is answered with the client's own status code echoed back, as RFC 6455 §5.5.1 asks for, and a close carrying no payload is still answered with an empty close. +- **A `304 Not Modified` keeps the headers the `200` would have sent.** The + revalidation response was built from an empty header map holding nothing but + `ETag`, so `Cache-Control`, `Vary`, `Last-Modified`, `Expires` and the + `Access-Control-*` headers a hook had added were thrown away: a browser + revalidating a cross-origin resource got a 304 with no + `Access-Control-Allow-Origin` and failed the CORS check, and a shared cache + lost the `Vary: Origin` telling it to key the entry by origin. The 304 now + carries everything the `200` did apart from the body and the headers that + describe it. ## [0.9.3] diff --git a/test/web.carp b/test/web.carp index c9c2307..916595c 100644 --- a/test/web.carp +++ b/test/web.carp @@ -71,6 +71,31 @@ @(Array.unsafe-first &(Map.get-with-default (Response.headers resp) name &[@""]))) +; builds the 304 for a conditional GET, running `ah` as the after-hooks +(defn cached-304 [ah] + (let [app (App.GET (App.create) + @"/" + (fn [-r -p] + (-> (Response.text @"hi") + (Response.with-header @"ETag" @"\"abc\"") + (Response.with-header @"Cache-Control" + @"max-age=60")))) + bh (the (Array (Fn [&Request &(Map String String)] (Maybe Response))) []) + raw @"GET / HTTP/1.1\r\nHost: x\r\nOrigin: http://a.test\r\nIf-None-Match: \"abc\"\r\n\r\n" + pair (web-build-response &app &bh ah &(String.to-bytes &raw))] + @(Pair.a &pair))) + +; like `cached-304`, with the CORS after-hook configured for `origin` +(defn cors-304 [origin] + (let-do [ah (the + (Array (Fn [&Request &(Map String String) Response] Response)) + [CORS.after-hook])] + (CORS.configure origin) + (cached-304 &ah))) + +(defn no-hooks [] + (the (Array (Fn [&Request &(Map String String) Response] Response)) [])) + ; matches `raw` against a single Server-Sent Events route at `pattern` (defn sse-match [raw pattern] (web-try-sse &(String.to-bytes raw) @@ -1298,6 +1323,56 @@ (Map.contains? (Response.headers &resp) "ETag")) "304 response preserves ETag header") + ; -- 304 keeps the caching and negotiation headers the 200 carried -- + (assert-equal test + "max-age=60" + &(header-val &(cached-304 &(no-hooks)) "Cache-Control") + "304 keeps Cache-Control") + (assert-equal test + "http://a.test" + &(header-val &(cors-304 @"http://a.test") "Access-Control-Allow-Origin") + "304 keeps the CORS after-hook's Access-Control-Allow-Origin") + (assert-equal test + "Origin" + &(header-val &(cors-304 @"http://a.test") "Vary") + "304 keeps the CORS after-hook's Vary") + + ; -- 304 drops the representation and framing headers -- + (assert-false test + (Map.contains? (Response.headers &(cached-304 &(no-hooks))) "Content-Type") + "304 drops Content-Type") + (assert-true test + (String.empty? (Response.body &(cached-304 &(no-hooks)))) + "304 has no body") + (assert-equal test + "0" + &(header-val + &(web-finalize-response (cached-304 &(no-hooks)) true) + "Content-Length") + "a finalized 304 sends Content-Length: 0") + + ; -- a sendfile 304 leaves the transfer machinery disarmed -- + (assert-equal test + 304 + @(Response.code + &(static-resp + "GET /static/index.html HTTP/1.1\r\nRange: bytes=0-1\r\nIf-Modified-Since: Fri, 31 Dec 2100 23:59:59 GMT\r\n\r\n")) + "a ranged conditional GET for a static file returns 304") + (assert-false test + (Map.contains? + (Response.headers + &(static-resp + "GET /static/index.html HTTP/1.1\r\nRange: bytes=0-1\r\nIf-Modified-Since: Fri, 31 Dec 2100 23:59:59 GMT\r\n\r\n")) + "X-Sendfile") + "a 304 for a static file drops X-Sendfile") + (assert-false test + (Map.contains? + (Response.headers + &(static-resp + "GET /static/index.html HTTP/1.1\r\nRange: bytes=0-1\r\nIf-Modified-Since: Fri, 31 Dec 2100 23:59:59 GMT\r\n\r\n")) + "X-Sendfile-Range") + "a 304 for a static file drops the X-Sendfile-Range marker") + ; -- No 304 when ETags differ -- (assert-equal test 200 diff --git a/web.carp b/web.carp index 7d4fda2..ff2b8bd 100644 --- a/web.carp +++ b/web.carp @@ -1881,15 +1881,18 @@ stream and any proxy in front of it from timing out.") false (= &inm (Array.unsafe-first &vals)))))) -; Build a 304 Not Modified response, preserving the ETag header. +; Turn a response into a 304 keeping the headers RFC 9110 §15.4.5 requires. (hidden web-not-modified) (defn web-not-modified [resp] - (let [etag-vals (Map.get-with-default (Response.headers &resp) "ETag" &[])] - (if (= (Array.length &etag-vals) 0) - (Response.init 304 @"Not Modified" @"HTTP/1.1" [] {} @"") - (let [k @"ETag" - v [@(Array.unsafe-first &etag-vals)]] - (Response.init 304 @"Not Modified" @"HTTP/1.1" [] (Map.put {} &k &v) @""))))) + (let [drop-keys [@"Content-Length" @"Content-Type" @"Transfer-Encoding" + @"X-Sendfile" @"X-Sendfile-Range"] + hdrs (Array.reduce &(fn [m k] (Map.remove m k)) + @(Response.headers &resp) + &drop-keys)] + (-> resp + (Response.with-status 304 @"Not Modified") + (Response.set-body @"") + (Response.set-headers hdrs)))) ; True when the response's Last-Modified is not newer than If-Modified-Since. (hidden web-if-modified-since?) From f72fd836086e83fb926af02dfee7a1f184aa6436 Mon Sep 17 00:00:00 2001 From: "carpentry-heartbeat[bot]" Date: Sat, 22 Aug 2026 17:56:16 +0200 Subject: [PATCH 2/2] Match the 304 drop list case-insensitively HTTP field names are case-insensitive, but `Map.remove` matches keys byte-for-byte. A handler that set `content-length` or `transfer-encoding` under a non-canonical spelling kept it on the 304, and `web-finalize-response` then added `Content-Length: 0` beside it. RFC 9112 6.3 has a recipient reject a message whose `Content-Length` fields disagree, and 6.1 forbids `Content-Length` next to `Transfer-Encoding` at all. Filter the header map on the lowercased key instead, the same shape `web-decode-body` already uses to strip `Transfer-Encoding` from a dechunked request. The four assertions added next to the existing 304 tests all fail without the filter; the nine already there pass either way, so nothing pinned this. --- test/web.carp | 33 +++++++++++++++++++++++++++++++++ web.carp | 14 +++++++++----- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/test/web.carp b/test/web.carp index 916595c..f995a5f 100644 --- a/test/web.carp +++ b/test/web.carp @@ -96,6 +96,13 @@ (defn no-hooks [] (the (Array (Fn [&Request &(Map String String) Response] Response)) [])) +; the 304 built from a 200 that also carried `name: value` +(defn not-modified-with [name value] + (web-not-modified + (-> (Response.text @"hi") + (Response.with-header @"ETag" @"\"abc\"") + (Response.with-header name value)))) + ; matches `raw` against a single Server-Sent Events route at `pattern` (defn sse-match [raw pattern] (web-try-sse &(String.to-bytes raw) @@ -1351,6 +1358,32 @@ "Content-Length") "a finalized 304 sends Content-Length: 0") + ; -- the drop list matches header names case-insensitively -- + (assert-false test + (Map.contains? + (Response.headers &(not-modified-with @"content-length" @"5")) + "content-length") + "304 drops a lowercase content-length") + (assert-false test + (Map.contains? + (Response.headers &(not-modified-with @"transfer-encoding" @"chunked")) + "transfer-encoding") + "304 drops a lowercase transfer-encoding") + (assert-false test + (Map.contains? + (Response.headers &(not-modified-with @"Content-type" @"text/plain")) + "Content-type") + "304 drops a mixed-case Content-type") + (assert-equal test + 1 + (Array.length + &(header-values-ci + (Response.headers + &(web-finalize-response (not-modified-with @"content-length" @"5") + true)) + "Content-Length")) + "a finalized 304 sends one Content-Length even when the handler set another") + ; -- a sendfile 304 leaves the transfer machinery disarmed -- (assert-equal test 304 diff --git a/web.carp b/web.carp index ff2b8bd..112f04d 100644 --- a/web.carp +++ b/web.carp @@ -1884,11 +1884,15 @@ stream and any proxy in front of it from timing out.") ; Turn a response into a 304 keeping the headers RFC 9110 §15.4.5 requires. (hidden web-not-modified) (defn web-not-modified [resp] - (let [drop-keys [@"Content-Length" @"Content-Type" @"Transfer-Encoding" - @"X-Sendfile" @"X-Sendfile-Range"] - hdrs (Array.reduce &(fn [m k] (Map.remove m k)) - @(Response.headers &resp) - &drop-keys)] + (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 @"")