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..f995a5f 100644 --- a/test/web.carp +++ b/test/web.carp @@ -71,6 +71,38 @@ @(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)) [])) + +; 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) @@ -1298,6 +1330,82 @@ (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") + + ; -- 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 + @(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..112f04d 100644 --- a/web.carp +++ b/web.carp @@ -1881,15 +1881,22 @@ 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 (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)))) ; True when the response's Last-Modified is not newer than If-Modified-Since. (hidden web-if-modified-since?)