diff --git a/docs/runtime/networking/fetch.mdx b/docs/runtime/networking/fetch.mdx index afe904b75e09..9177c45902de 100644 --- a/docs/runtime/networking/fetch.mdx +++ b/docs/runtime/networking/fetch.mdx @@ -260,11 +260,42 @@ const response = await fetch("http://example.com", { // Disable connection reuse for this request keepalive: false, + // How many redirects to follow (default: 20, maximum: 126) + maxRedirects: 5, + // Debug logging level verbose: true, // or "curl" for more detailed output }); ``` +### Redirects + +By default, `fetch` follows up to 20 redirects, the limit the [fetch specification](https://fetch.spec.whatwg.org/#http-redirect-fetch) mandates. The 21st redirect rejects with a `TypeError`: + +```ts +try { + await fetch("http://example.com/a-very-long-redirect-chain"); +} catch (error) { + error instanceof TypeError; // true + error.code; // "TooManyRedirects" +} +``` + +Use the `maxRedirects` option to raise or lower the limit. It accepts any integer from `0` (reject the first redirect) through `126`; larger values are clamped to `126`. + +```ts +// Follow at most 5 redirects +await fetch("http://example.com/", { maxRedirects: 5 }); +``` + +The standard `redirect` option is also available. `"error"` rejects with a `TypeError` whose `code` is `"UnexpectedRedirect"`. `"manual"` resolves with the raw redirect response, so its status and `Location` header are readable; browsers instead return an opaque-redirect filtered response whose status is `0` and whose headers are hidden, and Bun (like Node.js) does not apply that filtering. + +```ts +const response = await fetch("http://example.com/", { redirect: "manual" }); +response.status; // 302 +response.headers.get("location"); // the redirect target +``` + ### Protocol support Beyond HTTP(S), Bun's fetch supports several additional protocols: @@ -335,6 +366,7 @@ Bun's fetch implementation includes several specific error cases: - Using the `proxy` and `unix` options together throws an error - TLS certificate validation failures when `rejectUnauthorized` is true (or undefined) - S3 operations may throw specific errors related to authentication or permissions +- Redirect failures reject with a `TypeError` carrying a `code` property, such as `"TooManyRedirects"` or `"UnexpectedRedirect"` ### Content-Type handling diff --git a/packages/bun-types/globals.d.ts b/packages/bun-types/globals.d.ts index d88036b35841..f315010de299 100644 --- a/packages/bun-types/globals.d.ts +++ b/packages/bun-types/globals.d.ts @@ -2070,11 +2070,15 @@ interface BunFetchRequestInit extends RequestInit { /** * The maximum number of redirects to follow when `redirect` is `"follow"`. - * If the response chain redirects more than this many times, the request - * rejects with a "too many redirects" error. + * Once the chain redirects more than this many times, the request rejects + * with a `TypeError` whose `code` is `"TooManyRedirects"`. * Not part of the Fetch API specification. * - * @default 126 + * `0` rejects the first redirect rather than meaning "unlimited". Values + * above the maximum of `126` are clamped to `126`. The default matches the + * 20-redirect limit the Fetch specification mandates. + * + * @default 20 * @example * ```js * const response = await fetch("https://example.com/", { maxRedirects: 3 }); diff --git a/src/http/AsyncHTTP.rs b/src/http/AsyncHTTP.rs index c5f5a2c2056a..f5431ee9c70d 100644 --- a/src/http/AsyncHTTP.rs +++ b/src/http/AsyncHTTP.rs @@ -13,8 +13,8 @@ use bun_picohttp as picohttp; use crate::headers::{self, Headers}; use crate::{ - FetchRedirect, Flags, HTTPClient, HTTPRequestBody, HTTPVerboseLevel, InternalState, Method, - Signals, ThreadlocalAsyncHTTP, + DEFAULT_REDIRECT_COUNT, FetchRedirect, Flags, HTTPClient, HTTPRequestBody, HTTPVerboseLevel, + InternalState, Method, Signals, ThreadlocalAsyncHTTP, }; use crate::{HTTPClientResult, HTTPClientResultCallback}; @@ -165,9 +165,7 @@ fn make_client<'a>( url, connected_url: URL::default(), verbose: HTTPVerboseLevel::None, - // Note: DEFAULT_REDIRECT_COUNT (= 127) is crate-private in lib.rs; - // duplicated as a literal here. - remaining_redirect_count: 127, + remaining_redirect_count: DEFAULT_REDIRECT_COUNT, allow_retry: false, h2_retries: 0, redirect_type, @@ -484,6 +482,7 @@ impl<'a> AsyncHTTP<'a> { this.client.flags.disable_decompression = val; } if let Some(val) = options.max_redirects { + // +1 for the terminal check (see `DEFAULT_REDIRECT_COUNT`); clamped to fit `i8`. this.client.remaining_redirect_count = (val.min(126) + 1) as i8; } if let Some(val) = options.disable_keepalive { diff --git a/src/http/lib.rs b/src/http/lib.rs index eca6a662c26a..00680d414154 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -238,6 +238,9 @@ pub static EXPERIMENTAL_HTTP3_CLIENT_FROM_CLI: AtomicBool = AtomicBool::new(fals const MAX_REDIRECT_URL_LENGTH: usize = 128 * 1024; +/// caps redirects at 20; `do_redirect` decrements then checks for 0, so the stored budget is `+1`. +pub(crate) const DEFAULT_REDIRECT_COUNT: i8 = 20 + 1; + /// The static is exported to /// C++ via `BUN_DEFAULT_MAX_HTTP_HEADER_SIZE`; `AtomicUsize` has the same /// size/alignment as `usize` so the symbol layout is unchanged. @@ -2634,8 +2637,7 @@ impl<'a> HTTPClient<'a> { self.hostname = None; } - // TODO: should this check be before decrementing the redirect count? - // the current logic will allow one less redirect than requested + // Decrement-then-check: see `DEFAULT_REDIRECT_COUNT` for why every initializer stores `limit + 1`. if self.remaining_redirect_count == 0 { self.fail(crate::Error::TooManyRedirects); return; diff --git a/src/jsc/SystemError.rs b/src/jsc/SystemError.rs index 8856495836df..3267e883f20e 100644 --- a/src/jsc/SystemError.rs +++ b/src/jsc/SystemError.rs @@ -72,6 +72,10 @@ pub type Maybe = core::result::Result; // is ABI-identical to a non-null `JSGlobalObject*` with write provenance. unsafe extern "C" { safe fn SystemError__toErrorInstance(this: &SystemError, global: &JSGlobalObject) -> JSValue; + safe fn SystemError__toTypeErrorInstance( + this: &SystemError, + global: &JSGlobalObject, + ) -> JSValue; safe fn SystemError__toErrorInstanceWithInfoObject( this: &SystemError, global: &JSGlobalObject, @@ -91,6 +95,11 @@ impl SystemError { SystemError__toErrorInstance(&self, global) } + /// [`to_error_instance`](Self::to_error_instance) as a JS `TypeError` (same `code`/`path`/`errno` properties), for fetch-spec network errors. + pub fn to_type_error_instance(self, global: &JSGlobalObject) -> JSValue { + SystemError__toTypeErrorInstance(&self, global) + } + /// Like `to_error_instance` but populates the error's stack trace with async /// frames from the given promise's await chain. Use when creating an error /// from native code at the top of the event loop (threadpool callback) to diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 97fbbf7b30fd..15187b4672dd 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -2353,7 +2353,7 @@ JSC::EncodedJSValue JSGlobalObject__createOutOfMemoryError(JSC::JSGlobalObject* return JSValue::encode(exception); } -JSC::EncodedJSValue SystemError__toErrorInstance(const SystemError* arg0, JSC::JSGlobalObject* globalObject) +static JSC::EncodedJSValue systemErrorToErrorInstance(const SystemError* arg0, JSC::JSGlobalObject* globalObject, JSC::ErrorType errorType) { SystemError err = *arg0; @@ -2367,7 +2367,7 @@ JSC::EncodedJSValue SystemError__toErrorInstance(const SystemError* arg0, JSC::J auto& names = WebCore::builtinNames(vm); - JSC::JSObject* result = createError(globalObject, ErrorType::Error, message); + JSC::JSObject* result = createError(globalObject, errorType, message); auto clientData = WebCore::clientData(vm); @@ -2426,6 +2426,17 @@ JSC::EncodedJSValue SystemError__toErrorInstance(const SystemError* arg0, JSC::J return JSC::JSValue::encode(result); } +JSC::EncodedJSValue SystemError__toErrorInstance(const SystemError* arg0, JSC::JSGlobalObject* globalObject) +{ + return systemErrorToErrorInstance(arg0, globalObject, ErrorType::Error); +} + +// `SystemError__toErrorInstance` as a `TypeError`, for fetch-spec network errors. +JSC::EncodedJSValue SystemError__toTypeErrorInstance(const SystemError* arg0, JSC::JSGlobalObject* globalObject) +{ + return systemErrorToErrorInstance(arg0, globalObject, ErrorType::TypeError); +} + JSC::EncodedJSValue SystemError__toErrorInstanceWithInfoObject(const SystemError* arg0, JSC::JSGlobalObject* globalObject) { SystemError err = *arg0; diff --git a/src/jsc/bindings/headers.h b/src/jsc/bindings/headers.h index ab056ebc60bd..ddece38e8b43 100644 --- a/src/jsc/bindings/headers.h +++ b/src/jsc/bindings/headers.h @@ -99,6 +99,7 @@ CPP_DECL void WebCore__FetchHeaders__remove(WebCore::FetchHeaders* arg0, const Z CPP_DECL JSC::EncodedJSValue WebCore__FetchHeaders__toJS(WebCore::FetchHeaders* arg0, JSC::JSGlobalObject* arg1); CPP_DECL void WebCore__FetchHeaders__toUWSResponse(WebCore::FetchHeaders* arg0, UWSResponseKind kind, void* arg2); CPP_DECL JSC::EncodedJSValue SystemError__toErrorInstance(const SystemError* arg0, JSC::JSGlobalObject* arg1); +CPP_DECL JSC::EncodedJSValue SystemError__toTypeErrorInstance(const SystemError* arg0, JSC::JSGlobalObject* arg1); #pragma mark - JSC::JSCell diff --git a/src/runtime/webcore/Body.rs b/src/runtime/webcore/Body.rs index ca4d992d8c70..9882a9cbfc61 100644 --- a/src/runtime/webcore/Body.rs +++ b/src/runtime/webcore/Body.rs @@ -564,6 +564,8 @@ pub enum Tag { pub enum ValueError { AbortReason(CommonAbortReason), SystemError(SystemError), + /// [`SystemError`](Self::SystemError) surfaced as a JS `TypeError`; use over [`TypeError`](Self::TypeError) when callers also rely on the error's `code`. + SystemTypeError(SystemError), Message(BunString), /// Surfaces as a JS `TypeError`. The fetch spec maps every "network /// error" to TypeError, so use this for fetch-layer rejections that @@ -579,6 +581,7 @@ impl ValueError { match self { // The bun.String fields are dropped by the assignment below. ValueError::SystemError(_system_error) => {} + ValueError::SystemTypeError(_system_error) => {} ValueError::Message(message) => message.deref(), ValueError::TypeError(message) => message.deref(), ValueError::JSValue(v) => v.deinit(), @@ -612,6 +615,9 @@ impl ValueError { ValueError::SystemError(system_error) => { core::mem::take(system_error).to_error_instance(global_object) } + ValueError::SystemTypeError(system_error) => { + core::mem::take(system_error).to_type_error_instance(global_object) + } ValueError::Message(message) => message.to_error_instance(global_object), ValueError::TypeError(message) => message.to_type_error_instance(global_object), // do an early return in this case we don't need to create a new Strong @@ -628,6 +634,7 @@ impl ValueError { // `.clone()` on BunString/SystemError already bumps the refcount (paired // with their Drop deref); an extra `.ref_()` here would leak +1 per dupe. ValueError::SystemError(e) => ValueError::SystemError(e.clone()), + ValueError::SystemTypeError(e) => ValueError::SystemTypeError(e.clone()), ValueError::Message(m) => ValueError::Message(m.clone()), ValueError::TypeError(m) => ValueError::TypeError(m.clone()), ValueError::JSValue(js_ref) => { diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 55b0c70260dd..fb961925b18d 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -1291,9 +1291,7 @@ impl FetchTasklet { let fail = self.result.fail.unwrap(); - // Fetch-spec "network error" cases that callers feature-detect via - // `instanceof TypeError`. Keep this list narrow; the catch-all - // SystemError below is still a plain Error for backwards compat. + // Fetch-spec "network error" `TypeError`s; redirect failures go through `SystemTypeError` below so their `code` survives. if fail == http::Error::RequestBodyNotReusable { return BodyValueError::TypeError(BunString::static_( "Request body is a ReadableStream and cannot be replayed for this redirect", @@ -1567,6 +1565,19 @@ impl FetchTasklet { ..Default::default() }; + // : each of these is a network error, so a `TypeError`. + if matches!( + fail, + http::Error::TooManyRedirects + | http::Error::UnexpectedRedirect + | http::Error::RedirectURLInvalid + | http::Error::InvalidRedirectURL + | http::Error::RedirectURLTooLong + | http::Error::UnsupportedRedirectProtocol + ) { + return BodyValueError::SystemTypeError(fetch_error); + } + BodyValueError::SystemError(fetch_error) } diff --git a/test/js/web/fetch/fetch-redirect.test.ts b/test/js/web/fetch/fetch-redirect.test.ts index 73ea51d509ea..882c13b6a996 100644 --- a/test/js/web/fetch/fetch-redirect.test.ts +++ b/test/js/web/fetch/fetch-redirect.test.ts @@ -328,3 +328,104 @@ describe("fetch() follows a redirect whose Location scheme is not lowercase", () } }); }); + +// https://fetch.spec.whatwg.org/#http-redirect-fetch step 5: "If request's +// redirect count is 20, then return a network error." A network error rejects +// the fetch() promise with a TypeError. +describe("fetch() redirect limit", () => { + // `/0 -> /1 -> ... -> /hops` via 302, then a 200. `requests.count` is the + // number of round trips the client actually made. + function redirectChain(hops: number) { + const requests = { count: 0 }; + const server = Bun.serve({ + port: 0, + fetch(request: Request) { + requests.count++; + const n = Number(new URL(request.url).pathname.slice("/".length)); + if (n >= hops) return new Response("done"); + return new Response(null, { status: 302, headers: { Location: `/${n + 1}` } }); + }, + }); + return { server, requests, [Symbol.dispose]: () => server.stop(true) }; + } + + async function rejection(promise: Promise): Promise { + return await promise.then( + () => { + throw new Error("expected the fetch promise to reject"); + }, + (e: unknown) => e, + ); + } + + it.concurrent("follows exactly 20 redirects by default", async () => { + using chain = redirectChain(20); + const resp = await fetch(`${chain.server.url}0`); + expect(await resp.text()).toBe("done"); + expect(resp.status).toBe(200); + expect(chain.requests.count).toBe(21); + }); + + it.concurrent("rejects the 21st redirect with a TypeError", async () => { + using chain = redirectChain(21); + const err = await rejection(fetch(`${chain.server.url}0`)); + expect(err).toBeInstanceOf(TypeError); + expect(err.code).toBe("TooManyRedirects"); + expect(err.message).toContain("redirected too many times"); + // 20 redirects were followed; the 21st redirect response is the error. + expect(chain.requests.count).toBe(21); + }); + + it.concurrent("a self-redirect loop makes exactly 21 requests before rejecting", async () => { + let requests = 0; + using server = Bun.serve({ + port: 0, + fetch() { + requests++; + return new Response(null, { status: 302, headers: { Location: "/" } }); + }, + }); + const err = await rejection(fetch(server.url)); + expect(err).toBeInstanceOf(TypeError); + expect(err.code).toBe("TooManyRedirects"); + expect(requests).toBe(21); + }); + + it.concurrent("exceeding an explicit maxRedirects rejects with a TypeError", async () => { + using chain = redirectChain(3); + const err = await rejection(fetch(`${chain.server.url}0`, { maxRedirects: 2 })); + expect(err).toBeInstanceOf(TypeError); + expect(err.code).toBe("TooManyRedirects"); + expect(chain.requests.count).toBe(3); + }); + + it.concurrent('redirect: "error" rejects with a TypeError', async () => { + using server = Bun.serve({ + port: 0, + fetch: () => new Response(null, { status: 302, headers: { Location: "/elsewhere" } }), + }); + const err = await rejection(fetch(server.url, { redirect: "error" })); + expect(err).toBeInstanceOf(TypeError); + expect(err.code).toBe("UnexpectedRedirect"); + }); + + it.concurrent("a redirect to a non-HTTP(S) scheme rejects with a TypeError", async () => { + using server = Bun.serve({ + port: 0, + fetch: () => new Response(null, { status: 302, headers: { Location: "ftp://example.com/" } }), + }); + const err = await rejection(fetch(server.url)); + expect(err).toBeInstanceOf(TypeError); + expect(err.code).toBe("UnsupportedRedirectProtocol"); + }); + + it.concurrent("a redirect to an unparseable URL rejects with a TypeError", async () => { + using server = Bun.serve({ + port: 0, + fetch: () => new Response(null, { status: 302, headers: { Location: "http://[/" } }), + }); + const err = await rejection(fetch(server.url)); + expect(err).toBeInstanceOf(TypeError); + expect(err.code).toBe("RedirectURLInvalid"); + }); +});