Fetch redirect fix (#21064)

Co-authored-by: pfgithub <6010774+pfgithub@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Ciro Spaciari <ciro.spaciari@gmail.com>

src/bun.js/webcore/Request.zig

@@ -7,6 +7,7 @@ _headers: ?*FetchHeaders = null,
 signal: ?*AbortSignal = null,
 body: *Body.Value.HiveRef,
 method: Method = Method.GET,
+redirect: FetchRedirect = .follow,
 request_context: JSC.API.AnyRequestContext = JSC.API.AnyRequestContext.Null,
 https: bool = false,
 weak_ptr_data: WeakRef.Data = .empty,
@@ -348,10 +349,12 @@ pub fn finalize(this: *Request) void {
 }
 
 pub fn getRedirect(
-    _: *Request,
+    this: *Request,
     globalThis: *JSC.JSGlobalObject,
 ) JSC.JSValue {
-    return ZigString.init("follow").toJS(globalThis);
+    return switch (this.redirect) {
+        inline else => |tag| ZigString.static(@tagName(tag)).toJS(globalThis),
+    };
 }
 pub fn getReferrer(
     this: *Request,
@@ -499,7 +502,7 @@ const Fields = enum {
     // referrerPolicy,
     // mode,
     // credentials,
-    // redirect,
+    redirect,
     // integrity,
     // keepalive,
     signal,
@@ -578,6 +581,11 @@ pub fn constructInto(globalThis: *JSC.JSGlobalObject, arguments: []const JSC.JSV
                     fields.insert(.method);
                 }
 
+                if (!fields.contains(.redirect)) {
+                    req.redirect = request.redirect;
+                    fields.insert(.redirect);
+                }
+
                 if (!fields.contains(.headers)) {
                     if (request.cloneHeaders(globalThis)) |headers| {
                         req._headers = headers;
@@ -706,6 +714,14 @@ pub fn constructInto(globalThis: *JSC.JSGlobalObject, arguments: []const JSC.JSV
 
             if (globalThis.hasException()) return error.JSError;
         }
+
+        // Extract redirect option
+        if (!fields.contains(.redirect)) {
+            if (try value.getOptionalEnum(globalThis, "redirect", FetchRedirect)) |redirect_value| {
+                req.redirect = redirect_value;
+                fields.insert(.redirect);
+            }
+        }
     }
 
     if (globalThis.hasException()) {
@@ -921,6 +937,7 @@ pub fn cloneInto(
         .body = body,
         .url = if (preserve_url) original_url else this.url.dupeRef(),
         .method = this.method,
+        .redirect = this.redirect,
         ._headers = this.cloneHeaders(globalThis),
     };
 
@@ -948,6 +965,7 @@ const MimeType = bun.http.MimeType;
 const JSC = bun.JSC;
 
 const Method = @import("../../http/Method.zig").Method;
+const FetchRedirect = @import("../../http/FetchRedirect.zig").FetchRedirect;
 const FetchHeaders = bun.webcore.FetchHeaders;
 const AbortSignal = JSC.WebCore.AbortSignal;
 const Output = bun.Output;

src/bun.js/webcore/fetch.zig

@@ -1865,6 +1865,12 @@ pub fn Bun__fetch_(
 
     // redirect: "follow" | "error" | "manual" | undefined;
     redirect_type = extract_redirect_type: {
+        // First, try to use the Request object's redirect if available
+        if (request) |req| {
+            redirect_type = req.redirect;
+        }
+
+        // Then check options/init objects which can override the Request's redirect
         const objects_to_try = [_]JSValue{
             options_object orelse .zero,
             request_init_object orelse .zero,

test/no-validate-exceptions.txt

@@ -535,6 +535,7 @@ test/js/web/fetch/headers.test.ts
 test/js/web/fetch/headers.undici.test.ts
 test/js/web/fetch/stream-fast-path.test.ts
 test/js/web/fetch/utf8-bom.test.ts
+test/regression/issue/test-21049.test.ts
 test/js/web/html/FormData.test.ts
 test/js/web/request/request-clone-leak.test.ts
 test/js/web/request/request-subclass.test.ts

test/regression/issue/test-21049.test.ts

@@ -0,0 +1,169 @@
+import { expect, test } from "bun:test";
+import { bunEnv, bunExe } from "harness";
+
+test("fetch with Request object respects redirect: 'manual' option", async () => {
+  // Test server that redirects
+  await using server = Bun.serve({
+    port: 0,
+    fetch(req) {
+      const url = new URL(req.url);
+      if (url.pathname === "/redirect") {
+        return new Response(null, {
+          status: 302,
+          headers: {
+            Location: "/target",
+          },
+        });
+      }
+      if (url.pathname === "/target") {
+        return new Response("Target reached", { status: 200 });
+      }
+      return new Response("Not found", { status: 404 });
+    },
+  });
+
+  // Test 1: Direct fetch with redirect: "manual" (currently works)
+  const directResponse = await fetch(`${server.url}/redirect`, {
+    redirect: "manual",
+  });
+  expect(directResponse.status).toBe(302);
+  expect(directResponse.url).toBe(`${server.url}/redirect`);
+  expect(directResponse.headers.get("location")).toBe("/target");
+  expect(directResponse.redirected).toBe(false);
+
+  // Test 2: Fetch with Request object and redirect: "manual" (currently broken)
+  const request = new Request(`${server.url}/redirect`, {
+    redirect: "manual",
+  });
+  const requestResponse = await fetch(request);
+  expect(requestResponse.status).toBe(302);
+  expect(requestResponse.url).toBe(`${server.url}/redirect`); // This should be the original URL, not the target
+  expect(requestResponse.headers.get("location")).toBe("/target");
+  expect(requestResponse.redirected).toBe(false);
+
+  // Test 3: Verify the behavior matches Node.js and Deno
+  const testScript = `
+    async function main() {
+      const request = new Request("${server.url}/redirect", {
+        redirect: "manual",
+      });
+      const response = await fetch(request);
+      console.log(JSON.stringify({
+        status: response.status,
+        url: response.url,
+        redirected: response.redirected,
+        location: response.headers.get("location")
+      }));
+    }
+    main();
+  `;
+
+  // Run with Bun
+  await using bunProc = Bun.spawn({
+    cmd: [bunExe(), "-e", testScript],
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+
+  const [bunStdout, bunExitCode] = await Promise.all([new Response(bunProc.stdout).text(), bunProc.exited]);
+
+  expect(bunExitCode).toBe(0);
+  const bunResult = JSON.parse(bunStdout.trim());
+
+  // The bug: Bun follows the redirect even though redirect: "manual" was specified
+  // Expected: status=302, url=original, redirected=false
+  // Actual (bug): status=200, url=target, redirected=true
+  expect(bunResult).toEqual({
+    status: 302,
+    url: `${server.url}/redirect`,
+    redirected: false,
+    location: "/target",
+  });
+});
+
+// Additional test to verify it works with external redirects
+test("fetch with Request object respects redirect: 'manual' for external URLs", async () => {
+  // This test uses a real URL that redirects
+  const request = new Request("https://w3id.org/security/v1", {
+    redirect: "manual",
+  });
+
+  const response = await fetch(request);
+
+  // When redirect: "manual" is set, we should get the redirect response
+  expect([301, 302, 303, 307, 308]).toContain(response.status); // Any redirect status
+  expect(response.url).toBe("https://w3id.org/security/v1");
+  expect(response.redirected).toBe(false);
+  expect(response.headers.get("location")).toBeTruthy();
+});
+
+// Test edge case: fetch with options but no redirect should use Request's redirect
+test("fetch with Request respects redirect when fetch has other options but no redirect", async () => {
+  // Test server that redirects
+  await using server = Bun.serve({
+    port: 0,
+    fetch(req) {
+      const url = new URL(req.url);
+      if (url.pathname === "/redirect") {
+        return new Response(null, {
+          status: 302,
+          headers: {
+            Location: "/target",
+          },
+        });
+      }
+      if (url.pathname === "/target") {
+        return new Response("Target reached", {
+          status: 200,
+          headers: {
+            "X-Target": "true",
+          },
+        });
+      }
+      return new Response("Not found", { status: 404 });
+    },
+  });
+
+  // Create a Request with redirect: "manual"
+  const request = new Request(`${server.url}/redirect`, {
+    redirect: "manual",
+    headers: {
+      "X-Original": "request",
+    },
+  });
+
+  // Test 1: fetch with other options but NO redirect option
+  // Should use the Request's redirect: "manual"
+  const response1 = await fetch(request, {
+    headers: {
+      "X-Additional": "fetch-option",
+    },
+    // Note: no redirect option here
+  });
+
+  expect(response1.status).toBe(302);
+  expect(response1.url).toBe(`${server.url}/redirect`);
+  expect(response1.redirected).toBe(false);
+  expect(response1.headers.get("location")).toBe("/target");
+
+  // Test 2: fetch with explicit redirect option should override Request's redirect
+  const response2 = await fetch(request, {
+    headers: {
+      "X-Additional": "fetch-option",
+    },
+    redirect: "follow", // Explicitly override
+  });
+
+  expect(response2.status).toBe(200);
+  expect(response2.url).toBe(new URL("/target", server.url).href);
+  expect(response2.redirected).toBe(true);
+  expect(response2.headers.get("X-Target")).toBe("true");
+
+  // Test 3: fetch with empty options object should use Request's redirect
+  const response3 = await fetch(request, {});
+
+  expect(response3.status).toBe(302);
+  expect(response3.url).toBe(`${server.url}/redirect`);
+  expect(response3.redirected).toBe(false);
+});