Merge pull request #17 from carpentry-org/claude/response-pipeline-fixes

Fix three response pipeline bugs

Author: hellerve

CHANGELOG.md

@@ -29,6 +29,18 @@
   (§5.1). The upgrade handshake validates `Sec-WebSocket-Version: 13` and
   responds with 426 Upgrade Required if the version is missing or wrong (§4.2.1).
 
+### Fixed
+
+- **Content-Length no longer sent with chunked encoding.** `web-finalize-response`
+  now skips the `Content-Length` header when `Transfer-Encoding` is already set,
+  fixing an RFC 7230 §3.3.2 violation.
+- **`log-after` no longer crashes when `_start` is missing.** The after-hook used
+  `Maybe.unsafe-from` on the parsed start time, which panicked if `log-before`
+  was not registered. Now falls back to `0l`.
+- **File descriptor leak on `fstat` failure.** When `sendfile` opened a file but
+  `fstat` failed, the fd was stored in `ConnState` but never closed. The fd is
+  now closed immediately on `fstat` failure.
+
 ### Changed
 
 - `WSFrame` gains `rsv` (`Int`) and `masked` (`Bool`) fields in addition to

test/web.carp

@@ -461,4 +461,38 @@
         &(Result.unsafe-from-success
           (Request.parse
             "POST / HTTP/1.1\r\nContent-Type: application/json\r\n\r\n{}"))))
-    "decode-multipart-request empty for non-multipart"))
+    "decode-multipart-request empty for non-multipart")
+
+  ; ---------------------------------------------------------------------------
+  ; Response finalization tests
+  ; ---------------------------------------------------------------------------
+
+  ; -- finalize omits Content-Length for chunked responses --
+  (assert-false test
+    (Map.contains?
+      (Response.headers
+        &(web-finalize-response
+          (Response.chunked 200 @"text/plain" &[@"hello"])
+          true))
+      "Content-Length")
+    "finalize omits Content-Length when Transfer-Encoding is set")
+
+  ; -- finalize adds Content-Length for normal responses --
+  (assert-true test
+    (Map.contains?
+      (Response.headers &(web-finalize-response (Response.text @"hi") true))
+      "Content-Length")
+    "finalize adds Content-Length for normal responses")
+
+  ; ---------------------------------------------------------------------------
+  ; log-after safety tests
+  ; ---------------------------------------------------------------------------
+
+  ; -- log-after does not crash when _start is missing --
+  (assert-equal test
+    200
+    (let [req (Result.unsafe-from-success
+            (Request.parse "GET / HTTP/1.1\r\nHost: x\r\n\r\n"))
+          params (the (Map String String) {})]
+      @(Response.code &(log-after &req &params (Response.text @"ok"))))
+    "log-after does not crash without _start param"))

web.carp

@@ -939,14 +939,15 @@ responses.")
 
 (defn web-finalize-response [resp keep-alive]
   (let [body-len (String.length (Response.body &resp))
-        cl-k @"Content-Length"
-        cl-v [(Int.str body-len)]
+        has-te (Map.contains? (Response.headers &resp) "Transfer-Encoding")
         conn-k @"Connection"
         conn-v [(if keep-alive @"keep-alive" @"close")]
-        h (Map.put
-          (Map.put @(Response.headers &resp) &cl-k &cl-v)
-          &conn-k
-          &conn-v)]
+        h (let [base (Map.put @(Response.headers &resp) &conn-k &conn-v)]
+          (if has-te
+            base
+            (let [cl-k @"Content-Length"
+                  cl-v [(Int.str body-len)]]
+              (Map.put base &cl-k &cl-v))))]
     (Response.set-headers resp h)))
 
 (defn read-request-buf [client buf]
@@ -1675,17 +1676,19 @@ fallback.")
                                     (if (< ffd 0)
                                       -1l
                                       (let [sz (IO.Raw.fstat-size ffd)]
-                                        (do
-                                          (Map.put! (ConnState.sf-fds cs)
-                                                    &fd
-                                                    &ffd)
-                                          (Map.put! (ConnState.sf-offsets cs)
-                                                    &fd
-                                                    &0l)
-                                          (Map.put! (ConnState.sf-sizes cs)
-                                                    &fd
-                                                    &sz)
-                                          sz))))
+                                        (if (< sz 0l)
+                                          (do (ignore (IO.Raw.close-fd ffd)) -1l)
+                                          (do
+                                            (Map.put! (ConnState.sf-fds cs)
+                                                      &fd
+                                                      &ffd)
+                                            (Map.put! (ConnState.sf-offsets cs)
+                                                      &fd
+                                                      &0l)
+                                            (Map.put! (ConnState.sf-sizes cs)
+                                                      &fd
+                                                      &sz)
+                                            sz)))))
                                 (Maybe.Nothing) -1l)
                               actual-resp (if (and (Maybe.just? &sf-path)
                                                    (< sf-result 0l))
@@ -1925,16 +1928,17 @@ stdout. Reads the start time from `_start` in params, set by
   (GET \"/\" hello))
 ```")
 (defn log-after [req params resp]
-  (let [start (Maybe.unsafe-from (Long.from-string &(Map.get params "_start")))
-        now (Uint64.to-long (System.nanotime))
-        elapsed (/ (- now start) 1000000l)]
-    (do
-      (Log.info "%s /%s %d %ldms"
-                (Request.verb req)
-                &(web-request-path req)
-                (Response.code &resp)
-                elapsed)
-      resp)))
+  (let-do [start (match (Long.from-string &(Map.get params "_start"))
+             (Maybe.Just v) v
+             (Maybe.Nothing) 0l)
+           now (Uint64.to-long (System.nanotime))
+           elapsed (/ (- now start) 1000000l)]
+    (Log.info "%s /%s %d %ldms"
+              (Request.verb req)
+              &(web-request-path req)
+              (Response.code &resp)
+              elapsed)
+    resp))
 
 ; ---------------------------------------------------------------------------
 ; defserver: generate a main that builds an App and starts serving