diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ddd295..d0ec82c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,24 @@ # Changelog +## Unreleased + +### Fixed +- **A conditional `PUT`, `POST`, `PATCH` or `DELETE` is no longer told it + failed after it succeeded.** web evaluates `If-None-Match` after the handler + has run, so for a method that changes state the write had already happened, + and the response's `ETag` described the new representation. A conditional + create (`PUT` with `If-None-Match: *`) wrote and was then answered with a + `412`. web now sends such a response as the handler built it and answers + only a `GET` or `HEAD` with a `304`. A handler that needs a precondition + evaluates it before it acts, with http's `Request.preconditions` against the + resource's current validators. +- **A resumed download of a changed file is no longer spliced.** `If-Range` was + ignored, so a `Range` request for a static file that had changed got a `206` + with bytes of the new file, which the client appended to the old one's. A + `Range` whose `If-Range` names another `ETag` or `Last-Modified` now gets + the whole file with a `200`, as RFC 9110 §13.1.5 requires. A weak `ETag` + never matches. + ## [0.12.0] ### Changed diff --git a/test/web.carp b/test/web.carp index 5a129f9..729a1e8 100644 --- a/test/web.carp +++ b/test/web.carp @@ -167,30 +167,6 @@ (defn no-hooks-before [] (the (Array (Fn [&Request &(Map String String)] (Maybe 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))))) - ; `resp` as it goes on the wire, i.e. once its framing headers are filled in (defn framed [resp] (web-finalize-response resp true)) @@ -1317,6 +1293,80 @@ "HEAD /static/index.html HTTP/1.1\r\nRange: bytes=0-4\r\n\r\n"))) "HEAD range request has no body") + ; -- a Range whose If-Range names another version is ignored (RFC 9110 §13.1.5) -- + (assert-equal test + 200 + @(Response.code + &(static-resp + "HEAD /static/index.html HTTP/1.1\r\nRange: bytes=0-4\r\nIf-Range: \"changed\"\r\n\r\n")) + "an If-Range naming another ETag serves the whole file") + (assert-false test + (Map.contains? + (Response.headers + &(static-resp + "GET /static/index.html HTTP/1.1\r\nRange: bytes=0-4\r\nIf-Range: \"changed\"\r\n\r\n")) + "X-Sendfile-Range") + "an If-Range naming another ETag leaves a GET's range marker off") + (assert-equal test + 206 + @(Response.code + &(static-resp + &(fmt + "HEAD /static/index.html HTTP/1.1\r\nRange: bytes=0-4\r\nIf-Range: %s\r\n\r\n" + &(header-val + &(static-resp "HEAD /static/index.html HTTP/1.1\r\n\r\n") + "ETag")))) + "an If-Range naming the current ETag serves the range") + (assert-true test + (Map.contains? + (Response.headers + &(static-resp + &(fmt + "GET /static/index.html HTTP/1.1\r\nRange: bytes=0-4\r\nIf-Range: %s\r\n\r\n" + &(header-val + &(static-resp "HEAD /static/index.html HTTP/1.1\r\n\r\n") + "ETag")))) + "X-Sendfile-Range") + "an If-Range naming the current ETag arms a GET's range marker") + (assert-equal test + 200 + @(Response.code + &(static-resp + &(fmt + "HEAD /static/index.html HTTP/1.1\r\nRange: bytes=0-4\r\nIf-Range: W/%s\r\n\r\n" + &(header-val + &(static-resp "HEAD /static/index.html HTTP/1.1\r\n\r\n") + "ETag")))) + "a weak If-Range never matches, so the whole file is served") + (assert-equal test + 206 + @(Response.code + &(static-resp + &(fmt + "HEAD /static/index.html HTTP/1.1\r\nRange: bytes=0-4\r\nIf-Range: %s\r\n\r\n" + &(header-val + &(static-resp "HEAD /static/index.html HTTP/1.1\r\n\r\n") + "Last-Modified")))) + "an If-Range naming the current Last-Modified serves the range") + (assert-equal test + 200 + @(Response.code + &(static-resp + "HEAD /static/index.html HTTP/1.1\r\nRange: bytes=0-4\r\nIf-Range: Sat, 01 Jan 2000 00:00:00 GMT\r\n\r\n")) + "an If-Range naming another date serves the whole file") + (assert-equal test + 200 + @(Response.code + &(static-resp + "HEAD /static/index.html HTTP/1.1\r\nIf-Range: \"changed\"\r\n\r\n")) + "an If-Range without a Range changes nothing") + (assert-equal test + 200 + @(Response.code + &(static-resp + "HEAD /static/index.html HTTP/1.1\r\nRange: bytes=0-4\r\nif-range: \"changed\"\r\n\r\n")) + "a lower-case if-range is read too") + ; -- HEAD with an unsatisfiable range answers 416 -- (assert-equal test 416 @@ -1561,59 +1611,37 @@ (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) -- + ; -- 304 is a GET/HEAD outcome; any other method's handler has already acted, + ; so its response goes out as the handler built it -- (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 + 200 (conditional-code "PUT" "\"abc\"" "If-None-Match: \"abc\"\r\n") - "a matching If-None-Match on PUT returns 412") + "a matching If-None-Match on PUT leaves the handler's response alone") + (assert-equal test + 200 + (conditional-code "PUT" "\"abc\"" "If-None-Match: *\r\n") + "a conditional create on PUT gets the handler's response, not a 412") (assert-equal test - 412 + 200 (conditional-code "DELETE" "\"abc\"" "If-None-Match: *\r\n") - "a matching If-None-Match on DELETE returns 412") + "a matching If-None-Match on DELETE leaves the handler's response alone") + (assert-equal test + 200 + (conditional-code "POST" "\"abc\"" "If-None-Match: \"abc\"\r\n") + "a matching If-None-Match on POST leaves the handler's response alone") (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") + "hi" + (Response.body + &(conditional-resp "PUT" "\"abc\"" "" "If-None-Match: \"abc\"\r\n")) + "a matching If-None-Match on PUT keeps the handler's body") ; -- If-Modified-Since is GET/HEAD-only and yields to If-None-Match -- (assert-equal test @@ -1781,13 +1809,6 @@ (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 3a8ac6f..bec9fa6 100644 --- a/web.carp +++ b/web.carp @@ -1928,6 +1928,29 @@ stream and any proxy in front of it from timing out.") (Map.remove @(Response.headers &resp) &@"X-Sendfile-Range"))) +; The first value of response header `name`, if any. +(hidden web-response-header) +(defn web-response-header [resp name] + (let [vals (header-values-ci (Response.headers resp) name)] + (if (Array.empty? &vals) + (Maybe.Nothing) + (Maybe.Just @(Array.unsafe-first &vals))))) + +; Whether a Range may be served: true unless the request's If-Range names a +; version other than the response's (RFC 9110 §13.1.5), in which case the +; whole file goes out instead. The validators are read only for an If-Range. +(hidden web-if-range-current?) +(defn web-if-range-current? [req resp] + (if (Maybe.nothing? &(Request.header req "If-Range")) + true + (let [etag (match (web-response-header resp "ETag") + (Maybe.Just v) (Result.to-maybe (ETag.parse &v)) + (Maybe.Nothing) (Maybe.Nothing)) + modified (match (web-response-header resp "Last-Modified") + (Maybe.Just v) (Result.to-maybe (HttpDate.parse &v)) + (Maybe.Nothing) (Maybe.Nothing))] + (Request.if-range-matches? req &etag &modified)))) + ; Attach the request's Range header to a sendfile response as an internal ; marker, resolved against the file size at transfer setup. (hidden web-mark-range) @@ -1938,9 +1961,12 @@ stream and any proxy in front of it from timing out.") (match (Request.header req "Range") (Maybe.Nothing) resp (Maybe.Just rv) - (let [k @"X-Sendfile-Range" - v [rv]] - (Response.set-headers resp (Map.put @(Response.headers &resp) &k &v)))))) + (if (web-if-range-current? req &resp) + (let [k @"X-Sendfile-Range" + v [rv]] + (Response.set-headers resp + (Map.put @(Response.headers &resp) &k &v))) + resp)))) ; Resolve the marker attached by web-mark-range against a known file size. (hidden web-resolve-marker) @@ -2132,11 +2158,6 @@ stream and any proxy in front of it from timing out.") (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] @@ -2153,15 +2174,19 @@ stream and any proxy in front of it from timing out.") (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). +; If-None-Match takes precedence over If-Modified-Since (RFC 9110 §13.1.3); a +; non-2xx response ignores both (§13.2.1). Only a GET or HEAD is answered with +; a 304: any other method's handler has acted by the time its response is seen, +; so the response goes out as the handler built it, and a handler that needs a +; precondition evaluates it before acting. (hidden web-conditional-not-modified?) (defn web-conditional-not-modified? [req resp] (let [code @(Response.code resp)] (cond + (not (web-get-or-head? req)) false (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-if-modified-since? req resp) (web-etag-match? req resp)))) ; Forward-declare App.max-header-line so top-level validation helpers can @@ -2255,11 +2280,8 @@ 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) - ; a failed precondition: 304 for GET/HEAD, 412 for anything else cond-resp (if (web-conditional-not-modified? &req &ranged) - (if (web-get-or-head? &req) - (web-not-modified ranged) - (web-precondition-failed ranged)) + (web-not-modified ranged) ranged) ; HEAD: strip body, preserve Content-Length final (if is-head (web-strip-head-body cond-resp) cond-resp)]