fix(http): fix error handling in the request iterator (denoland/deno#8365)

If the request body is using chunked encoding, errors may 
be thrown in "request.finalize()".

In this case, we should untrack and close the connection.
This commit is contained in:
lideming 2020-11-19 00:47:47 +08:00 committed by denobot
parent 5923199fa6
commit 53fc2a46ed
3 changed files with 134 additions and 8 deletions

View File

@ -69,7 +69,7 @@ export function chunkedBodyReader(h: Headers, r: BufReader): Deno.Reader {
const [chunkSizeString] = line.split(";");
const chunkSize = parseInt(chunkSizeString, 16);
if (Number.isNaN(chunkSize) || chunkSize < 0) {
throw new Error("Invalid chunk size");
throw new Deno.errors.InvalidData("Invalid chunk size");
}
if (chunkSize > 0) {
if (chunkSize > buf.byteLength) {

View File

@ -151,10 +151,15 @@ export class Server implements AsyncIterable<ServerRequest> {
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`),
});
// Try to send the "400 Bad Request" before closing the connection.
try {
await writeResponse(writer, {
status: 400,
body: encode(`${error.message}\r\n\r\n`),
});
} catch (error) {
// The connection is broken.
}
}
break;
}
@ -175,8 +180,14 @@ export class Server implements AsyncIterable<ServerRequest> {
this.untrackConnection(request.conn);
return;
}
// Consume unread body and trailers if receiver didn't consume those data
await request.finalize();
try {
// Consume unread body and trailers if receiver didn't consume those data
await request.finalize();
} catch (error) {
// Invalid data was received or the connection was closed.
break;
}
}
this.untrackConnection(conn);
@ -212,9 +223,12 @@ export class Server implements AsyncIterable<ServerRequest> {
conn = await this.listener.accept();
} catch (error) {
if (
// The listener is closed:
error instanceof Deno.errors.BadResource ||
// TLS handshake errors:
error instanceof Deno.errors.InvalidData ||
error instanceof Deno.errors.UnexpectedEof
error instanceof Deno.errors.UnexpectedEof ||
error instanceof Deno.errors.ConnectionReset
) {
return mux.add(this.acceptConnAndIterateHttpRequests(mux));
}

View File

@ -564,6 +564,118 @@ Deno.test({
},
});
Deno.test({
name: "[http] finalizing invalid chunked data closes connection",
async fn(): Promise<void> {
const serverRoutine = async (): Promise<void> => {
const server = serve(":8124");
for await (const req of server) {
await req.respond({ status: 200, body: "Hello, world!" });
break;
}
server.close();
};
const p = serverRoutine();
const conn = await Deno.connect({
hostname: "127.0.0.1",
port: 8124,
});
await Deno.writeAll(
conn,
encode(
"PUT / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\nzzzzzzz\r\nhello",
),
);
await conn.closeWrite();
const responseString = decode(await Deno.readAll(conn));
assertEquals(
responseString,
"HTTP/1.1 200 OK\r\ncontent-length: 13\r\n\r\nHello, world!",
);
conn.close();
await p;
},
});
Deno.test({
name: "[http] finalizing chunked unexpected EOF closes connection",
async fn(): Promise<void> {
const serverRoutine = async (): Promise<void> => {
const server = serve(":8124");
for await (const req of server) {
await req.respond({ status: 200, body: "Hello, world!" });
break;
}
server.close();
};
const p = serverRoutine();
const conn = await Deno.connect({
hostname: "127.0.0.1",
port: 8124,
});
await Deno.writeAll(
conn,
encode("PUT / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n5\r\nHello"),
);
conn.closeWrite();
const responseString = decode(await Deno.readAll(conn));
assertEquals(
responseString,
"HTTP/1.1 200 OK\r\ncontent-length: 13\r\n\r\nHello, world!",
);
conn.close();
await p;
},
});
Deno.test({
name:
"[http] receiving bad request from a closed connection should not throw",
async fn(): Promise<void> {
const server = serve(":8124");
const serverRoutine = async (): Promise<void> => {
for await (const req of server) {
await req.respond({ status: 200, body: "Hello, world!" });
}
};
const p = serverRoutine();
const conn = await Deno.connect({
hostname: "127.0.0.1",
port: 8124,
});
await Deno.writeAll(
conn,
encode([
// A normal request is required:
"GET / HTTP/1.1",
"Host: localhost",
"",
// The bad request:
"GET / HTTP/1.1",
"Host: localhost",
"INVALID!HEADER!",
"",
"",
].join("\r\n")),
);
// After sending the two requests, don't receive the reponses.
// Closing the connection now.
conn.close();
// The server will write responses to the closed connection,
// the first few `write()` calls will not throws, until the server received
// the TCP RST. So we need the normal request before the bad request to
// make the server do a few writes before it writes that `400` response.
// Wait for server to handle requests.
await delay(10);
server.close();
await p;
},
});
Deno.test({
name: "serveTLS Invalid Cert",
fn: async (): Promise<void> => {