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

…oland#4551)

std/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;

std/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

std/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,

std/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