1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-29 02:29:06 -05:00

fix(std/http): close connection on .respond() error (#3475)

This commit is contained in:
Kevin (Kun) "Kassimo" Qian 2019-12-11 16:46:03 -08:00 committed by Ry Dahl
parent 407195ea87
commit c3c69aff7e
2 changed files with 90 additions and 6 deletions

View file

@ -107,7 +107,7 @@ export class ServerRequest {
conn!: Conn; conn!: Conn;
r!: BufReader; r!: BufReader;
w!: BufWriter; w!: BufWriter;
done: Deferred<void> = deferred(); done: Deferred<Error | undefined> = deferred();
public async *bodyStream(): AsyncIterableIterator<Uint8Array> { public async *bodyStream(): AsyncIterableIterator<Uint8Array> {
if (this.headers.has("content-length")) { if (this.headers.has("content-length")) {
@ -193,11 +193,24 @@ export class ServerRequest {
} }
async respond(r: Response): Promise<void> { async respond(r: Response): Promise<void> {
let err: Error | undefined;
try {
// Write our response! // Write our response!
await writeResponse(this.w, r); await writeResponse(this.w, r);
} catch (e) {
try {
// Eagerly close on error.
this.conn.close();
} catch {}
err = e;
}
// Signal that this request has been processed and the next pipelined // Signal that this request has been processed and the next pipelined
// request on the same connection can be accepted. // request on the same connection can be accepted.
this.done.resolve(); this.done.resolve(err);
if (err) {
// Error during responding, rethrow.
throw err;
}
} }
} }
@ -338,7 +351,13 @@ export class Server implements AsyncIterable<ServerRequest> {
// Wait for the request to be processed before we accept a new request on // Wait for the request to be processed before we accept a new request on
// this connection. // this connection.
await req!.done; const procError = await req!.done;
if (procError) {
// Something bad happened during response.
// (likely other side closed during pipelined req)
// req.done implies this connection already closed, so we can just return.
return;
}
} }
if (req! === Deno.EOF) { if (req! === Deno.EOF) {

View file

@ -17,7 +17,7 @@ import {
readRequest, readRequest,
parseHTTPVersion parseHTTPVersion
} from "./server.ts"; } from "./server.ts";
import { delay } from "../util/async.ts"; import { delay, deferred } from "../util/async.ts";
import { import {
BufReader, BufReader,
BufWriter, BufWriter,
@ -590,4 +590,69 @@ test({
} }
}); });
// TODO(kevinkassimo): create a test that works on Windows.
// The following test is to ensure that if an error occurs during respond
// would result in connection closed. (such that fd/resource is freed).
// On *nix, a delayed second attempt to write to a CLOSE_WAIT connection would
// receive a RST and thus trigger an error during response for us to test.
// We need to find a way to similarly trigger an error on Windows so that
// we can test if connection is closed.
if (Deno.build.os !== "win") {
test({
name: "[http] respond error handling",
async fn(): Promise<void> {
const connClosedPromise = deferred();
const serverRoutine = async (): Promise<void> => {
let reqCount = 0;
const server = serve(":8124");
const serverRid = server.listener["rid"];
let connRid = -1;
for await (const req of server) {
connRid = req.conn.rid;
reqCount++;
await req.body();
await connClosedPromise;
try {
await req.respond({
body: new TextEncoder().encode("Hello World")
});
await delay(100);
req.done = deferred();
// This duplicate respond is to ensure we get a write failure from the
// other side. Our client would enter CLOSE_WAIT stage after close(),
// meaning first server .send (.respond) after close would still work.
// However, a second send would fail under RST, which is similar
// to the scenario where a failure happens during .respond
await req.respond({
body: new TextEncoder().encode("Hello World")
});
} catch {
break;
}
}
server.close();
const resources = Deno.resources();
assert(reqCount === 1);
// Server should be gone
assert(!(serverRid in resources));
// The connection should be destroyed
assert(!(connRid in resources));
};
const p = serverRoutine();
const conn = await Deno.dial({
hostname: "127.0.0.1",
port: 8124
});
await Deno.writeAll(
conn,
new TextEncoder().encode("GET / HTTP/1.1\r\n\r\n")
);
conn.close(); // abruptly closing connection before response.
// conn on server side enters CLOSE_WAIT state.
connClosedPromise.resolve();
await p;
}
});
}
runIfMain(import.meta); runIfMain(import.meta);