Expand function fails on weird GET query params

[bartoszhernas]

Please avoid duplicates

• I checked all open bugs and none of them matched my problem.

Reproducible test case

Use url: include=record-labels,artists&include[music-videos]=artists

Nock Version

13.3.6

Node Version

v20.9.0

TypeScript Version

No response

What happened?

The expand function that is used to compare if the GET parameters are the same to match the interceptor will fail on input coming from Apple Music API.

Take this URL and try to call it with nock:

/v1/catalog/us/albums/1629747885?art[url]=f&extend=editorialArtwork,editorialVideo,extendedAssetUrls,offers,trackCount&fields[artists]=name,url&fields[curators]=name&fields[record-labels]=name,url&format[resources]=map&include=record-labels,artists&include[music-videos]=artists&include[playlists]=curator&include[songs]=artists,composers,albums&l=en-US&meta[albums:tracks]=popularity&platform=web&views=appears-on,audio-extras,more-by-artist,other-versions,related-videos,video-extras,you-might-also-like

it will fail with:

TypeError: Cannot create property 'music-videos' on string 'record-labels,artists'
    at expand (/Users/bartoszhernas/Developer/FYM/app/node_modules/.pnpm/nock@13.3.6_patch_hash=im4utkxp22iqg6dq2hrzjgeg4u/node_modules/nock/lib/common.js:723:27)
    at Object.dataEqual (/Users/bartoszhernas/Developer/FYM/app/node_modules/.pnpm/nock@13.3.6_patch_hash=im4utkxp22iqg6dq2hrzjgeg4u/node_modules/nock/lib/common.js:561:16)
    at Interceptor.matchQuery (/Users/bartoszhernas/Developer/FYM/app/node_modules/.pnpm/nock@13.3.6_patch_hash=im4utkxp22iqg6dq2hrzjgeg4u/node_modules/nock/lib/interceptor.js:442:19)
    at Interceptor.match (/Users/bartoszhernas/Developer/FYM/app/node_modules/.pnpm/nock@13.3.6_patch_hash=im4utkxp22iqg6dq2hrzjgeg4u/node_modules/nock/lib/interceptor.js:322:33)
    at /Users/bartoszhernas/Developer/FYM/app/node_modules/.pnpm/nock@13.3.6_patch_hash=im4utkxp22iqg6dq2hrzjgeg4u/node_modules/nock/lib/intercepted_request_router.js:304:9
    at Array.find (<anonymous>)
    at InterceptedRequestRouter.startPlayback (/Users/bartoszhernas/Developer/FYM/app/node_modules/.pnpm/nock@13.3.6_patch_hash=im4utkxp22iqg6dq2hrzjgeg4u/node_modules/nock/lib/intercepted_request_router.js:303:45)
    at InterceptedRequestRouter.maybeStartPlayback (/Users/bartoszhernas/Developer/FYM/app/node_modules/.pnpm/nock@13.3.6_patch_hash=im4utkxp22iqg6dq2hrzjgeg4u/node_modules/nock/lib/intercepted_request_router.js:268:12)
    at InterceptedRequestRouter.connectSocket (/Users/bartoszhernas/Developer/FYM/app/node_modules/.pnpm/nock@13.3.6_patch_hash=im4utkxp22iqg6dq2hrzjgeg4u/node_modules/nock/lib/intercepted_request_router.js:140:12)
    at /Users/bartoszhernas/Developer/FYM/app/node_modules/.pnpm/nock@13.3.6_patch_hash=im4utkxp22iqg6dq2hrzjgeg4u/node_modules/nock/lib/intercepted_request_router.js:78:33
    at processTicksAndRejections (node:internal/process/task_queues:77:11)

The culprit is that one of the params, specifically include is specifying the value as a string, and then as a objects.

this part of the URL will make code error out: include=record-labels,artists&include[music-videos]=artists

I am now looking into the proper fix for it

Would you be interested in contributing a fix?

• yes

[bartoszhernas]

The fix as a patch for pnpm:

diff --git a/lib/common.js b/lib/common.js
index 97d7c30e4602157b2bf1d475afa6b79525fda606..82ed39892109e51375acf86fa18ca8028796d78d 100644
--- a/lib/common.js
+++ b/lib/common.js
@@ -499,7 +499,7 @@ function isStream(obj) {
 function normalizeClientRequestArgs(input, options, cb) {
   if (typeof input === 'string') {
     input = urlToOptions(new url.URL(input))
-  } else if (input instanceof url.URL) {
+  } else if (input instanceof url.URL || input.constructor.name === 'URL') {
     input = urlToOptions(input)
   } else {
     cb = options
@@ -731,7 +731,7 @@ const expand = input => {
             resultPtr[part] = {}
           }
         }
-        resultPtr = resultPtr[part]
+        resultPtr = { __nock_value: resultPtr[part] }
       }
     }
   }

The first part is a fix for something else (Clickhouse Node JS library doesn't work with nock without this fix as it's uses URLImpl class for URLs. Checking constructor name worked better in this case).

The second part is fix to always make sure the value is an object.

[mikicho]

Thanks @bartoszhernas!
Would you mind creating a pull request and adding a test?

[bartoszhernas]

Two bug fixes with two corresponding tests added to PR.

I realised while writing test, that my original fix was really bad and was hiding the issue and braking URL comparison 🥶