Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement throwOnMaxRedirect option for RedirectHandler #2563

Merged
merged 7 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions docs/api/RedirectHandler.md
metcoder95 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# Class: RedirectHandler
mertcanaltin marked this conversation as resolved.
Show resolved Hide resolved

A class that handles redirection logic for HTTP requests.

## `new RedirectHandler(dispatch, maxRedirections, opts, handler, redirectionLimitReached)`

Arguments:

- **dispatch** `function` - The dispatch function to be called after every retry.
- **maxRedirections** `number` - Maximum number of redirections allowed.
- **opts** `object` - Options for handling redirection.
- **handler** `object` - An object containing handlers for different stages of the request lifecycle.
- **redirectionLimitReached** `boolean` (default: `false`) - A flag that the implementer can provide to enable or disable the feature. If set to `false`, it indicates that the caller doesn't want to use the feature and prefers the old behavior.

Returns: `RedirectHandler`

### Parameters

- **dispatch** `(options: Dispatch.DispatchOptions, handlers: Dispatch.DispatchHandlers) => Promise<Dispatch.DispatchResponse>` (required) - Dispatch function to be called after every redirection.
- **maxRedirections** `number` (required) - Maximum number of redirections allowed.
- **opts** `object` (required) - Options for handling redirection.
- **handler** `object` (required) - Handlers for different stages of the request lifecycle.
- **redirectionLimitReached** `boolean` (default: `false`) - A flag that the implementer can provide to enable or disable the feature. If set to `false`, it indicates that the caller doesn't want to use the feature and prefers the old behavior.

### Properties

- **location** `string` - The current redirection location.
- **abort** `function` - The abort function.
- **opts** `object` - The options for handling redirection.
- **maxRedirections** `number` - Maximum number of redirections allowed.
- **handler** `object` - Handlers for different stages of the request lifecycle.
- **history** `Array` - An array representing the history of URLs during redirection.
- **redirectionLimitReached** `boolean` - Indicates whether the redirection limit has been reached.

### Methods

#### `onConnect(abort)`

Called when the connection is established.

Parameters:

- **abort** `function` - The abort function.

#### `onUpgrade(statusCode, headers, socket)`

Called when an upgrade is requested.

Parameters:

- **statusCode** `number` - The HTTP status code.
- **headers** `object` - The headers received in the response.
- **socket** `object` - The socket object.

#### `onError(error)`

Called when an error occurs.

Parameters:

- **error** `Error` - The error that occurred.

#### `onHeaders(statusCode, headers, resume, statusText)`

Called when headers are received.

Parameters:

- **statusCode** `number` - The HTTP status code.
- **headers** `object` - The headers received in the response.
- **resume** `function` - The resume function.
- **statusText** `string` - The status text.

#### `onData(chunk)`

Called when data is received.

Parameters:

- **chunk** `Buffer` - The data chunk received.

#### `onComplete(trailers)`

Called when the request is complete.

Parameters:

- **trailers** `object` - The trailers received.

#### `onBodySent(chunk)`

Called when the request body is sent.

Parameters:

- **chunk** `Buffer` - The chunk of the request body sent.
1 change: 1 addition & 0 deletions docsify/sidebar.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
* [MIME Type Parsing](/docs/api/ContentType.md "Undici API - MIME Type Parsing")
* [CacheStorage](/docs/api/CacheStorage.md "Undici API - CacheStorage")
* [Util](/docs/api/Util.md "Undici API - Util")
* [RedirectHandler](/docs/api/RedirectHandler.md "Undici API - RedirectHandler")
* Best Practices
* [Proxy](/docs/best-practices/proxy.md "Connecting through a proxy")
* [Client Certificate](/docs/best-practices/client-certificate.md "Connect using a client certificate")
Expand Down
10 changes: 10 additions & 0 deletions lib/handler/RedirectHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class RedirectHandler {
this.maxRedirections = maxRedirections
this.handler = handler
this.history = []
this.redirectionLimitReached = false
mertcanaltin marked this conversation as resolved.
Show resolved Hide resolved

if (util.isStream(this.opts.body)) {
// TODO (fix): Provide some way for the user to cache the file to e.g. /tmp
Expand Down Expand Up @@ -91,6 +92,15 @@ class RedirectHandler {
? null
: parseLocation(statusCode, headers)

if (this.history.length >= this.maxRedirections && !this.redirectionLimitReached) {
if (this.request) {
this.request.abort()
mertcanaltin marked this conversation as resolved.
Show resolved Hide resolved
}

this.redirectionLimitReached = true
metcoder95 marked this conversation as resolved.
Show resolved Hide resolved
this.abort(new Error('max redirects'))
mertcanaltin marked this conversation as resolved.
Show resolved Hide resolved
}

if (this.opts.origin) {
this.history.push(new URL(this.opts.path, this.opts.origin))
}
Expand Down
24 changes: 24 additions & 0 deletions test/redirect-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,30 @@ for (const factory of [
t.equal(body.length, 0)
})

t.test('should follow a redirect chain up to the allowed number of times for redirectionLimitReached', async t => {
const server = await startRedirectingServer(t)

try {
const { statusCode, headers, body: bodyStream, context: { history } } = await request(t, server, undefined, `http://${server}/300`, {
mertcanaltin marked this conversation as resolved.
Show resolved Hide resolved
maxRedirections: 2,
redirectionLimitReached: 2
})

const body = await bodyStream.text()

t.equal(statusCode, 300)
t.equal(headers.location, `http://${server}/300/2`)
t.same(history.map(x => x.toString()), [`http://${server}/300`, `http://${server}/300/1`])
t.equal(body.length, 0)
} catch (error) {
if (error.message.startsWith('max redirects')) {
t.pass('Max redirects handled correctly')
} else {
t.fail(`Unexpected error: ${error.message}`)
}
}
})

t.test('when a Location response header is NOT present', async t => {
const redirectCodes = [300, 301, 302, 303, 307, 308]
const server = await startRedirectingWithoutLocationServer(t)
Expand Down
6 changes: 6 additions & 0 deletions types/dispatcher.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ declare namespace Dispatcher {
opaque?: unknown;
/** Default: 0 */
maxRedirections?: number;
/** Default: false */
redirectionLimitReached?: boolean;
/** Default: `null` */
responseHeader?: 'raw' | null;
}
Expand All @@ -141,6 +143,8 @@ declare namespace Dispatcher {
signal?: AbortSignal | EventEmitter | null;
/** Default: 0 */
maxRedirections?: number;
/** Default: false */
redirectionLimitReached?: boolean;
/** Default: `null` */
onInfo?: (info: { statusCode: number, headers: Record<string, string | string[]> }) => void;
/** Default: `null` */
Expand All @@ -164,6 +168,8 @@ declare namespace Dispatcher {
signal?: AbortSignal | EventEmitter | null;
/** Default: 0 */
maxRedirections?: number;
/** Default: false */
redirectionLimitReached?: boolean;
/** Default: `null` */
responseHeader?: 'raw' | null;
}
Expand Down
2 changes: 1 addition & 1 deletion types/handlers.d.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Dispatcher from "./dispatcher";

export declare class RedirectHandler implements Dispatcher.DispatchHandlers{
constructor (dispatch: Dispatcher, maxRedirections: number, opts: Dispatcher.DispatchOptions, handler: Dispatcher.DispatchHandlers)
constructor (dispatch: Dispatcher, maxRedirections: number, opts: Dispatcher.DispatchOptions, handler: Dispatcher.DispatchHandlers, redirectionLimitReached: boolean)
mertcanaltin marked this conversation as resolved.
Show resolved Hide resolved
}

export declare class DecoratorHandler implements Dispatcher.DispatchHandlers{
Expand Down
Loading