node:http: report each bad Content-Length with node's code and reason

packages/bun-uws/src/HttpParser.h

@@ -74,6 +74,7 @@ struct HttpResponseData;
     enum HttpParserError: uint8_t {
         HTTP_PARSER_ERROR_NONE = 0,
         HTTP_PARSER_ERROR_INVALID_CHUNKED_ENCODING = 1,
+        /* A non-digit byte in a Content-Length value (llhttp "Invalid character in Content-Length"). */
         HTTP_PARSER_ERROR_INVALID_CONTENT_LENGTH = 2,
         HTTP_PARSER_ERROR_INVALID_TRANSFER_ENCODING = 3,
         HTTP_PARSER_ERROR_MISSING_HOST_HEADER = 4,
@@ -106,6 +107,10 @@ struct HttpResponseData;
          * body. llhttp reports HPE_INVALID_CONTENT_LENGTH ("Content-Length can't
          * be present with Transfer-Encoding"). node:http compat only. */
         HTTP_PARSER_ERROR_TRAILER_CONTENT_LENGTH = 17,
+        HTTP_PARSER_ERROR_EMPTY_CONTENT_LENGTH = 18,
+        HTTP_PARSER_ERROR_CONTENT_LENGTH_OVERFLOW = 19,
+        /* A second Content-Length with a different value. The same value is accepted; llhttp rejects both. */
+        HTTP_PARSER_ERROR_DUPLICATE_CONTENT_LENGTH = 20,
     };
 
 
@@ -735,22 +740,37 @@ struct HttpResponseData;
          * personality so a client cannot stream unbounded extension bytes. */
         static const uint64_t MAX_CHUNK_EXTENSION_SIZE = 16 * 1024;
 
-        /* Returns UINT64_MAX on error. Maximum 999999999 is allowed. */
-        static uint64_t toUnsignedInteger(std::string_view str) {
-            /* We assume at least 64-bit integer giving us safely 999999999999999999 (18 number of 9s) */
-            if (str.length() > 18) {
-                return UINT64_MAX;
-            }
-
-            uint64_t unsignedIntegerValue = 0;
-            for (char c : str) {
-                /* As long as the letter is 0-9 we cannot overflow. */
-                if (c < '0' || c > '9') {
-                    return UINT64_MAX;
+        /* The limit is STATE_SIZE_MASK, not UINT64_MAX: remainingStreamingBytes shares its top bits with the chunked state flags. */
+        static HttpParserError parseContentLength(std::string_view str, uint64_t &value, bool acceptLong) {
+            uint64_t result = 0;
+            if (str.length() > 18) [[unlikely]] {
+                /* Only leading zeros keep a longer value at or below the limit. llhttp accepts them. */
+                if (!acceptLong) {
+                    return HTTP_PARSER_ERROR_CONTENT_LENGTH_OVERFLOW;
+                }
+                for (char c : str) {
+                    if (c < '0' || c > '9') {
+                        return HTTP_PARSER_ERROR_INVALID_CONTENT_LENGTH;
+                    }
+                    result = result * 10ull + ((unsigned int) c - (unsigned int) '0');
+                    if (result > STATE_SIZE_MASK) {
+                        return HTTP_PARSER_ERROR_CONTENT_LENGTH_OVERFLOW;
+                    }
+                }
+            } else {
+                /* 18 digits stay below 2^63, so one check after the loop is enough. */
+                for (char c : str) {
+                    if (c < '0' || c > '9') {
+                        return HTTP_PARSER_ERROR_INVALID_CONTENT_LENGTH;
+                    }
+                    result = result * 10ull + ((unsigned int) c - (unsigned int) '0');
+                }
+                if (result > STATE_SIZE_MASK) {
+                    return HTTP_PARSER_ERROR_CONTENT_LENGTH_OVERFLOW;
                 }
-                unsignedIntegerValue = unsignedIntegerValue * 10ull + ((unsigned int) c - (unsigned int) '0');
             }
-            return unsignedIntegerValue;
+            value = result;
+            return HTTP_PARSER_ERROR_NONE;
         }
 
         static inline uint64_t hasLess(uint64_t x, uint64_t n) {
@@ -1294,20 +1314,26 @@ struct HttpResponseData;
             /* RFC 9110 8.6 + RFC 9112 6.3: locate the Content-Length header and, in the
              * same pass, verify every Content-Length header carries the same non-empty
              * value. A single empty value or multiple differing values are ambiguous and
-             * must be rejected to prevent request smuggling. The bloom filter short-circuits
-             * the common "no Content-Length" case. */
+             * must be rejected to prevent request smuggling. The value is parsed here, in
+             * wire order and before the Transfer-Encoding and Host checks, like llhttp when
+             * Content-Length comes first. The bloom filter short-circuits the common "no
+             * Content-Length" case. */
             std::string_view contentLengthString;
+            uint64_t contentLength = 0;
             if (req->bf.mightHave("content-length")) {
                 for (HttpRequest::Header *h = req->headers; (++h)->key.length(); ) {
                     if (h->key.length() == 14 && !strncasecmp(h->key.data(), "content-length", 14)) {
+                        if (h->value.length() == 0) {
+                            return HttpParserResult::error(HTTP_ERROR_400_BAD_REQUEST, HTTP_PARSER_ERROR_EMPTY_CONTENT_LENGTH);
+                        }
                         if (contentLengthString.data() == nullptr) {
-                            if (h->value.length() == 0) {
-                                return HttpParserResult::error(HTTP_ERROR_400_BAD_REQUEST, HTTP_PARSER_ERROR_INVALID_CONTENT_LENGTH);
+                            if (HttpParserError contentLengthError = parseContentLength(h->value, contentLength, IsNodeHttp)) [[unlikely]] {
+                                return HttpParserResult::error(HTTP_ERROR_400_BAD_REQUEST, contentLengthError);
                             }
                             contentLengthString = h->value;
                         } else if (h->value.length() != contentLengthString.length() ||
                                    strncmp(h->value.data(), contentLengthString.data(), contentLengthString.length())) {
-                            return HttpParserResult::error(HTTP_ERROR_400_BAD_REQUEST, HTTP_PARSER_ERROR_INVALID_CONTENT_LENGTH);
+                            return HttpParserResult::error(HTTP_ERROR_400_BAD_REQUEST, HTTP_PARSER_ERROR_DUPLICATE_CONTENT_LENGTH);
                         }
                     }
                 }
@@ -1412,17 +1438,9 @@ struct HttpResponseData;
             const char *querySeparatorPtr = (const char *) memchr(req->headers->value.data(), '?', req->headers->value.length());
             req->querySeparator = (unsigned int) ((querySeparatorPtr ? querySeparatorPtr : req->headers->value.data() + req->headers->value.length()) - req->headers->value.data());
 
-            // lets check if content len is valid before calling requestHandler
+            /* parseContentLength() kept this at or below STATE_SIZE_MASK, so it cannot reach a chunked flag bit. */
             if(contentLengthStringLen) {
-                remainingStreamingBytes = toUnsignedInteger(contentLengthString);
-                /* remainingStreamingBytes is overloaded: for Content-Length it holds the raw byte
-                 * count, for Transfer-Encoding: chunked it holds the ChunkedEncoding state word.
-                 * isParsingChunkedEncoding() distinguishes the two by testing the flag bits, so a
-                 * Content-Length value must never reach a flag bit. UINT64_MAX (parse error) is
-                 * also caught by this since UINT64_MAX > STATE_SIZE_MASK. */
-                if (remainingStreamingBytes > STATE_SIZE_MASK) [[unlikely]] {
-                    return HttpParserResult::error(HTTP_ERROR_400_BAD_REQUEST, HTTP_PARSER_ERROR_INVALID_CONTENT_LENGTH);
-                }
+                remainingStreamingBytes = contentLength;
             }
 
             /* If returned socket is not what we put in we need

src/js/node/_http_server.ts

@@ -1317,6 +1317,9 @@ enum HttpParserError {
   HTTP_PARSER_ERROR_TRAILER_FIELDS_TOO_LARGE = 15,
   HTTP_PARSER_ERROR_CHUNK_TERMINATOR_EXPECTED = 16,
   HTTP_PARSER_ERROR_TRAILER_CONTENT_LENGTH = 17,
+  HTTP_PARSER_ERROR_EMPTY_CONTENT_LENGTH = 18,
+  HTTP_PARSER_ERROR_CONTENT_LENGTH_OVERFLOW = 19,
+  HTTP_PARSER_ERROR_DUPLICATE_CONTENT_LENGTH = 20,
 }
 // Native callback fired when the HTTP parser rejects incoming bytes. Builds
 // the same error object Node's parser produces and routes it through
@@ -1348,25 +1351,41 @@ function onServerClientError(
     return;
   }
 
-  let err;
+  // err.reason is set where the parser error maps to exactly one llhttp reason.
+  let err, reason;
   switch (errorCode) {
     case HttpParserError.HTTP_PARSER_ERROR_INVALID_CHUNKED_ENCODING:
       err = $HPE_INVALID_CHUNK_SIZE("Parse Error: Invalid character in chunk size");
       break;
     case HttpParserError.HTTP_PARSER_ERROR_INVALID_CONTENT_LENGTH:
-      err = $HPE_UNEXPECTED_CONTENT_LENGTH("Parse Error");
+      reason = "Invalid character in Content-Length";
+      err = $HPE_INVALID_CONTENT_LENGTH(`Parse Error: ${reason}`);
+      break;
+    case HttpParserError.HTTP_PARSER_ERROR_EMPTY_CONTENT_LENGTH:
+      reason = "Empty Content-Length";
+      err = $HPE_INVALID_CONTENT_LENGTH(`Parse Error: ${reason}`);
+      break;
+    case HttpParserError.HTTP_PARSER_ERROR_CONTENT_LENGTH_OVERFLOW:
+      reason = "Content-Length overflow";
+      err = $HPE_INVALID_CONTENT_LENGTH(`Parse Error: ${reason}`);
+      break;
+    case HttpParserError.HTTP_PARSER_ERROR_DUPLICATE_CONTENT_LENGTH:
+      reason = "Duplicate Content-Length";
+      err = $HPE_UNEXPECTED_CONTENT_LENGTH(`Parse Error: ${reason}`);
       break;
     case HttpParserError.HTTP_PARSER_ERROR_INVALID_TRANSFER_ENCODING:
       err = $HPE_INVALID_TRANSFER_ENCODING("Parse Error: Request has invalid `Transfer-Encoding`");
       break;
     case HttpParserError.HTTP_PARSER_ERROR_TRAILER_CONTENT_LENGTH:
-      err = $HPE_INVALID_CONTENT_LENGTH("Parse Error: Content-Length can't be present with Transfer-Encoding");
+      reason = "Content-Length can't be present with Transfer-Encoding";
+      err = $HPE_INVALID_CONTENT_LENGTH(`Parse Error: ${reason}`);
       break;
     case HttpParserError.HTTP_PARSER_ERROR_INVALID_REQUEST:
       err = $HPE_INVALID_CONSTANT("Parse Error: Expected HTTP/");
       break;
     case HttpParserError.HTTP_PARSER_ERROR_INVALID_EOF:
-      err = $HPE_INVALID_EOF_STATE("Parse Error: Invalid EOF state");
+      reason = "Invalid EOF state";
+      err = $HPE_INVALID_EOF_STATE(`Parse Error: ${reason}`);
       break;
     case HttpParserError.HTTP_PARSER_ERROR_INVALID_METHOD:
       err = $HPE_INVALID_METHOD("Parse Error: Invalid method encountered");
@@ -1377,24 +1396,29 @@ function onServerClientError(
       break;
     case HttpParserError.HTTP_PARSER_ERROR_REQUEST_HEADER_FIELDS_TOO_LARGE:
     case HttpParserError.HTTP_PARSER_ERROR_TRAILER_FIELDS_TOO_LARGE:
-      err = $HPE_HEADER_OVERFLOW("Parse Error: Header overflow");
+      reason = "Header overflow";
+      err = $HPE_HEADER_OVERFLOW(`Parse Error: ${reason}`);
       err.bytesParsed = rawPacket.byteLength;
       break;
     case HttpParserError.HTTP_PARSER_ERROR_INVALID_HTTP_VERSION:
       err = $HPE_INVALID_VERSION("Parse Error: Invalid HTTP version");
       break;
     case HttpParserError.HTTP_PARSER_ERROR_LF_EXPECTED:
-      err = $HPE_LF_EXPECTED("Parse Error: Missing expected LF after header value");
+      reason = "Missing expected LF after header value";
+      err = $HPE_LF_EXPECTED(`Parse Error: ${reason}`);
       break;
     case HttpParserError.HTTP_PARSER_ERROR_CHUNK_EXTENSIONS_OVERFLOW:
-      err = $HPE_CHUNK_EXTENSIONS_OVERFLOW("Parse Error: Chunk extensions overflow");
+      reason = "Chunk extensions overflow";
+      err = $HPE_CHUNK_EXTENSIONS_OVERFLOW(`Parse Error: ${reason}`);
       break;
     case HttpParserError.HTTP_PARSER_ERROR_PAUSED_H2_UPGRADE:
-      err = $HPE_PAUSED_H2_UPGRADE("Parse Error: Pause on PRI/Upgrade");
+      reason = "Pause on PRI/Upgrade";
+      err = $HPE_PAUSED_H2_UPGRADE(`Parse Error: ${reason}`);
       err.bytesParsed = 24;
       break;
     case HttpParserError.HTTP_PARSER_ERROR_CLOSED_CONNECTION:
-      err = $HPE_CLOSED_CONNECTION("Parse Error: Data after `Connection: close`");
+      reason = "Data after `Connection: close`";
+      err = $HPE_CLOSED_CONNECTION(`Parse Error: ${reason}`);
       break;
     case HttpParserError.HTTP_PARSER_ERROR_CHUNK_TERMINATOR_EXPECTED:
       err = $HPE_STRICT("Parse Error: Expected LF after chunk data");
@@ -1403,6 +1427,7 @@ function onServerClientError(
       err = $HPE_INTERNAL("Parse Error");
       break;
   }
+  if (reason !== undefined) err.reason = reason;
   err.rawPacket = Buffer.from(rawPacket);
   socketOnError.$call(nodeSocket, err);
 }

test/js/bun/http/request-smuggling.test.ts

@@ -353,6 +353,71 @@ test("rejects empty-valued Content-Length followed by smuggled Content-Length",
   expect(seen).not.toContain("GET /admin");
 });
 
+// RFC 9110 8.6: Content-Length = 1*DIGIT, so each zero-padded value below is 5.
+test.each([
+  [18, "HTTP/1.1 200", "000000000000000005", ["hello"]],
+  [19, "HTTP/1.1 400", "0000000000000000005", []],
+])("Bun.serve answers a %d-byte zero-padded Content-Length with %s", async (_, status, value, bodies) => {
+  const seen: string[] = [];
+  await using server = Bun.serve({
+    port: 0,
+    async fetch(req) {
+      seen.push(await req.text());
+      return new Response("OK");
+    },
+  });
+
+  const { promise, resolve, reject } = Promise.withResolvers<string>();
+  const client = net.connect(server.port, "127.0.0.1", () => {
+    client.write(`POST /a HTTP/1.1\r\nHost: x\r\nContent-Length: ${value}\r\n\r\nhello`);
+  });
+  let raw = "";
+  client.on("data", data => {
+    raw += data;
+    if (raw.includes("\r\n\r\n")) resolve(raw);
+  });
+  client.on("error", reject);
+  client.on("close", () => resolve(raw));
+  const response = await promise;
+  client.destroy();
+
+  expect(response).toStartWith(status);
+  expect(seen).toEqual(bodies);
+});
+
+test("node:http frames a zero-padded Content-Length by its digits (llhttp parity)", async () => {
+  const hits: { url: string; body: string }[] = [];
+  await using server = createServer((req, res) => {
+    let body = "";
+    req.on("data", d => (body += d));
+    req.on("end", () => {
+      hits.push({ url: req.url!, body });
+      res.end("ok");
+    });
+  });
+  await once(server.listen(0, "127.0.0.1"), "listening");
+  const port = (server.address() as net.AddressInfo).port;
+
+  const { promise, resolve, reject } = Promise.withResolvers<string>();
+  const client = net.connect(port, "127.0.0.1", () => {
+    client.write(
+      "POST /a HTTP/1.1\r\nHost: x\r\nContent-Length: 0000000000000000005\r\n\r\nhello" +
+        "GET /b HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n",
+    );
+  });
+  let raw = "";
+  client.on("data", data => (raw += data));
+  client.on("error", reject);
+  client.on("close", () => resolve(raw));
+  const response = await promise;
+
+  expect(hits.sort((a, b) => a.url.localeCompare(b.url))).toEqual([
+    { url: "/a", body: "hello" },
+    { url: "/b", body: "" },
+  ]);
+  expect(response.match(/HTTP\/1\.1 200/g)).toHaveLength(2);
+});
+
 test("accepts valid Transfer-Encoding: chunked", async () => {
   let receivedBody = "";
 

test/js/bun/test/parallel/test-http-should-emit-clientError-when-Content-Length-is-invalid.ts

@@ -25,4 +25,4 @@ const client = connect(server.address().port, () => {
 });
 
 const err = (await promise) as Error;
-expect(err.code).toBe("HPE_UNEXPECTED_CONTENT_LENGTH");
+expect(err.code).toBe("HPE_INVALID_CONTENT_LENGTH");