diff options
| author | Divy Srivastava <dj.srivastava23@gmail.com> | 2022-07-12 23:01:37 +0530 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2022-07-12 13:31:37 -0400 |
| commit | 5b26a4a30e67b56232dd306b4735b759df9b16d1 (patch) | |
| tree | 840612397bf0593cb7e73ac727bdfb75619e43e8 | |
| parent | dcd05d4cb9780b7ab6753de5267bc59d834d8022 (diff) | |
fix(ext/http): reading headers with ongoing body reader (#15161)
| -rw-r--r-- | cli/tests/unit/http_test.ts | 62 | ||||
| -rw-r--r-- | ext/fetch/23_request.js | 6 | ||||
| -rw-r--r-- | ext/http/lib.rs | 18 |
3 files changed, 77 insertions, 9 deletions
diff --git a/cli/tests/unit/http_test.ts b/cli/tests/unit/http_test.ts index 779f11e56..9afe22553 100644 --- a/cli/tests/unit/http_test.ts +++ b/cli/tests/unit/http_test.ts @@ -73,6 +73,68 @@ Deno.test({ permissions: { net: true } }, async function httpServerBasic() { await promise; }); +// https://github.com/denoland/deno/issues/15107 +Deno.test( + { permissions: { net: true } }, + async function httpLazyHeadersIssue15107() { + let headers: Headers; + const promise = (async () => { + const listener = Deno.listen({ port: 2333 }); + const conn = await listener.accept(); + listener.close(); + const httpConn = Deno.serveHttp(conn); + const e = await httpConn.nextRequest(); + assert(e); + const { request } = e; + request.text(); + headers = request.headers; + httpConn.close(); + })(); + + const conn = await Deno.connect({ port: 2333 }); + // Send GET request with a body + content-length. + const encoder = new TextEncoder(); + const body = + `GET / HTTP/1.1\r\nHost: 127.0.0.1:2333\r\nContent-Length: 5\r\n\r\n12345`; + const writeResult = await conn.write(encoder.encode(body)); + assertEquals(body.length, writeResult); + await promise; + conn.close(); + assertEquals(headers!.get("content-length"), "5"); + }, +); + +Deno.test( + { permissions: { net: true } }, + async function httpReadHeadersAfterClose() { + const promise = (async () => { + const listener = Deno.listen({ port: 2334 }); + const conn = await listener.accept(); + listener.close(); + const httpConn = Deno.serveHttp(conn); + const e = await httpConn.nextRequest(); + assert(e); + const { request, respondWith } = e; + + await request.text(); // Read body + await respondWith(new Response("Hello World")); // Closes request + + assertThrows(() => request.headers, TypeError, "request closed"); + httpConn.close(); + })(); + + const conn = await Deno.connect({ port: 2334 }); + // Send GET request with a body + content-length. + const encoder = new TextEncoder(); + const body = + `GET / HTTP/1.1\r\nHost: 127.0.0.1:2333\r\nContent-Length: 5\r\n\r\n12345`; + const writeResult = await conn.write(encoder.encode(body)); + assertEquals(body.length, writeResult); + await promise; + conn.close(); + }, +); + Deno.test( { permissions: { net: true } }, async function httpServerGetRequestBody() { diff --git a/ext/fetch/23_request.js b/ext/fetch/23_request.js index 3058db6f7..cbc6a887b 100644 --- a/ext/fetch/23_request.js +++ b/ext/fetch/23_request.js @@ -81,7 +81,11 @@ headerListInner: null, get headerList() { if (this.headerListInner === null) { - this.headerListInner = headerList(); + try { + this.headerListInner = headerList(); + } catch { + throw new TypeError("cannot read headers: request closed"); + } } return this.headerListInner; }, diff --git a/ext/http/lib.rs b/ext/http/lib.rs index 6a900eb8e..27d277654 100644 --- a/ext/http/lib.rs +++ b/ext/http/lib.rs @@ -44,6 +44,7 @@ use hyper::header::HeaderValue; use hyper::server::conn::Http; use hyper::service::Service; use hyper::Body; +use hyper::HeaderMap; use hyper::Request; use hyper::Response; use serde::Serialize; @@ -341,7 +342,7 @@ impl Resource for HttpStreamResource { /// The read half of an HTTP stream. pub enum HttpRequestReader { Headers(Request<Body>), - Body(Peekable<Body>), + Body(HeaderMap<HeaderValue>, Peekable<Body>), Closed, } @@ -450,7 +451,7 @@ fn req_url( } fn req_headers( - req: &hyper::Request<hyper::Body>, + header_map: &HeaderMap<HeaderValue>, ) -> Vec<(ByteString, ByteString)> { // We treat cookies specially, because we don't want them to get them // mangled by the `Headers` object in JS. What we do is take all cookie @@ -459,8 +460,8 @@ fn req_headers( let cookie_sep = "; ".as_bytes(); let mut cookies = vec![]; - let mut headers = Vec::with_capacity(req.headers().len()); - for (name, value) in req.headers().iter() { + let mut headers = Vec::with_capacity(header_map.len()); + for (name, value) in header_map.iter() { if name == hyper::header::COOKIE { cookies.push(value.as_bytes()); } else { @@ -557,7 +558,8 @@ fn op_http_headers( .try_borrow() .ok_or_else(|| http_error("already in use"))?; match &*rd { - HttpRequestReader::Headers(request) => Ok(req_headers(request)), + HttpRequestReader::Headers(request) => Ok(req_headers(request.headers())), + HttpRequestReader::Body(headers, _) => Ok(req_headers(headers)), _ => unreachable!(), } } @@ -829,13 +831,13 @@ async fn op_http_read( let body = loop { match &mut *rd { HttpRequestReader::Headers(_) => {} - HttpRequestReader::Body(body) => break body, + HttpRequestReader::Body(_, body) => break body, HttpRequestReader::Closed => return Ok(0), } match take(&mut *rd) { HttpRequestReader::Headers(request) => { - let body = request.into_body().peekable(); - *rd = HttpRequestReader::Body(body); + let (parts, body) = request.into_parts(); + *rd = HttpRequestReader::Body(parts.headers, body.peekable()); } _ => unreachable!(), }; |