diff --git a/README.md b/README.md index 2ac58b6695e..fea71cb4be6 100644 --- a/README.md +++ b/README.md @@ -127,7 +127,6 @@ Arguments: * **options** [`RequestOptions`](./docs/docs/api/Dispatcher.md#parameter-requestoptions) * **dispatcher** `Dispatcher` - Default: [getGlobalDispatcher](#undicigetglobaldispatcher) * **method** `String` - Default: `PUT` if `options.body`, otherwise `GET` - * **maxRedirections** `Integer` - Default: `0` Returns a promise with the result of the `Dispatcher.request` method. @@ -143,7 +142,6 @@ Arguments: * **options** [`StreamOptions`](./docs/docs/api/Dispatcher.md#parameter-streamoptions) * **dispatcher** `Dispatcher` - Default: [getGlobalDispatcher](#undicigetglobaldispatcher) * **method** `String` - Default: `PUT` if `options.body`, otherwise `GET` - * **maxRedirections** `Integer` - Default: `0` * **factory** `Dispatcher.stream.factory` Returns a promise with the result of the `Dispatcher.stream` method. @@ -160,7 +158,6 @@ Arguments: * **options** [`PipelineOptions`](./docs/docs/api/Dispatcher.md#parameter-pipelineoptions) * **dispatcher** `Dispatcher` - Default: [getGlobalDispatcher](#undicigetglobaldispatcher) * **method** `String` - Default: `PUT` if `options.body`, otherwise `GET` - * **maxRedirections** `Integer` - Default: `0` * **handler** `Dispatcher.pipeline.handler` Returns: `stream.Duplex` @@ -178,7 +175,6 @@ Arguments: * **url** `string | URL | UrlObject` * **options** [`ConnectOptions`](./docs/docs/api/Dispatcher.md#parameter-connectoptions) * **dispatcher** `Dispatcher` - Default: [getGlobalDispatcher](#undicigetglobaldispatcher) - * **maxRedirections** `Integer` - Default: `0` * **callback** `(err: Error | null, data: ConnectData | null) => void` (optional) Returns a promise with the result of the `Dispatcher.connect` method. @@ -338,7 +334,6 @@ Arguments: * **url** `string | URL | UrlObject` * **options** [`UpgradeOptions`](./docs/docs/api/Dispatcher.md#parameter-upgradeoptions) * **dispatcher** `Dispatcher` - Default: [getGlobalDispatcher](#undicigetglobaldispatcher) - * **maxRedirections** `Integer` - Default: `0` * **callback** `(error: Error | null, data: UpgradeData) => void` (optional) Returns a promise with the result of the `Dispatcher.upgrade` method. diff --git a/docs/docs/api/Agent.md b/docs/docs/api/Agent.md index dd5d99bc1e1..e3e3e26a93b 100644 --- a/docs/docs/api/Agent.md +++ b/docs/docs/api/Agent.md @@ -19,8 +19,6 @@ Returns: `Agent` Extends: [`PoolOptions`](Pool.md#parameter-pooloptions) * **factory** `(origin: URL, opts: Object) => Dispatcher` - Default: `(origin, opts) => new Pool(origin, opts)` -* **maxRedirections** `Integer` - Default: `0`. The number of HTTP redirection to follow unless otherwise specified in `DispatchOptions`. -* **interceptors** `{ Agent: DispatchInterceptor[] }` - Default: `[RedirectInterceptor]` - A list of interceptors that are applied to the dispatch method. Additional logic can be applied (such as, but not limited to: 302 status code handling, authentication, cookies, compression and caching). Note that the behavior of interceptors is Experimental and might change at any given time. ## Instance Properties @@ -51,7 +49,6 @@ Implements [`Dispatcher.dispatch(options, handler)`](Dispatcher.md#dispatcherdis Extends: [`DispatchOptions`](Dispatcher.md#parameter-dispatchoptions) * **origin** `string | URL` -* **maxRedirections** `Integer`. Implements [`Dispatcher.destroy([error, callback])`](Dispatcher.md#dispatcherdestroyerror-callback-promise). diff --git a/docs/docs/api/Client.md b/docs/docs/api/Client.md index 03342f59959..4b193bda508 100644 --- a/docs/docs/api/Client.md +++ b/docs/docs/api/Client.md @@ -29,8 +29,6 @@ Returns: `Client` * **pipelining** `number | null` (optional) - Default: `1` - The amount of concurrent requests to be sent over the single TCP/TLS connection according to [RFC7230](https://tools.ietf.org/html/rfc7230#section-6.3.2). Carefully consider your workload and environment before enabling concurrent requests as pipelining may reduce performance if used incorrectly. Pipelining is sensitive to network stack settings as well as head of line blocking caused by e.g. long running requests. Set to `0` to disable keep-alive connections. * **connect** `ConnectOptions | Function | null` (optional) - Default: `null`. * **strictContentLength** `Boolean` (optional) - Default: `true` - Whether to treat request content length mismatches as errors. If true, an error is thrown when the request content-length header doesn't match the length of the request body. - -* **interceptors** `{ Client: DispatchInterceptor[] }` - Default: `[RedirectInterceptor]` - A list of interceptors that are applied to the dispatch method. Additional logic can be applied (such as, but not limited to: 302 status code handling, authentication, cookies, compression and caching). Note that the behavior of interceptors is Experimental and might change at any given time. **Note: this is deprecated in favor of [Dispatcher#compose](./Dispatcher.md#dispatcher). Support will be droped in next major.** * **autoSelectFamily**: `boolean` (optional) - Default: depends on local Node version, on Node 18.13.0 and above is `false`. Enables a family autodetection algorithm that loosely implements section 5 of [RFC 8305](https://tools.ietf.org/html/rfc8305#section-5). See [here](https://nodejs.org/api/net.html#socketconnectoptions-connectlistener) for more details. This option is ignored if not supported by the current Node version. * **autoSelectFamilyAttemptTimeout**: `number` - Default: depends on local Node version, on Node 18.13.0 and above is `250`. The amount of time in milliseconds to wait for a connection attempt to finish before trying the next address when using the `autoSelectFamily` option. See [here](https://nodejs.org/api/net.html#socketconnectoptions-connectlistener) for more details. * **allowH2**: `boolean` - Default: `false`. Enables support for H2 if the server has assigned bigger priority to it through ALPN negotiation. diff --git a/docs/docs/api/DispatchInterceptor.md b/docs/docs/api/DispatchInterceptor.md deleted file mode 100644 index 7dfc260e32a..00000000000 --- a/docs/docs/api/DispatchInterceptor.md +++ /dev/null @@ -1,60 +0,0 @@ -# Interface: DispatchInterceptor - -Extends: `Function` - -A function that can be applied to the `Dispatcher.Dispatch` function before it is invoked with a dispatch request. - -This allows one to write logic to intercept both the outgoing request, and the incoming response. - -### Parameter: `Dispatcher.Dispatch` - -The base dispatch function you are decorating. - -### ReturnType: `Dispatcher.Dispatch` - -A dispatch function that has been altered to provide additional logic - -### Basic Example - -Here is an example of an interceptor being used to provide a JWT bearer token - -```js -'use strict' - -const insertHeaderInterceptor = dispatch => { - return function InterceptedDispatch(opts, handler){ - opts.headers.push('Authorization', 'Bearer [Some token]') - return dispatch(opts, handler) - } -} - -const client = new Client('https://localhost:3000', { - interceptors: { Client: [insertHeaderInterceptor] } -}) - -``` - -### Basic Example 2 - -Here is a contrived example of an interceptor stripping the headers from a response. - -```js -'use strict' - -const clearHeadersInterceptor = dispatch => { - const { DecoratorHandler } = require('undici') - class ResultInterceptor extends DecoratorHandler { - onHeaders (statusCode, headers, resume) { - return super.onHeaders(statusCode, [], resume) - } - } - return function InterceptedDispatch(opts, handler){ - return dispatch(opts, new ResultInterceptor(handler)) - } -} - -const client = new Client('https://localhost:3000', { - interceptors: { Client: [clearHeadersInterceptor] } -}) - -``` diff --git a/docs/docs/api/EnvHttpProxyAgent.md b/docs/docs/api/EnvHttpProxyAgent.md index a4932de8be7..7d431030f67 100644 --- a/docs/docs/api/EnvHttpProxyAgent.md +++ b/docs/docs/api/EnvHttpProxyAgent.md @@ -133,7 +133,6 @@ Implements [`Dispatcher.dispatch(options, handler)`](Dispatcher.md#dispatcherdis Extends: [`DispatchOptions`](Dispatcher.md#parameter-dispatchoptions) * **origin** `string | URL` -* **maxRedirections** `Integer`. Implements [`Dispatcher.destroy([error, callback])`](Dispatcher.md#dispatcherdestroyerror-callback-promise). diff --git a/docs/docs/api/Pool.md b/docs/docs/api/Pool.md index 8fcabac3154..6b08294b61c 100644 --- a/docs/docs/api/Pool.md +++ b/docs/docs/api/Pool.md @@ -19,7 +19,6 @@ Extends: [`ClientOptions`](Client.md#parameter-clientoptions) * **factory** `(origin: URL, opts: Object) => Dispatcher` - Default: `(origin, opts) => new Client(origin, opts)` * **connections** `number | null` (optional) - Default: `null` - The number of `Client` instances to create. When set to `null`, the `Pool` instance will create an unlimited amount of `Client` instances. -* **interceptors** `{ Pool: DispatchInterceptor[] } }` - Default: `{ Pool: [] }` - A list of interceptors that are applied to the dispatch method. Additional logic can be applied (such as, but not limited to: 302 status code handling, authentication, cookies, compression and caching). ## Instance Properties diff --git a/index.js b/index.js index e421f93e4ae..444706560ae 100644 --- a/index.js +++ b/index.js @@ -21,7 +21,6 @@ const RetryHandler = require('./lib/handler/retry-handler') const { getGlobalDispatcher, setGlobalDispatcher } = require('./lib/global') const DecoratorHandler = require('./lib/handler/decorator-handler') const RedirectHandler = require('./lib/handler/redirect-handler') -const createRedirectInterceptor = require('./lib/interceptor/redirect-interceptor') Object.assign(Dispatcher.prototype, api) @@ -37,7 +36,6 @@ module.exports.RetryHandler = RetryHandler module.exports.DecoratorHandler = DecoratorHandler module.exports.RedirectHandler = RedirectHandler -module.exports.createRedirectInterceptor = createRedirectInterceptor module.exports.interceptors = { redirect: require('./lib/interceptor/redirect'), retry: require('./lib/interceptor/retry'), diff --git a/lib/api/api-connect.js b/lib/api/api-connect.js index f9dbbf64fe8..c8b86dd7d53 100644 --- a/lib/api/api-connect.js +++ b/lib/api/api-connect.js @@ -95,7 +95,9 @@ function connect (opts, callback) { try { const connectHandler = new ConnectHandler(opts, callback) - this.dispatch({ ...opts, method: 'CONNECT' }, connectHandler) + const connectOptions = { ...opts, method: 'CONNECT' } + + this.dispatch(connectOptions, connectHandler) } catch (err) { if (typeof callback !== 'function') { throw err diff --git a/lib/api/api-pipeline.js b/lib/api/api-pipeline.js index 322334e7848..1516e5dbd51 100644 --- a/lib/api/api-pipeline.js +++ b/lib/api/api-pipeline.js @@ -5,15 +5,15 @@ const { Duplex, PassThrough } = require('node:stream') +const assert = require('node:assert') +const { AsyncResource } = require('node:async_hooks') const { InvalidArgumentError, InvalidReturnValueError, RequestAbortedError } = require('../core/errors') const util = require('../core/util') -const { AsyncResource } = require('node:async_hooks') const { addSignal, removeSignal } = require('./abort-signal') -const assert = require('node:assert') const kResume = Symbol('resume') diff --git a/lib/api/api-request.js b/lib/api/api-request.js index 31cc3e5cd00..b988c2e7496 100644 --- a/lib/api/api-request.js +++ b/lib/api/api-request.js @@ -1,11 +1,11 @@ 'use strict' const assert = require('node:assert') +const { AsyncResource } = require('node:async_hooks') const { Readable } = require('./readable') const { InvalidArgumentError, RequestAbortedError } = require('../core/errors') const util = require('../core/util') const { getResolveErrorBodyCallback } = require('./util') -const { AsyncResource } = require('node:async_hooks') class RequestHandler extends AsyncResource { constructor (opts, callback) { @@ -189,7 +189,9 @@ function request (opts, callback) { } try { - this.dispatch(opts, new RequestHandler(opts, callback)) + const handler = new RequestHandler(opts, callback) + + this.dispatch(opts, handler) } catch (err) { if (typeof callback !== 'function') { throw err diff --git a/lib/api/api-stream.js b/lib/api/api-stream.js index fba2266dd7d..50f61632365 100644 --- a/lib/api/api-stream.js +++ b/lib/api/api-stream.js @@ -2,10 +2,10 @@ const assert = require('node:assert') const { finished, PassThrough } = require('node:stream') +const { AsyncResource } = require('node:async_hooks') const { InvalidArgumentError, InvalidReturnValueError } = require('../core/errors') const util = require('../core/util') const { getResolveErrorBodyCallback } = require('./util') -const { AsyncResource } = require('node:async_hooks') const { addSignal, removeSignal } = require('./abort-signal') class StreamHandler extends AsyncResource { @@ -207,7 +207,9 @@ function stream (opts, factory, callback) { } try { - this.dispatch(opts, new StreamHandler(opts, factory, callback)) + const handler = new StreamHandler(opts, factory, callback) + + this.dispatch(opts, handler) } catch (err) { if (typeof callback !== 'function') { throw err diff --git a/lib/api/api-upgrade.js b/lib/api/api-upgrade.js index 019fe1efa2d..6c2076d5ac1 100644 --- a/lib/api/api-upgrade.js +++ b/lib/api/api-upgrade.js @@ -2,9 +2,9 @@ const { InvalidArgumentError, SocketError } = require('../core/errors') const { AsyncResource } = require('node:async_hooks') +const assert = require('node:assert') const util = require('../core/util') const { addSignal, removeSignal } = require('./abort-signal') -const assert = require('node:assert') class UpgradeHandler extends AsyncResource { constructor (opts, callback) { @@ -91,11 +91,13 @@ function upgrade (opts, callback) { try { const upgradeHandler = new UpgradeHandler(opts, callback) - this.dispatch({ + const upgradeOpts = { ...opts, method: opts.method || 'GET', upgrade: opts.protocol || 'Websocket' - }, upgradeHandler) + } + + this.dispatch(upgradeOpts, upgradeHandler) } catch (err) { if (typeof callback !== 'function') { throw err diff --git a/lib/core/symbols.js b/lib/core/symbols.js index c8ba5dd8ec5..1c3732620b1 100644 --- a/lib/core/symbols.js +++ b/lib/core/symbols.js @@ -52,7 +52,6 @@ module.exports = { kMaxRequests: Symbol('maxRequestsPerClient'), kProxy: Symbol('proxy agent options'), kCounter: Symbol('socket request counter'), - kInterceptors: Symbol('dispatch interceptors'), kMaxResponseSize: Symbol('max response size'), kHTTP2Session: Symbol('http2Session'), kHTTP2SessionState: Symbol('http2Session state'), diff --git a/lib/dispatcher/agent.js b/lib/dispatcher/agent.js index 98f1486cac0..9e362fa0682 100644 --- a/lib/dispatcher/agent.js +++ b/lib/dispatcher/agent.js @@ -1,17 +1,15 @@ 'use strict' const { InvalidArgumentError } = require('../core/errors') -const { kClients, kRunning, kClose, kDestroy, kDispatch, kInterceptors } = require('../core/symbols') +const { kClients, kRunning, kClose, kDestroy, kDispatch } = require('../core/symbols') const DispatcherBase = require('./dispatcher-base') const Pool = require('./pool') const Client = require('./client') const util = require('../core/util') -const createRedirectInterceptor = require('../interceptor/redirect-interceptor') const kOnConnect = Symbol('onConnect') const kOnDisconnect = Symbol('onDisconnect') const kOnConnectionError = Symbol('onConnectionError') -const kMaxRedirections = Symbol('maxRedirections') const kOnDrain = Symbol('onDrain') const kFactory = Symbol('factory') const kOptions = Symbol('options') @@ -23,7 +21,7 @@ function defaultFactory (origin, opts) { } class Agent extends DispatcherBase { - constructor ({ factory = defaultFactory, maxRedirections = 0, connect, ...options } = {}) { + constructor ({ factory = defaultFactory, connect, ...options } = {}) { super() if (typeof factory !== 'function') { @@ -34,23 +32,11 @@ class Agent extends DispatcherBase { throw new InvalidArgumentError('connect must be a function or an object') } - if (!Number.isInteger(maxRedirections) || maxRedirections < 0) { - throw new InvalidArgumentError('maxRedirections must be a positive number') - } - if (connect && typeof connect !== 'function') { connect = { ...connect } } - this[kInterceptors] = options.interceptors?.Agent && Array.isArray(options.interceptors.Agent) - ? options.interceptors.Agent - : [createRedirectInterceptor({ maxRedirections })] - this[kOptions] = { ...util.deepClone(options), connect } - this[kOptions].interceptors = options.interceptors - ? { ...options.interceptors } - : undefined - this[kMaxRedirections] = maxRedirections this[kFactory] = factory this[kClients] = new Map() diff --git a/lib/dispatcher/balanced-pool.js b/lib/dispatcher/balanced-pool.js index 15a7e7b5879..0b1decf3014 100644 --- a/lib/dispatcher/balanced-pool.js +++ b/lib/dispatcher/balanced-pool.js @@ -13,7 +13,7 @@ const { kGetDispatcher } = require('./pool-base') const Pool = require('./pool') -const { kUrl, kInterceptors } = require('../core/symbols') +const { kUrl } = require('../core/symbols') const { parseOrigin } = require('../core/util') const kFactory = Symbol('factory') @@ -53,9 +53,6 @@ class BalancedPool extends PoolBase { throw new InvalidArgumentError('factory must be a function.') } - this[kInterceptors] = opts.interceptors?.BalancedPool && Array.isArray(opts.interceptors.BalancedPool) - ? opts.interceptors.BalancedPool - : [] this[kFactory] = factory for (const upstream of upstreams) { diff --git a/lib/dispatcher/client.js b/lib/dispatcher/client.js index cb61206b1ed..87714d5009d 100644 --- a/lib/dispatcher/client.js +++ b/lib/dispatcher/client.js @@ -43,13 +43,11 @@ const { kBodyTimeout, kStrictContentLength, kConnector, - kMaxRedirections, kMaxRequests, kCounter, kClose, kDestroy, kDispatch, - kInterceptors, kLocalAddress, kMaxResponseSize, kOnError, @@ -59,7 +57,6 @@ const { } = require('../core/symbols.js') const connectH1 = require('./client-h1.js') const connectH2 = require('./client-h2.js') -let deprecatedInterceptorWarned = false const kClosedResolve = Symbol('kClosedResolve') @@ -77,7 +74,6 @@ class Client extends DispatcherBase { * @param {import('../../types/client.js').Client.Options} options */ constructor (url, { - interceptors, maxHeaderSize, headersTimeout, socketTimeout, @@ -95,7 +91,6 @@ class Client extends DispatcherBase { tls, strictContentLength, maxCachedSessions, - maxRedirections, connect, maxRequestsPerClient, localAddress, @@ -164,10 +159,6 @@ class Client extends DispatcherBase { throw new InvalidArgumentError('connect must be a function or an object') } - if (maxRedirections != null && (!Number.isInteger(maxRedirections) || maxRedirections < 0)) { - throw new InvalidArgumentError('maxRedirections must be a positive number') - } - if (maxRequestsPerClient != null && (!Number.isInteger(maxRequestsPerClient) || maxRequestsPerClient < 0)) { throw new InvalidArgumentError('maxRequestsPerClient must be a positive number') } @@ -208,18 +199,6 @@ class Client extends DispatcherBase { }) } - if (interceptors?.Client && Array.isArray(interceptors.Client)) { - this[kInterceptors] = interceptors.Client - if (!deprecatedInterceptorWarned) { - deprecatedInterceptorWarned = true - process.emitWarning('Client.Options#interceptor is deprecated. Use Dispatcher#compose instead.', { - code: 'UNDICI-CLIENT-INTERCEPTOR-DEPRECATED' - }) - } - } else { - this[kInterceptors] = [createRedirectInterceptor({ maxRedirections })] - } - this[kUrl] = util.parseOrigin(url) this[kConnector] = connect this[kPipelining] = pipelining != null ? pipelining : 1 @@ -236,7 +215,6 @@ class Client extends DispatcherBase { this[kBodyTimeout] = bodyTimeout != null ? bodyTimeout : 300e3 this[kHeadersTimeout] = headersTimeout != null ? headersTimeout : 300e3 this[kStrictContentLength] = strictContentLength == null ? true : strictContentLength - this[kMaxRedirections] = maxRedirections this[kMaxRequests] = maxRequestsPerClient this[kClosedResolve] = null this[kMaxResponseSize] = maxResponseSize > -1 ? maxResponseSize : -1 @@ -362,8 +340,6 @@ class Client extends DispatcherBase { } } -const createRedirectInterceptor = require('../interceptor/redirect-interceptor.js') - function onError (client, err) { if ( client[kRunning] === 0 && diff --git a/lib/dispatcher/dispatcher-base.js b/lib/dispatcher/dispatcher-base.js index bd860acdcf4..afe4e9086db 100644 --- a/lib/dispatcher/dispatcher-base.js +++ b/lib/dispatcher/dispatcher-base.js @@ -6,11 +6,10 @@ const { ClientClosedError, InvalidArgumentError } = require('../core/errors') -const { kDestroy, kClose, kClosed, kDestroyed, kDispatch, kInterceptors } = require('../core/symbols') +const { kDestroy, kClose, kClosed, kDestroyed, kDispatch } = require('../core/symbols') const kOnDestroyed = Symbol('onDestroyed') const kOnClosed = Symbol('onClosed') -const kInterceptedDispatch = Symbol('Intercepted Dispatch') class DispatcherBase extends Dispatcher { constructor () { @@ -30,23 +29,6 @@ class DispatcherBase extends Dispatcher { return this[kClosed] } - get interceptors () { - return this[kInterceptors] - } - - set interceptors (newInterceptors) { - if (newInterceptors) { - for (let i = newInterceptors.length - 1; i >= 0; i--) { - const interceptor = this[kInterceptors][i] - if (typeof interceptor !== 'function') { - throw new InvalidArgumentError('interceptor must be an function') - } - } - } - - this[kInterceptors] = newInterceptors - } - close (callback) { if (callback === undefined) { return new Promise((resolve, reject) => { @@ -142,20 +124,6 @@ class DispatcherBase extends Dispatcher { }) } - [kInterceptedDispatch] (opts, handler) { - if (!this[kInterceptors] || this[kInterceptors].length === 0) { - this[kInterceptedDispatch] = this[kDispatch] - return this[kDispatch](opts, handler) - } - - let dispatch = this[kDispatch].bind(this) - for (let i = this[kInterceptors].length - 1; i >= 0; i--) { - dispatch = this[kInterceptors][i](dispatch) - } - this[kInterceptedDispatch] = dispatch - return dispatch(opts, handler) - } - dispatch (opts, handler) { if (!handler || typeof handler !== 'object') { throw new InvalidArgumentError('handler must be an object') @@ -174,7 +142,7 @@ class DispatcherBase extends Dispatcher { throw new ClientClosedError() } - return this[kInterceptedDispatch](opts, handler) + return this[kDispatch](opts, handler) } catch (err) { if (typeof handler.onError !== 'function') { throw new InvalidArgumentError('invalid onError method') diff --git a/lib/dispatcher/pool.js b/lib/dispatcher/pool.js index 2d84cd96488..9c59a4003c9 100644 --- a/lib/dispatcher/pool.js +++ b/lib/dispatcher/pool.js @@ -12,7 +12,7 @@ const { InvalidArgumentError } = require('../core/errors') const util = require('../core/util') -const { kUrl, kInterceptors } = require('../core/symbols') +const { kUrl } = require('../core/symbols') const buildConnector = require('../core/connect') const kOptions = Symbol('options') @@ -63,9 +63,6 @@ class Pool extends PoolBase { }) } - this[kInterceptors] = options.interceptors?.Pool && Array.isArray(options.interceptors.Pool) - ? options.interceptors.Pool - : [] this[kConnections] = connections || null this[kUrl] = util.parseOrigin(origin) this[kOptions] = { ...util.deepClone(options), connect, allowH2 } diff --git a/lib/handler/redirect-handler.js b/lib/handler/redirect-handler.js index 16a7b2150a9..3db7a476d45 100644 --- a/lib/handler/redirect-handler.js +++ b/lib/handler/redirect-handler.js @@ -24,6 +24,15 @@ class BodyAsyncIterable { } class RedirectHandler { + static buildDispatch (dispatcher, maxRedirections) { + if (maxRedirections != null && (!Number.isInteger(maxRedirections) || maxRedirections < 0)) { + throw new InvalidArgumentError('maxRedirections must be a positive number') + } + + const dispatch = dispatcher.dispatch.bind(dispatcher) + return (opts, originalHandler) => dispatch(opts, new RedirectHandler(dispatch, maxRedirections, opts, originalHandler)) + } + constructor (dispatch, maxRedirections, opts, handler) { if (maxRedirections != null && (!Number.isInteger(maxRedirections) || maxRedirections < 0)) { throw new InvalidArgumentError('maxRedirections must be a positive number') diff --git a/lib/interceptor/redirect-interceptor.js b/lib/interceptor/redirect-interceptor.js deleted file mode 100644 index 896ee8db939..00000000000 --- a/lib/interceptor/redirect-interceptor.js +++ /dev/null @@ -1,21 +0,0 @@ -'use strict' - -const RedirectHandler = require('../handler/redirect-handler') - -function createRedirectInterceptor ({ maxRedirections: defaultMaxRedirections }) { - return (dispatch) => { - return function Intercept (opts, handler) { - const { maxRedirections = defaultMaxRedirections } = opts - - if (!maxRedirections) { - return dispatch(opts, handler) - } - - const redirectHandler = new RedirectHandler(dispatch, maxRedirections, opts, handler) - opts = { ...opts, maxRedirections: 0 } // Stop sub dispatcher from also redirecting. - return dispatch(opts, redirectHandler) - } - } -} - -module.exports = createRedirectInterceptor diff --git a/lib/interceptor/redirect.js b/lib/interceptor/redirect.js index d2e789d8efb..55bad59c6ee 100644 --- a/lib/interceptor/redirect.js +++ b/lib/interceptor/redirect.js @@ -1,24 +1,21 @@ 'use strict' + const RedirectHandler = require('../handler/redirect-handler') -module.exports = opts => { - const globalMaxRedirections = opts?.maxRedirections - return dispatch => { - return function redirectInterceptor (opts, handler) { - const { maxRedirections = globalMaxRedirections, ...baseOpts } = opts +function createRedirectInterceptor ({ maxRedirections: defaultMaxRedirections } = {}) { + return (dispatch) => { + return function Intercept (opts, handler) { + const { maxRedirections = defaultMaxRedirections, ...rest } = opts - if (!maxRedirections) { + if (maxRedirections == null || maxRedirections === 0) { return dispatch(opts, handler) } - const redirectHandler = new RedirectHandler( - dispatch, - maxRedirections, - opts, - handler - ) - - return dispatch(baseOpts, redirectHandler) + const dispatchOpts = { ...rest, maxRedirections: 0 } // Stop sub dispatcher from also redirecting. + const redirectHandler = new RedirectHandler(dispatch, maxRedirections, dispatchOpts, handler) + return dispatch(dispatchOpts, redirectHandler) } } } + +module.exports = createRedirectInterceptor diff --git a/test/interceptor.js b/test/interceptor.js deleted file mode 100644 index 55a657120ea..00000000000 --- a/test/interceptor.js +++ /dev/null @@ -1,198 +0,0 @@ -'use strict' - -const { strictEqual, deepStrictEqual } = require('node:assert') -const { describe, beforeEach, afterEach, test } = require('node:test') -const { createServer } = require('node:http') -const { Agent, request } = require('../index') -const DecoratorHandler = require('../lib/handler/decorator-handler') - -const defaultOpts = { keepAliveTimeout: 10, keepAliveMaxTimeout: 10 } - -describe('interceptors', () => { - let server - beforeEach(async () => { - server = createServer((req, res) => { - res.setHeader('Content-Type', 'text/plain') - res.end('hello') - }) - await new Promise((resolve) => { server.listen(0, resolve) }) - }) - afterEach(async () => { - await new Promise((resolve) => server.close(resolve)) - }) - - test('interceptors are applied on client from an agent', async () => { - const interceptors = [] - const buildInterceptor = dispatch => { - const interceptorContext = { requestCount: 0 } - interceptors.push(interceptorContext) - return (opts, handler) => { - interceptorContext.requestCount++ - return dispatch(opts, handler) - } - } - - const opts = { interceptors: { Client: [buildInterceptor] }, ...defaultOpts } - const agent = new Agent(opts) - const origin = new URL(`http://localhost:${server.address().port}`) - await Promise.all([ - request(origin, { dispatcher: agent }), - request(origin, { dispatcher: agent }) - ]) - - // Assert that the requests are run on different interceptors (different Clients) - const requestCounts = interceptors.map(x => x.requestCount) - deepStrictEqual(requestCounts, [1, 1]) - }) - - test('interceptors are applied in the correct order', async () => { - const setHeaderInterceptor = (dispatch) => { - return (opts, handler) => { - opts.headers.push('foo', 'bar') - return dispatch(opts, handler) - } - } - - const assertHeaderInterceptor = (dispatch) => { - return (opts, handler) => { - deepStrictEqual(opts.headers, ['foo', 'bar']) - return dispatch(opts, handler) - } - } - - const opts = { interceptors: { Pool: [setHeaderInterceptor, assertHeaderInterceptor] }, ...defaultOpts } - const agent = new Agent(opts) - const origin = new URL(`http://localhost:${server.address().port}`) - await request(origin, { dispatcher: agent, headers: [] }) - }) - - test('interceptors handlers are called in reverse order', async () => { - const clearResponseHeadersInterceptor = (dispatch) => { - return (opts, handler) => { - class ResultInterceptor extends DecoratorHandler { - onHeaders (statusCode, headers, resume) { - return super.onHeaders(statusCode, [], resume) - } - } - - return dispatch(opts, new ResultInterceptor(handler)) - } - } - - const assertHeaderInterceptor = (dispatch) => { - return (opts, handler) => { - class ResultInterceptor extends DecoratorHandler { - onHeaders (statusCode, headers, resume) { - deepStrictEqual(headers, []) - return super.onHeaders(statusCode, headers, resume) - } - } - - return dispatch(opts, new ResultInterceptor(handler)) - } - } - - const opts = { interceptors: { Agent: [assertHeaderInterceptor, clearResponseHeadersInterceptor] }, ...defaultOpts } - const agent = new Agent(opts) - const origin = new URL(`http://localhost:${server.address().port}`) - await request(origin, { dispatcher: agent, headers: [] }) - }) -}) - -describe('interceptors with NtlmRequestHandler', () => { - class FakeNtlmRequestHandler { - constructor (dispatch, opts, handler) { - this.dispatch = dispatch - this.opts = opts - this.handler = handler - this.requestCount = 0 - } - - onConnect (...args) { - return this.handler.onConnect(...args) - } - - onError (...args) { - return this.handler.onError(...args) - } - - onUpgrade (...args) { - return this.handler.onUpgrade(...args) - } - - onHeaders (statusCode, headers, resume, statusText) { - this.requestCount++ - if (this.requestCount < 2) { - // Do nothing - } else { - return this.handler.onHeaders(statusCode, headers, resume, statusText) - } - } - - onData (...args) { - if (this.requestCount < 2) { - // Do nothing - } else { - return this.handler.onData(...args) - } - } - - onComplete (...args) { - if (this.requestCount < 2) { - this.dispatch(this.opts, this) - } else { - return this.handler.onComplete(...args) - } - } - - onBodySent (...args) { - if (this.requestCount < 2) { - // Do nothing - } else { - return this.handler.onBodySent(...args) - } - } - } - let server - - beforeEach(async () => { - // This Test is important because NTLM and Negotiate require several - // http requests in sequence to run on the same keepAlive socket - - const socketRequestCountSymbol = Symbol('Socket Request Count') - server = createServer((req, res) => { - if (req.socket[socketRequestCountSymbol] === undefined) { - req.socket[socketRequestCountSymbol] = 0 - } - req.socket[socketRequestCountSymbol]++ - res.setHeader('Content-Type', 'text/plain') - - // Simulate NTLM/Negotiate logic, by returning 200 - // on the second request of each socket - if (req.socket[socketRequestCountSymbol] >= 2) { - res.statusCode = 200 - res.end() - } else { - res.statusCode = 401 - res.end() - } - }) - await new Promise((resolve) => { server.listen(0, resolve) }) - }) - afterEach(async () => { - await new Promise((resolve) => server.close(resolve)) - }) - - test('Retry interceptor on Client will use the same socket', async () => { - const interceptor = dispatch => { - return (opts, handler) => { - return dispatch(opts, new FakeNtlmRequestHandler(dispatch, opts, handler)) - } - } - const opts = { interceptors: { Client: [interceptor] }, ...defaultOpts } - const agent = new Agent(opts) - const origin = new URL(`http://localhost:${server.address().port}`) - const { statusCode } = await request(origin, { dispatcher: agent, headers: [] }) - strictEqual(statusCode, 200) - }) -}) diff --git a/test/interceptors/redirect.js b/test/interceptors/redirect.js index 239c9597568..0d8c333cf7d 100644 --- a/test/interceptors/redirect.js +++ b/test/interceptors/redirect.js @@ -1,7 +1,9 @@ 'use strict' -const { tspl } = require('@matteo.collina/tspl') const { test, after } = require('node:test') +const { createServer } = require('node:http') +const { once } = require('node:events') +const { tspl } = require('@matteo.collina/tspl') const undici = require('../..') const { startRedirectingServer, @@ -10,7 +12,9 @@ const { startRedirectingWithoutLocationServer, startRedirectingWithAuthorization, startRedirectingWithCookie, - startRedirectingWithQueryParams + startRedirectingWithQueryParams, + startServer, + startRedirectingWithRelativePath } = require('../utils/redirecting-servers') const { createReadable, createReadableStream } = require('../utils/stream') @@ -20,14 +24,16 @@ const { for (const factory of [ (server, opts) => - new undici.Agent(opts).compose(redirect(opts?.maxRedirections)), + new undici.Agent(opts).compose( + redirect({ maxRedirections: opts?.maxRedirections }) + ), (server, opts) => new undici.Pool(`http://${server}`, opts).compose( - redirect(opts?.maxRedirections) + redirect({ maxRedirections: opts?.maxRedirections }) ), (server, opts) => new undici.Client(`http://${server}`, opts).compose( - redirect(opts?.maxRedirections) + redirect({ maxRedirections: opts?.maxRedirections }) ) ]) { const request = (t, server, opts, ...args) => { @@ -584,7 +590,7 @@ test('should follow redirections when going cross origin', async t => { context: { history } } = await undici.request(`http://${server1}`, { method: 'POST', - maxRedirections: 10 + dispatcher: new undici.Agent({}).compose(redirect({ maxRedirections: 10 })) }) const body = await bodyStream.text() @@ -613,7 +619,9 @@ test('should handle errors (callback)', async t => { undici.request( 'http://localhost:0', { - maxRedirections: 10 + dispatcher: new undici.Agent({}).compose( + redirect({ maxRedirections: 10 }) + ) }, error => { t.match(error.code, /EADDRNOTAVAIL|ECONNREFUSED/) @@ -627,7 +635,11 @@ test('should handle errors (promise)', async t => { t = tspl(t, { plan: 1 }) try { - await undici.request('http://localhost:0', { maxRedirections: 10 }) + await undici.request('http://localhost:0', { + dispatcher: new undici.Agent({}).compose( + redirect({ maxRedirections: 10 }) + ) + }) t.fail('Did not throw') } catch (error) { t.match(error.code, /EADDRNOTAVAIL|ECONNREFUSED/) @@ -641,7 +653,7 @@ test('removes authorization header on third party origin', async t => { const [server1] = await startRedirectingWithAuthorization('secret') const { body: bodyStream } = await undici.request(`http://${server1}`, { - maxRedirections: 10, + dispatcher: new undici.Agent({}).compose(redirect({ maxRedirections: 10 })), headers: { authorization: 'secret' } @@ -658,7 +670,7 @@ test('removes cookie header on third party origin', async t => { t = tspl(t, { plan: 1 }) const [server1] = await startRedirectingWithCookie('a=b') const { body: bodyStream } = await undici.request(`http://${server1}`, { - maxRedirections: 10, + dispatcher: new undici.Agent({}).compose(redirect({ maxRedirections: 10 })), headers: { cookie: 'a=b' } @@ -670,3 +682,94 @@ test('removes cookie header on third party origin', async t => { await t.completed }) + +test('should upgrade the connection when no redirects are present', async t => { + t = tspl(t, { plan: 2 }) + + const server = await startServer((req, res) => { + if (req.url === '/') { + res.statusCode = 301 + res.setHeader('Location', `http://${server}/end`) + res.end('REDIRECT') + return + } + + res.statusCode = 101 + res.setHeader('Connection', 'upgrade') + res.setHeader('Upgrade', req.headers.upgrade) + res.end('') + }) + + const { headers, socket } = await undici.upgrade(`http://${server}/`, { + method: 'GET', + protocol: 'foo/1', + dispatcher: new undici.Client(`http://${server}/`).compose(redirect({ maxRedirections: 10 })) + }) + + socket.end() + + t.strictEqual(headers.connection, 'upgrade') + t.strictEqual(headers.upgrade, 'foo/1') + + await t.completed +}) + +test('should redirect to relative URL according to RFC 7231', async t => { + t = tspl(t, { plan: 2 }) + + const server = await startRedirectingWithRelativePath() + + const { statusCode, body } = await undici.request(`http://${server}`, { + dispatcher: new undici.Client(`http://${server}/`).compose(redirect({ maxRedirections: 3 })) + }) + + const finalPath = await body.text() + + t.strictEqual(statusCode, 200) + t.strictEqual(finalPath, '/absolute/b') +}) + +test('Cross-origin redirects clear forbidden headers', async (t) => { + const { strictEqual } = tspl(t, { plan: 6 }) + + const server1 = createServer((req, res) => { + strictEqual(req.headers.cookie, undefined) + strictEqual(req.headers.authorization, undefined) + strictEqual(req.headers['proxy-authorization'], undefined) + + res.end('redirected') + }).listen(0) + + const server2 = createServer((req, res) => { + strictEqual(req.headers.authorization, 'test') + strictEqual(req.headers.cookie, 'ddd=dddd') + + res.writeHead(302, { + ...req.headers, + Location: `http://localhost:${server1.address().port}` + }) + res.end() + }).listen(0) + + t.after(() => { + server1.close() + server2.close() + }) + + await Promise.all([ + once(server1, 'listening'), + once(server2, 'listening') + ]) + + const res = await undici.request(`http://localhost:${server2.address().port}`, { + dispatcher: new undici.Agent({}).compose(redirect({ maxRedirections: 1 })), + headers: { + Authorization: 'test', + Cookie: 'ddd=dddd', + 'Proxy-Authorization': 'test' + } + }) + + const text = await res.body.text() + strictEqual(text, 'redirected') +}) diff --git a/test/jest/interceptor.test.js b/test/jest/interceptor.test.js deleted file mode 100644 index 84e6210fb8d..00000000000 --- a/test/jest/interceptor.test.js +++ /dev/null @@ -1,197 +0,0 @@ -'use strict' - -const { createServer } = require('node:http') -const { Agent, request } = require('../../index') -const DecoratorHandler = require('../../lib/handler/decorator-handler') -/* global expect */ - -const defaultOpts = { keepAliveTimeout: 10, keepAliveMaxTimeout: 10 } - -describe('interceptors', () => { - let server - beforeEach(async () => { - server = createServer((req, res) => { - res.setHeader('Content-Type', 'text/plain') - res.end('hello') - }) - await new Promise((resolve) => { server.listen(0, resolve) }) - }) - afterEach(async () => { - await new Promise((resolve) => server.close(resolve)) - }) - - test('interceptors are applied on client from an agent', async () => { - const interceptors = [] - const buildInterceptor = dispatch => { - const interceptorContext = { requestCount: 0 } - interceptors.push(interceptorContext) - return (opts, handler) => { - interceptorContext.requestCount++ - return dispatch(opts, handler) - } - } - - const opts = { interceptors: { Client: [buildInterceptor] }, ...defaultOpts } - const agent = new Agent(opts) - const origin = new URL(`http://localhost:${server.address().port}`) - await Promise.all([ - request(origin, { dispatcher: agent }), - request(origin, { dispatcher: agent }) - ]) - - // Assert that the requests are run on different interceptors (different Clients) - const requestCounts = interceptors.map(x => x.requestCount) - expect(requestCounts).toEqual([1, 1]) - }) - - test('interceptors are applied in the correct order', async () => { - const setHeaderInterceptor = (dispatch) => { - return (opts, handler) => { - opts.headers.push('foo', 'bar') - return dispatch(opts, handler) - } - } - - const assertHeaderInterceptor = (dispatch) => { - return (opts, handler) => { - expect(opts.headers).toEqual(['foo', 'bar']) - return dispatch(opts, handler) - } - } - - const opts = { interceptors: { Pool: [setHeaderInterceptor, assertHeaderInterceptor] }, ...defaultOpts } - const agent = new Agent(opts) - const origin = new URL(`http://localhost:${server.address().port}`) - await request(origin, { dispatcher: agent, headers: [] }) - }) - - test('interceptors handlers are called in reverse order', async () => { - const clearResponseHeadersInterceptor = (dispatch) => { - return (opts, handler) => { - class ResultInterceptor extends DecoratorHandler { - onHeaders (statusCode, headers, resume) { - return super.onHeaders(statusCode, [], resume) - } - } - - return dispatch(opts, new ResultInterceptor(handler)) - } - } - - const assertHeaderInterceptor = (dispatch) => { - return (opts, handler) => { - class ResultInterceptor extends DecoratorHandler { - onHeaders (statusCode, headers, resume) { - expect(headers).toEqual([]) - return super.onHeaders(statusCode, headers, resume) - } - } - - return dispatch(opts, new ResultInterceptor(handler)) - } - } - - const opts = { interceptors: { Agent: [assertHeaderInterceptor, clearResponseHeadersInterceptor] }, ...defaultOpts } - const agent = new Agent(opts) - const origin = new URL(`http://localhost:${server.address().port}`) - await request(origin, { dispatcher: agent, headers: [] }) - }) -}) - -describe('interceptors with NtlmRequestHandler', () => { - class FakeNtlmRequestHandler { - constructor (dispatch, opts, handler) { - this.dispatch = dispatch - this.opts = opts - this.handler = handler - this.requestCount = 0 - } - - onConnect (...args) { - return this.handler.onConnect(...args) - } - - onError (...args) { - return this.handler.onError(...args) - } - - onUpgrade (...args) { - return this.handler.onUpgrade(...args) - } - - onHeaders (statusCode, headers, resume, statusText) { - this.requestCount++ - if (this.requestCount < 2) { - // Do nothing - } else { - return this.handler.onHeaders(statusCode, headers, resume, statusText) - } - } - - onData (...args) { - if (this.requestCount < 2) { - // Do nothing - } else { - return this.handler.onData(...args) - } - } - - onComplete (...args) { - if (this.requestCount < 2) { - this.dispatch(this.opts, this) - } else { - return this.handler.onComplete(...args) - } - } - - onBodySent (...args) { - if (this.requestCount < 2) { - // Do nothing - } else { - return this.handler.onBodySent(...args) - } - } - } - let server - - beforeEach(async () => { - // This Test is important because NTLM and Negotiate require several - // http requests in sequence to run on the same keepAlive socket - - const socketRequestCountSymbol = Symbol('Socket Request Count') - server = createServer((req, res) => { - if (req.socket[socketRequestCountSymbol] === undefined) { - req.socket[socketRequestCountSymbol] = 0 - } - req.socket[socketRequestCountSymbol]++ - res.setHeader('Content-Type', 'text/plain') - - // Simulate NTLM/Negotiate logic, by returning 200 - // on the second request of each socket - if (req.socket[socketRequestCountSymbol] >= 2) { - res.statusCode = 200 - res.end() - } else { - res.statusCode = 401 - res.end() - } - }) - await new Promise((resolve) => { server.listen(0, resolve) }) - }) - afterEach(async () => { - await new Promise((resolve) => server.close(resolve)) - }) - - test('Retry interceptor on Client will use the same socket', async () => { - const interceptor = dispatch => { - return (opts, handler) => { - return dispatch(opts, new FakeNtlmRequestHandler(dispatch, opts, handler)) - } - } - const opts = { interceptors: { Client: [interceptor] }, ...defaultOpts } - const agent = new Agent(opts) - const origin = new URL(`http://localhost:${server.address().port}`) - const { statusCode } = await request(origin, { dispatcher: agent, headers: [] }) - expect(statusCode).toEqual(200) - }) -}) diff --git a/test/node-test/agent.js b/test/node-test/agent.js index 74a3ae26e77..c5238d7b570 100644 --- a/test/node-test/agent.js +++ b/test/node-test/agent.js @@ -662,11 +662,8 @@ test('stream: fails with invalid onInfo', async (t) => { }) test('constructor validations', t => { - const p = tspl(t, { plan: 4 }) + const p = tspl(t, { plan: 1 }) p.throws(() => new Agent({ factory: 'ASD' }), errors.InvalidArgumentError, 'throws on invalid opts argument') - p.throws(() => new Agent({ maxRedirections: 'ASD' }), errors.InvalidArgumentError, 'throws on invalid opts argument') - p.throws(() => new Agent({ maxRedirections: -1 }), errors.InvalidArgumentError, 'throws on invalid opts argument') - p.throws(() => new Agent({ maxRedirections: null }), errors.InvalidArgumentError, 'throws on invalid opts argument') }) test('dispatch validations', async t => { diff --git a/test/redirect-cross-origin-header.js b/test/redirect-cross-origin-header.js deleted file mode 100644 index 451563cadc5..00000000000 --- a/test/redirect-cross-origin-header.js +++ /dev/null @@ -1,52 +0,0 @@ -'use strict' - -const { test } = require('node:test') -const { tspl } = require('@matteo.collina/tspl') -const { createServer } = require('node:http') -const { once } = require('node:events') -const { request } = require('..') - -test('Cross-origin redirects clear forbidden headers', async (t) => { - const { strictEqual } = tspl(t, { plan: 6 }) - - const server1 = createServer((req, res) => { - strictEqual(req.headers.cookie, undefined) - strictEqual(req.headers.authorization, undefined) - strictEqual(req.headers['proxy-authorization'], undefined) - - res.end('redirected') - }).listen(0) - - const server2 = createServer((req, res) => { - strictEqual(req.headers.authorization, 'test') - strictEqual(req.headers.cookie, 'ddd=dddd') - - res.writeHead(302, { - ...req.headers, - Location: `http://localhost:${server1.address().port}` - }) - res.end() - }).listen(0) - - t.after(() => { - server1.close() - server2.close() - }) - - await Promise.all([ - once(server1, 'listening'), - once(server2, 'listening') - ]) - - const res = await request(`http://localhost:${server2.address().port}`, { - maxRedirections: 1, - headers: { - Authorization: 'test', - Cookie: 'ddd=dddd', - 'Proxy-Authorization': 'test' - } - }) - - const text = await res.body.text() - strictEqual(text, 'redirected') -}) diff --git a/test/redirect-pipeline.js b/test/redirect-pipeline.js index 98896fae19c..52e4545e0d6 100644 --- a/test/redirect-pipeline.js +++ b/test/redirect-pipeline.js @@ -2,13 +2,14 @@ const { tspl } = require('@matteo.collina/tspl') const { test } = require('node:test') -const { pipeline: undiciPipeline } = require('..') +const { pipeline: undiciPipeline, Client, interceptors } = require('..') const { pipeline: streamPipelineCb } = require('node:stream') const { promisify } = require('node:util') const { createReadable, createWritable } = require('./utils/stream') const { startRedirectingServer } = require('./utils/redirecting-servers') const streamPipeline = promisify(streamPipelineCb) +const redirect = interceptors.redirect test('should not follow redirection by default if not using RedirectAgent', async t => { t = tspl(t, { plan: 3 }) @@ -18,7 +19,9 @@ test('should not follow redirection by default if not using RedirectAgent', asyn await streamPipeline( createReadable('REQUEST'), - undiciPipeline(`http://${serverRoot}/`, {}, ({ statusCode, headers, body }) => { + undiciPipeline(`http://${serverRoot}/`, { + dispatcher: new Client(`http://${serverRoot}/`).compose(redirect({ maxRedirections: null })) + }, ({ statusCode, headers, body }) => { t.strictEqual(statusCode, 302) t.strictEqual(headers.location, `http://${serverRoot}/302/1`) @@ -38,7 +41,7 @@ test('should not follow redirects when using RedirectAgent within pipeline', asy await streamPipeline( createReadable('REQUEST'), - undiciPipeline(`http://${serverRoot}/`, { maxRedirections: 1 }, ({ statusCode, headers, body }) => { + undiciPipeline(`http://${serverRoot}/`, { dispatcher: new Client(`http://${serverRoot}/`).compose(redirect({ maxRedirections: 1 })) }, ({ statusCode, headers, body }) => { t.strictEqual(statusCode, 302) t.strictEqual(headers.location, `http://${serverRoot}/302/1`) diff --git a/test/redirect-relative.js b/test/redirect-relative.js deleted file mode 100644 index e4d45406e75..00000000000 --- a/test/redirect-relative.js +++ /dev/null @@ -1,23 +0,0 @@ -'use strict' - -const { tspl } = require('@matteo.collina/tspl') -const { test } = require('node:test') -const { request } = require('..') -const { - startRedirectingWithRelativePath -} = require('./utils/redirecting-servers') - -test('should redirect to relative URL according to RFC 7231', async t => { - t = tspl(t, { plan: 2 }) - - const server = await startRedirectingWithRelativePath() - - const { statusCode, body } = await request(`http://${server}`, { - maxRedirections: 3 - }) - - const finalPath = await body.text() - - t.strictEqual(statusCode, 200) - t.strictEqual(finalPath, '/absolute/b') -}) diff --git a/test/redirect-request.js b/test/redirect-request.js index e7554b929f6..c7264bc5faf 100644 --- a/test/redirect-request.js +++ b/test/redirect-request.js @@ -13,11 +13,12 @@ const { startRedirectingWithQueryParams } = require('./utils/redirecting-servers') const { createReadable, createReadableStream } = require('./utils/stream') +const redirect = undici.interceptors.redirect for (const factory of [ - (server, opts) => new undici.Agent(opts), - (server, opts) => new undici.Pool(`http://${server}`, opts), - (server, opts) => new undici.Client(`http://${server}`, opts) + (server, opts) => new undici.Agent(opts).compose(redirect({ maxRedirections: opts?.maxRedirections })), + (server, opts) => new undici.Pool(`http://${server}`, opts).compose(redirect({ maxRedirections: opts?.maxRedirections })), + (server, opts) => new undici.Client(`http://${server}`, opts).compose(redirect({ maxRedirections: opts?.maxRedirections })) ]) { const request = (t, server, opts, ...args) => { const dispatcher = factory(server, opts) @@ -90,7 +91,7 @@ for (const factory of [ const server = await startRedirectingServer() - const { statusCode, headers, body: bodyStream, context: { history } } = await request(t, server, { maxRedirections: 10 }, `http://${server}/300?key=value`) + const { statusCode, headers, body: bodyStream, context: { history } } = await request(t, server, undefined, `http://${server}/300?key=value`, { maxRedirections: 10 }) const body = await bodyStream.text() t.strictEqual(statusCode, 200) @@ -380,10 +381,9 @@ for (const factory of [ t = tspl(t, { plan: 1 }) try { - await request(t, 'localhost', { + await request(t, 'localhost', undefined, 'http://localhost', { + method: 'GET', maxRedirections: 'INVALID' - }, 'http://localhost', { - method: 'GET' }) t.fail('Did not throw') @@ -441,7 +441,7 @@ test('should follow redirections when going cross origin', async t => { const { statusCode, headers, body: bodyStream, context: { history } } = await undici.request(`http://${server1}`, { method: 'POST', - maxRedirections: 10 + dispatcher: new undici.Agent({}).compose(redirect({ maxRedirections: 10 })) }) const body = await bodyStream.text() @@ -467,7 +467,7 @@ test('should handle errors (callback)', async t => { undici.request( 'http://localhost:0', { - maxRedirections: 10 + dispatcher: new undici.Agent({}).compose(redirect({ maxRedirections: 10 })) }, error => { t.match(error.code, /EADDRNOTAVAIL|ECONNREFUSED/) @@ -481,7 +481,7 @@ test('should handle errors (promise)', async t => { t = tspl(t, { plan: 1 }) try { - await undici.request('http://localhost:0', { maxRedirections: 10 }) + await undici.request('http://localhost:0', { dispatcher: new undici.Agent({}).compose(redirect({ maxRedirections: 10 })) }) t.fail('Did not throw') } catch (error) { t.match(error.code, /EADDRNOTAVAIL|ECONNREFUSED/) @@ -495,7 +495,7 @@ test('removes authorization header on third party origin', async t => { const [server1] = await startRedirectingWithAuthorization('secret') const { body: bodyStream } = await undici.request(`http://${server1}`, { - maxRedirections: 10, + dispatcher: new undici.Agent({}).compose(redirect({ maxRedirections: 10 })), headers: { authorization: 'secret' } @@ -512,7 +512,7 @@ test('removes cookie header on third party origin', async t => { t = tspl(t, { plan: 1 }) const [server1] = await startRedirectingWithCookie('a=b') const { body: bodyStream } = await undici.request(`http://${server1}`, { - maxRedirections: 10, + dispatcher: new undici.Agent({}).compose(redirect({ maxRedirections: 10 })), headers: { cookie: 'a=b' } diff --git a/test/redirect-stream.js b/test/redirect-stream.js index c50e5033135..f45456d331e 100644 --- a/test/redirect-stream.js +++ b/test/redirect-stream.js @@ -2,7 +2,7 @@ const { tspl } = require('@matteo.collina/tspl') const { test, describe } = require('node:test') -const { stream } = require('..') +const { stream, Agent, Client, interceptors: { redirect } } = require('..') const { startRedirectingServer, startRedirectingWithBodyServer, @@ -21,7 +21,7 @@ test('should always have a history with the final URL even if no redirections we await stream( `http://${server}/200?key=value`, - { opaque: body, maxRedirections: 10 }, + { opaque: body, dispatcher: new Client(`http://${server}/`).compose(redirect({ maxRedirections: 10 })) }, ({ statusCode, headers, opaque, context: { history } }) => { t.strictEqual(statusCode, 200) t.strictEqual(headers.location, undefined) @@ -36,13 +36,13 @@ test('should always have a history with the final URL even if no redirections we t.strictEqual(body.join(''), `GET /5 key=value :: host@${server} connection@keep-alive`) }) -test('should not follow redirection by default if not using RedirectAgent', async t => { +test('should not follow redirection by default if max redirect = 0', async t => { t = tspl(t, { plan: 3 }) const body = [] const server = await startRedirectingServer() - await stream(`http://${server}`, { opaque: body }, ({ statusCode, headers, opaque }) => { + await stream(`http://${server}`, { opaque: body, dispatcher: new Agent({}).compose(redirect({ maxRedirections: 0 })) }, ({ statusCode, headers, opaque }) => { t.strictEqual(statusCode, 302) t.strictEqual(headers.location, `http://${server}/302/1`) @@ -60,7 +60,7 @@ test('should follow redirection after a HTTP 300', async t => { await stream( `http://${server}/300?key=value`, - { opaque: body, maxRedirections: 10 }, + { opaque: body, dispatcher: new Client(`http://${server}/`).compose(redirect({ maxRedirections: 10 })) }, ({ statusCode, headers, opaque, context: { history } }) => { t.strictEqual(statusCode, 200) t.strictEqual(headers.location, undefined) @@ -88,7 +88,7 @@ test('should follow redirection after a HTTP 301', async t => { await stream( `http://${server}/301`, - { method: 'POST', body: 'REQUEST', opaque: body, maxRedirections: 10 }, + { method: 'POST', body: 'REQUEST', opaque: body, dispatcher: new Client(`http://${server}/`).compose(redirect({ maxRedirections: 10 })) }, ({ statusCode, headers, opaque }) => { t.strictEqual(statusCode, 200) t.strictEqual(headers.location, undefined) @@ -108,7 +108,7 @@ test('should follow redirection after a HTTP 302', async t => { await stream( `http://${server}/302`, - { method: 'PUT', body: Buffer.from('REQUEST'), opaque: body, maxRedirections: 10 }, + { method: 'PUT', body: Buffer.from('REQUEST'), opaque: body, dispatcher: new Client(`http://${server}/`).compose(redirect({ maxRedirections: 10 })) }, ({ statusCode, headers, opaque }) => { t.strictEqual(statusCode, 200) t.strictEqual(headers.location, undefined) @@ -126,7 +126,7 @@ test('should follow redirection after a HTTP 303 changing method to GET', async const body = [] const server = await startRedirectingServer() - await stream(`http://${server}/303`, { opaque: body, maxRedirections: 10 }, ({ statusCode, headers, opaque }) => { + await stream(`http://${server}/303`, { opaque: body, dispatcher: new Client(`http://${server}/`).compose(redirect({ maxRedirections: 10 })) }, ({ statusCode, headers, opaque }) => { t.strictEqual(statusCode, 200) t.strictEqual(headers.location, undefined) @@ -163,7 +163,7 @@ test('should remove Host and request body related headers when following HTTP 30 '4' ], opaque: body, - maxRedirections: 10 + dispatcher: new Client(`http://${server}/`).compose(redirect({ maxRedirections: 10 })) }, ({ statusCode, headers, opaque }) => { t.strictEqual(statusCode, 200) @@ -196,7 +196,7 @@ test('should remove Host and request body related headers when following HTTP 30 'X-Bar': '4' }, opaque: body, - maxRedirections: 10 + dispatcher: new Client(`http://${server}/`).compose(redirect({ maxRedirections: 10 })) }, ({ statusCode, headers, opaque }) => { t.strictEqual(statusCode, 200) @@ -217,7 +217,7 @@ test('should follow redirection after a HTTP 307', async t => { await stream( `http://${server}/307`, - { method: 'DELETE', opaque: body, maxRedirections: 10 }, + { method: 'DELETE', opaque: body, dispatcher: new Client(`http://${server}/`).compose(redirect({ maxRedirections: 10 })) }, ({ statusCode, headers, opaque }) => { t.strictEqual(statusCode, 200) t.strictEqual(headers.location, undefined) @@ -237,7 +237,7 @@ test('should follow redirection after a HTTP 308', async t => { await stream( `http://${server}/308`, - { method: 'OPTIONS', opaque: body, maxRedirections: 10 }, + { method: 'OPTIONS', opaque: body, dispatcher: new Client(`http://${server}/`).compose(redirect({ maxRedirections: 10 })) }, ({ statusCode, headers, opaque }) => { t.strictEqual(statusCode, 200) t.strictEqual(headers.location, undefined) @@ -257,7 +257,7 @@ test('should ignore HTTP 3xx response bodies', async t => { await stream( `http://${server}/`, - { opaque: body, maxRedirections: 10 }, + { opaque: body, dispatcher: new Client(`http://${server}/`).compose(redirect({ maxRedirections: 10 })) }, ({ statusCode, headers, opaque, context: { history } }) => { t.strictEqual(statusCode, 200) t.strictEqual(headers.location, undefined) @@ -278,7 +278,7 @@ test('should follow a redirect chain up to the allowed number of times', async t await stream( `http://${server}/300`, - { opaque: body, maxRedirections: 2 }, + { opaque: body, dispatcher: new Client(`http://${server}/`).compose(redirect({ maxRedirections: 2 })) }, ({ statusCode, headers, opaque, context: { history } }) => { t.strictEqual(statusCode, 300) t.strictEqual(headers.location, `http://${server}/300/3`) @@ -299,7 +299,7 @@ test('should follow redirections when going cross origin', async t => { await stream( `http://${server1}`, - { method: 'POST', opaque: body, maxRedirections: 10 }, + { method: 'POST', opaque: body, dispatcher: new Agent({}).compose(redirect({ maxRedirections: 10 })) }, ({ statusCode, headers, opaque, context: { history } }) => { t.strictEqual(statusCode, 200) t.strictEqual(headers.location, undefined) diff --git a/test/redirect-upgrade.js b/test/redirect-upgrade.js deleted file mode 100644 index 5e331797379..00000000000 --- a/test/redirect-upgrade.js +++ /dev/null @@ -1,37 +0,0 @@ -'use strict' - -const { tspl } = require('@matteo.collina/tspl') -const { test } = require('node:test') -const { upgrade } = require('..') -const { startServer } = require('./utils/redirecting-servers') - -test('should upgrade the connection when no redirects are present', async t => { - t = tspl(t, { plan: 2 }) - - const server = await startServer((req, res) => { - if (req.url === '/') { - res.statusCode = 301 - res.setHeader('Location', `http://${server}/end`) - res.end('REDIRECT') - return - } - - res.statusCode = 101 - res.setHeader('Connection', 'upgrade') - res.setHeader('Upgrade', req.headers.upgrade) - res.end('') - }) - - const { headers, socket } = await upgrade(`http://${server}/`, { - method: 'GET', - protocol: 'foo/1', - maxRedirections: 10 - }) - - socket.end() - - t.strictEqual(headers.connection, 'upgrade') - t.strictEqual(headers.upgrade, 'foo/1') - - await t.completed -})