1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-23 15:49:44 -05:00

fix(std/http): fix error handling in the request iterator (#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 GitHub
parent b6fa6d6aac
commit 60d9ab08db
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
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 [chunkSizeString] = line.split(";");
const chunkSize = parseInt(chunkSizeString, 16); const chunkSize = parseInt(chunkSizeString, 16);
if (Number.isNaN(chunkSize) || chunkSize < 0) { 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 > 0) {
if (chunkSize > buf.byteLength) { if (chunkSize > buf.byteLength) {

View file

@ -151,10 +151,15 @@ export class Server implements AsyncIterable<ServerRequest> {
error instanceof Deno.errors.UnexpectedEof error instanceof Deno.errors.UnexpectedEof
) { ) {
// An error was thrown while parsing request headers. // An error was thrown while parsing request headers.
await writeResponse(writer, { // Try to send the "400 Bad Request" before closing the connection.
status: 400, try {
body: encode(`${error.message}\r\n\r\n`), await writeResponse(writer, {
}); status: 400,
body: encode(`${error.message}\r\n\r\n`),
});
} catch (error) {
// The connection is broken.
}
} }
break; break;
} }
@ -175,8 +180,14 @@ export class Server implements AsyncIterable<ServerRequest> {
this.untrackConnection(request.conn); this.untrackConnection(request.conn);
return; 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); this.untrackConnection(conn);
@ -212,9 +223,12 @@ export class Server implements AsyncIterable<ServerRequest> {
conn = await this.listener.accept(); conn = await this.listener.accept();
} catch (error) { } catch (error) {
if ( if (
// The listener is closed:
error instanceof Deno.errors.BadResource || error instanceof Deno.errors.BadResource ||
// TLS handshake errors:
error instanceof Deno.errors.InvalidData || 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)); 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({ Deno.test({
name: "serveTLS Invalid Cert", name: "serveTLS Invalid Cert",
fn: async (): Promise<void> => { fn: async (): Promise<void> => {