fix: add option ignoreTrailingSlash to MockAgent and .intercept() (…

…nodejs#3655)

* fix: mock interceptor should ignore trailing slashes

* only normalize path if it is a string

* put tests into mock-interceptors.js

* make ignoreTrailingSlashes an option of MockAgent

* rename to ignoreTrailingSlash

* make ignoreTrailingSlashes an option of .intercept

* Apply suggestions from code review

* add documentation

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.

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<string, string | RegExp | (body: string) => 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<string, any> | 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`
 

lib/mock/mock-client.js

@@ -10,7 +10,8 @@ const {
   kOriginalClose,
   kOrigin,
   kOriginalDispatch,
-  kConnected
+  kConnected,
+  kIgnoreTrailingSlash
 } = 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[kIgnoreTrailingSlash] = opts.ignoreTrailingSlash ?? 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 && { ignoreTrailingSlash: this[kIgnoreTrailingSlash], ...opts },
+      this[kDispatches]
+    )
   }
 
   async [kClose] () {

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)
   }
 

lib/mock/mock-pool.js

@@ -10,7 +10,8 @@ const {
   kOriginalClose,
   kOrigin,
   kOriginalDispatch,
-  kConnected
+  kConnected,
+  kIgnoreTrailingSlash
 } = 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[kIgnoreTrailingSlash] = opts.ignoreTrailingSlash ?? 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 && { ignoreTrailingSlash: this[kIgnoreTrailingSlash], ...opts },
+      this[kDispatches]
+    )
   }
 
   async [kClose] () {

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'),
+  kIgnoreTrailingSlash: Symbol('ignore trailing slash')
 }

lib/mock/mock-utils.js

@@ -133,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}'`)
   }
@@ -161,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)
@@ -181,8 +189,24 @@ function deleteMockDispatch (mockDispatches, key) {
   }
 }
 
+/**
+ * @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 = '/'
+  }
+
+  return path
+}
+
 function buildKey (opts) {
   const { path, method, body, headers, query } = opts
+
   return {
     path,
     method,

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,

test/mock-interceptor.js

@@ -259,6 +259,79 @@ describe('MockInterceptor - replyContentLength', () => {
   })
 })
 
+describe('https://github.com/nodejs/undici/issues/3649', () => {
+  [
+    ['/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 ignoreTrailingSlash as MockAgent option /${index}`, async (t) => {
+      t = tspl(t, { plan: 1 })
+
+      const mockAgent = new MockAgent({ ignoreTrailingSlash: true })
+      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 })
+    })
+
+    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: interceptPath, ignoreTrailingSlash: true }).reply(200, { ok: true })
+
+      const res = await fetch(new URL(fetchedPath, 'https://localhost'), { dispatcher: mockAgent })
+
+      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 }))
+      })
+    }
+  })
+})
+
 describe('MockInterceptor - different payloads', () => {
   [
     // Buffer

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 */
+    ignoreTrailingSlash?: boolean;
   }
 }