Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[http] add handling of bad request #419

Merged
merged 18 commits into from
May 22, 2019
14 changes: 11 additions & 3 deletions http/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ export class ServerRequest {
}
}

async function readRequest(
export async function readRequest(
bufr: BufReader
): Promise<[ServerRequest, BufState]> {
const req = new ServerRequest();
Expand Down Expand Up @@ -235,7 +235,11 @@ export class Server implements AsyncIterable<ServerRequest> {
let req: ServerRequest;

while (!this.closing) {
[req, bufStateErr] = await readRequest(bufr);
try {
[req, bufStateErr] = await readRequest(bufr);
} catch (err) {
bufStateErr = err;
}
if (bufStateErr) break;
req.w = w;
yield req;
Expand All @@ -247,7 +251,11 @@ export class Server implements AsyncIterable<ServerRequest> {
if (bufStateErr === "EOF") {
// The connection was gracefully closed.
} else if (bufStateErr instanceof Error) {
// TODO(ry): send something back like a HTTP 500 status.
// An error was thrown while parsing request headers.
await writeResponse(req.w, {
status: 400,
body: new TextEncoder().encode(`${bufStateErr.message}\r\n\r\n`)
});
} else if (this.closing) {
// There are more requests incoming but the server is closing.
// TODO(ry): send a back a HTTP 503 Service Unavailable status.
Expand Down
88 changes: 86 additions & 2 deletions http/server_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,13 @@

const { Buffer } = Deno;
import { test, runIfMain } from "../testing/mod.ts";
import { assertEquals } from "../testing/asserts.ts";
import { Response, ServerRequest, writeResponse } from "./server.ts";
import { assert, assertEquals } from "../testing/asserts.ts";
import {
Response,
ServerRequest,
writeResponse,
readRequest
} from "./server.ts";
import { BufReader, BufWriter } from "../io/bufio.ts";
import { StringReader } from "../io/readers.ts";

Expand Down Expand Up @@ -283,4 +288,83 @@ test(async function writeStringReaderResponse(): Promise<void> {
assertEquals(decoder.decode(line), "0");
});

test(async function readRequestError(): Promise<void> {
let input = `GET / HTTP/1.1
malformedHeader
`;
const reader = new BufReader(new StringReader(input));
let err;
try {
await readRequest(reader);
} catch (e) {
err = e;
}
assert(err instanceof Error);
assertEquals(err.message, "malformed MIME header line: malformedHeader");
});

// Ported from Go
// https://github.com/golang/go/blob/go1.12.5/src/net/http/request_test.go#L377-L443
// TODO(zekth) fix tests
test(async function testReadRequestError(): Promise<void> {
const testCases = {
0: {
in: "GET / HTTP/1.1\r\nheader: foo\r\n\r\n",
headers: [{ key: "header", value: "foo" }],
err: null
},
1: { in: "GET / HTTP/1.1\r\nheader:foo\r\n", err: "EOF", headers: [] },
2: { in: "", err: "EOF", headers: [] },
// 3: {
// in: "HEAD / HTTP/1.1\r\nContent-Length:4\r\n\r\n",
// err: "http: method cannot contain a Content-Length"
// },
4: {
in: "HEAD / HTTP/1.1\r\n\r\n",
headers: [],
err: null
}
// Multiple Content-Length values should either be
// deduplicated if same or reject otherwise
// See Issue 16490.
// 5: {
// in:
// "POST / HTTP/1.1\r\nContent-Length: 10\r\nContent-Length: 0\r\n\r\nGopher hey\r\n",
// err: "cannot contain multiple Content-Length headers"
// },
// 6: {
// in:
// "POST / HTTP/1.1\r\nContent-Length: 10\r\nContent-Length: 6\r\n\r\nGopher\r\n",
// err: "cannot contain multiple Content-Length headers"
// },
// 7: {
// in:
// "PUT / HTTP/1.1\r\nContent-Length: 6 \r\nContent-Length: 6\r\nContent-Length:6\r\n\r\nGopher\r\n",
// err: null,
// headers: [{ key: "Content-Length", value: "6" }]
// },
// 8: {
// in: "PUT / HTTP/1.1\r\nContent-Length: 1\r\nContent-Length: 6 \r\n\r\n",
// err: "cannot contain multiple Content-Length headers"
// },
// 9: {
// in: "POST / HTTP/1.1\r\nContent-Length:\r\nContent-Length: 3\r\n\r\n",
// err: "cannot contain multiple Content-Length headers"
// },
// 10: {
// in: "HEAD / HTTP/1.1\r\nContent-Length:0\r\nContent-Length: 0\r\n\r\n",
// headers: [{ key: "Content-Length", value: "0" }],
// err: null
// }
};
for (const p in testCases) {
const test = testCases[p];
const reader = new BufReader(new StringReader(test.in));
const [req, err] = await readRequest(reader);
assertEquals(test.err, err);
for (const h of test.headers) {
assertEquals(req.headers.get(h.key), h.value);
}
}
});
runIfMain(import.meta);