Align fetch() redirect behavior with spec (#5729)

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>

2 files changed, 162 insertions, 20 deletions

diff --git a/src/http_client_async.zig b/src/http_client_async.zig
index a78358066..00f11626f 100644
--- a/src/http_client_async.zig
+++ b/src/http_client_async.zig
@@ -3407,12 +3407,16 @@ pub fn handleResponseMetadata(
         this.proxy_tunneling = false;
     }
 
+    const status_code = response.status_code;
+
     // if is no redirect or if is redirect == "manual" just proceed
-    const is_redirect = response.status_code >= 300 and response.status_code <= 399;
+    const is_redirect = status_code >= 300 and status_code <= 399;
     if (is_redirect) {
         if (this.redirect_type == FetchRedirect.follow and location.len > 0 and this.remaining_redirect_count > 0) {
-            switch (response.status_code) {
+            switch (status_code) {
                 302, 301, 307, 308, 303 => {
+                    var is_same_origin = true;
+
                     if (strings.indexOf(location, "://")) |i| {
                         var url_buf = URLBufferPool.get(default_allocator);
 
@@ -3443,7 +3447,9 @@ pub fn handleResponseMetadata(
                             bun.copy(u8, &url_buf.data, location);
                         }
 
-                        this.url = URL.parse(url_buf.data[0..url_buf_len]);
+                        const new_url = URL.parse(url_buf.data[0..url_buf_len]);
+                        is_same_origin = strings.eqlCaseInsensitiveASCII(strings.withoutTrailingSlash(new_url.origin), strings.withoutTrailingSlash(this.url.origin), true);
+                        this.url = new_url;
                         this.redirect = url_buf;
                     } else if (strings.hasPrefixComptime(location, "//")) {
                         var url_buf = URLBufferPool.get(default_allocator);
@@ -3467,7 +3473,9 @@ pub fn handleResponseMetadata(
                             url_buf_len += "https:".len;
                         }
 
-                        this.url = URL.parse(url_buf.data[0..url_buf_len]);
+                        const new_url = URL.parse(url_buf.data[0..url_buf_len]);
+                        is_same_origin = strings.eqlCaseInsensitiveASCII(strings.withoutTrailingSlash(new_url.origin), strings.withoutTrailingSlash(this.url.origin), true);
+                        this.url = new_url;
                         this.redirect = url_buf;
                     } else {
                         var url_buf = URLBufferPool.get(default_allocator);
@@ -3488,28 +3496,41 @@ pub fn handleResponseMetadata(
                             ) catch return error.RedirectURLTooLong);
                         }
 
+                        is_same_origin = strings.eqlCaseInsensitiveASCII(strings.withoutTrailingSlash(this.url.origin), strings.withoutTrailingSlash(original_url.origin), true);
                         deferred_redirect.* = this.redirect;
                         this.redirect = url_buf;
                     }
 
-                    // Note: RFC 1945 and RFC 2068 specify that the client is not allowed to change
-                    // the method on the redirected request. However, most existing user agent
-                    // implementations treat 302 as if it were a 303 response, performing a GET on
-                    // the Location field-value regardless of the original request method. The
-                    // status codes 303 and 307 have been added for servers that wish to make
-                    // unambiguously clear which kind of reaction is expected of the client.
-                    if (response.status_code == 302) {
-                        switch (this.method) {
-                            .GET, .HEAD => {},
-                            else => {
-                                this.method = .GET;
-                            },
-                        }
+                    // If one of the following is true
+                    // - internalResponse’s status is 301 or 302 and request’s method is `POST`
+                    // - internalResponse’s status is 303 and request’s method is not `GET` or `HEAD`
+                    // then:
+                    if (((status_code == 301 or status_code == 302) and this.method == .POST) or
+                        (status_code == 303 and this.method != .GET and this.method != .HEAD))
+                    {
+                        // - Set request’s method to `GET` and request’s body to null.
+                        this.method = .GET;
+                        // - For each headerName of request-body-header name, delete headerName from request’s header list.
+                        this.header_entries.len = 0;
                     }
 
-                    // https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/303
-                    if (response.status_code == 303 and this.method != .HEAD) {
-                        this.method = .GET;
+                    // https://fetch.spec.whatwg.org/#concept-http-redirect-fetch
+                    // If request’s current URL’s origin is not same origin with
+                    // locationURL’s origin, then for each headerName of CORS
+                    // non-wildcard request-header name, delete headerName from
+                    // request’s header list.
+                    if (!is_same_origin and this.header_entries.len > 0) {
+                        const authorization_header_hash = comptime hashHeaderConst("Authorization");
+                        for (this.header_entries.items(.name), 0..) |name_ptr, i| {
+                            const name = this.headerStr(name_ptr);
+                            if (name.len == "Authorization".len) {
+                                const hash = hashHeaderName(name);
+                                if (hash == authorization_header_hash) {
+                                    this.header_entries.swapRemove(i);
+                                    break;
+                                }
+                            }
+                        }
                     }
 
                     return error.Redirect;
diff --git a/test/js/web/fetch/fetch.test.ts b/test/js/web/fetch/fetch.test.ts
index fc4ce7a18..23e78ac48 100644
--- a/test/js/web/fetch/fetch.test.ts
+++ b/test/js/web/fetch/fetch.test.ts
@@ -1475,3 +1475,124 @@ it("should work with http 100 continue on the same buffer", async () => {
     server?.close();
   }
 });
+
+describe("should strip headers", () => {
+  it("status code 303", async () => {
+    const server = Bun.serve({
+      port: 0,
+      async fetch(request: Request) {
+        if (request.url.endsWith("/redirect")) {
+          return new Response("hello", {
+            headers: {
+              ...request.headers,
+              "Location": "/redirected",
+            },
+            status: 303,
+          });
+        }
+
+        return new Response("hello", {
+          headers: request.headers,
+        });
+      },
+    });
+
+    const { headers, url, redirected } = await fetch(`http://${server.hostname}:${server.port}/redirect`, {
+      method: "POST",
+      headers: {
+        "I-Am-Here": "yes",
+      },
+    });
+
+    expect(headers.get("I-Am-Here")).toBeNull();
+    expect(url).toEndWith("/redirected");
+    expect(redirected).toBe(true);
+    server.stop(true);
+  });
+
+  it("cross-origin status code 302", async () => {
+    const server1 = Bun.serve({
+      port: 0,
+      async fetch(request: Request) {
+        if (request.url.endsWith("/redirect")) {
+          return new Response("hello", {
+            headers: {
+              ...request.headers,
+              "Location": `http://${server2.hostname}:${server2.port}/redirected`,
+            },
+            status: 302,
+          });
+        }
+
+        return new Response("hello", {
+          headers: request.headers,
+        });
+      },
+    });
+
+    const server2 = Bun.serve({
+      port: 0,
+      async fetch(request: Request, server) {
+        if (request.url.endsWith("/redirect")) {
+          return new Response("hello", {
+            headers: {
+              ...request.headers,
+              "Location": `http://${server.hostname}:${server.port}/redirected`,
+            },
+            status: 302,
+          });
+        }
+
+        return new Response("hello", {
+          headers: request.headers,
+        });
+      },
+    });
+
+    const { headers, url, redirected } = await fetch(`http://${server1.hostname}:${server1.port}/redirect`, {
+      method: "GET",
+      headers: {
+        "Authorization": "yes",
+      },
+    });
+
+    expect(headers.get("Authorization")).toBeNull();
+    expect(url).toEndWith("/redirected");
+    expect(redirected).toBe(true);
+    server1.stop(true);
+    server2.stop(true);
+  });
+});
+
+it("same-origin status code 302 should not strip headers", async () => {
+  const server = Bun.serve({
+    port: 0,
+    async fetch(request: Request, server) {
+      if (request.url.endsWith("/redirect")) {
+        return new Response("hello", {
+          headers: {
+            ...request.headers,
+            "Location": `http://${server.hostname}:${server.port}/redirected`,
+          },
+          status: 302,
+        });
+      }
+
+      return new Response("hello", {
+        headers: request.headers,
+      });
+    },
+  });
+
+  const { headers, url, redirected } = await fetch(`http://${server.hostname}:${server.port}/redirect`, {
+    method: "GET",
+    headers: {
+      "Authorization": "yes",
+    },
+  });
+
+  expect(headers.get("Authorization")).toEqual("yes");
+  expect(url).toEndWith("/redirected");
+  expect(redirected).toBe(true);
+  server.stop(true);
+});