From 3241d119591b376de95d50da966c3d1ec76e84d1 Mon Sep 17 00:00:00 2001 From: Veit Heller Date: Tue, 8 Sep 2026 10:43:07 +0200 Subject: [PATCH] Decode a multipart upload from the request bytes, not from a String --- CHANGELOG.md | 17 +++++ README.md | 16 +++++ test/smoke-server.carp | 21 ++++++ test/smoke.sh | 39 +++++++---- test/web.carp | 110 ++++++++++++++++++++++++++++++- web.carp | 146 ++++++++++++++++++++++++++++++++++++----- 6 files changed, 320 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85b37e2..fbafed5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,24 @@ now rejected by `Form.decode-multipart` instead of decoding to a nameless part with an empty body, and the whole body fails with it. +### Added +- **`Form.decode-multipart-request-bytes` decodes a `multipart/form-data` body + as bytes**, yielding `BinaryPart`s whose bodies are `(Array Byte)`. It reads + the raw request bytes the server now keeps alongside the parsed request, so a + binary upload survives. Reach for it over `Form.decode-multipart-request` for + anything that can carry a file. + ### Fixed +- **A binary upload no longer decodes to zero parts.** The request buffer was + turned into a `String` before parsing, so a `multipart/form-data` body was + cut at its first NUL byte, taking the closing delimiter with it; the handler + was then handed a `Result.Success` carrying no parts and had nothing to + check. A 123-byte body measured 109 as a `String` and decoded to 0 parts + where the same body as text gave 1. +- **A chunked request body is dechunked as bytes.** The dechunker ran on the + request's `String` body, so a chunked binary upload was answered with a 400 + rather than decoded. Chunk framing is now walked over the buffer's bytes, + which is also the only chunk decoder left in the request path. - **An `If-Modified-Since` in either obsolete date format is understood.** A client sending `Sunday, 06-Nov-94 08:49:37 GMT` or `Sun Nov 6 08:49:37 1994` had its conditional request thrown away and got the whole body back instead diff --git a/README.md b/README.md index 4c7c642..1538dcc 100644 --- a/README.md +++ b/README.md @@ -156,6 +156,22 @@ The error handler receives the request, status code, and reason phrase. `Form.decode` decodes `application/x-www-form-urlencoded` bodies. Handles `+` as space and percent-encoding. +### File uploads + +```clojure +(defn handle-upload [req params] + (match (Form.decode-multipart-request-bytes req) + (Result.Success parts) + (Response.text (fmt "got %d parts" (Array.length &parts))) + (Result.Error e) (Response.bad-request))) +``` + +`Form.decode-multipart-request-bytes` decodes a `multipart/form-data` body into +`BinaryPart`s, whose bodies are `(Array Byte)`. Use it for anything that can +carry a file. `Form.decode-multipart-request` returns `FormPart`s instead, +whose bodies are `String`s and so end at their first NUL byte, which leaves a +binary upload truncated or with no parts at all. + ### Chunked responses ```clojure diff --git a/test/smoke-server.carp b/test/smoke-server.carp index 416b033..36e3ebf 100644 --- a/test/smoke-server.carp +++ b/test/smoke-server.carp @@ -50,6 +50,27 @@ (Map.get-with-default &(Form.decode-request r) "a" &@"none")))) + (App.POST @"/upload" + (fn [r p] + (match (Form.decode-multipart-request-bytes r) + (Result.Success parts) + (if (Array.empty? &parts) + (Response.text @"parts=0") + (let-do [b (BinaryPart.body (Array.unsafe-nth &parts + 0)) + sum 0] + (for [i 0 (Array.length b)] + (set! sum + (+ sum + (Byte.to-int @(Array.unsafe-nth b + i))))) + (Response.text + (fmt "parts=%d len=%d sum=%d" + (Array.length &parts) + (Array.length b) + sum)))) + (Result.Error e) + (Response.text (fmt "err=%s" &e))))) (App.POST @"/meta" (fn [r p] (Response.text diff --git a/test/smoke.sh b/test/smoke.sh index 818d568..3d0da44 100755 --- a/test/smoke.sh +++ b/test/smoke.sh @@ -66,13 +66,28 @@ curl -sf --max-time 30 -H 'Transfer-Encoding: chunked' \ || fail "chunked POST errored" cmp -s "$WORK/body" "$WORK/chunked" || fail "chunked body was not decoded intact" -# 4. normalization: the handler sees a plain body with a Content-Length +# 4. a binary multipart upload keeps every byte, NULs included. The body is +# A NUL B NUL C: 5 bytes summing to 198, which a String-based decode cuts to +# one byte and reports as zero parts. +check "binary multipart upload" +printf 'A\000B\000C' > "$WORK/bin" +UP=$(curl -sf --max-time 30 -F "f=@$WORK/bin" "$BASE/upload") +[ "$UP" = "parts=1 len=5 sum=198" ] || fail "binary multipart upload (got: $UP)" + +# 5. the same upload, chunked, goes through the byte-level dechunker +check "chunked binary multipart upload" +UPC=$(curl -sf --max-time 30 -H 'Transfer-Encoding: chunked' \ + -F "f=@$WORK/bin" "$BASE/upload") +[ "$UPC" = "parts=1 len=5 sum=198" ] \ + || fail "chunked binary multipart upload (got: $UPC)" + +# 6. normalization: the handler sees a plain body with a Content-Length check "normalization" META=$(curl -sf --max-time 10 -H 'Transfer-Encoding: chunked' \ --data-binary 'hello world' "$BASE/meta") [ "$META" = "plain 11" ] || fail "normalization (got: $META)" -# 5. the server answers Expect: 100-continue with an interim response +# 7. the server answers Expect: 100-continue with an interim response check "100-continue" curl -sf --max-time 30 -H 'Expect: 100-continue' \ --data-binary @"$WORK/body" -o /dev/null "$BASE/echo" -v 2> "$WORK/expect.log" \ @@ -80,44 +95,44 @@ curl -sf --max-time 30 -H 'Expect: 100-continue' \ grep -q 'HTTP/1.1 100 Continue' "$WORK/expect.log" \ || fail "no 100 Continue interim response" -# 6. keep-alive: two requests reuse one connection +# 8. keep-alive: two requests reuse one connection check "keep-alive" curl -sf --max-time 10 -o /dev/null -o /dev/null -v "$BASE/ok" "$BASE/ok" \ 2> "$WORK/ka.log" || fail "keep-alive requests errored" grep -qi 're-us' "$WORK/ka.log" || fail "connection was not reused" -# 7. malformed framing is rejected with a 400 +# 9. malformed framing is rejected with a 400 check "malformed framing" CODE=$(curl -s --max-time 10 -o /dev/null -w '%{http_code}' \ -H 'Transfer-Encoding: gzip' --data-binary 'x' "$BASE/echo") [ "$CODE" = "400" ] || fail "non-chunked Transfer-Encoding not rejected (got: $CODE)" -# 8. a byte range yields 206 Partial Content with the requested bytes +# 10. a byte range yields 206 Partial Content with the requested bytes check "byte range" RH=$(curl -s --max-time 10 -D - -o "$WORK/range" -r 0-4 "$BASE/static/index.html") echo "$RH" | grep -qi '206 Partial Content' || fail "range request not 206" echo "$RH" | grep -qi 'Content-Range: bytes 0-4/11' || fail "wrong Content-Range" [ "$(cat "$WORK/range")" = "root " ] || fail "range body wrong (got: $(cat "$WORK/range"))" -# 9. an open-ended range reassembles to the whole file +# 11. an open-ended range reassembles to the whole file check "open range" [ "$(curl -s --max-time 10 -r 0- "$BASE/static/index.html")" \ = "$(cat test/static-fixtures/index.html)" ] || fail "open range mismatch" -# 10. a range past the end is rejected with 416 +# 12. a range past the end is rejected with 416 check "unsatisfiable range" CODE=$(curl -s --max-time 10 -o /dev/null -w '%{http_code}' \ -r 100-200 "$BASE/static/index.html") [ "$CODE" = "416" ] || fail "unsatisfiable range not 416 (got: $CODE)" -# 11. a range-set is answered with its first satisfiable range +# 13. a range-set is answered with its first satisfiable range check "multi-range" RH=$(curl -s --max-time 10 -D - -o "$WORK/multi" \ -H 'Range: bytes=0-3, -2' "$BASE/static/index.html") echo "$RH" | grep -qi 'Content-Range: bytes 0-3/11' || fail "multi-range Content-Range" [ "$(cat "$WORK/multi")" = "root" ] || fail "multi-range body (got: $(cat "$WORK/multi"))" -# 12. headers whose byte length exceeds their character count are answered, +# 14. headers whose byte length exceeds their character count are answered, # not aborted on WIDE=$(printf '\303\244\303\244\303\244\303\244\303\244') check "non-ASCII Range" @@ -133,7 +148,7 @@ BODY=$(curl -s --max-time 10 -H "Content-Type: $WIDE$WIDE$WIDE$WIDE" \ check "server survived" [ "$(curl -sf --max-time 10 "$BASE/ok")" = "ok" ] || fail "server died on a non-ASCII header" -# 13. a Server-Sent Events stream opens, pushes on connect, and keeps ticking. +# 15. a Server-Sent Events stream opens, pushes on connect, and keeps ticking. # curl is cut off by --max-time because the stream never ends. check "SSE stream" curl -s --max-time 5 --no-buffer -H 'Last-Event-ID: 77' \ @@ -150,7 +165,7 @@ grep -q '^data: 77$' "$WORK/sse.body" || fail "Last-Event-ID did not reach the h TICKS=$(grep -c '^data: tick$' "$WORK/sse.body" || true) [ "${TICKS:-0}" -ge 2 ] || fail "stream stopped ticking (got $TICKS ticks)" -# 14. a tick that queues nothing sends a keep-alive comment instead +# 16. a tick that queues nothing sends a keep-alive comment instead check "SSE keep-alive comment" curl -s --max-time 5 --no-buffer -o "$WORK/quiet.body" "$BASE/quiet" || true grep -q '^data: hi$' "$WORK/quiet.body" || fail "no connect event on the quiet stream" @@ -160,7 +175,7 @@ KEEPS=$(grep -c '^:$' "$WORK/quiet.body" || true) check "server survived the stream" [ "$(curl -sf --max-time 10 "$BASE/ok")" = "ok" ] || fail "server died on an SSE stream" -# 15. the upgrade handshake and the frame loop, driven by a client that shares +# 17. the upgrade handshake and the frame loop, driven by a client that shares # no code with the server (python3 standard library only) check "WebSocket end to end" python3 test/ws-smoke.py "$PORT" || fail "WebSocket checks failed" diff --git a/test/web.carp b/test/web.carp index 371b89f..5a129f9 100644 --- a/test/web.carp +++ b/test/web.carp @@ -102,6 +102,71 @@ (defn no-hooks [] (the (Array (Fn [&Request &(Map String String) Response] Response)) [])) +(defn push-bytes! [out bs] + (for [i 0 (Array.length bs)] + (Array.push-back! out @(Array.unsafe-nth bs i)))) + +; a multipart body carrying one file part whose content is `payload` verbatim +(defn mp-body [payload] + (let-do [body (the (Array Byte) [])] + (push-bytes! + &body + &(String.to-bytes + "--bb\r\nContent-Disposition: form-data; name=\"f\"; filename=\"x.bin\"\r\nContent-Type: application/octet-stream\r\n\r\n")) + (push-bytes! &body payload) + (push-bytes! &body &(String.to-bytes "\r\n--bb--\r\n")) + body)) + +; that body framed with Content-Length +(defn mp-request [payload] + (let-do [body (mp-body payload) + buf (String.to-bytes + &(fmt + "POST /up HTTP/1.1\r\nHost: x\r\nContent-Type: multipart/form-data; boundary=bb\r\nContent-Length: %d\r\n\r\n" + (Array.length &body)))] + (push-bytes! &buf &body) + buf)) + +; and framed with chunked transfer-encoding, in two chunks +(defn mp-request-chunked [payload] + (let-do [body (mp-body payload) + n (Array.length &body) + half (/ n 2) + buf (String.to-bytes + "POST /up HTTP/1.1\r\nHost: x\r\nContent-Type: multipart/form-data; boundary=bb\r\nTransfer-Encoding: chunked\r\n\r\n")] + (push-bytes! &buf &(String.to-bytes &(fmt "%x\r\n" half))) + (push-bytes! &buf &(Array.slice &body 0 half)) + (push-bytes! &buf &(String.to-bytes "\r\n")) + (push-bytes! &buf &(String.to-bytes &(fmt "%x\r\n" (- n half)))) + (push-bytes! &buf &(Array.slice &body half n)) + (push-bytes! &buf &(String.to-bytes "\r\n0\r\n\r\n")) + buf)) + +; the body of the first part a byte-level decode yields, as a byte count and a +; checksum, so that a NUL in it cannot be lost in the comparison +(defn upload-report [req -params] + (match (Form.decode-multipart-request-bytes req) + (Result.Success parts) + (if (Array.empty? &parts) + (Response.text @"parts=0") + (let-do [b (BinaryPart.body (Array.unsafe-nth &parts 0)) + sum 0] + (for [i 0 (Array.length b)] + (set! sum (+ sum (Byte.to-int @(Array.unsafe-nth b i))))) + (Response.text + (fmt "parts=%d len=%d sum=%d" (Array.length &parts) (Array.length b) sum)))) + (Result.Error e) (Response.text (fmt "err=%s" &e)))) + +; what the upload route reports for the raw request buffer `buf` +(defn upload-body [buf] + (let [app (App.POST (App.create) @"/up" upload-report) + pair (web-build-response &app &(no-hooks-before) &(no-hooks) buf) + resp @(Pair.a &pair)] + @(Response.body &resp))) + +(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] @@ -2927,4 +2992,47 @@ (assert-equal test 0 (dispatch-tag &(dispatch &(dual-app) "BREW / HTTP/1.1\r\nHost: x\r\n\r\n")) - "a malformed request line is rejected before any probe")) + "a malformed request line is rejected before any probe") + + ; -- binary multipart uploads survive the request buffer -- + + (assert-equal test + "parts=1 len=5 sum=10" + &(upload-body &(mp-request &[1b 2b 3b 4b 0b])) + "a payload ending in NUL keeps every byte") + + (assert-equal test + "parts=1 len=5 sum=10" + &(upload-body &(mp-request &[1b 0b 2b 3b 4b])) + "a NUL in the middle of a payload neither truncates nor drops the part") + + (assert-equal test + "parts=1 len=4 sum=0" + &(upload-body &(mp-request &[0b 0b 0b 0b])) + "a payload that is nothing but NULs still decodes") + + (assert-equal test + "parts=1 len=5 sum=10" + &(upload-body &(mp-request-chunked &[1b 0b 2b 3b 4b])) + "a chunked binary upload is dechunked as bytes") + + (assert-equal test + "parts=1 len=3 sum=6" + &(upload-body &(mp-request &[1b 2b 3b])) + "a payload with no NUL is unaffected") + + (assert-equal test + 1 + (Array.length + &(Result.unsafe-from-success + (Form.decode-multipart &(String.from-bytes &(mp-body &[1b 2b 3b])) "bb"))) + "the String-based decode still works for text bodies") + + (assert-equal test + 0 + (Array.length + &(Result.unsafe-from-success + (Form.decode-multipart + &(String.from-bytes &(mp-body &[1b 0b 2b 3b 4b])) + "bb"))) + "the String-based decode still loses a binary part, which is the reason the byte-based one exists")) diff --git a/web.carp b/web.carp index 737ce29..b48a920 100644 --- a/web.carp +++ b/web.carp @@ -637,6 +637,17 @@ with a `Content-Range` header (or `416` when unsatisfiable).") (with-header @"ETag" etag) (with-header @"Last-Modified" (web-http-date mt)))))))))) +; The raw bytes of the body of the request being served right now, kept +; alongside the parsed `Request` because a `Request` body is a `String` and so +; ends at its first NUL byte. Set once per request, just before the hooks and +; the handler run. One event loop is single-threaded and a worker is its own +; process, so exactly one request is in flight per process at a time. +(hidden *web-raw-body*) +(def *web-raw-body* (the (Array Byte) [])) + +(hidden web-set-raw-body!) +(defn web-set-raw-body! [bs] (set! *web-raw-body* bs)) + (doc Form "provides helpers for parsing `application/x-www-form-urlencoded` and `multipart/form-data` request bodies.") (defmodule Form @@ -717,7 +728,48 @@ when the request is not `multipart/form-data` or carries no boundary. (Response.text (fmt \"got %d parts\" (Array.length &parts))) (Result.Error _) (Response.bad-request))) ```") - (defn decode-multipart-request [req] (Request.multipart-data req))) + (defn decode-multipart-request [req] (Request.multipart-data req)) + + (doc decode-multipart-request-bytes "decodes the multipart form body of a +request as bytes, via http’s +[`Multipart.parse-bytes`](Multipart.html#parse-bytes). Returns +`(Result (Array BinaryPart) String)`; fails when the request is not +`multipart/form-data` or carries no boundary. + +Prefer this over +[`decode-multipart-request`](#decode-multipart-request) for anything that can +carry a file. A `Request` body is a `String` and so ends at its first NUL byte, +which silently truncates a binary upload and can leave it with no parts at all; +this reads the raw request bytes the server kept alongside the request, so the +upload survives. + +``` +(defn upload [req params] + (match (Form.decode-multipart-request-bytes req) + (Result.Success parts) + (Response.text (fmt \"got %d parts\" (Array.length &parts))) + (Result.Error _) (Response.bad-request))) +``` + +Only valid while the request is being served, which is the whole of a handler +and of a before- or after-hook. It reads the body of the request in flight, so +holding on to the parts is fine but calling it later is not.") + (defn decode-multipart-request-bytes [req] + (match (Request.header req "Content-Type") + (Maybe.Nothing) (Result.Error @"multipart: no Content-Type header") + (Maybe.Just ct) + (match (MediaType.parse &ct) + (Result.Error e) (Result.Error e) + (Result.Success mt) + (if (/= &(MediaType.mime &mt) "multipart/form-data") + (Result.Error + (fmt "multipart: expected multipart/form-data, got '%s'" + &(MediaType.mime &mt))) + (match (MediaType.param &mt "boundary") + (Maybe.Nothing) + (Result.Error @"multipart: no boundary parameter") + (Maybe.Just b) + (Multipart.parse-bytes &*web-raw-body* &b))))))) (doc WSEvent "represents an event on a WebSocket connection. @@ -1487,6 +1539,61 @@ stream and any proxy in front of it from timing out.") expects bad)))))) +; Dechunk a chunked request body at byte level, so that a binary upload +; survives. Nothing when the chunk framing is malformed or incomplete. +(hidden web-dechunk-bytes) +(defn web-dechunk-bytes [buf body-start] + (let-do [len (Array.length buf) + pos body-start + out (the (Array Byte) []) + ok true + done false] + (while-do (not done) + (let [nl (web-crlf-index buf pos len)] + (if (< nl 0) + (do (set! ok false) (set! done true)) + (let [size (web-parse-chunk-size buf pos nl) + start (+ nl 2)] + (cond + (< size 0) (do (set! ok false) (set! done true)) + (= size 0) (set! done true) + (> (+ start size) len) (do (set! ok false) (set! done true)) + (do + (for [i start (+ start size)] + (Array.push-back! &out @(Array.unsafe-nth buf i))) + (set! pos (+ start (+ size 2))))))))) + (if ok (Maybe.Just out) (Maybe.Nothing)))) + +; The raw bytes of the body carried by `buf`, dechunked when the request is +; chunked. Nothing when the framing is malformed or the body is short. +(hidden web-request-body-bytes) +(defn web-request-body-bytes [buf] + (match (web-parse-framing buf) + (Maybe.Nothing) (the (Maybe (Array Byte)) (Maybe.Nothing)) + (Maybe.Just f) + (if @(WebFraming.malformed &f) + (Maybe.Nothing) + (let [start @(WebFraming.body-start &f) + len (Array.length buf)] + (if @(WebFraming.chunked &f) + (web-dechunk-bytes buf start) + (match-ref (WebFraming.content-length &f) + (Maybe.Just cl) + (if (> (+ start @cl) len) + (Maybe.Nothing) + (Maybe.Just (Array.slice buf start (+ start @cl)))) + (Maybe.Nothing) (Maybe.Just (Array.slice buf start len)))))))) + +; Keep the body bytes of the request in `buf` for the handler. A buffer whose +; framing does not parse leaves an empty body rather than the previous +; request's, so a handler can never read one request's bytes from another. +(hidden web-stash-raw-body!) +(defn web-stash-raw-body! [buf] + (web-set-raw-body! + (match (web-request-body-bytes buf) + (Maybe.Just bs) bs + (Maybe.Nothing) (the (Array Byte) [])))) + ; Whether `buf` holds a whole HTTP request: 1 complete, 0 needs more bytes, ; -1 malformed (400). Paired with the chunk cursor for `web-chunked-status`. (hidden web-request-status) @@ -2100,14 +2207,17 @@ stream and any proxy in front of it from timing out.") (the (Maybe Response) (Maybe.Nothing))))))))))) ; Dechunk a chunked request body and normalize its framing headers: -; Transfer-Encoding removed, Content-Length set, trailers discarded. +; Transfer-Encoding removed, Content-Length set, trailers discarded. The +; dechunking is done on `buf`'s bytes rather than on the request's `String` +; body, which a binary upload's first NUL byte would have cut short. (hidden web-decode-body) -(defn web-decode-body [chunked? req] - (if chunked? - (match (TransferEncoding.dechunk (Request.body &req)) - (Result.Error e) (Result.Error e) - (Result.Success decoded) - (let [cl-v [(Int.str (String.length &decoded))] +(defn web-decode-body [buf req] + (if (web-chunked-buf? buf) + (match (web-request-body-bytes buf) + (Maybe.Nothing) (Result.Error @"malformed chunked body") + (Maybe.Just bytes) + (let [decoded (String.from-bytes &bytes) + cl-v [(Int.str (Array.length &bytes))] hdrs (Map.kv-reduce &(fn [acc k v] (if (= &(String.ascii-to-lower k) "transfer-encoding") @@ -2123,8 +2233,8 @@ stream and any proxy in front of it from timing out.") ; Build the response for a parsed request. Returns `(Pair Response Bool)`. (hidden web-respond) -(defn web-respond [app before-hooks after-hooks req chunked?] - (let [decoded (web-decode-body chunked? req)] +(defn web-respond [app before-hooks after-hooks req buf] + (let [decoded (web-decode-body buf req)] (match decoded (Result.Error _) (Pair.init (Response.bad-request) false) (Result.Success req) @@ -2167,7 +2277,9 @@ stream and any proxy in front of it from timing out.") (match (Request.parse &(String.from-bytes buf)) (Result.Error _) (Pair.init (Response.bad-request) false) (Result.Success req) - (web-respond app before-hooks after-hooks req (web-chunked-buf? buf)))) + (do + (web-stash-raw-body! buf) + (web-respond app before-hooks after-hooks req buf)))) ; What a complete request buffer resolves to. (hidden WebDispatch) @@ -2203,11 +2315,13 @@ stream and any proxy in front of it from timing out.") @(Pair.a (Pair.b &si)) @(Pair.b (Pair.b &si))) (Maybe.Nothing) - (let [pair (web-respond app - before-hooks - after-hooks - req - (web-chunked-buf? buf))] + (let [pair (do + (web-stash-raw-body! buf) + (web-respond app + before-hooks + after-hooks + req + buf))] (WebDispatch.Respond @(Pair.a &pair) @(Pair.b &pair))))))))) ; Check if a response has an X-Sendfile header and extract the path.