From 10cf8adc76a313f6375bdd9b9a770f2b766b886e Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Tue, 1 Oct 2024 01:10:57 +0200 Subject: [PATCH 1/8] fix: mock interceptor should ignore trailing slashes --- lib/mock/mock-utils.js | 11 ++++- test/issue-3649.js | 102 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 test/issue-3649.js diff --git a/lib/mock/mock-utils.js b/lib/mock/mock-utils.js index 56be2c8a06e..5e89397c684 100644 --- a/lib/mock/mock-utils.js +++ b/lib/mock/mock-utils.js @@ -178,7 +178,16 @@ function deleteMockDispatch (mockDispatches, key) { } function buildKey (opts) { - const { path, method, body, headers, query } = opts + let { path, method, body, headers, query } = opts + + while (path.endsWith('/')) { + path = path.slice(0, -1) + } + + if (path.length === 0) { + path = '/' + } + return { path, method, diff --git a/test/issue-3649.js b/test/issue-3649.js new file mode 100644 index 00000000000..2daa6f6cb0a --- /dev/null +++ b/test/issue-3649.js @@ -0,0 +1,102 @@ +'use strict' + +const { tspl } = require('@matteo.collina/tspl') +const { describe, test, beforeEach } = require('node:test') +const { MockAgent, fetch, setGlobalDispatcher, getGlobalDispatcher } = require('..') + +describe('https://github.com/nodejs/undici/issues/3649', () => { + const undiciGlobalDispatcher = getGlobalDispatcher() + if (!undiciGlobalDispatcher) throw new Error('Could not find the global Undici dispatcher') + + let mockAgent + + beforeEach(() => { + mockAgent = new MockAgent() + mockAgent.disableNetConnect() + setGlobalDispatcher(mockAgent) + }) + + test('MockAgent should match with or without trailing slash /1', async (t) => { + t = tspl(t, { plan: 1 }) + + mockAgent + .get('https://localhost') + .intercept({ path: '/api/some-path' }).reply(200, { ok: true }) + + const res = await fetch(new URL('/api/some-path', 'https://localhost')) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) + + test('MockAgent should match with or without trailing slash /2', async (t) => { + t = tspl(t, { plan: 1 }) + + mockAgent + .get('https://localhost') + .intercept({ path: '/api/some-path' }).reply(200, { ok: true }) + + const res = await fetch(new URL('/api/some-path/', 'https://localhost')) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) + + test('MockAgent should match with or without trailing slash /3', async (t) => { + t = tspl(t, { plan: 1 }) + + mockAgent + .get('https://localhost') + .intercept({ path: '/api/some-path/' }).reply(200, { ok: true }) + + const res = await fetch(new URL('/api/some-path', 'https://localhost')) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) + + test('MockAgent should match with or without trailing slash /4', async (t) => { + t = tspl(t, { plan: 1 }) + + mockAgent + .get('https://localhost') + .intercept({ path: '/api/some-path/' }).reply(200, { ok: true }) + + const res = await fetch(new URL('/api/some-path/', 'https://localhost')) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) + + test('MockAgent should match with or without trailing slash /5', async (t) => { + t = tspl(t, { plan: 1 }) + + mockAgent + .get('https://localhost') + .intercept({ path: '/api/some-path////' }).reply(200, { ok: true }) + + const res = await fetch(new URL('/api/some-path//', 'https://localhost')) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) + + test('MockAgent should match with or without trailing slash /6', async (t) => { + t = tspl(t, { plan: 1 }) + + mockAgent + .get('https://localhost') + .intercept({ path: '/' }).reply(200, { ok: true }) + + const res = await fetch(new URL('', 'https://localhost')) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) + + test('MockAgent should match with or without trailing slash /7', async (t) => { + t = tspl(t, { plan: 1 }) + + mockAgent + .get('https://localhost') + .intercept({ path: '' }).reply(200, { ok: true }) + + const res = await fetch(new URL('/', 'https://localhost')) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) +}) From b58016c26e2cb607edb37d48d29a96b814889cf7 Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Tue, 1 Oct 2024 01:19:45 +0200 Subject: [PATCH 2/8] only normalize path if it is a string --- lib/mock/mock-utils.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/mock/mock-utils.js b/lib/mock/mock-utils.js index 5e89397c684..c5512845970 100644 --- a/lib/mock/mock-utils.js +++ b/lib/mock/mock-utils.js @@ -180,12 +180,15 @@ function deleteMockDispatch (mockDispatches, key) { function buildKey (opts) { let { path, method, body, headers, query } = opts - while (path.endsWith('/')) { - path = path.slice(0, -1) - } + // normalize path if it is a string + if (typeof path === 'string') { + while (path.endsWith('/')) { + path = path.slice(0, -1) + } - if (path.length === 0) { - path = '/' + if (path.length === 0) { + path = '/' + } } return { From 809df7629b6e088bf90dc42fa5bc5ee159d62167 Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Tue, 1 Oct 2024 01:30:24 +0200 Subject: [PATCH 3/8] put tests into mock-interceptors.js --- test/issue-3649.js | 102 -------------------------------------- test/mock-interceptor.js | 103 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 102 insertions(+), 103 deletions(-) delete mode 100644 test/issue-3649.js diff --git a/test/issue-3649.js b/test/issue-3649.js deleted file mode 100644 index 2daa6f6cb0a..00000000000 --- a/test/issue-3649.js +++ /dev/null @@ -1,102 +0,0 @@ -'use strict' - -const { tspl } = require('@matteo.collina/tspl') -const { describe, test, beforeEach } = require('node:test') -const { MockAgent, fetch, setGlobalDispatcher, getGlobalDispatcher } = require('..') - -describe('https://github.com/nodejs/undici/issues/3649', () => { - const undiciGlobalDispatcher = getGlobalDispatcher() - if (!undiciGlobalDispatcher) throw new Error('Could not find the global Undici dispatcher') - - let mockAgent - - beforeEach(() => { - mockAgent = new MockAgent() - mockAgent.disableNetConnect() - setGlobalDispatcher(mockAgent) - }) - - test('MockAgent should match with or without trailing slash /1', async (t) => { - t = tspl(t, { plan: 1 }) - - mockAgent - .get('https://localhost') - .intercept({ path: '/api/some-path' }).reply(200, { ok: true }) - - const res = await fetch(new URL('/api/some-path', 'https://localhost')) - - t.deepStrictEqual(await res.json(), { ok: true }) - }) - - test('MockAgent should match with or without trailing slash /2', async (t) => { - t = tspl(t, { plan: 1 }) - - mockAgent - .get('https://localhost') - .intercept({ path: '/api/some-path' }).reply(200, { ok: true }) - - const res = await fetch(new URL('/api/some-path/', 'https://localhost')) - - t.deepStrictEqual(await res.json(), { ok: true }) - }) - - test('MockAgent should match with or without trailing slash /3', async (t) => { - t = tspl(t, { plan: 1 }) - - mockAgent - .get('https://localhost') - .intercept({ path: '/api/some-path/' }).reply(200, { ok: true }) - - const res = await fetch(new URL('/api/some-path', 'https://localhost')) - - t.deepStrictEqual(await res.json(), { ok: true }) - }) - - test('MockAgent should match with or without trailing slash /4', async (t) => { - t = tspl(t, { plan: 1 }) - - mockAgent - .get('https://localhost') - .intercept({ path: '/api/some-path/' }).reply(200, { ok: true }) - - const res = await fetch(new URL('/api/some-path/', 'https://localhost')) - - t.deepStrictEqual(await res.json(), { ok: true }) - }) - - test('MockAgent should match with or without trailing slash /5', async (t) => { - t = tspl(t, { plan: 1 }) - - mockAgent - .get('https://localhost') - .intercept({ path: '/api/some-path////' }).reply(200, { ok: true }) - - const res = await fetch(new URL('/api/some-path//', 'https://localhost')) - - t.deepStrictEqual(await res.json(), { ok: true }) - }) - - test('MockAgent should match with or without trailing slash /6', async (t) => { - t = tspl(t, { plan: 1 }) - - mockAgent - .get('https://localhost') - .intercept({ path: '/' }).reply(200, { ok: true }) - - const res = await fetch(new URL('', 'https://localhost')) - - t.deepStrictEqual(await res.json(), { ok: true }) - }) - - test('MockAgent should match with or without trailing slash /7', async (t) => { - t = tspl(t, { plan: 1 }) - - mockAgent - .get('https://localhost') - .intercept({ path: '' }).reply(200, { ok: true }) - - const res = await fetch(new URL('/', 'https://localhost')) - - t.deepStrictEqual(await res.json(), { ok: true }) - }) -}) diff --git a/test/mock-interceptor.js b/test/mock-interceptor.js index 0d16290f579..bcd8ab4dd7a 100644 --- a/test/mock-interceptor.js +++ b/test/mock-interceptor.js @@ -6,6 +6,7 @@ const { MockInterceptor, MockScope } = require('../lib/mock/mock-interceptor') const MockAgent = require('../lib/mock/mock-agent') const { kDispatchKey } = require('../lib/mock/mock-symbols') const { InvalidArgumentError } = require('../lib/core/errors') +const { fetch } = require('../lib/web/fetch/index') describe('MockInterceptor - path', () => { test('should remove hash fragment from paths', t => { @@ -14,7 +15,7 @@ describe('MockInterceptor - path', () => { path: '#foobar', method: '' }, []) - t.strictEqual(mockInterceptor[kDispatchKey].path, '') + t.strictEqual(mockInterceptor[kDispatchKey].path, '/') }) }) @@ -257,3 +258,103 @@ describe('MockInterceptor - replyContentLength', () => { t.ok(result instanceof MockInterceptor) }) }) + +describe('https://github.com/nodejs/undici/issues/3649', () => { + test('MockAgent should match with or without trailing slash /1', async (t) => { + t = tspl(t, { plan: 1 }) + + const mockAgent = new MockAgent() + mockAgent.disableNetConnect() + mockAgent + .get('https://localhost') + .intercept({ path: '/api/some-path' }).reply(200, { ok: true }) + + const res = await fetch(new URL('/api/some-path', 'https://localhost'), { dispatcher: mockAgent }) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) + + test('MockAgent should match with or without trailing slash /2', async (t) => { + t = tspl(t, { plan: 1 }) + + const mockAgent = new MockAgent() + mockAgent.disableNetConnect() + mockAgent + .get('https://localhost') + .intercept({ path: '/api/some-path' }).reply(200, { ok: true }) + + const res = await fetch(new URL('/api/some-path/', 'https://localhost'), { dispatcher: mockAgent }) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) + + test('MockAgent should match with or without trailing slash /3', async (t) => { + t = tspl(t, { plan: 1 }) + + const mockAgent = new MockAgent() + mockAgent.disableNetConnect() + mockAgent + .get('https://localhost') + .intercept({ path: '/api/some-path/' }).reply(200, { ok: true }) + + const res = await fetch(new URL('/api/some-path', 'https://localhost'), { dispatcher: mockAgent }) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) + + test('MockAgent should match with or without trailing slash /4', async (t) => { + t = tspl(t, { plan: 1 }) + + const mockAgent = new MockAgent() + mockAgent.disableNetConnect() + mockAgent + .get('https://localhost') + .intercept({ path: '/api/some-path/' }).reply(200, { ok: true }) + + const res = await fetch(new URL('/api/some-path/', 'https://localhost'), { dispatcher: mockAgent }) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) + + test('MockAgent should match with or without trailing slash /5', async (t) => { + t = tspl(t, { plan: 1 }) + + const mockAgent = new MockAgent() + mockAgent.disableNetConnect() + mockAgent + .get('https://localhost') + .intercept({ path: '/api/some-path////' }).reply(200, { ok: true }) + + const res = await fetch(new URL('/api/some-path//', 'https://localhost'), { dispatcher: mockAgent }) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) + + test('MockAgent should match with or without trailing slash /6', async (t) => { + t = tspl(t, { plan: 1 }) + + const mockAgent = new MockAgent() + mockAgent.disableNetConnect() + mockAgent + .get('https://localhost') + .intercept({ path: '/' }).reply(200, { ok: true }) + + const res = await fetch(new URL('', 'https://localhost'), { dispatcher: mockAgent }) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) + + test('MockAgent should match with or without trailing slash /7', async (t) => { + t = tspl(t, { plan: 1 }) + + const mockAgent = new MockAgent() + mockAgent.disableNetConnect() + mockAgent + .get('https://localhost') + .intercept({ path: '' }).reply(200, { ok: true }) + + const res = await fetch(new URL('/', 'https://localhost'), { dispatcher: mockAgent }) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) +}) From 729a2ad9ed90b6f3c0dbdb046b6caba380bea0ab Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Wed, 2 Oct 2024 21:10:59 +0200 Subject: [PATCH 4/8] make ignoreTrailingSlashes an option of MockAgent --- lib/mock/mock-client.js | 9 +++- lib/mock/mock-pool.js | 9 +++- lib/mock/mock-symbols.js | 3 +- lib/mock/mock-utils.js | 9 ++-- test/mock-interceptor.js | 102 +++++++++------------------------------ types/mock-agent.d.ts | 3 ++ 6 files changed, 46 insertions(+), 89 deletions(-) diff --git a/lib/mock/mock-client.js b/lib/mock/mock-client.js index fd840fba908..80afd97b868 100644 --- a/lib/mock/mock-client.js +++ b/lib/mock/mock-client.js @@ -10,7 +10,8 @@ const { kOriginalClose, kOrigin, kOriginalDispatch, - kConnected + kConnected, + kIgnoreTrailingSlashes } = require('./mock-symbols') const { MockInterceptor } = require('./mock-interceptor') const Symbols = require('../core/symbols') @@ -29,6 +30,7 @@ class MockClient extends Client { this[kMockAgent] = opts.agent this[kOrigin] = origin + this[kIgnoreTrailingSlashes] = opts.ignoreTrailingSlashes ?? false this[kDispatches] = [] this[kConnected] = 1 this[kOriginalDispatch] = this.dispatch @@ -46,7 +48,10 @@ class MockClient extends Client { * Sets up the base interceptor for mocking replies from undici. */ intercept (opts) { - return new MockInterceptor(opts, this[kDispatches]) + return new MockInterceptor( + opts && { ignoreTrailingSlashes: this[kIgnoreTrailingSlashes], ...opts }, + this[kDispatches] + ) } async [kClose] () { diff --git a/lib/mock/mock-pool.js b/lib/mock/mock-pool.js index bc68a37faa3..95b6525e536 100644 --- a/lib/mock/mock-pool.js +++ b/lib/mock/mock-pool.js @@ -10,7 +10,8 @@ const { kOriginalClose, kOrigin, kOriginalDispatch, - kConnected + kConnected, + kIgnoreTrailingSlashes } = require('./mock-symbols') const { MockInterceptor } = require('./mock-interceptor') const Symbols = require('../core/symbols') @@ -29,6 +30,7 @@ class MockPool extends Pool { this[kMockAgent] = opts.agent this[kOrigin] = origin + this[kIgnoreTrailingSlashes] = opts.ignoreTrailingSlashes ?? false this[kDispatches] = [] this[kConnected] = 1 this[kOriginalDispatch] = this.dispatch @@ -46,7 +48,10 @@ class MockPool extends Pool { * Sets up the base interceptor for mocking replies from undici. */ intercept (opts) { - return new MockInterceptor(opts, this[kDispatches]) + return new MockInterceptor( + opts && { ignoreTrailingSlashes: this[kIgnoreTrailingSlashes], ...opts }, + this[kDispatches] + ) } async [kClose] () { diff --git a/lib/mock/mock-symbols.js b/lib/mock/mock-symbols.js index 2a8336b75f3..32449e9f36e 100644 --- a/lib/mock/mock-symbols.js +++ b/lib/mock/mock-symbols.js @@ -20,5 +20,6 @@ module.exports = { kIsMockActive: Symbol('is mock active'), kNetConnect: Symbol('net connect'), kGetNetConnect: Symbol('get net connect'), - kConnected: Symbol('connected') + kConnected: Symbol('connected'), + kIgnoreTrailingSlashes: Symbol('ignore trailing slashes') } diff --git a/lib/mock/mock-utils.js b/lib/mock/mock-utils.js index 7b434f2d938..9fafa7c1b63 100644 --- a/lib/mock/mock-utils.js +++ b/lib/mock/mock-utils.js @@ -6,7 +6,8 @@ const { kMockAgent, kOriginalDispatch, kOrigin, - kGetNetConnect + kGetNetConnect, + kIgnoreTrailingSlashes } = require('./mock-symbols') const { serializePathWithQuery } = require('../core/util') const { STATUS_CODES } = require('node:http') @@ -182,10 +183,10 @@ function deleteMockDispatch (mockDispatches, key) { } function buildKey (opts) { - let { path, method, body, headers, query } = opts + let { path, method, body, headers, query, ignoreTrailingSlashes = false } = opts // normalize path if it is a string - if (typeof path === 'string') { + if (ignoreTrailingSlashes && typeof path === 'string') { while (path.endsWith('/')) { path = path.slice(0, -1) } @@ -243,7 +244,7 @@ async function getResponse (body) { */ function mockDispatch (opts, handler) { // Get mock dispatch from built key - const key = buildKey(opts) + const key = buildKey({ ignoreTrailingSlashes: this[kIgnoreTrailingSlashes], ...opts }) const mockDispatch = getMockDispatch(this[kDispatches], key) mockDispatch.timesInvoked++ diff --git a/test/mock-interceptor.js b/test/mock-interceptor.js index fa0723f5abc..4c2936f3b42 100644 --- a/test/mock-interceptor.js +++ b/test/mock-interceptor.js @@ -15,7 +15,7 @@ describe('MockInterceptor - path', () => { path: '#foobar', method: '' }, []) - t.strictEqual(mockInterceptor[kDispatchKey].path, '/') + t.strictEqual(mockInterceptor[kDispatchKey].path, '') }) }) @@ -260,74 +260,30 @@ describe('MockInterceptor - replyContentLength', () => { }) describe('https://github.com/nodejs/undici/issues/3649', () => { - test('MockAgent should match with or without trailing slash /1', async (t) => { - t = tspl(t, { plan: 1 }) - - const mockAgent = new MockAgent() - mockAgent.disableNetConnect() - mockAgent - .get('https://localhost') - .intercept({ path: '/api/some-path' }).reply(200, { ok: true }) - - const res = await fetch(new URL('/api/some-path', 'https://localhost'), { dispatcher: mockAgent }) - - t.deepStrictEqual(await res.json(), { ok: true }) - }) - - test('MockAgent should match with or without trailing slash /2', async (t) => { - t = tspl(t, { plan: 1 }) - - const mockAgent = new MockAgent() - mockAgent.disableNetConnect() - mockAgent - .get('https://localhost') - .intercept({ path: '/api/some-path' }).reply(200, { ok: true }) - - const res = await fetch(new URL('/api/some-path/', 'https://localhost'), { dispatcher: mockAgent }) - - t.deepStrictEqual(await res.json(), { ok: true }) - }) - - test('MockAgent should match with or without trailing slash /3', async (t) => { - t = tspl(t, { plan: 1 }) - - const mockAgent = new MockAgent() - mockAgent.disableNetConnect() - mockAgent - .get('https://localhost') - .intercept({ path: '/api/some-path/' }).reply(200, { ok: true }) - - const res = await fetch(new URL('/api/some-path', 'https://localhost'), { dispatcher: mockAgent }) - - t.deepStrictEqual(await res.json(), { ok: true }) - }) - - test('MockAgent should match with or without trailing slash /4', async (t) => { - t = tspl(t, { plan: 1 }) - - const mockAgent = new MockAgent() - mockAgent.disableNetConnect() - mockAgent - .get('https://localhost') - .intercept({ path: '/api/some-path/' }).reply(200, { ok: true }) - - const res = await fetch(new URL('/api/some-path/', 'https://localhost'), { dispatcher: mockAgent }) - - t.deepStrictEqual(await res.json(), { ok: true }) - }) - - test('MockAgent should match with or without trailing slash /5', async (t) => { - t = tspl(t, { plan: 1 }) + [ + ['/api/some-path', '/api/some-path'], + ['/api/some-path/', '/api/some-path'], + ['/api/some-path', '/api/some-path/'], + ['/api/some-path/', '/api/some-path/'], + ['/api/some-path////', '/api/some-path//'], + ['', ''], + ['/', ''], + ['', '/'], + ['/', '/'] + ].forEach(([interceptPath, fetchedPath], index) => { + test(`MockAgent should match with or without trailing slash by setting ignoreTrailingSlashes as MockAgent option /${index}`, async (t) => { + t = tspl(t, { plan: 1 }) - const mockAgent = new MockAgent() - mockAgent.disableNetConnect() - mockAgent - .get('https://localhost') - .intercept({ path: '/api/some-path////' }).reply(200, { ok: true }) + const mockAgent = new MockAgent({ ignoreTrailingSlashes: true }) + mockAgent.disableNetConnect() + mockAgent + .get('https://localhost') + .intercept({ path: interceptPath }).reply(200, { ok: true }) - const res = await fetch(new URL('/api/some-path//', 'https://localhost'), { dispatcher: mockAgent }) + const res = await fetch(new URL(fetchedPath, 'https://localhost'), { dispatcher: mockAgent }) - t.deepStrictEqual(await res.json(), { ok: true }) + t.deepStrictEqual(await res.json(), { ok: true }) + }) }) test('MockAgent should match with or without trailing slash /6', async (t) => { @@ -343,20 +299,6 @@ describe('https://github.com/nodejs/undici/issues/3649', () => { t.deepStrictEqual(await res.json(), { ok: true }) }) - - test('MockAgent should match with or without trailing slash /7', async (t) => { - t = tspl(t, { plan: 1 }) - - const mockAgent = new MockAgent() - mockAgent.disableNetConnect() - mockAgent - .get('https://localhost') - .intercept({ path: '' }).reply(200, { ok: true }) - - const res = await fetch(new URL('/', 'https://localhost'), { dispatcher: mockAgent }) - - t.deepStrictEqual(await res.json(), { ok: true }) - }) }) describe('MockInterceptor - different payloads', () => { diff --git a/types/mock-agent.d.ts b/types/mock-agent.d.ts index da40a43b744..26733eb4914 100644 --- a/types/mock-agent.d.ts +++ b/types/mock-agent.d.ts @@ -46,5 +46,8 @@ declare namespace MockAgent { export interface Options extends Agent.Options { /** A custom agent to be encapsulated by the MockAgent. */ agent?: Dispatcher; + + /** Ignore trailing slashes in the path */ + ignoreTrailingSlashes?: boolean; } } From 8c268efe84ba658e17aaaf464ddd5fd21eb7518e Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Wed, 2 Oct 2024 21:19:32 +0200 Subject: [PATCH 5/8] rename to ignoreTrailingSlash --- lib/mock/mock-client.js | 6 +++--- lib/mock/mock-pool.js | 6 +++--- lib/mock/mock-symbols.js | 2 +- lib/mock/mock-utils.js | 8 ++++---- test/mock-interceptor.js | 4 ++-- types/mock-agent.d.ts | 2 +- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/mock/mock-client.js b/lib/mock/mock-client.js index 80afd97b868..f8a786ced8b 100644 --- a/lib/mock/mock-client.js +++ b/lib/mock/mock-client.js @@ -11,7 +11,7 @@ const { kOrigin, kOriginalDispatch, kConnected, - kIgnoreTrailingSlashes + kIgnoreTrailingSlash } = require('./mock-symbols') const { MockInterceptor } = require('./mock-interceptor') const Symbols = require('../core/symbols') @@ -30,7 +30,7 @@ class MockClient extends Client { this[kMockAgent] = opts.agent this[kOrigin] = origin - this[kIgnoreTrailingSlashes] = opts.ignoreTrailingSlashes ?? false + this[kIgnoreTrailingSlash] = opts.ignoreTrailingSlash ?? false this[kDispatches] = [] this[kConnected] = 1 this[kOriginalDispatch] = this.dispatch @@ -49,7 +49,7 @@ class MockClient extends Client { */ intercept (opts) { return new MockInterceptor( - opts && { ignoreTrailingSlashes: this[kIgnoreTrailingSlashes], ...opts }, + opts && { ignoreTrailingSlash: this[kIgnoreTrailingSlash], ...opts }, this[kDispatches] ) } diff --git a/lib/mock/mock-pool.js b/lib/mock/mock-pool.js index 95b6525e536..a266211ac70 100644 --- a/lib/mock/mock-pool.js +++ b/lib/mock/mock-pool.js @@ -11,7 +11,7 @@ const { kOrigin, kOriginalDispatch, kConnected, - kIgnoreTrailingSlashes + kIgnoreTrailingSlash } = require('./mock-symbols') const { MockInterceptor } = require('./mock-interceptor') const Symbols = require('../core/symbols') @@ -30,7 +30,7 @@ class MockPool extends Pool { this[kMockAgent] = opts.agent this[kOrigin] = origin - this[kIgnoreTrailingSlashes] = opts.ignoreTrailingSlashes ?? false + this[kIgnoreTrailingSlash] = opts.ignoreTrailingSlash ?? false this[kDispatches] = [] this[kConnected] = 1 this[kOriginalDispatch] = this.dispatch @@ -49,7 +49,7 @@ class MockPool extends Pool { */ intercept (opts) { return new MockInterceptor( - opts && { ignoreTrailingSlashes: this[kIgnoreTrailingSlashes], ...opts }, + opts && { ignoreTrailingSlash: this[kIgnoreTrailingSlash], ...opts }, this[kDispatches] ) } diff --git a/lib/mock/mock-symbols.js b/lib/mock/mock-symbols.js index 32449e9f36e..492edddabf5 100644 --- a/lib/mock/mock-symbols.js +++ b/lib/mock/mock-symbols.js @@ -21,5 +21,5 @@ module.exports = { kNetConnect: Symbol('net connect'), kGetNetConnect: Symbol('get net connect'), kConnected: Symbol('connected'), - kIgnoreTrailingSlashes: Symbol('ignore trailing slashes') + kIgnoreTrailingSlash: Symbol('ignore trailing slash') } diff --git a/lib/mock/mock-utils.js b/lib/mock/mock-utils.js index 9fafa7c1b63..2096957f388 100644 --- a/lib/mock/mock-utils.js +++ b/lib/mock/mock-utils.js @@ -7,7 +7,7 @@ const { kOriginalDispatch, kOrigin, kGetNetConnect, - kIgnoreTrailingSlashes + kIgnoreTrailingSlash } = require('./mock-symbols') const { serializePathWithQuery } = require('../core/util') const { STATUS_CODES } = require('node:http') @@ -183,10 +183,10 @@ function deleteMockDispatch (mockDispatches, key) { } function buildKey (opts) { - let { path, method, body, headers, query, ignoreTrailingSlashes = false } = opts + let { path, method, body, headers, query, ignoreTrailingSlash = false } = opts // normalize path if it is a string - if (ignoreTrailingSlashes && typeof path === 'string') { + if (ignoreTrailingSlash && typeof path === 'string') { while (path.endsWith('/')) { path = path.slice(0, -1) } @@ -244,7 +244,7 @@ async function getResponse (body) { */ function mockDispatch (opts, handler) { // Get mock dispatch from built key - const key = buildKey({ ignoreTrailingSlashes: this[kIgnoreTrailingSlashes], ...opts }) + const key = buildKey({ ignoreTrailingSlash: this[kIgnoreTrailingSlash], ...opts }) const mockDispatch = getMockDispatch(this[kDispatches], key) mockDispatch.timesInvoked++ diff --git a/test/mock-interceptor.js b/test/mock-interceptor.js index 4c2936f3b42..ceda8f2e908 100644 --- a/test/mock-interceptor.js +++ b/test/mock-interceptor.js @@ -271,10 +271,10 @@ describe('https://github.com/nodejs/undici/issues/3649', () => { ['', '/'], ['/', '/'] ].forEach(([interceptPath, fetchedPath], index) => { - test(`MockAgent should match with or without trailing slash by setting ignoreTrailingSlashes as MockAgent option /${index}`, async (t) => { + test(`MockAgent should match with or without trailing slash by setting ignoreTrailingSlash as MockAgent option /${index}`, async (t) => { t = tspl(t, { plan: 1 }) - const mockAgent = new MockAgent({ ignoreTrailingSlashes: true }) + const mockAgent = new MockAgent({ ignoreTrailingSlash: true }) mockAgent.disableNetConnect() mockAgent .get('https://localhost') diff --git a/types/mock-agent.d.ts b/types/mock-agent.d.ts index 26733eb4914..311b28b2db0 100644 --- a/types/mock-agent.d.ts +++ b/types/mock-agent.d.ts @@ -48,6 +48,6 @@ declare namespace MockAgent { agent?: Dispatcher; /** Ignore trailing slashes in the path */ - ignoreTrailingSlashes?: boolean; + ignoreTrailingSlash?: boolean; } } From 7cd005e86dbb121ca5bcf07c461f58c01151d4ab Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Thu, 3 Oct 2024 11:01:21 +0200 Subject: [PATCH 6/8] make ignoreTrailingSlashes an option of .intercept --- lib/mock/mock-interceptor.js | 10 +++-- lib/mock/mock-utils.js | 47 ++++++++++++-------- test/mock-interceptor-unused-assertions.js | 1 + test/mock-interceptor.js | 51 +++++++++++++++++----- 4 files changed, 77 insertions(+), 32 deletions(-) diff --git a/lib/mock/mock-interceptor.js b/lib/mock/mock-interceptor.js index 4304c8fb8ec..1ea7aac486d 100644 --- a/lib/mock/mock-interceptor.js +++ b/lib/mock/mock-interceptor.js @@ -7,7 +7,8 @@ const { kDefaultHeaders, kDefaultTrailers, kContentLength, - kMockDispatch + kMockDispatch, + kIgnoreTrailingSlash } = require('./mock-symbols') const { InvalidArgumentError } = require('../core/errors') const { serializePathWithQuery } = require('../core/util') @@ -85,6 +86,7 @@ class MockInterceptor { this[kDispatchKey] = buildKey(opts) this[kDispatches] = mockDispatches + this[kIgnoreTrailingSlash] = opts.ignoreTrailingSlash ?? false this[kDefaultHeaders] = {} this[kDefaultTrailers] = {} this[kContentLength] = false @@ -137,7 +139,7 @@ class MockInterceptor { } // Add usual dispatch data, but this time set the data parameter to function that will eventually provide data. - const newMockDispatch = addMockDispatch(this[kDispatches], this[kDispatchKey], wrappedDefaultsCallback) + const newMockDispatch = addMockDispatch(this[kDispatches], this[kDispatchKey], wrappedDefaultsCallback, { ignoreTrailingSlash: this[kIgnoreTrailingSlash] }) return new MockScope(newMockDispatch) } @@ -154,7 +156,7 @@ class MockInterceptor { // Send in-already provided data like usual const dispatchData = this.createMockScopeDispatchData(replyParameters) - const newMockDispatch = addMockDispatch(this[kDispatches], this[kDispatchKey], dispatchData) + const newMockDispatch = addMockDispatch(this[kDispatches], this[kDispatchKey], dispatchData, { ignoreTrailingSlash: this[kIgnoreTrailingSlash] }) return new MockScope(newMockDispatch) } @@ -166,7 +168,7 @@ class MockInterceptor { throw new InvalidArgumentError('error must be defined') } - const newMockDispatch = addMockDispatch(this[kDispatches], this[kDispatchKey], { error }) + const newMockDispatch = addMockDispatch(this[kDispatches], this[kDispatchKey], { error }, { ignoreTrailingSlash: this[kIgnoreTrailingSlash] }) return new MockScope(newMockDispatch) } diff --git a/lib/mock/mock-utils.js b/lib/mock/mock-utils.js index 2096957f388..e74ffe64cdc 100644 --- a/lib/mock/mock-utils.js +++ b/lib/mock/mock-utils.js @@ -6,8 +6,7 @@ const { kMockAgent, kOriginalDispatch, kOrigin, - kGetNetConnect, - kIgnoreTrailingSlash + kGetNetConnect } = require('./mock-symbols') const { serializePathWithQuery } = require('../core/util') const { STATUS_CODES } = require('node:http') @@ -134,8 +133,16 @@ function getMockDispatch (mockDispatches, key) { const basePath = key.query ? serializePathWithQuery(key.path, key.query) : key.path const resolvedPath = typeof basePath === 'string' ? safeUrl(basePath) : basePath + const resolvedPathWithoutTrailingSlash = removeTrailingSlash(resolvedPath) + // Match path - let matchedMockDispatches = mockDispatches.filter(({ consumed }) => !consumed).filter(({ path }) => matchValue(safeUrl(path), resolvedPath)) + let matchedMockDispatches = mockDispatches + .filter(({ consumed }) => !consumed) + .filter(({ path, ignoreTrailingSlash }) => { + return ignoreTrailingSlash + ? matchValue(removeTrailingSlash(safeUrl(path)), resolvedPathWithoutTrailingSlash) + : matchValue(safeUrl(path), resolvedPath) + }) if (matchedMockDispatches.length === 0) { throw new MockNotMatchedError(`Mock dispatch not matched for path '${resolvedPath}'`) } @@ -162,8 +169,8 @@ function getMockDispatch (mockDispatches, key) { return matchedMockDispatches[0] } -function addMockDispatch (mockDispatches, key, data) { - const baseData = { timesInvoked: 0, times: 1, persist: false, consumed: false } +function addMockDispatch (mockDispatches, key, data, opts) { + const baseData = { timesInvoked: 0, times: 1, persist: false, consumed: false, ...opts } const replyData = typeof data === 'function' ? { callback: data } : { ...data } const newMockDispatch = { ...baseData, ...key, pending: true, data: { error: null, ...replyData } } mockDispatches.push(newMockDispatch) @@ -182,20 +189,24 @@ function deleteMockDispatch (mockDispatches, key) { } } -function buildKey (opts) { - let { path, method, body, headers, query, ignoreTrailingSlash = false } = opts - - // normalize path if it is a string - if (ignoreTrailingSlash && typeof path === 'string') { - while (path.endsWith('/')) { - path = path.slice(0, -1) - } +/** + * @param {string} path Path to remove trailing slash from + */ +function removeTrailingSlash (path) { + while (path.endsWith('/')) { + path = path.slice(0, -1) + } - if (path.length === 0) { - path = '/' - } + if (path.length === 0) { + path = '/' } + return path +} + +function buildKey (opts) { + const { path, method, body, headers, query } = opts + return { path, method, @@ -244,7 +255,7 @@ async function getResponse (body) { */ function mockDispatch (opts, handler) { // Get mock dispatch from built key - const key = buildKey({ ignoreTrailingSlash: this[kIgnoreTrailingSlash], ...opts }) + const key = buildKey(opts) const mockDispatch = getMockDispatch(this[kDispatches], key) mockDispatch.timesInvoked++ @@ -309,7 +320,7 @@ function mockDispatch (opts, handler) { deleteMockDispatch(mockDispatches, key) } - function resume () {} + function resume () { } return true } diff --git a/test/mock-interceptor-unused-assertions.js b/test/mock-interceptor-unused-assertions.js index 318907a8527..ddb5aeabab1 100644 --- a/test/mock-interceptor-unused-assertions.js +++ b/test/mock-interceptor-unused-assertions.js @@ -266,6 +266,7 @@ test('returns unused interceptors', t => { persist: false, consumed: false, pending: true, + ignoreTrailingSlash: false, path: '/', method: 'GET', body: undefined, diff --git a/test/mock-interceptor.js b/test/mock-interceptor.js index ceda8f2e908..7b17efe1a6e 100644 --- a/test/mock-interceptor.js +++ b/test/mock-interceptor.js @@ -284,20 +284,51 @@ describe('https://github.com/nodejs/undici/issues/3649', () => { t.deepStrictEqual(await res.json(), { ok: true }) }) - }) - test('MockAgent should match with or without trailing slash /6', async (t) => { - t = tspl(t, { plan: 1 }) + test(`MockAgent should match with or without trailing slash by setting ignoreTrailingSlash as intercept option /${index}`, async (t) => { + t = tspl(t, { plan: 1 }) - const mockAgent = new MockAgent() - mockAgent.disableNetConnect() - mockAgent - .get('https://localhost') - .intercept({ path: '/' }).reply(200, { ok: true }) + const mockAgent = new MockAgent() + mockAgent.disableNetConnect() + mockAgent + .get('https://localhost') + .intercept({ path: interceptPath, ignoreTrailingSlash: true }).reply(200, { ok: true }) + + const res = await fetch(new URL(fetchedPath, 'https://localhost'), { dispatcher: mockAgent }) - const res = await fetch(new URL('', 'https://localhost'), { dispatcher: mockAgent }) + t.deepStrictEqual(await res.json(), { ok: true }) + }) - t.deepStrictEqual(await res.json(), { ok: true }) + if ( + (interceptPath === fetchedPath && (interceptPath !== '' && fetchedPath !== '')) || + (interceptPath === '/' && fetchedPath === '') + ) { + test(`MockAgent should should match on strict equal cases of paths when ignoreTrailingSlash is not set /${index}`, async (t) => { + t = tspl(t, { plan: 1 }) + + const mockAgent = new MockAgent() + mockAgent.disableNetConnect() + mockAgent + .get('https://localhost') + .intercept({ path: interceptPath }).reply(200, { ok: true }) + + const res = await fetch(new URL(fetchedPath, 'https://localhost'), { dispatcher: mockAgent }) + + t.deepStrictEqual(await res.json(), { ok: true }) + }) + } else { + test(`MockAgent should should reject on not strict equal cases of paths when ignoreTrailingSlash is not set /${index}`, async (t) => { + t = tspl(t, { plan: 1 }) + + const mockAgent = new MockAgent() + mockAgent.disableNetConnect() + mockAgent + .get('https://localhost') + .intercept({ path: interceptPath }).reply(200, { ok: true }) + + t.rejects(fetch(new URL(fetchedPath, 'https://localhost'), { dispatcher: mockAgent })) + }) + } }) }) From 08f8ad7e45133ccb320545735b741e38409f47ac Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Thu, 3 Oct 2024 11:06:38 +0200 Subject: [PATCH 7/8] Apply suggestions from code review --- lib/mock/mock-utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mock/mock-utils.js b/lib/mock/mock-utils.js index e74ffe64cdc..b19aaaf8e11 100644 --- a/lib/mock/mock-utils.js +++ b/lib/mock/mock-utils.js @@ -320,7 +320,7 @@ function mockDispatch (opts, handler) { deleteMockDispatch(mockDispatches, key) } - function resume () { } + function resume () {} return true } From 538feb161e9483852ac43db5e67a3e108c076d41 Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Thu, 3 Oct 2024 11:14:50 +0200 Subject: [PATCH 8/8] add documentation --- docs/docs/api/MockAgent.md | 2 ++ docs/docs/api/MockPool.md | 1 + 2 files changed, 3 insertions(+) diff --git a/docs/docs/api/MockAgent.md b/docs/docs/api/MockAgent.md index 85ae69046e7..e5713db2712 100644 --- a/docs/docs/api/MockAgent.md +++ b/docs/docs/api/MockAgent.md @@ -18,6 +18,8 @@ Extends: [`AgentOptions`](Agent.md#parameter-agentoptions) * **agent** `Agent` (optional) - Default: `new Agent([options])` - a custom agent encapsulated by the MockAgent. +* **ignoreTrailingSlash** `boolean` (optional) - Default: `false` - set the default value for `ignoreTrailingSlash` for interceptors. + ### Example - Basic MockAgent instantiation This will instantiate the MockAgent. It will not do anything until registered as the agent to use with requests and mock interceptions are added. diff --git a/docs/docs/api/MockPool.md b/docs/docs/api/MockPool.md index 18b97d958f6..ac8185ffd76 100644 --- a/docs/docs/api/MockPool.md +++ b/docs/docs/api/MockPool.md @@ -58,6 +58,7 @@ Returns: `MockInterceptor` corresponding to the input options. * **body** `string | RegExp | (body: string) => boolean` - (optional) - a matcher for the HTTP request body. * **headers** `Record boolean`> - (optional) - a matcher for the HTTP request headers. To be intercepted, a request must match all defined headers. Extra headers not defined here may (or may not) be included in the request and do not affect the interception in any way. * **query** `Record | null` - (optional) - a matcher for the HTTP request query string params. Only applies when a `string` was provided for `MockPoolInterceptOptions.path`. +* **ignoreTrailingSlash** `boolean` - (optional) - set to `true` if the matcher should also match by ignoring potential trailing slashes in `MockPoolInterceptOptions.path`. ### Return: `MockInterceptor`