From cbfb7bf5593e7451739ba1e5908d0fdb225e1927 Mon Sep 17 00:00:00 2001 From: "carpentry-heartbeat[bot]" Date: Mon, 24 Aug 2026 06:32:08 +0200 Subject: [PATCH] Frame a 304 the way it is defined, without a Content-Length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CHANGELOG.md | 8 ++++++++ test/web.carp | 57 ++++++++++++++++++++++++++++++++++++++++++--------- web.carp | 9 ++++++-- 3 files changed, 62 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0676fb..1ec9d25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,14 @@ its own for handlers that need an `id`, a `retry` time, or a comment. ### Fixed +- **A `304 Not Modified` no longer claims the cached resource is empty.** A + conditional `GET` or `HEAD` that hit the cache went out with + `Content-Length: 0`, so a client that trusted it learned the representation + it already held was zero bytes long. A `304`, a `204 No Content` and a `1xx` + now carry no `Content-Length` at all, as RFC 9110 §8.6 and §15.4.5 ask; the + connection is still reused, since those statuses end at the header block + rather than at a length. A `HEAD` that is answered normally still reports the + length its `GET` would have had. - **A conditional request is matched the way `If-None-Match` is defined.** The header used to be compared as one whole string against the response's `ETag`, so `If-None-Match: *` never matched, a client holding several cached variants diff --git a/test/web.carp b/test/web.carp index 9c2d352..584501e 100644 --- a/test/web.carp +++ b/test/web.carp @@ -126,6 +126,9 @@ (CORS.configure @"http://a.test") @(Pair.a &(web-build-response &app &bh &ah &(String.to-bytes &raw))))) +; `resp` as it goes on the wire, i.e. once its framing headers are filled in +(defn framed [resp] (web-finalize-response resp true)) + ; the 304 built from a 200 that also carried `name: value` (defn not-modified-with [name value] (web-not-modified @@ -1583,12 +1586,18 @@ (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) + (assert-false test + (Map.contains? + (Response.headers &(framed (cached-304 &(no-hooks)))) "Content-Length") - "a finalized 304 sends Content-Length: 0") + "a finalized 304 from a GET sends no Content-Length") + (assert-false test + (Map.contains? + (Response.headers + &(framed + (conditional-resp "HEAD" "\"abc\"" "" "If-None-Match: \"abc\"\r\n"))) + "Content-Length") + "a finalized 304 from a HEAD sends no Content-Length") ; -- the drop list matches header names case-insensitively -- (assert-false test @@ -1607,14 +1616,42 @@ "Content-type") "304 drops a mixed-case Content-type") (assert-equal test - 1 + 0 (Array.length &(header-values-ci - (Response.headers - &(web-finalize-response (not-modified-with @"content-length" @"5") - true)) + (Response.headers &(framed (not-modified-with @"content-length" @"5"))) "Content-Length")) - "a finalized 304 sends one Content-Length even when the handler set another") + "a finalized 304 sends no Content-Length even when the handler set one") + + ; -- 204 and 1xx cannot carry content either (RFC 9110 §8.6) -- + (assert-false test + (Map.contains? + (Response.headers + &(framed (Response.with-status (Response.text @"") 204 @"No Content"))) + "Content-Length") + "a 204 sends no Content-Length") + (assert-false test + (Map.contains? + (Response.headers + &(framed (Response.with-status (Response.text @"hi") 100 @"Continue"))) + "Content-Length") + "a 1xx sends no Content-Length") + + ; -- a status that can carry content is still framed -- + (assert-equal test + "2" + &(header-val + &(framed + (conditional-resp "HEAD" "\"abc\"" "" "If-None-Match: \"z\"\r\n")) + "Content-Length") + "a HEAD 200 keeps the length its GET would have had") + (assert-equal test + "0" + &(header-val + &(framed + (conditional-resp "PUT" "\"abc\"" "" "If-None-Match: \"abc\"\r\n")) + "Content-Length") + "a 412 frames the empty body it does carry") ; -- a sendfile 304 leaves the transfer machinery disarmed -- (assert-equal test diff --git a/web.carp b/web.carp index 1bf65c7..8edd918 100644 --- a/web.carp +++ b/web.carp @@ -1495,6 +1495,10 @@ stream and any proxy in front of it from timing out.") &(String.split-by &(String.ascii-to-lower c) &[\,]))) &conns))))) +; Statuses that cannot carry content (RFC 9110 §8.6, §15.4.5). +(hidden web-bodyless-status?) +(defn web-bodyless-status? [code] (or (< code 200) (= code 204) (= code 304))) + (defn web-finalize-response [resp keep-alive] (let [body-len (String.length (Response.body &resp)) has-te (Map.contains? (Response.headers &resp) "Transfer-Encoding") @@ -1508,7 +1512,7 @@ stream and any proxy in front of it from timing out.") (let [d-k @"Date" d-v [(web-http-date (Long.from-int (System.time)))]] (Map.put withconn &d-k &d-v)))) - h (if (or has-te has-cl) + h (if (or has-te has-cl (web-bodyless-status? @(Response.code &resp))) dated (let [cl-k @"Content-Length" cl-v [(Int.str body-len)]] @@ -1860,7 +1864,8 @@ stream and any proxy in front of it from timing out.") (web-head-partial (web-strip-range-marker resp) start end sz) (RangeResolution.Unsatisfiable) (web-range-not-satisfiable sz)))))) (Maybe.Nothing) - (if (Map.contains? (Response.headers &resp) "Transfer-Encoding") + (if (or (Map.contains? (Response.headers &resp) "Transfer-Encoding") + (web-bodyless-status? @(Response.code &resp))) (Response.set-body resp @"") (let [body-len (String.length (Response.body &resp)) hdrs @(Response.headers &resp)