Skip to content

Commit

Permalink
Revert "Respond with 400 on request parse failure" (denoland#4593)
Browse files Browse the repository at this point in the history
readRequest should not write a response.

This reverts commit 017a611.
  • Loading branch information
ry committed Apr 2, 2020
1 parent 7a9273d commit c8fc29f
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 50 deletions.
35 changes: 7 additions & 28 deletions std/http/io.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,38 +342,17 @@ export function parseHTTPVersion(vers: string): [number, number] {

export async function readRequest(
conn: Deno.Conn,
// 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
bufr: BufReader
): Promise<ServerRequest | Deno.EOF> {
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 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();

const req = new ServerRequest();
req.conn = conn;
req.r = reader;
req.w = writer;
req.r = bufr;
[req.method, req.url, req.proto] = firstLine.split(" ", 3);
[req.protoMinor, req.protoMajor] = parseHTTPVersion(req.proto);
req.headers = headers;
Expand Down
18 changes: 1 addition & 17 deletions std/http/io_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
assert,
assertNotEOF,
assertNotEquals,
assertMatch,
} from "../testing/asserts.ts";
import {
bodyReader,
Expand Down Expand Up @@ -350,28 +349,13 @@ malformedHeader
`;
const reader = new BufReader(new StringReader(input));
let err;
let responseString: string;
try {
// 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
);
await readRequest(mockConn(), 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
Expand Down
4 changes: 2 additions & 2 deletions std/http/mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ export function mockConn(base: Partial<Deno.Conn> = {}): Deno.Conn {
read: (): Promise<number | Deno.EOF> => {
return Promise.resolve(0);
},
write: (p: Uint8Array): Promise<number> => {
return Promise.resolve(p.length);
write: (): Promise<number> => {
return Promise.resolve(-1);
},
close: (): void => {},
...base,
Expand Down
7 changes: 4 additions & 3 deletions std/http/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,21 +146,22 @@ export class Server implements AsyncIterable<ServerRequest> {
private async *iterateHttpRequests(
conn: Conn
): AsyncIterableIterator<ServerRequest> {
const reader = new BufReader(conn);
const writer = new BufWriter(conn);
const bufr = new BufReader(conn);
const w = new BufWriter(conn);
let req: ServerRequest | Deno.EOF = Deno.EOF;
let err: Error | undefined;

while (!this.closing) {
try {
req = await readRequest(conn, reader, writer);
req = await readRequest(conn, bufr);
} 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
Expand Down

0 comments on commit c8fc29f

Please sign in to comment.