Feat: add Axios built-in redirects support#10991
Conversation
# Conflicts: # lib/utils.js
…-redirects # Conflicts: # lib/adapters/http.js
…-redirects # Conflicts: # README.md # index.d.cts # index.d.ts # lib/adapters/fetch.js # lib/adapters/http.js # lib/core/Axios.js # lib/core/dispatchRequest.js # lib/core/settle.js # lib/defaults/transitional.js # package-lock.json # tests/unit/adapters/http.test.js
…-redirects # Conflicts: # lib/adapters/fetch.js
…-redirects # Conflicts: # README.md # index.d.cts # index.d.ts # lib/defaults/transitional.js
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
21 issues found
Confidence score: 2/5
- Several high-confidence, user-impacting regressions are present (sev 7–8), so merge risk is high rather than a minor polish pass.
- In
lib/core/dispatchWithRedirects.js, the redirect rewrite condition uses||instead of&&, which can incorrectly rewrite 301/302 flows and changeHEADrequests toGET. lib/helpers/isSubdomain.jsandlib/helpers/BufferedStream.jsintroduce concrete runtime behavior breaks (valid subdomains rejected; redirect buffering rejecting supported body types), andtests/e2e/node/redirects.test.jshas a no-op assertion that weakens detection of these failures.- Pay close attention to
lib/core/dispatchWithRedirects.js,lib/helpers/isSubdomain.js,lib/helpers/BufferedStream.js, andindex.d.cts- redirect semantics and type/config correctness are currently fragile.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/helpers/textEncoder.js">
<violation number="1" location="lib/helpers/textEncoder.js:20">
P3: Fallback UTF-8 encoding uses deprecated `unescape(...)`; use the modern percent-decoding pattern for consistency and future compatibility.</violation>
</file>
<file name="lib/adapters/http.js">
<violation number="1" location="lib/adapters/http.js:479">
P2: `useAxiosRedirects` is read from `config.transitional` directly, bypassing own-property checks and weakening prototype-pollution hardening.</violation>
</file>
<file name="index.d.cts">
<violation number="1" location="index.d.cts:501">
P2: `AxiosBufferingConfig` has a typo (`timout`) that makes the typed option ineffective at runtime.</violation>
<violation number="2" location="index.d.cts:537">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**
`beforeRedirect` type has misplaced parentheses, turning a union of two callback signatures into a single function that returns `void | ((redirectMeta) => false | void)`. The `)` should close the first callback before the `|`, not after the second callback.</violation>
</file>
<file name="lib/utils.js">
<violation number="1" location="lib/utils.js:742">
P2: `list()` incorrectly ignores supported falsy inputs (`0`, `false`) due to a truthiness guard.</violation>
</file>
<file name="lib/core/dispatchWithRedirects.js">
<violation number="1" location="lib/core/dispatchWithRedirects.js:46">
P2: Malformed `Location` headers can throw raw `TypeError` during URL parsing; wrap parsing and throw `AxiosError` (`ERR_INVALID_URL`) for consistent error handling.</violation>
<violation number="2" location="lib/core/dispatchWithRedirects.js:49">
P1: Redirect method rewrite condition is logically incorrect (`||` instead of `&&`), so 301/302 always enter rewrite path and can incorrectly change `HEAD` requests to `GET`.</violation>
</file>
<file name="lib/adapters/fetch.js">
<violation number="1" location="lib/adapters/fetch.js:432">
P2: The new ERR_STREAM_FLUSHED branch drops axios error context by throwing `AxiosError.from(err.cause)` without `config`/`request`, which can break interceptors/retry logic expecting `error.config`.</violation>
</file>
<file name="lib/core/dispatchRequest.js">
<violation number="1" location="lib/core/dispatchRequest.js:93">
P2: `useAxiosRedirects` is read via prototype chain; inherited values can unexpectedly enable redirect flow.</violation>
</file>
<file name="lib/helpers/BufferedStream.js">
<violation number="1" location="lib/helpers/BufferedStream.js:18">
P1: Constructor validation is too strict: it rejects Request/ReadableStream/blob-like bodies that redirect buffering intentionally supports, causing runtime failure before streaming begins.</violation>
</file>
<file name="tests/e2e/node/redirects.test.js">
<violation number="1" location="tests/e2e/node/redirects.test.js:160">
P1: `toBeDefined` matcher is called as a property access instead of a method call, making the assertion a no-op.</violation>
</file>
<file name="lib/cancel/CanceledError.js">
<violation number="1" location="lib/cancel/CanceledError.js:26">
P3: Comment typo: 'coping' should be 'copying'</violation>
</file>
<file name="lib/helpers/readBlob.js">
<violation number="1" location="lib/helpers/readBlob.js:23">
P2: `reader.cancel()` in `finally` can mask the original stream read error if cancel also rejects.</violation>
</file>
<file name="lib/helpers/asyncTimeout.js">
<violation number="1" location="lib/helpers/asyncTimeout.js:8">
P2: Abort path does not unsubscribe the signal listener, causing a listener leak on canceled timeouts.</violation>
</file>
<file name="lib/platform/browser/index.js">
<violation number="1" location="lib/platform/browser/index.js:18">
P2: `toBase64` uses `String.fromCharCode(...bytes)`, which can fail for large inputs due to spread argument limits.</violation>
</file>
<file name="lib/helpers/isSubdomain.js">
<violation number="1" location="lib/helpers/isSubdomain.js:3">
P1: Incorrect subdomain boundary check — `diff > 2` rejects single-character subdomain labels. For example, `isSubdomain('a.example.com', 'example.com')` incorrectly returns `false`. The condition should be `diff > 0` to allow any valid subdomain label of one or more characters plus the dot separator.</violation>
</file>
<file name="package.json">
<violation number="1" location="package.json:55">
P2: The `react-native` field is missing the `./lib/core/dispatchWithRedirects.js` → `./lib/helpers/null.js` mapping that was added to the `browser` field. The `browser` and `react-native` sections were previously identical, and the PR description states this feature is not supported in client environments. Without this mapping, Metro or other React Native bundlers would attempt to resolve and load the actual `dispatchWithRedirects.js` module (imported at the top level from the core `dispatchRequest.js`), contrary to the stated intent.</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:2395">
P2: Config property location inconsistency: `beforeRedirect` appears as a top-level config property in the type signature but is nested under `transitional` in the usage example</violation>
<violation number="2" location="README.md:2397">
P3: Inconsistent backtick fence length: the `beforeRedirect` hook code block opens with 3 backticks (```ts) but closes with 4 backticks (````)</violation>
<violation number="3" location="README.md:2469">
P2: Grammar error: 'unless you performs' should use the base verb form 'perform' after the subject 'you'</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| responseDetails: { headers: Record<string, string>; statusCode: HttpStatusCode }, | ||
| requestDetails: { headers: Record<string, string>; url: string; method: string }, | ||
| ) => void; | ||
| ) => void | ((redirectMeta: AxiosRedirectMeta) => false | void)); |
There was a problem hiding this comment.
P1: Custom agent: Flag AI Slop and Fabricated Changes
beforeRedirect type has misplaced parentheses, turning a union of two callback signatures into a single function that returns void | ((redirectMeta) => false | void). The ) should close the first callback before the |, not after the second callback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At index.d.cts, line 537:
<comment>`beforeRedirect` type has misplaced parentheses, turning a union of two callback signatures into a single function that returns `void | ((redirectMeta) => false | void)`. The `)` should close the first callback before the `|`, not after the second callback.</comment>
<file context>
@@ -505,11 +530,11 @@ declare namespace axios {
responseDetails: { headers: Record<string, string>; statusCode: HttpStatusCode },
requestDetails: { headers: Record<string, string>; url: string; method: string },
- ) => void;
+ ) => void | ((redirectMeta: AxiosRedirectMeta) => false | void));
socketPath?: string | null;
allowedSocketPaths?: string | string[] | null;
</file context>
| ) => void | ((redirectMeta: AxiosRedirectMeta) => false | void)); | |
| ) => void) | ((redirectMeta: AxiosRedirectMeta) => false | void); |
| const originalUrl = new URL(buildFullPath(config.baseURL, config.url, true), platform.origin); | ||
| const redirectedUrl = new URL(loc, originalUrl); | ||
|
|
||
| if (status === 303 || ((status === 301 || status === 302) && (method !== 'GET' || method !== 'HEAD'))) { |
There was a problem hiding this comment.
P1: Redirect method rewrite condition is logically incorrect (|| instead of &&), so 301/302 always enter rewrite path and can incorrectly change HEAD requests to GET.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/core/dispatchWithRedirects.js, line 49:
<comment>Redirect method rewrite condition is logically incorrect (`||` instead of `&&`), so 301/302 always enter rewrite path and can incorrectly change `HEAD` requests to `GET`.</comment>
<file context>
@@ -0,0 +1,206 @@
+ const originalUrl = new URL(buildFullPath(config.baseURL, config.url, true), platform.origin);
+ const redirectedUrl = new URL(loc, originalUrl);
+
+ if (status === 303 || ((status === 301 || status === 302) && (method !== 'GET' || method !== 'HEAD'))) {
+ method = 'GET';
+ data = undefined;
</file context>
| if (status === 303 || ((status === 301 || status === 302) && (method !== 'GET' || method !== 'HEAD'))) { | |
| if (status === 303 || ((status === 301 || status === 302) && (method !== 'GET' && method !== 'HEAD'))) { |
| @@ -0,0 +1,6 @@ | |||
| const isSubdomain = (sub, domain, psl) => { | |||
| let diff = sub.length - domain.length; | |||
| return diff > 2 && sub[diff - 1] === "." && sub.endsWith(domain); | |||
There was a problem hiding this comment.
P1: Incorrect subdomain boundary check — diff > 2 rejects single-character subdomain labels. For example, isSubdomain('a.example.com', 'example.com') incorrectly returns false. The condition should be diff > 0 to allow any valid subdomain label of one or more characters plus the dot separator.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/helpers/isSubdomain.js, line 3:
<comment>Incorrect subdomain boundary check — `diff > 2` rejects single-character subdomain labels. For example, `isSubdomain('a.example.com', 'example.com')` incorrectly returns `false`. The condition should be `diff > 0` to allow any valid subdomain label of one or more characters plus the dot separator.</comment>
<file context>
@@ -0,0 +1,6 @@
+const isSubdomain = (sub, domain, psl) => {
+ let diff = sub.length - domain.length;
+ return diff > 2 && sub[diff - 1] === "." && sub.endsWith(domain);
+}
+
</file context>
| limit = 0, | ||
| signal | ||
| } = {}) { | ||
| if (!utils.isAsyncIterable(source)) { |
There was a problem hiding this comment.
P1: Constructor validation is too strict: it rejects Request/ReadableStream/blob-like bodies that redirect buffering intentionally supports, causing runtime failure before streaming begins.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/helpers/BufferedStream.js, line 18:
<comment>Constructor validation is too strict: it rejects Request/ReadableStream/blob-like bodies that redirect buffering intentionally supports, causing runtime failure before streaming begins.</comment>
<file context>
@@ -0,0 +1,192 @@
+ limit = 0,
+ signal
+ } = {}) {
+ if (!utils.isAsyncIterable(source)) {
+ throw new Error('source must be async iterable');
+ }
</file context>
| expect(beforeRedirect).toHaveBeenCalled(); | ||
| const callArgs = beforeRedirect.mock.calls[0][0]; | ||
|
|
||
| expect(callArgs.agent).toBeDefined; |
There was a problem hiding this comment.
P1: toBeDefined matcher is called as a property access instead of a method call, making the assertion a no-op.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/e2e/node/redirects.test.js, line 160:
<comment>`toBeDefined` matcher is called as a property access instead of a method call, making the assertion a no-op.</comment>
<file context>
@@ -0,0 +1,1131 @@
+ expect(beforeRedirect).toHaveBeenCalled();
+ const callArgs = beforeRedirect.mock.calls[0][0];
+
+ expect(callArgs.agent).toBeDefined;
+ })
+
</file context>
| expect(callArgs.agent).toBeDefined; | |
| expect(callArgs.agent).toBeDefined(); |
|
|
||
| ```ts | ||
| { | ||
| beforeRedirect?: (redirectMeta: AxiosRedirectMeta) => false | void |
There was a problem hiding this comment.
P2: Config property location inconsistency: beforeRedirect appears as a top-level config property in the type signature but is nested under transitional in the usage example
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 2395:
<comment>Config property location inconsistency: `beforeRedirect` appears as a top-level config property in the type signature but is nested under `transitional` in the usage example</comment>
<file context>
@@ -2352,6 +2352,199 @@ const { data, headers, status } = await axios.post('https://httpbin.org/post', f
+
+```ts
+{
+ beforeRedirect?: (redirectMeta: AxiosRedirectMeta) => false | void
+}
+````
</file context>
| } | ||
|
|
||
| // fallback (UTF-8) | ||
| const utf8 = unescape(encodeURIComponent(str)); |
There was a problem hiding this comment.
P3: Fallback UTF-8 encoding uses deprecated unescape(...); use the modern percent-decoding pattern for consistency and future compatibility.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/helpers/textEncoder.js, line 20:
<comment>Fallback UTF-8 encoding uses deprecated `unescape(...)`; use the modern percent-decoding pattern for consistency and future compatibility.</comment>
<file context>
@@ -0,0 +1,34 @@
+ }
+
+ // fallback (UTF-8)
+ const utf8 = unescape(encodeURIComponent(str));
+ const {length} = utf8;
+ const arr = new Uint8Array(length);
</file context>
| return err; | ||
| } | ||
|
|
||
| // Avoid coping unappropriated DOMException code |
There was a problem hiding this comment.
P3: Comment typo: 'coping' should be 'copying'
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/cancel/CanceledError.js, line 26:
<comment>Comment typo: 'coping' should be 'copying'</comment>
<file context>
@@ -17,6 +17,19 @@ class CanceledError extends AxiosError {
+ return err;
+ }
+
+ // Avoid coping unappropriated DOMException code
+ if (err instanceof Error && err.name === 'AbortError') {
+ return new this(err.message, null, ...args);
</file context>
| // Avoid coping unappropriated DOMException code | |
| // Avoid copying unappropriated DOMException code |
| { | ||
| beforeRedirect?: (redirectMeta: AxiosRedirectMeta) => false | void | ||
| } | ||
| ```` |
There was a problem hiding this comment.
P3: Inconsistent backtick fence length: the beforeRedirect hook code block opens with 3 backticks (```ts) but closes with 4 backticks (````)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 2397:
<comment>Inconsistent backtick fence length: the `beforeRedirect` hook code block opens with 3 backticks (```ts) but closes with 4 backticks (````)</comment>
<file context>
@@ -2352,6 +2352,199 @@ const { data, headers, status } = await axios.post('https://httpbin.org/post', f
+{
+ beforeRedirect?: (redirectMeta: AxiosRedirectMeta) => false | void
+}
+````
+
+```
</file context>
| ```` |
There was a problem hiding this comment.
I think this is heading in the correct direction. I ran a security scan on it, and I think there are a couple of things we will need: mainly credential stripping, reading config property-guarded values, and some buffer bounding before following.
Also, please look at the Cubic review.
|
|
||
| let loc = response.headers.get('Location'); | ||
|
|
||
| if (maxRedirects >= 0 && redirectIndex >= maxRedirects) { |
There was a problem hiding this comment.
By looking at it maxRedirects: 0 throws Too many redirects instead of returning the 3xx response for normal validateStatus handling, lets preserve existing maxRedirects: 0 semantics.
| } | ||
| }); | ||
|
|
||
| if (beforeRedirect) { |
There was a problem hiding this comment.
Since beforeRedirect can mutate config.url after we have computed redirectTo, bypassing protocol allow-list, downgrade checks, and sanitisation, validate the final dispatch URL after the hook, or make redirectTo the only mutable target.
| throw new AxiosError('Protocol downgrade is not allowed', AxiosError.ERR_REDIRECT); | ||
| } | ||
|
|
||
| if( |
There was a problem hiding this comment.
Authorization/Cookie headers are preserved for redirects from a host to any subdomain via isSubdomain, which is weaker than the current follow-redirects cross-host stripping and can leak credentials to, for example, evil.api.example.com. Please strip sensitive headers whenever host/origin changes by default; require an explicit opt-in hook to preserve them.
| }); | ||
| } | ||
|
|
||
| ret = await iterator.next(); |
There was a problem hiding this comment.
abort/cancel does not interrupt an in-flight iterator.next(), so a stalled upload source can hang cancellation until it yields. Race reads with the signal and cleans up the iterator/source.
|
|
||
| timeout <= 0 && (limit = Infinity); | ||
|
|
||
| const isBufferingNeeded = limit && |
There was a problem hiding this comment.
Request/ReadableStream bodies are selected for buffering, but BufferedStream rejects non-async-iterables at lib/helpers/BufferedStream.js:18, let's align accepted body types or narrow the buffering functionality.
| }); | ||
| } | ||
|
|
||
| export default (initialConfig, dispatch) => { |
There was a problem hiding this comment.
Part of issue mentioned in LN24
| let ret; | ||
| let shouldSanitize; | ||
|
|
||
| const redirectedURLStr = redirectedUrl + ''; |
There was a problem hiding this comment.
sanitize() clears redirectedUrl.username/password but does not write the sanitized URL back to redirectedConfig.url. A relative redirect from a URL containing userinfo can still regenerate Basic auth in the next dispatch. After clearing, we should always assign.
| sensitiveHeaders && headers.delete(sensitiveHeaders); | ||
| } | ||
|
|
||
| return dispatchWithRedirects(dispatch, { |
There was a problem hiding this comment.
beforeRedirect can mutate config.url after protocol, downgrade, and sanitizer inputs are computed. The actual dispatched URL can differ from the checked URL.
| if (configTransport) { | ||
| transport = configTransport; | ||
| } else if (config.maxRedirects === 0) { | ||
| } else if (useAxiosRedirects || config.maxRedirects === 0) { |
There was a problem hiding this comment.
Streamed upload maxBodyLength is only enforced for config.maxRedirects === 0. The new useAxiosRedirects native transport path and HTTP/2 path can send past maxBodyLength, regressing the streamed-upload limit advisories we have received in the past.
| fetchOptions: { | ||
| redirect: platform.isNode ? 'manual' : undefined | ||
| }, | ||
| validateStatus: (status) => isRedirect(status) || !originalValidateFn || originalValidateFn(status), |
There was a problem hiding this comment.
Redirect responses are buffered before following. A malicious server can send 302 Location: /next and then stream an unbounded body forever, causing memory growth or hang before axios follows.
2 participants
The PR adds experimental support for built-in redirects in Axios, allowing handling of redirects over HTTP/2, since
follow-redirectsdoesn't support it. In addition, it also allows you to handle fetch adapter redirects on the server in the same way.The feature is activated only if
transitional.useAxiosRedirectsis set. In such a case, Axios follows redirects by itself, outside of an adapter. Thus, there is no need for separate logic to support redirects for each adapter.When the solution is stabilized, it is assumed that in future major versions it will replace the
follow-redirectspackage.The solution also avoids buffering the entire request stream into RAM. Instead, it utilizes a small, temporary buffer only during the initial phase of reading the payload. Since the server usually responds with a redirect at the beginning of the request, this allows you to avoid unnecessary buffering. That was a serious problem when working with large requests (like uploading files), forcing the user to disable redirects for such requests.
Closes #7358