From c40d5040cd577aa4ebe552242a06163fbcbc3d4b Mon Sep 17 00:00:00 2001
From: Luca Casonato <hello@lcas.dev>
Date: Mon, 11 Oct 2021 18:39:55 +0200
Subject: [PATCH] fix(http): don't expose body on GET/HEAD requests (#12260)

GET/HEAD requests can't have bodies according to `fetch` spec. This
commit changes the HTTP server to hide request bodies for requests with
GET or HEAD methods.
---
 cli/tests/unit/http_test.ts | 34 ++++++++++++++++++++++++++++++++++
 ext/http/01_http.js         |  7 ++++++-
 ext/http/lib.rs             | 11 +++++------
 3 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/cli/tests/unit/http_test.ts b/cli/tests/unit/http_test.ts
index fe6f1aba2b..0f23a2bb56 100644
--- a/cli/tests/unit/http_test.ts
+++ b/cli/tests/unit/http_test.ts
@@ -67,6 +67,40 @@ unitTest({ permissions: { net: true } }, async function httpServerBasic() {
   await promise;
 });
 
+unitTest(
+  { permissions: { net: true } },
+  async function httpServerGetRequestBody() {
+    const promise = (async () => {
+      const listener = Deno.listen({ port: 4501 });
+      const conn = await listener.accept();
+      listener.close();
+      const httpConn = Deno.serveHttp(conn);
+      const e = await httpConn.nextRequest();
+      assert(e);
+      const { request, respondWith } = e;
+      assertEquals(request.body, null);
+      await respondWith(new Response("", { headers: {} }));
+      httpConn.close();
+    })();
+
+    const conn = await Deno.connect({ port: 4501 });
+    // Send GET request with a body + content-length.
+    const encoder = new TextEncoder();
+    const body =
+      `GET / HTTP/1.1\r\nHost: 127.0.0.1:4501\r\nContent-Length: 5\r\n\r\n12345`;
+    const writeResult = await conn.write(encoder.encode(body));
+    assertEquals(body.length, writeResult);
+
+    const resp = new Uint8Array(200);
+    const readResult = await conn.read(resp);
+    assertEquals(readResult, 115);
+
+    conn.close();
+
+    await promise;
+  },
+);
+
 unitTest(
   { permissions: { net: true } },
   async function httpServerStreamResponse() {
diff --git a/ext/http/01_http.js b/ext/http/01_http.js
index 8c4f08aab7..4e6b8fc003 100644
--- a/ext/http/01_http.js
+++ b/ext/http/01_http.js
@@ -93,7 +93,12 @@
       let body = null;
       if (typeof requestRid === "number") {
         SetPrototypeAdd(this.managedResources, requestRid);
-        body = createRequestBodyStream(this, requestRid);
+        // There might be a body, but we don't expose it for GET/HEAD requests.
+        // It will be closed automatically once the request has been handled and
+        // the response has been sent.
+        if (method !== "GET" && method !== "HEAD") {
+          body = createRequestBodyStream(this, requestRid);
+        }
       }
 
       const innerRequest = newInnerRequest(
diff --git a/ext/http/lib.rs b/ext/http/lib.rs
index a4e9085376..09874fccc1 100644
--- a/ext/http/lib.rs
+++ b/ext/http/lib.rs
@@ -29,6 +29,7 @@ use hyper::http;
 use hyper::server::conn::Http;
 use hyper::service::Service as HyperService;
 use hyper::Body;
+use hyper::Method;
 use hyper::Request;
 use hyper::Response;
 use serde::Deserialize;
@@ -243,13 +244,11 @@ fn prepare_next_request(
   let url = req_url(&req, scheme, addr)?;
 
   let is_websocket = is_websocket_request(&req);
-  let has_body = if let Some(exact_size) = req.size_hint().exact() {
-    exact_size > 0
-  } else {
-    true
-  };
+  let can_have_body = !matches!(*req.method(), Method::GET | Method::HEAD);
+  let has_body =
+    is_websocket || (can_have_body && req.size_hint().exact() != Some(0));
 
-  let maybe_request_rid = if is_websocket || has_body {
+  let maybe_request_rid = if has_body {
     let request_rid = state.resource_table.add(RequestResource {
       conn_rid,
       inner: AsyncRefCell::new(RequestOrStreamReader::Request(Some(req))),