Bun.serve: ignore leading empty lines before the request-line

packages/bun-uws/src/HttpParser.h

@@ -1116,21 +1116,20 @@ struct HttpResponseData;
              * request-line, like Node/llhttp - e.g. a stray "\r\n" sent on an
              * idle keep-alive connection must not be treated as a bad request.
              * llhttp's s_start state loops on '\r' and '\n' independently, so a
-             * leading bare LF (or bare CR) is also tolerated. Node-compat only:
-             * Bun.serve keeps rejecting a request that does not start with the
-             * request-line, so this leniency is not a Bun-native default. */
-            if constexpr (IsNodeHttp) {
-                if (isNewline((unsigned char) data[0])) [[unlikely]] {
-                    /* The enclosing loop only runs while length is non-zero, so the
-                     * first byte is known to be one; re-test only after advancing. */
-                    do {
-                        data += 1;
-                        length -= 1;
-                        consumedTotal += 1;
-                    } while (length && isNewline((unsigned char) data[0]));
-                    if (length == 0) {
-                        break;
-                    }
+             * leading bare LF (or bare CR) is also tolerated. Applies to Bun.serve
+             * as well: the RFC's own motivating case (a stray CRLF after a POST
+             * body in front of the next keep-alive request) otherwise kills that
+             * request with a 400. */
+            if (isNewline((unsigned char) data[0])) [[unlikely]] {
+                /* The enclosing loop only runs while length is non-zero, so the
+                 * first byte is known to be one; re-test only after advancing. */
+                do {
+                    data += 1;
+                    length -= 1;
+                    consumedTotal += 1;
+                } while (length && isNewline((unsigned char) data[0]));
+                if (length == 0) {
+                    break;
                 }
             }
             auto result = getHeaders(data, data + length, req->headers, reserved, req->ancientHttp, isConnectRequest, useStrictMethodValidation, useInsecureHTTPParser, maxHeaderSize);

test/js/bun/http/bun-serve-leading-crlf.test.ts

@@ -0,0 +1,58 @@
+import { describe, expect, it } from "bun:test";
+import { connect } from "node:net";
+
+/* RFC 9112 2.2: "a server that is expecting to receive and parse a request-line
+ * SHOULD ignore at least one empty line (CRLF) received prior to the
+ * request-line." The motivating case the RFC gives is a client that sends an
+ * extra CRLF after a POST body, which then sits in front of the next request
+ * on the same keep-alive connection. */
+
+describe("Bun.serve: empty line(s) before the request-line are ignored (RFC 9112 2.2)", () => {
+  async function run(bytes: string) {
+    const events: string[] = [];
+    await using server = Bun.serve({
+      hostname: "127.0.0.1",
+      port: 0,
+      async fetch(req) {
+        events.push(`request ${req.method} ${new URL(req.url).pathname}`);
+        await req.text();
+        return new Response("ok");
+      },
+    });
+
+    const raw = await new Promise<string>(resolve => {
+      const out: Buffer[] = [];
+      const sock = connect(server.port, "127.0.0.1", () => sock.end(Buffer.from(bytes, "latin1")));
+      const done = () => {
+        sock.destroy();
+        resolve(Buffer.concat(out).toString("latin1"));
+      };
+      sock.on("data", d => out.push(d));
+      sock.on("close", done);
+      sock.on("error", done);
+    });
+    const statuses = (raw.match(/HTTP\/1\.[01] (\d{3})/g) ?? []).map(m => m.slice(-3));
+    return { events, statuses };
+  }
+
+  it.each([
+    ["one CRLF", "\r\n"],
+    ["one bare LF", "\n"],
+    ["multiple CRLF", "\r\n\r\n\r\n"],
+  ])("leading %s before a single request", async (_, prefix) => {
+    const { events, statuses } = await run(prefix + "GET /a HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n");
+    expect({ events, statuses }).toEqual({ events: ["request GET /a"], statuses: ["200"] });
+  });
+
+  it("stray CRLF after a POST body on a keep-alive connection", async () => {
+    const { events, statuses } = await run(
+      "POST /a HTTP/1.1\r\nHost: x\r\nContent-Length: 2\r\n\r\nhi" +
+        "\r\n" +
+        "GET /b HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n",
+    );
+    expect({ events, statuses }).toEqual({
+      events: ["request POST /a", "request GET /b"],
+      statuses: ["200", "200"],
+    });
+  });
+});

test/js/bun/http/http-spec.ts

@@ -120,6 +120,21 @@ const testCases: TestCase[] = [
     description: "Valid GET request",
     expectedStatus: [[200, 299]],
   },
+  {
+    request: "\r\nGET / HTTP/1.1\r\nHost: example.com\r\n\r\n",
+    description: "Leading empty line before request-line (RFC 9112 2.2)",
+    expectedStatus: [[200, 299]],
+  },
+  {
+    request: "\r\n\r\nGET / HTTP/1.1\r\nHost: example.com\r\n\r\n",
+    description: "Multiple leading empty lines before request-line",
+    expectedStatus: [[200, 299]],
+  },
+  {
+    request: "\nGET / HTTP/1.1\r\nHost: example.com\r\n\r\n",
+    description: "Leading bare LF before request-line",
+    expectedStatus: [[200, 299]],
+  },
   {
     request: "GET / HTTP/1.0\r\nHost: example.com\r\n\r\n",
     description: "Valid GET request with HTTP/1.0",

test/js/bun/http/serve.test.ts

@@ -3938,20 +3938,19 @@ describe("a client half-close after the request does not truncate a large respon
   });
 });
 
-// The node:http compat parser tolerates empty lines (and a bare CR/LF) before the
-// request-line like llhttp's s_start state. That leniency must stay behind the
-// node-http flag: Bun.serve still rejects a request that does not begin with the
-// request-line.
+// RFC 9112 2.2: a server SHOULD ignore at least one empty line (CRLF) received
+// prior to the request-line. llhttp's s_start state loops on '\r' and '\n'
+// independently, so a leading bare LF or bare CR is tolerated the same way.
 it.each([
   ["CRLF", "\r\n"],
   ["bare LF", "\n"],
   ["bare CR", "\r"],
-])("Bun.serve rejects a leading %s before the request-line", async (_label, prefix) => {
+])("Bun.serve ignores a leading %s before the request-line", async (_label, prefix) => {
   using server = serve({ port: 0, fetch: () => new Response("ok") });
 
   const { promise, resolve, reject } = Promise.withResolvers<string>();
   const socket = connect(server.port, "127.0.0.1", () => {
-    socket.write(`${prefix}GET / HTTP/1.1\r\nHost: localhost\r\n\r\n`);
+    socket.write(`${prefix}GET / HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n`);
   });
   let received = "";
   socket.on("data", chunk => {
@@ -3962,5 +3961,5 @@ it.each([
   const statusLine = (await promise).split("\r\n")[0];
   socket.destroy();
 
-  expect(statusLine).toBe("HTTP/1.1 400 Bad Request");
+  expect(statusLine).toBe("HTTP/1.1 200 OK");
 });