diff --git a/src/bun.js/webcore/Request.zig b/src/bun.js/webcore/Request.zig index fb2b249e1c..ad5ab8d920 100644 --- a/src/bun.js/webcore/Request.zig +++ b/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; diff --git a/src/bun.js/webcore/fetch.zig b/src/bun.js/webcore/fetch.zig index 5c84f06066..d91ac40176 100644 --- a/src/bun.js/webcore/fetch.zig +++ b/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, diff --git a/test/no-validate-exceptions.txt b/test/no-validate-exceptions.txt index db4370fec0..6e2885312c 100644 --- a/test/no-validate-exceptions.txt +++ b/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 diff --git a/test/regression/issue/test-21049.test.ts b/test/regression/issue/test-21049.test.ts new file mode 100644 index 0000000000..61b32a3b9b --- /dev/null +++ b/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); +});