fix(cli/fetch): set null body for null-body status (#5980)

3 files changed, 75 insertions, 25 deletions

diff --git a/cli/js/web/fetch.ts b/cli/js/web/fetch.ts
index 2f01e7ad7..c63fa698f 100644
--- a/cli/js/web/fetch.ts
+++ b/cli/js/web/fetch.ts
@@ -12,6 +12,9 @@ import { DomFileImpl } from "./dom_file.ts";
 import { getHeaderValueParams } from "./util.ts";
 import { ReadableStreamImpl } from "./streams/readable_stream.ts";
 
+const NULL_BODY_STATUS = [/* 101, */ 204, 205, 304];
+const REDIRECT_STATUS = [301, 302, 303, 307, 308];
+
 const responseData = new WeakMap();
 export class Response extends Body.Body implements domTypes.Response {
   readonly type: ResponseType;
@@ -45,7 +48,7 @@ export class Response extends Body.Body implements domTypes.Response {
     }
 
     // null body status
-    if (body && [/* 101, */ 204, 205, 304].includes(status)) {
+    if (body && NULL_BODY_STATUS.includes(status)) {
       throw new TypeError("Response with null body status cannot have body");
     }
 
@@ -288,32 +291,43 @@ export async function fetch(
     }
   }
 
+  let responseBody;
   let responseInit: ResponseInit = {};
   while (remRedirectCount) {
     const fetchResponse = await sendFetchReq(url, method, headers, body);
 
-    const responseBody = new ReadableStreamImpl({
-      async pull(controller: ReadableStreamDefaultController): Promise<void> {
-        try {
-          const b = new Uint8Array(1024 * 32);
-          const result = await read(fetchResponse.bodyRid, b);
-          if (result === null) {
+    if (
+      NULL_BODY_STATUS.includes(fetchResponse.status) ||
+      REDIRECT_STATUS.includes(fetchResponse.status)
+    ) {
+      // We won't use body of received response, so close it now
+      // otherwise it will be kept in resource table.
+      close(fetchResponse.bodyRid);
+      responseBody = null;
+    } else {
+      responseBody = new ReadableStreamImpl({
+        async pull(controller: ReadableStreamDefaultController): Promise<void> {
+          try {
+            const b = new Uint8Array(1024 * 32);
+            const result = await read(fetchResponse.bodyRid, b);
+            if (result === null) {
+              controller.close();
+              return close(fetchResponse.bodyRid);
+            }
+
+            controller.enqueue(b.subarray(0, result));
+          } catch (e) {
+            controller.error(e);
             controller.close();
-            return close(fetchResponse.bodyRid);
+            close(fetchResponse.bodyRid);
           }
-
-          controller.enqueue(b.subarray(0, result));
-        } catch (e) {
-          controller.error(e);
-          controller.close();
+        },
+        cancel(): void {
+          // When reader.cancel() is called
           close(fetchResponse.bodyRid);
-        }
-      },
-      cancel(): void {
-        // When reader.cancel() is called
-        close(fetchResponse.bodyRid);
-      },
-    });
+        },
+      });
+    }
 
     responseInit = {
       status: fetchResponse.status,
@@ -329,10 +343,7 @@ export async function fetch(
 
     const response = new Response(responseBody, responseInit);
 
-    if ([301, 302, 303, 307, 308].includes(fetchResponse.status)) {
-      // We won't use body of received response, so close it now
-      // otherwise it will be kept in resource table.
-      close(fetchResponse.bodyRid);
+    if (REDIRECT_STATUS.includes(fetchResponse.status)) {
       // We're in a redirect status
       switch ((init && init.redirect) || "follow") {
         case "error":
diff --git a/cli/tests/unit/fetch_test.ts b/cli/tests/unit/fetch_test.ts
index 5fd1cc469..c1dde92a9 100644
--- a/cli/tests/unit/fetch_test.ts
+++ b/cli/tests/unit/fetch_test.ts
@@ -713,3 +713,41 @@ unitTest(
     await res.body.cancel();
   }
 );
+
+unitTest(
+  { perms: { net: true } },
+  async function fetchNullBodyStatus(): Promise<void> {
+    const nullBodyStatus = [204, 205, 304];
+
+    for (const status of nullBodyStatus) {
+      const headers = new Headers([["x-status", String(status)]]);
+      const res = await fetch("http://localhost:4545/cli/tests/echo_server", {
+        body: "deno",
+        method: "POST",
+        headers,
+      });
+      assertEquals(res.body, null);
+      assertEquals(res.status, status);
+    }
+  }
+);
+
+unitTest(
+  { perms: { net: true } },
+  function fetchResponseConstructorNullBody(): void {
+    const nullBodyStatus = [204, 205, 304];
+
+    for (const status of nullBodyStatus) {
+      try {
+        new Response("deno", { status });
+        fail("Response with null body status cannot have body");
+      } catch (e) {
+        assert(e instanceof TypeError);
+        assertEquals(
+          e.message,
+          "Response with null body status cannot have body"
+        );
+      }
+    }
+  }
+);
diff --git a/tools/http_server.py b/tools/http_server.py
index 0c4d152f2..76f8e6e69 100755
--- a/tools/http_server.py
+++ b/tools/http_server.py
@@ -194,8 +194,9 @@ class ContentTypeHandler(QuietSimpleHTTPRequestHandler):
     def do_POST(self):
         # Simple echo server for request reflection
         if "echo_server" in self.path:
+            status = int(self.headers.getheader('x-status', "200"))
             self.protocol_version = 'HTTP/1.1'
-            self.send_response(200, 'OK')
+            self.send_response(status, 'OK')
             if self.headers.has_key('content-type'):
                 self.send_header('content-type',
                                  self.headers.getheader('content-type'))