From 3c88dffd3262e62f7def6f7437c6ce8a27f6cd73 Mon Sep 17 00:00:00 2001 From: Aaron O'Mullan Date: Sat, 25 Sep 2021 13:22:19 +0200 Subject: [PATCH] fix(http): panic when responding to a closed conn (#12216) Our oneshot receiver in `HyperService::call` would unwrap and panic, the `.await` on the oneshot receiver happens when the sender is dropped. The sender is dropped in `op_http_response` because: 1. We take `ResponseSenderResource` 2. Then get `ConnResource` and early exit on failure (conn already closed) 3. The taken sender then gets dropped in this early exit before any response is sent over the channel Fallbacking to returning a dummy response to hyper seems to be a fine quickfix --- cli/tests/unit/http_test.ts | 46 +++++++++++++++++++++++++++++++++++++ ext/http/lib.rs | 9 +++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/cli/tests/unit/http_test.ts b/cli/tests/unit/http_test.ts index f93d28efaf..2dc08cfa1d 100644 --- a/cli/tests/unit/http_test.ts +++ b/cli/tests/unit/http_test.ts @@ -936,3 +936,49 @@ unitTest( await promise; }, ); + +// https://github.com/denoland/deno/pull/12216 +unitTest( + { permissions: { net: true } }, + async function droppedConnSenderNoPanic() { + async function server(listener: Deno.Listener) { + const conn = await listener.accept(); + const http = Deno.serveHttp(conn); + + for (;;) { + const req = await http.nextRequest(); + if (req == null) break; + + nextloop() + .then(() => { + http.close(); + return req.respondWith(new Response("boom")); + }) + .catch(() => {}); + } + + try { + http.close(); + } catch { + "nop"; + } + + listener.close(); + } + + async function client() { + const resp = await fetch("http://127.0.0.1:8000/"); + await resp.body?.cancel(); + } + + function nextloop() { + return new Promise((resolve) => setTimeout(resolve, 0)); + } + + async function main() { + const listener = Deno.listen({ port: 8000 }); + await Promise.all([server(listener), client()]); + } + await main(); + }, +); diff --git a/ext/http/lib.rs b/ext/http/lib.rs index 9aef3d3702..aabf5d839c 100644 --- a/ext/http/lib.rs +++ b/ext/http/lib.rs @@ -108,7 +108,14 @@ impl HyperService> for Service { response_tx: resp_tx, }); - async move { Ok(resp_rx.await.unwrap()) }.boxed_local() + async move { + resp_rx.await.or_else(|_| + // Fallback dummy response in case sender was dropped due to closed conn + Response::builder() + .status(hyper::StatusCode::INTERNAL_SERVER_ERROR) + .body(vec![].into())) + } + .boxed_local() } }