Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
57 changes: 47 additions & 10 deletions test/web.carp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
9 changes: 7 additions & 2 deletions web.carp
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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)]]
Expand Down Expand Up @@ -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)
Expand Down