fix: make sure array search params using different syntaxes are

supported

Author: dario-piotrowicz

lib/mock/mock-utils.js

@@ -92,6 +92,38 @@ function matchHeaders (mockDispatch, headers) {
   return true
 }
 
+function normalizeQueryParams (query) {
+  if (typeof query !== 'string') {
+    return query
+  }
+
+  const originalQp = new URLSearchParams(query)
+  const normalizedQp = new URLSearchParams()
+
+  for (let [key, value] of originalQp.entries()) {
+    key = key.replace('[]', '')
+
+    const valueRepresentsString = /^(['"]).*\1$/.test(value)
+    if (valueRepresentsString) {
+      normalizedQp.append(key, value)
+      continue
+    }
+
+    if (value.includes(',')) {
+      const values = value.split(',')
+      for (const v of values) {
+        normalizedQp.append(key, v)
+      }
+      continue
+    }
+
+    normalizedQp.append(key, value)
+  }
+
+  normalizedQp.sort()
+  return normalizedQp
+}
+
 function safeUrl (path) {
   if (typeof path !== 'string') {
     return path
@@ -103,8 +135,7 @@ function safeUrl (path) {
     return path
   }
 
-  const qp = new URLSearchParams(pathSegments.pop())
-  qp.sort()
+  const qp = normalizeQueryParams(pathSegments.pop())
   return [...pathSegments, qp.toString()].join('?')
 }
 
@@ -395,5 +426,6 @@ module.exports = {
   checkNetConnect,
   buildAndValidateMockOptions,
   getHeaderByName,
-  buildHeadersFromArray
+  buildHeadersFromArray,
+  normalizeQueryParams
 }

test/mock-agent.js

@@ -2888,3 +2888,55 @@ test('MockAgent - headers should be array of strings (fetch)', async (t) => {
 
   t.deepStrictEqual(response.headers.getSetCookie(), ['foo=bar', 'bar=baz', 'baz=qux'])
 })
+
+// https://github.com/nodejs/undici/issues/4146
+;[
+  '/foo?array=item1&array=item2',
+  '/foo?array[]=item1&array[]=item2',
+  '/foo?array=item1,item2'
+].forEach(path => {
+  test(`MockAgent - multi value query parameter "${path}"`, async (t) => {
+    t = tspl(t, { plan: 4 })
+
+    const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
+      res.setHeader('content-type', 'text/plain')
+      res.end('should not be called')
+      t.fail('should not be called')
+      t.end()
+    })
+    after(() => server.close())
+
+    await promisify(server.listen.bind(server))(0)
+
+    const baseUrl = `http://localhost:${server.address().port}`
+
+    const mockAgent = new MockAgent()
+    after(() => mockAgent.close())
+    const mockPool = mockAgent.get(baseUrl)
+
+    mockPool.intercept({
+      path: '/foo',
+      method: 'GET',
+      query: {
+        array: ['item1', 'item2']
+      }
+    }).reply(200, { foo: 'bar' }, {
+      headers: { 'content-type': 'application/json' },
+      trailers: { 'Content-MD5': 'test' }
+    })
+
+    const { statusCode, headers, trailers, body } = await mockAgent.request({
+      origin: baseUrl,
+      path,
+      method: 'GET'
+    })
+    t.strictEqual(statusCode, 200)
+    t.strictEqual(headers['content-type'], 'application/json')
+    t.deepStrictEqual(trailers, { 'content-md5': 'test' })
+
+    const jsonResponse = JSON.parse(await getResponse(body))
+    t.deepStrictEqual(jsonResponse, {
+      foo: 'bar'
+    })
+  })
+})

test/mock-utils.js

@@ -9,7 +9,8 @@ const {
   getResponseData,
   getStatusText,
   getHeaderByName,
-  buildHeadersFromArray
+  buildHeadersFromArray,
+  normalizeQueryParams
 } = require('../lib/mock/mock-utils')
 
 test('deleteMockDispatch - should do nothing if not able to find mock dispatch', (t) => {
@@ -242,3 +243,33 @@ describe('buildHeadersFromArray', () => {
     t.strictEqual(headers.key, 'value')
   })
 })
+
+describe.only('normalizeQueryParams', () => {
+  test('it should handle basic cases', (t) => {
+    t = tspl(t, { plan: 4 })
+
+    t.deepStrictEqual(normalizeQueryParams('').toString(), '')
+    t.deepStrictEqual(normalizeQueryParams('a').toString(), 'a=')
+    t.deepStrictEqual(normalizeQueryParams('b=2&c=3&a=1').toString(), 'a=1&b=2&c=3')
+    t.deepStrictEqual(normalizeQueryParams('lang=en_EN&id=123').toString(), 'id=123&lang=en_EN')
+  })
+
+  // https://github.com/nodejs/undici/issues/4146
+  test('it should handle multiple values set using different syntaxes', (t) => {
+    t = tspl(t, { plan: 3 })
+
+    t.deepStrictEqual(normalizeQueryParams('a=1&a=2&a=3').toString(), 'a=1&a=2&a=3')
+    t.deepStrictEqual(normalizeQueryParams('a[]=1&a[]=2&a[]=3').toString(), 'a=1&a=2&a=3')
+    t.deepStrictEqual(normalizeQueryParams('a=1,2,3').toString(), 'a=1&a=2&a=3')
+  })
+
+  test('should handle edge case scenarios', (t) => {
+    t = tspl(t, { plan: 4 })
+
+    t.deepStrictEqual(normalizeQueryParams('a="b[]"').toString(), `a=${encodeURIComponent('"b[]"')}`)
+    t.deepStrictEqual(normalizeQueryParams('a="1,2,3"').toString(), `a=${encodeURIComponent('"1,2,3"')}`)
+    const encodedSingleQuote = '%27'
+    t.deepStrictEqual(normalizeQueryParams("a='b[]'").toString(), `a=${encodedSingleQuote}${encodeURIComponent('b[]')}${encodedSingleQuote}`)
+    t.deepStrictEqual(normalizeQueryParams("a='1,2,3'").toString(), `a=${encodedSingleQuote}${encodeURIComponent('1,2,3')}${encodedSingleQuote}`)
+  })
+})