diff --git a/CHANGELOG.md b/CHANGELOG.md index 006ebe6..b0676fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,20 @@ its own for handlers that need an `id`, a `retry` time, or a comment. ### Fixed +- **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 + (`"a", "b"`) re-downloaded the full body every time, and a weak validator + (`W/"x"`) never matched its strong spelling. All three now match, as RFC 9110 + §13.1.2 asks for. A matching `If-None-Match` on a method other than `GET` or + `HEAD` is answered with `412 Precondition Failed` instead of a `304 Not + Modified`, and `If-Modified-Since` is ignored on those methods. A response + that is not a `2xx` ignores both headers, so a `404` stays a `404`. The + `412`, like the `304`, keeps every header the response had built up — a + `Set-Cookie` the handler issued, an `Access-Control-Allow-Origin` an + after-hook added — and drops only the ones that described the body it no + longer carries. A response `ETag` or `Last-Modified` is found whatever its + capitalisation, the way the request headers are already read. - **A file whose extension is uppercase gets its real content type.** A path ending in `.JPG`, `.PNG`, `.HTML` or any other spelling of a known extension that is not all lower case was served as `application/octet-stream`, so a diff --git a/test/web.carp b/test/web.carp index 5c55737..9c2d352 100644 --- a/test/web.carp +++ b/test/web.carp @@ -102,6 +102,30 @@ (defn no-hooks [] (the (Array (Fn [&Request &(Map String String) Response] Response)) [])) +; the response a `verb` request carrying `hdrs` gets from a route whose 200 +; sets an ETag, `X-Custom` and a cookie, behind the CORS after-hook +(defn cors-conditional [verb hdrs] + (let-do [app (App.route (App.create) + @verb + @"/" + (fn [-r -p] + (-> (Response.text @"hi") + (Response.with-header @"ETag" @"\"abc\"") + (Response.with-header @"X-Custom" @"yes") + (Response.set-simple-cookie @"sid" @"s1")))) + bh (the + (Array (Fn [&Request &(Map String String)] (Maybe Response))) + []) + ah (the + (Array (Fn [&Request &(Map String String) Response] Response)) + [CORS.after-hook]) + raw (fmt + "%s / HTTP/1.1\r\nHost: x\r\nOrigin: http://a.test\r\n%s\r\n" + verb + hdrs)] + (CORS.configure @"http://a.test") + @(Pair.a &(web-build-response &app &bh &ah &(String.to-bytes &raw))))) + ; the 304 built from a 200 that also carried `name: value` (defn not-modified-with [name value] (web-not-modified @@ -109,6 +133,41 @@ (Response.with-header @"ETag" @"\"abc\"") (Response.with-header name value)))) +; the response a `verb` request carrying the raw header lines `hdrs` gets from +; a route whose 200 has ETag `etag` and Last-Modified `lm`; either header is +; left off when its argument is empty +(defn conditional-resp [verb etag lm hdrs] + (let [e @etag + l @lm + app (App.route (App.create) + @verb + @"/" + (fn [-r -p] + (let [base (Response.text @"hi") + tagged (if (String.empty? &e) + base + (Response.with-header base @"ETag" @&e))] + (if (String.empty? &l) + tagged + (Response.with-header tagged @"Last-Modified" @&l))))) + bh (the (Array (Fn [&Request &(Map String String)] (Maybe Response))) []) + raw (fmt "%s / HTTP/1.1\r\nHost: x\r\n%s\r\n" verb hdrs) + pair (web-build-response &app &bh &(no-hooks) &(String.to-bytes &raw))] + @(Pair.a &pair))) + +(defn conditional-code [verb etag hdrs] + @(Response.code &(conditional-resp verb etag "" hdrs))) + +; the code for a GET whose If-None-Match is `inm` against ETag `etag` +(defn inm-code [inm etag] + (conditional-code "GET" etag &(fmt "If-None-Match: %s\r\n" inm))) + +; the code for a `verb` request carrying `hdrs` against a 200 whose ETag is +; "abc" and whose Last-Modified is older than every date used below +(defn dated-code [verb hdrs] + @(Response.code + &(conditional-resp verb "\"abc\"" "Sun, 01 Jan 2023 00:00:00 GMT" hdrs))) + ; matches `raw` against a single Server-Sent Events route at `pattern` (defn sse-match [raw pattern] (web-try-sse &(String.to-bytes raw) @@ -1310,6 +1369,159 @@ (web-etag-match? &req &resp)) "etag-match? false when ETags differ") + ; -- the response side of the lookup is case-insensitive too -- + (assert-true test + (let [req (parse-req + "GET / HTTP/1.1\r\nHost: x\r\nIf-None-Match: \"abc\"\r\n\r\n") + resp (Response.with-header (Response.text @"hi") @"etag" @"\"abc\"")] + (web-etag-match? &req &resp)) + "etag-match? finds a lower-case ETag on the response") + (assert-true test + (let [req (parse-req + "GET / HTTP/1.1\r\nHost: x\r\nIf-Modified-Since: Sun, 06 Nov 1994 08:49:37 GMT\r\n\r\n") + resp (Response.with-header (Response.text @"hi") + @"last-modified" + @"Sun, 06 Nov 1994 08:49:37 GMT")] + (web-if-modified-since? &req &resp)) + "if-modified-since? finds a lower-case Last-Modified on the response") + + ; -- an entity-tag list is split on commas outside the quoted tags -- + (assert-equal test + &[@"\"a\"" @"W/\"b\"" @"\"c,d\""] + &(web-parse-etag-list " \"a\" , W/\"b\",\"c,d\" ") + "the entity-tag list keeps quoted commas and drops the OWS") + (assert-equal test + &(the (Array String) []) + &(web-parse-etag-list "\"unterminated") + "an unterminated entity-tag yields no tags") + + ; -- If-None-Match: * matches any current representation (RFC 9110 §13.1.2) -- + (assert-equal test 304 (inm-code "*" "\"abc\"") "* matches a tagged 200") + (assert-equal test 304 (inm-code "*" "") "* matches a 200 with no ETag") + (assert-equal test + 404 + @(Response.code + &(static-resp "GET /static/nope.html HTTP/1.1\r\nIf-None-Match: *\r\n\r\n")) + "* on a request that answers 404 leaves the 404 alone") + + ; -- every member of the list is compared, not just the first -- + (assert-equal test + 304 + (inm-code "\"z\", \"abc\"" "\"abc\"") + "a list matching on its second tag returns 304") + (assert-equal test + 304 + (inm-code " \"z\" ,\t\"abc\" " "\"abc\"") + "OWS around the list members is ignored") + (assert-equal test + 200 + (inm-code "\"z\", \"y\"" "\"abc\"") + "a list matching nothing returns 200") + (assert-equal test + 304 + (inm-code "\"a,b\"" "\"a,b\"") + "a comma inside a quoted tag does not split it") + (assert-equal test + 200 + (inm-code "\"a,b\"" "\"a\"") + "a quoted comma is part of the tag, not a list separator") + (assert-equal test + 200 + (inm-code "" "\"abc\"") + "an empty If-None-Match matches nothing") + + ; -- If-None-Match uses the weak comparison function (RFC 9110 §13.1.2) -- + (assert-equal test + 304 + (inm-code "W/\"abc\"" "\"abc\"") + "a weak request tag matches a strong ETag") + (assert-equal test + 304 + (inm-code "\"abc\"" "W/\"abc\"") + "a strong request tag matches a weak ETag") + (assert-equal test + 304 + (inm-code "W/\"abc\"" "W/\"abc\"") + "two weak tags match") + (assert-equal test + 200 + (inm-code "W/\"abc\"" "\"abcd\"") + "the weak comparison still compares the whole opaque tag") + + ; -- 304 is a GET/HEAD outcome; other methods get 412 (RFC 9110 §13.2.2) -- + (assert-equal test + 304 + (conditional-code "HEAD" "\"abc\"" "If-None-Match: \"abc\"\r\n") + "a matching If-None-Match on HEAD returns 304") + (assert-equal test + 412 + (conditional-code "PUT" "\"abc\"" "If-None-Match: \"abc\"\r\n") + "a matching If-None-Match on PUT returns 412") + (assert-equal test + 412 + (conditional-code "DELETE" "\"abc\"" "If-None-Match: *\r\n") + "a matching If-None-Match on DELETE returns 412") + (assert-equal test + 200 + (conditional-code "PUT" "\"abc\"" "If-None-Match: \"z\"\r\n") + "a failing If-None-Match on PUT leaves the 200 alone") + (assert-true test + (String.empty? + (Response.body + &(conditional-resp "PUT" "\"abc\"" "" "If-None-Match: \"abc\"\r\n"))) + "a 412 has no body") + (assert-false test + (Map.contains? + (Response.headers + &(conditional-resp "PUT" "\"abc\"" "" "If-None-Match: \"abc\"\r\n")) + "Content-Type") + "a 412 drops Content-Type") + + ; -- a 412 keeps every header that was not describing the body -- + (assert-true test + (Map.contains? + (Response.headers + &(conditional-resp "PUT" "\"abc\"" "" "If-None-Match: \"abc\"\r\n")) + "ETag") + "a 412 carries the ETag its precondition was tested against") + (assert-equal test + "http://a.test" + &(header-val + &(cors-conditional "PUT" "If-None-Match: \"abc\"\r\n") + "Access-Control-Allow-Origin") + "a 412 keeps the CORS after-hook's Access-Control-Allow-Origin") + (assert-equal test + "yes" + &(header-val + &(cors-conditional "PUT" "If-None-Match: \"abc\"\r\n") + "X-Custom") + "a 412 keeps a header the handler set") + (assert-true test + (Map.contains? + (Response.headers &(cors-conditional "PUT" "If-None-Match: \"abc\"\r\n")) + "Set-Cookie") + "a 412 keeps a cookie the handler set") + + ; -- If-Modified-Since is GET/HEAD-only and yields to If-None-Match -- + (assert-equal test + 304 + (dated-code "GET" "If-Modified-Since: Fri, 31 Dec 2100 23:59:59 GMT\r\n") + "an If-Modified-Since later than Last-Modified returns 304") + (assert-equal test + 200 + (dated-code "GET" + "If-None-Match: \"z\"\r\nIf-Modified-Since: Fri, 31 Dec 2100 23:59:59 GMT\r\n") + "a failing If-None-Match wins over a matching If-Modified-Since") + (assert-equal test + 304 + (dated-code "GET" + "If-None-Match: \"abc\"\r\nIf-Modified-Since: Sun, 01 Jan 2023 00:00:00 GMT\r\n") + "a matching If-None-Match returns 304 whatever If-Modified-Since says") + (assert-equal test + 200 + (dated-code "PUT" "If-Modified-Since: Fri, 31 Dec 2100 23:59:59 GMT\r\n") + "If-Modified-Since is ignored on a method other than GET or HEAD") + ; -- 304 Not Modified on matching ETag -- (assert-equal test 304 diff --git a/web.carp b/web.carp index 9ffe203..1bf65c7 100644 --- a/web.carp +++ b/web.carp @@ -1870,23 +1870,77 @@ stream and any proxy in front of it from timing out.") (Response.set-body @"") (Response.set-headers (Map.put hdrs &cl-k &cl-v))))))) -; Check If-None-Match against a response's ETag header. -; Returns true if the request's If-None-Match matches the response's ETag. +; An entity-tag's opaque tag, weak marker removed (RFC 9110 §8.8.3.2). +(hidden web-etag-opaque) +(defn web-etag-opaque [tag] + (if (web-starts-with? tag "W/") + (String.byte-slice tag 2 (String.length tag)) + @tag)) + +; Split an If-None-Match value into entity-tags; a quoted tag may hold commas, +; so the scan is quote-aware and skips anything malformed. +(hidden web-parse-etag-list) +(defn web-parse-etag-list [s] + (let-do [len (String.length s) + out (the (Array String) []) + i 0] + (while (< i len) + (let [c (String.char-at s i)] + (if (or (= c \,) (or (= c \space) (= c \tab))) + (set! i (+ i 1)) + (let-do [start i] + (when (and (= c \W) + (and (< (+ i 1) len) (= (String.char-at s (+ i 1)) \/))) + (set! i (+ i 2))) + (if (and (< i len) (= (String.char-at s i) \")) + (let-do [j (+ i 1)] + (while (and (< j len) (/= (String.char-at s j) \")) + (set! j (+ j 1))) + (if (< j len) + (do + (Array.push-back! &out (String.byte-slice s start (+ j 1))) + (set! i (+ j 1))) + (set! i len))) + (while (and (< i len) (/= (String.char-at s i) \,)) + (set! i (+ i 1)))))))) + out)) + +; True when one entity-tag in `inm` weakly matches the opaque tag `tag`. +(hidden web-etag-list-match?) +(defn web-etag-list-match? [inm tag] + (let-do [tags (web-parse-etag-list inm) + found false] + (for [i 0 (Array.length &tags)] + (when (= tag &(web-etag-opaque (Array.unsafe-nth &tags i))) + (set! found true))) + found)) + +; True when If-None-Match matches the response's ETag under the weak +; comparison function, or is `*` (RFC 9110 §13.1.2). (hidden web-etag-match?) (defn web-etag-match? [req resp] - (match (Request.header req "If-None-Match") - (Maybe.Nothing) false - (Maybe.Just inm) - (match (Map.get-maybe (Response.headers resp) "ETag") - (Maybe.Nothing) false - (Maybe.Just vals) - (if (= (Array.length &vals) 0) - false - (= &inm (Array.unsafe-first &vals)))))) - -; 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 [inms (header-values-ci (Request.headers req) "If-None-Match")] + (cond + (Array.empty? &inms) false + (Array.any? &(fn [v] (= &(String.trim v) "*")) &inms) true + (let [vals (header-values-ci (Response.headers resp) "ETag")] + (if (Array.empty? &vals) + false + (let-do [tag (web-etag-opaque (Array.unsafe-first &vals)) + found false] + (for [i 0 (Array.length &inms)] + (when (web-etag-list-match? (Array.unsafe-nth &inms i) &tag) + (set! found true))) + found)))))) + +(hidden web-get-or-head?) +(defn web-get-or-head? [req] + (let [v (Request.verb req)] (or (= v "GET") (= v "HEAD")))) + +; Restatus with an empty body, dropping the headers that framed the old one +; plus the sendfile markers that would otherwise put a file back in its place. +(hidden web-bodyless) +(defn web-bodyless [resp code reason] (let [drop-keys [@"content-length" @"content-type" @"transfer-encoding" @"x-sendfile" @"x-sendfile-range"] hdrs (Map.kv-reduce @@ -1897,10 +1951,19 @@ stream and any proxy in front of it from timing out.") (the (Map String (Array String)) {}) (Response.headers &resp))] (-> resp - (Response.with-status 304 @"Not Modified") + (Response.with-status code reason) (Response.set-body @"") (Response.set-headers hdrs)))) +; Turn a response into a 304 keeping the headers RFC 9110 §15.4.5 requires. +(hidden web-not-modified) +(defn web-not-modified [resp] (web-bodyless resp 304 @"Not Modified")) + +; The 412 a failed precondition owes a non-GET/HEAD request (RFC 9110 §15.5.13). +(hidden web-precondition-failed) +(defn web-precondition-failed [resp] + (web-bodyless resp 412 @"Precondition Failed")) + ; True when the response's Last-Modified is not newer than If-Modified-Since. (hidden web-if-modified-since?) (defn web-if-modified-since? [req resp] @@ -1910,21 +1973,23 @@ stream and any proxy in front of it from timing out.") (match (web-parse-http-date &ims) (Maybe.Nothing) false (Maybe.Just since) - (match (Map.get-maybe (Response.headers resp) "Last-Modified") - (Maybe.Nothing) false - (Maybe.Just vals) - (if (= (Array.length &vals) 0) - false - (match (web-parse-http-date (Array.unsafe-first &vals)) - (Maybe.Nothing) false - (Maybe.Just mtime) (not (Long.< since mtime)))))))) - -; If-None-Match takes precedence over If-Modified-Since (RFC 9110 §13.1.3). + (let [vals (header-values-ci (Response.headers resp) "Last-Modified")] + (if (Array.empty? &vals) + false + (match (web-parse-http-date (Array.unsafe-first &vals)) + (Maybe.Nothing) false + (Maybe.Just mtime) (not (Long.< since mtime)))))))) + +; If-None-Match takes precedence over If-Modified-Since (RFC 9110 §13.1.3), +; which applies to GET and HEAD only; a non-2xx response ignores both (§13.2.1). (hidden web-conditional-not-modified?) (defn web-conditional-not-modified? [req resp] - (match (Request.header req "If-None-Match") - (Maybe.Just _) (web-etag-match? req resp) - (Maybe.Nothing) (web-if-modified-since? req resp))) + (let [code @(Response.code resp)] + (cond + (or (< code 200) (>= code 300)) false + (Array.empty? &(header-values-ci (Request.headers req) "If-None-Match")) + (and (web-get-or-head? req) (web-if-modified-since? req resp)) + (web-etag-match? req resp)))) ; Forward-declare App.max-header-line so top-level validation helpers can ; reference it before the main (defmodule App ...) block. @@ -2020,9 +2085,11 @@ stream and any proxy in front of it from timing out.") (~(Route.handler r) &req ¶ms)))) after-resp (web-run-after after-hooks &req ¶ms resp) ranged (web-mark-range &req after-resp) - ; conditional GET: 304 when If-None-Match/If-Modified-Since match + ; a failed precondition: 304 for GET/HEAD, 412 for anything else cond-resp (if (web-conditional-not-modified? &req &ranged) - (web-not-modified ranged) + (if (web-get-or-head? &req) + (web-not-modified ranged) + (web-precondition-failed ranged)) ranged) ; HEAD: strip body, preserve Content-Length final (if is-head (web-strip-head-body cond-resp) cond-resp)]