`Error: Query params cannot be passed when url already contains "?" or "#".` at redirection

[nandenjin]

Bug Description

When use options.query, options.maxRedirections and the target server returns redirection URL contains ? like https://example.com?param=here, undici raises Error: Query params cannot be passed when url already contains "?" or "#". to redirected URL.

It seems that undici tries to add query parameters for URLs after redirections. (To be clear, I think it should not - it is not intended behavior for undici)

Reproducible By

I found this issue when I tried to send request to WebApp built with Google Apps Script, so I created an endpoint just for testing. It redirects you once then shows query params you passed first.

https://script.google.com/macros/s/AKfycbzf56MFzf5Mdvop2eldVkhVf-vBvazOA86Noc0R7Km6vUgYUd0Ufm7_zIKd0yx2VpO1mA/exec

And you can repoduce the issue by:

const { request } = require('undici');

request(
  `https://script.google.com/macros/s/AKfycbzf56MFzf5Mdvop2eldVkhVf-vBvazOA86Noc0R7Km6vUgYUd0Ufm7_zIKd0yx2VpO1mA/exec`,
  {
    maxRedirectioins: 1,
    query: { some_params: 'here' }
  }
)

Expected Behavior

No error raised up. options.query works only for the first URL, not for URLs as redirected target.

Logs & Screenshots

Logs for sample code to reproduce (given above):

nandenjin@nandenjin-hp:~/workspace$ node sample.js
/home/nandenjin/workspace/node_modules/undici/lib/core/util.js:33
    throw new Error('Query params cannot be passed when url already contains "?" or "#".')
          ^

Error: Query params cannot be passed when url already contains "?" or "#".
    at Object.buildURL (/home/nandenjin/workspace/node_modules/undici/lib/core/util.js:33:11)
    at new Request (/home/nandenjin/workspace/node_modules/undici/lib/core/request.js:132:30)
    at Client.[dispatch] (/home/nandenjin/workspace/node_modules/undici/lib/client.js:263:21)
    at Intercept (/home/nandenjin/workspace/node_modules/undici/lib/interceptor/redirectInterceptor.js:11:16)
    at Client.[Intercepted Dispatch] (/home/nandenjin/workspace/node_modules/undici/lib/dispatcher-base.js:157:12)
    at Client.dispatch (/home/nandenjin/workspace/node_modules/undici/lib/dispatcher-base.js:178:40)
    at Pool.[dispatch] (/home/nandenjin/workspace/node_modules/undici/lib/pool-base.js:143:28)
    at Pool.[Intercepted Dispatch] (/home/nandenjin/workspace/node_modules/undici/lib/dispatcher-base.js:149:29)
    at Pool.dispatch (/home/nandenjin/workspace/node_modules/undici/lib/dispatcher-base.js:178:40)
    at Agent.[dispatch] (/home/nandenjin/workspace/node_modules/undici/lib/agent.js:118:23)
nandenjin@nandenjin-hp:~/workspace$ 

Environment

• node -v: v16.13.2
• [email protected]
• Ubuntu 18.04 on WSL, Windows 11

Additional context

I investigated the issue by injecting console.log manually at node_modules/undici/lib/core/util.js:

function buildURL (url, queryParams) {
  console.log('🌟', url, queryParams) // INJECTED

  if (url.includes('?') || url.includes('#')) {
    throw new Error('Query params cannot be passed when url already contains "?" or "#".')
  }

  const stringified = stringify(queryParams)
  // ...

then I received below...

nandenjin@nandenjin-hp:~/workspace$ node ss.js 
🌟 /macros/s/AKfycbzf56MFzf5Mdvop2eldVkhVf-vBvazOA86Noc0R7Km6vUgYUd0Ufm7_zIKd0yx2VpO1mA/exec { someParams: 'here' }
🌟 /macros/echo?user_content_key=VV95Xwwp7kvFldrlylaO2_8q6G_Gj_Apm-MSzm5RieVmFh0wvL0roDb2HJ9BLUfjoGZ5eyURPOj1zgHLFkAyKiO_HQR7M-D2OJmA1Yb3SEsKFZqtv3DaNYcMrmhZHmUMWojr9NvTBuBLhyHCd5hHawMD2MvukPNcPvDu1Ie3pXq060MQR7vk1WNb2Z-d0HdIDeR-fjYnob9M0sAc9guvMow5yZeqNY5O0ng1hKCdVZqw_Ab5fU_nTM--1TMf6UgRc_a8jUBzMEzqPlr3aBED9g&lib=Mwk9ESj_JQAtSSPGKH5n5UClFdB15v-vk { someParams: 'here' }
/home/nandenjin/workspace/node_modules/undici/lib/core/util.js:33
    throw new Error('Query params cannot be passed when url already contains "?" or "#".')
          ^

Error: Query params cannot be passed when url already contains "?" or "#".

[nandenjin]

For those who are facing same issue: Passing URL instance with setting searchParams prop works well.

const url = new URL("https://script.google.com/macros/s/AKfycbzf56MFzf5Mdvop2eldVkhVf-vBvazOA86Noc0R7Km6vUgYUd0Ufm7_zIKd0yx2VpO1mA/exec")
url.searchParams.set('some_params', 'here')

// Works for me
request(url, {
  maxRedirections: 5,
})

[mcollina]

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

[IlyasShabi]

@mcollina I would like to help on this

[mcollina]

go for it!