feat(std/http/server): Respond with 400 on request parse failure (#4614)

2 files changed, 49 insertions, 14 deletions

diff --git a/std/http/server.ts b/std/http/server.ts
index 00f401f62..4fbd6f9c9 100644
--- a/std/http/server.ts
+++ b/std/http/server.ts
@@ -1,4 +1,5 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
+import { encode } from "../encoding/utf8.ts";
 import { BufReader, BufWriter } from "../io/bufio.ts";
 import { assert } from "../testing/asserts.ts";
 import { deferred, Deferred, MuxAsyncIterator } from "../util/async.ts";
@@ -146,36 +147,45 @@ export class Server implements AsyncIterable<ServerRequest> {
   private async *iterateHttpRequests(
     conn: Conn
   ): AsyncIterableIterator<ServerRequest> {
-    const bufr = new BufReader(conn);
-    const w = new BufWriter(conn);
-    let req: ServerRequest | Deno.EOF = Deno.EOF;
-    let err: Error | undefined;
+    const reader = new BufReader(conn);
+    const writer = new BufWriter(conn);
 
     while (!this.closing) {
+      let request: ServerRequest | Deno.EOF;
       try {
-        req = await readRequest(conn, bufr);
-      } catch (e) {
-        err = e;
+        request = await readRequest(conn, reader);
+      } catch (error) {
+        if (
+          error instanceof Deno.errors.InvalidData ||
+          error instanceof Deno.errors.UnexpectedEof
+        ) {
+          // An error was thrown while parsing request headers.
+          await writeResponse(writer, {
+            status: 400,
+            body: encode(`${error.message}\r\n\r\n`),
+          });
+        }
+        break;
       }
-      if (err != null || req === Deno.EOF) {
+      if (request == Deno.EOF) {
         break;
       }
 
-      req.w = w;
-      yield req;
+      request.w = writer;
+      yield request;
 
       // Wait for the request to be processed before we accept a new request on
       // this connection.
-      const procError = await req.done;
-      if (procError) {
+      const responseError = await request.done;
+      if (responseError) {
         // Something bad happened during response.
         // (likely other side closed during pipelined req)
         // req.done implies this connection already closed, so we can just return.
-        this.untrackConnection(req.conn);
+        this.untrackConnection(request.conn);
         return;
       }
       // Consume unread body and trailers if receiver didn't consume those data
-      await req.finalize();
+      await request.finalize();
     }
 
     this.untrackConnection(conn);
diff --git a/std/http/server_test.ts b/std/http/server_test.ts
index e70c241c7..12849d6e7 100644
--- a/std/http/server_test.ts
+++ b/std/http/server_test.ts
@@ -9,6 +9,7 @@ import { TextProtoReader } from "../textproto/mod.ts";
 import {
   assert,
   assertEquals,
+  assertMatch,
   assertNotEOF,
   assertStrContains,
   assertThrowsAsync,
@@ -519,3 +520,27 @@ test({
     await p;
   },
 });
+
+test({
+  name: "[http] request error gets 400 response",
+  async fn(): Promise<void> {
+    const server = serve(":8124");
+    const entry = server[Symbol.asyncIterator]().next();
+    const conn = await Deno.connect({
+      hostname: "127.0.0.1",
+      port: 8124,
+    });
+    await Deno.writeAll(
+      conn,
+      encode("GET / HTTP/1.1\r\nmalformedHeader\r\n\r\n\r\n\r\n")
+    );
+    const responseString = decode(await Deno.readAll(conn));
+    assertMatch(
+      responseString,
+      /^HTTP\/1\.1 400 Bad Request\r\ncontent-length: \d+\r\n\r\n.*\r\n\r\n$/ms
+    );
+    conn.close();
+    server.close();
+    assert((await entry).done);
+  },
+});