summaryrefslogtreecommitdiff
path: root/ext/http/http_next.rs
diff options
context:
space:
mode:
authorMatt Mastracci <matthew@mastracci.com>2023-08-20 19:35:26 -0600
committerGitHub <noreply@github.com>2023-08-21 01:35:26 +0000
commit576d0db372c3f4c9b01caecdbe2360a73de6d36d (patch)
tree7b083558f91a156181f870ae4c039cbcf8cd5ff8 /ext/http/http_next.rs
parentefdf0bbd9b76b8b1b5d0374a63358192534f22ad (diff)
fix(ext/http): ensure request body resource lives as long as response is alive (#20206)
Deno.serve's fast streaming implementation was not keeping the request body resource ID alive. We were taking the `Rc<Resource>` from the resource table during the response, so a hairpin duplex response that fed back the request body would work. However, if any JS code attempted to read from the request body (which requires the resource ID to be valid), the response would fail with a difficult-to-diagnose "EOF" error. This was affecting more complex duplex uses of `Deno.fetch` (though as far as I can tell was unreported). Simple test: ```ts const reader = request.body.getReader(); return new Response( new ReadableStream({ async pull(controller) { const { done, value } = await reader.read(); if (done) { controller.close(); } else { controller.enqueue(value); } }, }), ``` And then attempt to use the stream in duplex mode: ```ts async function testDuplex( reader: ReadableStreamDefaultReader<Uint8Array>, writable: WritableStreamDefaultWriter<Uint8Array>, ) { await writable.write(new Uint8Array([1])); const chunk1 = await reader.read(); assert(!chunk1.done); assertEquals(chunk1.value, new Uint8Array([1])); await writable.write(new Uint8Array([2])); const chunk2 = await reader.read(); assert(!chunk2.done); assertEquals(chunk2.value, new Uint8Array([2])); await writable.close(); const chunk3 = await reader.read(); assert(chunk3.done); } ``` In older versions of Deno, this would just lock up. I believe after 23ff0e722e3c4b0827940853c53c5ee2ede5ec9f, it started throwing a more explicit error: ``` httpServerStreamDuplexJavascript => ./cli/tests/unit/serve_test.ts:1339:6 error: TypeError: request or response body error: error reading a body from connection: Connection reset by peer (os error 54) at async Object.pull (ext:deno_web/06_streams.js:810:27) ```
Diffstat (limited to 'ext/http/http_next.rs')
-rw-r--r--ext/http/http_next.rs21
1 files changed, 16 insertions, 5 deletions
diff --git a/ext/http/http_next.rs b/ext/http/http_next.rs
index 60ef83b0f..17e9befe2 100644
--- a/ext/http/http_next.rs
+++ b/ext/http/http_next.rs
@@ -14,6 +14,7 @@ use crate::slab::slab_drop;
use crate::slab::slab_get;
use crate::slab::slab_init;
use crate::slab::slab_insert;
+use crate::slab::HttpRequestBodyAutocloser;
use crate::slab::SlabId;
use crate::websocket_upgrade::WebSocketUpgrade;
use crate::LocalExecutor;
@@ -376,13 +377,20 @@ pub fn op_http_get_request_headers<'scope>(
#[op(fast)]
pub fn op_http_read_request_body(
- state: &mut OpState,
+ state: Rc<RefCell<OpState>>,
slab_id: SlabId,
) -> ResourceId {
let mut http = slab_get(slab_id);
- let incoming = http.take_body();
- let body_resource = Rc::new(HttpRequestBody::new(incoming));
- state.resource_table.add_rc(body_resource)
+ let rid = if let Some(incoming) = http.take_body() {
+ let body_resource = Rc::new(HttpRequestBody::new(incoming));
+ state.borrow_mut().resource_table.add_rc(body_resource)
+ } else {
+ // This should not be possible, but rather than panicking we'll return an invalid
+ // resource value to JavaScript.
+ ResourceId::MAX
+ };
+ http.put_resource(HttpRequestBodyAutocloser::new(rid, state.clone()));
+ rid
}
#[op2(fast)]
@@ -577,6 +585,7 @@ fn set_response(
response_fn: impl FnOnce(Compression) -> ResponseBytesInner,
) {
let mut http = slab_get(slab_id);
+ let resource = http.take_resource();
let compression = is_request_compressible(&http.request_parts().headers);
let response = http.response();
let compression = modify_compressibility_from_response(
@@ -584,7 +593,9 @@ fn set_response(
length,
response.headers_mut(),
);
- response.body_mut().initialize(response_fn(compression));
+ response
+ .body_mut()
+ .initialize(response_fn(compression), resource);
// The Javascript code should never provide a status that is invalid here (see 23_response.js), so we
// will quitely ignore invalid values.