Query params: Arrays and comma-separated strings are equal #1552
Labels
Comments
|
Hmmm maybe related to #507? Thanks for the bug report and the test case! |
mastermatt
added a commit
to mastermatt/nock
that referenced
this issue
Jul 18, 2019
Drop `deep-equal` and `qs` dependency. Replace `qs.parse` with `querystring.parse` from the std lib. The main difference between the two being that `qs` "unpacks" nested objects when the search keys use JSON path notation eg "foo[bar][0]". This has no affect on URL encoded form bodies during matching because we manually convert the notation to nested objects for comparison now, which means users can now provide expectations in either form. Similarly, when matching search params using a provided object, there is no net affect. The only breaking change is when a user provides a callback function to `.query()`. Replace `deep-equal` with a custom utility function in common.js. Doing so not only dropped a dependency, it also achieved a more generic utility for our usec ase with less code. Interceptor matching had some refactoring: - Query matching was moved to its own method in an effort to isolate concerns. - Matching the request method was moved to the top of the match method. - Debugging statements were updated as a baby step towards having more insight into why a match was missed. Fixes nock#507 Fixes nock#1552 BREAKING CHANGE: The only argument passed to the Interceptor.query callback now receives the output from querystring.parse instead of qs.parse. This means that instead of nested objects the argument will be a flat object.
mastermatt
added a commit
that referenced
this issue
Jul 28, 2019
* refactor(match_body): options obj as arg instead of context Also don't bother calling `transformRequestBodyFunction` or `matchBody` if there is no request body on the Interceptor. * perf: prefer regexp.test over match when applicable * refactor: overhaul body and query matching Drop `deep-equal` and `qs` dependency. Replace `qs.parse` with `querystring.parse` from the std lib. The main difference between the two being that `qs` "unpacks" nested objects when the search keys use JSON path notation eg "foo[bar][0]". This has no affect on URL encoded form bodies during matching because we manually convert the notation to nested objects for comparison now, which means users can now provide expectations in either form. Similarly, when matching search params using a provided object, there is no net affect. The only breaking change is when a user provides a callback function to `.query()`. Replace `deep-equal` with a custom utility function in common.js. Doing so not only dropped a dependency, it also achieved a more generic utility for our usec ase with less code. Interceptor matching had some refactoring: - Query matching was moved to its own method in an effort to isolate concerns. - Matching the request method was moved to the top of the match method. - Debugging statements were updated as a baby step towards having more insight into why a match was missed. Fixes #507 Fixes #1552 BREAKING CHANGE: The only argument passed to the Interceptor.query callback now receives the output from querystring.parse instead of qs.parse. This means that instead of nested objects the argument will be a flat object.
|
🎉 This issue has been resolved in version 11.0.0-beta.30 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This issue has been resolved in version 11.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
gr2m
pushed a commit
that referenced
this issue
Sep 4, 2019
* refactor(match_body): options obj as arg instead of context Also don't bother calling `transformRequestBodyFunction` or `matchBody` if there is no request body on the Interceptor. * perf: prefer regexp.test over match when applicable * refactor: overhaul body and query matching Drop `deep-equal` and `qs` dependency. Replace `qs.parse` with `querystring.parse` from the std lib. The main difference between the two being that `qs` "unpacks" nested objects when the search keys use JSON path notation eg "foo[bar][0]". This has no affect on URL encoded form bodies during matching because we manually convert the notation to nested objects for comparison now, which means users can now provide expectations in either form. Similarly, when matching search params using a provided object, there is no net affect. The only breaking change is when a user provides a callback function to `.query()`. Replace `deep-equal` with a custom utility function in common.js. Doing so not only dropped a dependency, it also achieved a more generic utility for our usec ase with less code. Interceptor matching had some refactoring: - Query matching was moved to its own method in an effort to isolate concerns. - Matching the request method was moved to the top of the match method. - Debugging statements were updated as a baby step towards having more insight into why a match was missed. Fixes #507 Fixes #1552 BREAKING CHANGE: The only argument passed to the Interceptor.query callback now receives the output from querystring.parse instead of qs.parse. This means that instead of nested objects the argument will be a flat object.
gr2m
pushed a commit
that referenced
this issue
Sep 4, 2019
* refactor(match_body): options obj as arg instead of context Also don't bother calling `transformRequestBodyFunction` or `matchBody` if there is no request body on the Interceptor. * perf: prefer regexp.test over match when applicable * refactor: overhaul body and query matching Drop `deep-equal` and `qs` dependency. Replace `qs.parse` with `querystring.parse` from the std lib. The main difference between the two being that `qs` "unpacks" nested objects when the search keys use JSON path notation eg "foo[bar][0]". This has no affect on URL encoded form bodies during matching because we manually convert the notation to nested objects for comparison now, which means users can now provide expectations in either form. Similarly, when matching search params using a provided object, there is no net affect. The only breaking change is when a user provides a callback function to `.query()`. Replace `deep-equal` with a custom utility function in common.js. Doing so not only dropped a dependency, it also achieved a more generic utility for our usec ase with less code. Interceptor matching had some refactoring: - Query matching was moved to its own method in an effort to isolate concerns. - Matching the request method was moved to the top of the match method. - Debugging statements were updated as a baby step towards having more insight into why a match was missed. Fixes #507 Fixes #1552 BREAKING CHANGE: The only argument passed to the Interceptor.query callback now receives the output from querystring.parse instead of qs.parse. This means that instead of nested objects the argument will be a flat object.
gr2m
pushed a commit
that referenced
this issue
Sep 5, 2019
* refactor(match_body): options obj as arg instead of context Also don't bother calling `transformRequestBodyFunction` or `matchBody` if there is no request body on the Interceptor. * perf: prefer regexp.test over match when applicable * refactor: overhaul body and query matching Drop `deep-equal` and `qs` dependency. Replace `qs.parse` with `querystring.parse` from the std lib. The main difference between the two being that `qs` "unpacks" nested objects when the search keys use JSON path notation eg "foo[bar][0]". This has no affect on URL encoded form bodies during matching because we manually convert the notation to nested objects for comparison now, which means users can now provide expectations in either form. Similarly, when matching search params using a provided object, there is no net affect. The only breaking change is when a user provides a callback function to `.query()`. Replace `deep-equal` with a custom utility function in common.js. Doing so not only dropped a dependency, it also achieved a more generic utility for our usec ase with less code. Interceptor matching had some refactoring: - Query matching was moved to its own method in an effort to isolate concerns. - Matching the request method was moved to the top of the match method. - Debugging statements were updated as a baby step towards having more insight into why a match was missed. Fixes #507 Fixes #1552 BREAKING CHANGE: The only argument passed to the Interceptor.query callback now receives the output from querystring.parse instead of qs.parse. This means that instead of nested objects the argument will be a flat object.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What is the expected behavior?
Nock should not consider comma-separated query params to match array-based query params
What is the actual behavior?
If nock expects a comma-separated string as a query parameter, and an array-based parameter is set in the tested code, the two params are seen to be equal.
Possible solution
The problem, I believe, happens in matchStringOrRegexp. The interceptor sees the expected value is the comma-separated string (a string), it passes that and the
qs.parsed query param (an array) to matchStringOrRegexp.matchStringOrRegexpwill typecast the array to a string, meaning they will match:matchStringOrRegexp([ 'abc', '123' ], 'abc,123') => trueHow to reproduce the issue
https://runkit.com/jonogilmour/node-nock-query-issue
Does the bug have a test case?
Versions
The text was updated successfully, but these errors were encountered: