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)