feat(http/server): Respond with 400 on request parse failure (denolan…

…d/deno#4551)

http/io.ts

@@ -342,17 +342,38 @@ export function parseHTTPVersion(vers: string): [number, number] {
 
 export async function readRequest(
   conn: Deno.Conn,
-  bufr: BufReader
+  // The reader and writer buffers may be constructed externally so they can be
+  // shared by requests on the same connection -- see `Server`.
+  reader?: BufReader,
+  writer?: BufWriter
 ): Promise<ServerRequest | Deno.EOF> {
-  const tp = new TextProtoReader(bufr);
-  const firstLine = await tp.readLine(); // e.g. GET /index.html HTTP/1.0
-  if (firstLine === Deno.EOF) return Deno.EOF;
-  const headers = await tp.readMIMEHeader();
-  if (headers === Deno.EOF) throw new Deno.errors.UnexpectedEof();
+  reader = reader ?? new BufReader(conn);
+  writer = writer ?? new BufWriter(conn);
+  const tp = new TextProtoReader(reader);
+  let firstLine: string | Deno.EOF;
+  let headers: Headers | Deno.EOF;
+  try {
+    firstLine = await tp.readLine(); // e.g. GET /index.html HTTP/1.0
+    if (firstLine == Deno.EOF) {
+      return Deno.EOF;
+    }
+    headers = await tp.readMIMEHeader();
+    if (headers == Deno.EOF) {
+      throw new Deno.errors.UnexpectedEof();
+    }
+  } catch (error) {
+    // An error was thrown while parsing request headers.
+    await writeResponse(writer, {
+      status: 400,
+      body: encoder.encode(`${error.message}\r\n\r\n`),
+    });
+    throw error;
+  }
 
   const req = new ServerRequest();
   req.conn = conn;
-  req.r = bufr;
+  req.r = reader;
+  req.w = writer;
   [req.method, req.url, req.proto] = firstLine.split(" ", 3);
   [req.protoMinor, req.protoMajor] = parseHTTPVersion(req.proto);
   req.headers = headers;

http/io_test.ts

@@ -5,6 +5,7 @@ import {
   assert,
   assertNotEOF,
   assertNotEquals,
+  assertMatch,
 } from "../testing/asserts.ts";
 import {
   bodyReader,
@@ -349,13 +350,28 @@ malformedHeader
 `;
   const reader = new BufReader(new StringReader(input));
   let err;
+  let responseString: string;
   try {
-    await readRequest(mockConn(), reader);
+    // Capture whatever `readRequest()` attempts to write to the connection on
+    // error. We expect it to be a 400 response.
+    await readRequest(
+      mockConn({
+        write(p: Uint8Array): Promise<number> {
+          responseString = decode(p);
+          return Promise.resolve(p.length);
+        },
+      }),
+      reader
+    );
   } catch (e) {
     err = e;
   }
   assert(err instanceof Error);
   assertEquals(err.message, "malformed MIME header line: malformedHeader");
+  assertMatch(
+    responseString!,
+    /^HTTP\/1\.1 400 Bad Request\r\ncontent-length: \d+\r\n\r\n.*\r\n\r\n$/ms
+  );
 });
 
 // Ported from Go

http/mock.ts

@@ -17,8 +17,8 @@ export function mockConn(base: Partial<Deno.Conn> = {}): Deno.Conn {
     read: (): Promise<number | Deno.EOF> => {
       return Promise.resolve(0);
     },
-    write: (): Promise<number> => {
-      return Promise.resolve(-1);
+    write: (p: Uint8Array): Promise<number> => {
+      return Promise.resolve(p.length);
     },
     close: (): void => {},
     ...base,

http/server.ts

@@ -146,22 +146,21 @@ export class Server implements AsyncIterable<ServerRequest> {
   private async *iterateHttpRequests(
     conn: Conn
   ): AsyncIterableIterator<ServerRequest> {
-    const bufr = new BufReader(conn);
-    const w = new BufWriter(conn);
+    const reader = new BufReader(conn);
+    const writer = new BufWriter(conn);
     let req: ServerRequest | Deno.EOF = Deno.EOF;
     let err: Error | undefined;
 
     while (!this.closing) {
       try {
-        req = await readRequest(conn, bufr);
+        req = await readRequest(conn, reader, writer);
       } catch (e) {
         err = e;
       }
       if (err != null || req === Deno.EOF) {
         break;
       }
 
-      req.w = w;
       yield req;
 
       // Wait for the request to be processed before we accept a new request on