diff --git a/docs/docs/api/Agent.md b/docs/docs/api/Agent.md index dd5d99bc1e1..86ce0bb62ee 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 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/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/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..3adb6be15dd 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') @@ -240,7 +240,9 @@ class PipelineHandler extends AsyncResource { function pipeline (opts, handler) { try { const pipelineHandler = new PipelineHandler(opts, handler) + this.dispatch({ ...opts, body: pipelineHandler.req }, pipelineHandler) + return pipelineHandler.ret } catch (err) { return new PassThrough().destroy(err) diff --git a/lib/api/api-request.js b/lib/api/api-request.js index 31cc3e5cd00..8cc63be2e97 100644 --- a/lib/api/api-request.js +++ b/lib/api/api-request.js @@ -1,11 +1,13 @@ 'use strict' +const { AsyncResource } = require('node:async_hooks') const assert = require('node:assert') -const { Readable } = require('./readable') + const { InvalidArgumentError, RequestAbortedError } = require('../core/errors') const util = require('../core/util') + +const { Readable } = require('./readable') const { getResolveErrorBodyCallback } = require('./util') -const { AsyncResource } = require('node:async_hooks') class RequestHandler extends AsyncResource { constructor (opts, callback) { @@ -189,7 +191,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..c004ec79433 100644 --- a/lib/api/api-stream.js +++ b/lib/api/api-stream.js @@ -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..1965ecd4430 100644 --- a/lib/api/api-upgrade.js +++ b/lib/api/api-upgrade.js @@ -1,10 +1,10 @@ 'use strict' -const { InvalidArgumentError, SocketError } = require('../core/errors') +const assert = require('node:assert') const { AsyncResource } = require('node:async_hooks') +const { InvalidArgumentError, SocketError } = require('../core/errors') 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..d88227c4e33 100644 --- a/lib/dispatcher/client.js +++ b/lib/dispatcher/client.js @@ -49,7 +49,6 @@ const { kClose, kDestroy, kDispatch, - kInterceptors, kLocalAddress, kMaxResponseSize, kOnError, @@ -59,7 +58,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 +75,6 @@ class Client extends DispatcherBase { * @param {import('../../types/client.js').Client.Options} options */ constructor (url, { - interceptors, maxHeaderSize, headersTimeout, socketTimeout, @@ -208,18 +205,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 @@ -362,8 +347,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..87f14aef3c0 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,15 +63,9 @@ 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 } - this[kOptions].interceptors = options.interceptors - ? { ...options.interceptors } - : undefined this[kFactory] = factory } diff --git a/lib/dispatcher/proxy-agent.js b/lib/dispatcher/proxy-agent.js index 226b67846da..1924bcc896d 100644 --- a/lib/dispatcher/proxy-agent.js +++ b/lib/dispatcher/proxy-agent.js @@ -1,6 +1,6 @@ 'use strict' -const { kProxy, kClose, kDestroy, kInterceptors } = require('../core/symbols') +const { kProxy, kClose, kDestroy } = require('../core/symbols') const { URL } = require('node:url') const Agent = require('./agent') const Pool = require('./pool') @@ -40,9 +40,6 @@ class ProxyAgent extends DispatcherBase { const { href, origin, port, protocol, username, password, hostname: proxyHostname } = url this[kProxy] = { uri: href, protocol } - this[kInterceptors] = opts.interceptors?.ProxyAgent && Array.isArray(opts.interceptors.ProxyAgent) - ? opts.interceptors.ProxyAgent - : [] this[kRequestTls] = opts.requestTls this[kProxyTls] = opts.proxyTls this[kProxyHeaders] = opts.headers || {} 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..996313754dc 100644 --- a/test/interceptors/redirect.js +++ b/test/interceptors/redirect.js @@ -1,6 +1,8 @@ 'use strict' const { tspl } = require('@matteo.collina/tspl') +const { createServer } = require('node:http') +const { once } = require('node:events') const { test, after } = require('node:test') const undici = require('../..') const { @@ -10,7 +12,8 @@ const { startRedirectingWithoutLocationServer, startRedirectingWithAuthorization, startRedirectingWithCookie, - startRedirectingWithQueryParams + startRedirectingWithQueryParams, + startRedirectingWithRelativePath } = require('../utils/redirecting-servers') const { createReadable, createReadableStream } = require('../utils/stream') @@ -570,6 +573,63 @@ for (const factory of [ t.strictEqual(body.length, 0) 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 request( + t, + server, + undefined, + `http://${server}`, + { + maxRedirections: 3 + } + ) + + const finalPath = await body.text() + + t.strictEqual(statusCode, 200) + t.strictEqual(finalPath, '/absolute/b') + }) + + // test('should follow redirections when going cross origin', { only: true }, async t => { + // t = tspl(t, { plan: 4 }) + + // const [server1, server2, server3] = await startRedirectingChainServers() + + // const { + // statusCode, + // headers, + // body: bodyStream, + // context: { history } + // } = await request(t, server1, undefined, `http://${server1}`, { + // method: 'POST', + // maxRedirections: 10 + // }) + + // const body = await bodyStream.text() + + // // console.log('history') + // t.strictEqual(statusCode, 200) + // t.ok(!headers.location) + // t.deepStrictEqual( + // history.map(x => x.toString()), + // [ + // `http://${server1}/`, + // `http://${server2}/`, + // `http://${server3}/`, + // `http://${server2}/end`, + // `http://${server3}/end`, + // `http://${server1}/end` + // ] + // ) + // t.strictEqual(body, 'POST') + + // await t.completed + // }) } test('should follow redirections when going cross origin', async t => { 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-request.js b/test/redirect-request.js index e7554b929f6..fdcea5dd065 100644 --- a/test/redirect-request.js +++ b/test/redirect-request.js @@ -90,7 +90,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 +380,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') diff --git a/test/redirect-stream.js b/test/redirect-stream.js index c50e5033135..3d03cc946d2 100644 --- a/test/redirect-stream.js +++ b/test/redirect-stream.js @@ -36,7 +36,7 @@ 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 = [] diff --git a/test/types/client.test-d.ts b/test/types/client.test-d.ts index c416d779bec..d9c1e447813 100644 --- a/test/types/client.test-d.ts +++ b/test/types/client.test-d.ts @@ -68,18 +68,6 @@ expectAssignable(new Client(new URL('http://localhost'), {})) expectAssignable(new Client('', { autoSelectFamilyAttemptTimeout: 300e3 })) - expectAssignable(new Client('', { - interceptors: { - Client: [(dispatcher) => { - expectAssignable(dispatcher); - return (opts, handlers) => { - expectAssignable(opts); - expectAssignable(handlers); - return dispatcher(opts, handlers) - } - }] - } - })) } { diff --git a/test/types/interceptor.test-d.ts b/test/types/interceptor.test-d.ts deleted file mode 100644 index ba242bfcadb..00000000000 --- a/test/types/interceptor.test-d.ts +++ /dev/null @@ -1,5 +0,0 @@ -import {expectAssignable} from "tsd"; -import Undici from "../.."; -import Dispatcher from "../../types/dispatcher"; - -expectAssignable(Undici.createRedirectInterceptor({ maxRedirections: 3 })) diff --git a/types/agent.d.ts b/types/agent.d.ts index 58081ce9372..4ff6baccd86 100644 --- a/types/agent.d.ts +++ b/types/agent.d.ts @@ -20,8 +20,6 @@ declare namespace Agent { factory?(origin: string | URL, opts: Object): Dispatcher; /** Integer. Default: `0` */ maxRedirections?: number; - - interceptors?: { Agent?: readonly Dispatcher.DispatchInterceptor[] } & Pool.Options["interceptors"] } export interface DispatchOptions extends Dispatcher.DispatchOptions { diff --git a/types/client.d.ts b/types/client.d.ts index d0a5379f33c..be07a47d351 100644 --- a/types/client.d.ts +++ b/types/client.d.ts @@ -28,12 +28,7 @@ export class Client extends Dispatcher { } export declare namespace Client { - export interface OptionsInterceptors { - Client: readonly Dispatcher.DispatchInterceptor[]; - } export interface Options { - /** TODO */ - interceptors?: OptionsInterceptors; /** The maximum length of request headers in bytes. Default: Node.js' `--max-http-header-size` or `16384` (16KiB). */ maxHeaderSize?: number; /** The amount of time, in milliseconds, the parser will wait to receive the complete HTTP headers (Node 14 and above only). Default: `300e3` milliseconds (300s). */ diff --git a/types/index.d.ts b/types/index.d.ts index 5b540211df7..6d7931fa644 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -40,7 +40,6 @@ declare namespace Undici { var RedirectHandler: typeof import ('./handlers').RedirectHandler var DecoratorHandler: typeof import ('./handlers').DecoratorHandler var RetryHandler: typeof import ('./retry-handler').default - var createRedirectInterceptor: typeof import ('./interceptors').default.createRedirectInterceptor var BalancedPool: typeof import('./balanced-pool').default; var Client: typeof import('./client').default; var buildConnector: typeof import('./connector').default; diff --git a/types/pool.d.ts b/types/pool.d.ts index bad5ba0308e..d24cfd06870 100644 --- a/types/pool.d.ts +++ b/types/pool.d.ts @@ -33,7 +33,5 @@ declare namespace Pool { factory?(origin: URL, opts: object): Dispatcher; /** The max number of clients to create. `null` if no limit. Default `null`. */ connections?: number | null; - - interceptors?: { Pool?: readonly Dispatcher.DispatchInterceptor[] } & Client.Options["interceptors"] } }