Skip to content

Commit 35221ce

Browse files
mastermattmastermatt
authored
refactor: overhaul body and query matching (#1632)
* 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.
1 parent 88e85ac commit 35221ce

File tree

8 files changed

+178
-156
lines changed

8 files changed

+178
-156
lines changed

‎lib/common.js‎

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,9 +424,7 @@ function matchStringOrRegexp(target, pattern) {
424424
const str =
425425
(!_.isUndefined(target) && target.toString && target.toString()) || ''
426426

427-
return pattern instanceof RegExp
428-
? str.match(pattern)
429-
: str === String(pattern)
427+
return pattern instanceof RegExp ? pattern.test(str) : str === String(pattern)
430428
}
431429

432430
/**
@@ -546,6 +544,53 @@ function urlToOptions(url) {
546544
return options
547545
}
548546

547+
/**
548+
* Determines if request data matches the expected schema.
549+
*
550+
* Used for comparing decoded search parameters, request body JSON objects,
551+
* and URL decoded request form bodies.
552+
*
553+
* Performs a general recursive strict comparision with two caveats:
554+
* - The expected data can use regexp to compare values
555+
* - JSON path notation and nested objects are considered equal
556+
*/
557+
const dataEqual = (expected, actual) =>
558+
deepEqual(expand(expected), expand(actual))
559+
560+
/**
561+
* Converts flat objects whose keys use JSON path notation to nested objects.
562+
*
563+
* The input object is not mutated.
564+
*
565+
* @example
566+
* { 'foo[bar][0]': 'baz' } -> { foo: { bar: [ 'baz' ] } }
567+
*/
568+
const expand = input =>
569+
Object.entries(input).reduce((acc, [k, v]) => _.set(acc, k, v), {})
570+
571+
/**
572+
* Performs a recursive strict comparison between two values.
573+
*
574+
* Expected values or leaf nodes of expected object values that are RegExp use test() for comparison.
575+
*/
576+
const deepEqual = (expected, actual) => {
577+
debug('deepEqual comparing', typeof expected, expected, typeof actual, actual)
578+
if (expected instanceof RegExp) {
579+
return expected.test(actual)
580+
}
581+
582+
if (Array.isArray(expected) || _.isPlainObject(expected)) {
583+
const expKeys = Object.keys(expected)
584+
if (expKeys.length !== Object.keys(actual).length) {
585+
return false
586+
}
587+
588+
return expKeys.every(key => deepEqual(expected[key], actual[key]))
589+
}
590+
591+
return expected === actual
592+
}
593+
549594
exports.normalizeClientRequestArgs = normalizeClientRequestArgs
550595
exports.normalizeRequestOptions = normalizeRequestOptions
551596
exports.normalizeOrigin = normalizeOrigin
@@ -567,3 +612,4 @@ exports.percentDecode = percentDecode
567612
exports.matchStringOrRegexp = matchStringOrRegexp
568613
exports.formatQueryValue = formatQueryValue
569614
exports.isStream = isStream
615+
exports.dataEqual = dataEqual

‎lib/interceptor.js‎

Lines changed: 53 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
'use strict'
22

3-
const matchBody = require('./match_body')
4-
const common = require('./common')
5-
const _ = require('lodash')
63
const debug = require('debug')('nock.interceptor')
74
const stringify = require('json-stringify-safe')
8-
const qs = require('qs')
5+
const _ = require('lodash')
6+
const querystring = require('querystring')
97
const url = require('url')
108

9+
const common = require('./common')
10+
const matchBody = require('./match_body')
11+
1112
let fs
1213
try {
1314
fs = require('fs')
@@ -228,11 +229,16 @@ Interceptor.prototype.match = function match(options, body) {
228229
}
229230

230231
const method = (options.method || 'GET').toUpperCase()
231-
let { path } = options
232+
let { path = '' } = options
232233
let matches
233234
let matchKey
234235
const { proto } = options
235236

237+
if (this.method !== method) {
238+
debug(`Method did not match. Request ${method} Interceptor ${this.method}`)
239+
return false
240+
}
241+
236242
if (this.scope.transformPathFunction) {
237243
path = this.scope.transformPathFunction(path)
238244
}
@@ -277,6 +283,25 @@ Interceptor.prototype.match = function match(options, body) {
277283
return false
278284
}
279285

286+
// Match query strings when using query()
287+
if (this.queries === null) {
288+
debug('query matching skipped')
289+
} else {
290+
// can't rely on pathname or search being in the options, but path has a default
291+
const [pathname, search] = path.split('?')
292+
const matchQueries = this.matchQuery({ search })
293+
294+
debug(matchQueries ? 'query matching succeeded' : 'query matching failed')
295+
296+
if (!matchQueries) {
297+
return false
298+
}
299+
300+
// If the query string was explicitly checked then subsequent checks against
301+
// the path using a callback or regexp only validate the pathname.
302+
path = pathname
303+
}
304+
280305
// If we have a filtered scope then we use it instead reconstructing
281306
// the scope from the request options (proto, host and port) as these
282307
// two won't necessarily match and we have to remove the scope that was
@@ -287,93 +312,26 @@ Interceptor.prototype.match = function match(options, body) {
287312
matchKey = common.normalizeOrigin(proto, options.host, options.port)
288313
}
289314

290-
// Match query strings when using query()
291-
let matchQueries = true
292-
let queryIndex = -1
293-
let queryString
294-
let queries
295-
296-
if (this.queries) {
297-
queryIndex = path.indexOf('?')
298-
queryString = queryIndex !== -1 ? path.slice(queryIndex + 1) : ''
299-
queries = qs.parse(queryString)
300-
301-
// Only check for query string matches if this.queries is an object
302-
if (_.isObject(this.queries)) {
303-
if (_.isFunction(this.queries)) {
304-
matchQueries = this.queries(queries)
305-
} else {
306-
// Make sure that you have an equal number of keys. We are
307-
// looping through the passed query params and not the expected values
308-
// if the user passes fewer query params than expected but all values
309-
// match this will throw a false positive. Testing that the length of the
310-
// passed query params is equal to the length of expected keys will prevent
311-
// us from doing any value checking BEFORE we know if they have all the proper
312-
// params
313-
debug('this.queries: %j', this.queries)
314-
debug('queries: %j', queries)
315-
if (_.size(this.queries) !== _.size(queries)) {
316-
matchQueries = false
317-
} else {
318-
const self = this
319-
_.forOwn(queries, function matchOneKeyVal(val, key) {
320-
const expVal = self.queries[key]
321-
let isMatch
322-
if (val === undefined || expVal === undefined) {
323-
isMatch = false
324-
} else if (expVal instanceof RegExp) {
325-
isMatch = common.matchStringOrRegexp(val, expVal)
326-
} else if (_.isArray(expVal) || _.isObject(expVal)) {
327-
isMatch = _.isEqual(val, expVal)
328-
} else {
329-
isMatch = common.matchStringOrRegexp(val, expVal)
330-
}
331-
matchQueries = matchQueries && !!isMatch
332-
})
333-
}
334-
debug('matchQueries: %j', matchQueries)
335-
}
336-
}
337-
338-
// Remove the query string from the path
339-
if (queryIndex !== -1) {
340-
path = path.substr(0, queryIndex)
341-
}
342-
}
343-
344315
if (typeof this.uri === 'function') {
345316
matches =
346-
matchQueries &&
347-
method === this.method &&
348317
common.matchStringOrRegexp(matchKey, this.basePath) &&
349318
// This is a false positive, as `uri` is not bound to `this`.
350319
// eslint-disable-next-line no-useless-call
351320
this.uri.call(this, path)
352321
} else {
353322
matches =
354-
method === this.method &&
355323
common.matchStringOrRegexp(matchKey, this.basePath) &&
356-
common.matchStringOrRegexp(path, this.path) &&
357-
matchQueries
324+
common.matchStringOrRegexp(path, this.path)
358325
}
359326

360-
// special logger for query()
361-
if (queryIndex !== -1) {
362-
this.scope.logger(
363-
`matching ${matchKey}${path}?${queryString} to ${
364-
this._key
365-
} with query(${stringify(this.queries)}): ${matches}`
366-
)
367-
} else {
368-
this.scope.logger(`matching ${matchKey}${path} to ${this._key}: ${matches}`)
369-
}
327+
this.scope.logger(`matching ${matchKey}${path} to ${this._key}: ${matches}`)
370328

371-
if (matches) {
329+
if (matches && this._requestBody !== undefined) {
372330
if (this.scope.transformRequestBodyFunction) {
373331
body = this.scope.transformRequestBodyFunction(body, this._requestBody)
374332
}
375333

376-
matches = matchBody.call(options, this._requestBody, body)
334+
matches = matchBody(options, this._requestBody, body)
377335
if (!matches) {
378336
this.scope.logger("bodies don't match: \n", this._requestBody, '\n', body)
379337
}
@@ -406,11 +364,11 @@ Interceptor.prototype.matchAddress = function matchAddress(options) {
406364
const matchKey = `${method} ${proto}://${options.host}${path}`
407365

408366
if (isRegex && !isRegexBasePath) {
409-
return !!matchKey.match(comparisonKey) && !!path.match(this.path)
367+
return !!matchKey.match(comparisonKey) && this.path.test(path)
410368
}
411369

412370
if (isRegexBasePath) {
413-
return !!matchKey.match(this.scope.basePath) && !!path.match(this.path)
371+
return this.scope.basePath.test(matchKey) && !!path.match(this.path)
414372
}
415373

416374
return comparisonKey === matchKey
@@ -420,6 +378,22 @@ Interceptor.prototype.matchHostName = function matchHostName(options) {
420378
return options.hostname === this.scope.urlParts.hostname
421379
}
422380

381+
Interceptor.prototype.matchQuery = function matchQuery(options) {
382+
if (this.queries === true) {
383+
return true
384+
}
385+
386+
const reqQueries = querystring.parse(options.search)
387+
debug('Interceptor queries: %j', this.queries)
388+
debug(' Request queries: %j', reqQueries)
389+
390+
if (_.isFunction(this.queries)) {
391+
return this.queries(reqQueries)
392+
}
393+
394+
return common.dataEqual(this.queries, reqQueries)
395+
}
396+
423397
Interceptor.prototype.filteringPath = function filteringPath(...args) {
424398
this.scope.filteringPath(...args)
425399
return this
@@ -482,7 +456,7 @@ Interceptor.prototype.query = function query(queries) {
482456
if (queries instanceof url.URLSearchParams) {
483457
// Normalize the data into the shape that is matched against.
484458
// Duplicate keys are handled by combining the values into an array.
485-
queries = qs.parse(queries.toString())
459+
queries = querystring.parse(queries.toString())
486460
} else if (!_.isPlainObject(queries)) {
487461
throw Error(`Argument Error: ${queries}`)
488462
}

‎lib/match_body.js‎

Lines changed: 11 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,13 @@
11
'use strict'
22

3-
const deepEqual = require('deep-equal')
4-
const qs = require('qs')
53
const _ = require('lodash')
6-
const common = require('./common')
7-
8-
module.exports = function matchBody(spec, body) {
9-
if (typeof spec === 'undefined') {
10-
return true
11-
}
4+
const querystring = require('querystring')
125

13-
const options = this || {}
6+
const common = require('./common')
147

8+
module.exports = function matchBody(options, spec, body) {
159
if (spec instanceof RegExp) {
16-
return body.match(spec)
10+
return spec.test(body)
1711
}
1812

1913
if (Buffer.isBuffer(spec)) {
@@ -30,9 +24,8 @@ module.exports = function matchBody(spec, body) {
3024
''
3125
).toString()
3226

33-
const isMultipart = contentType.indexOf('multipart') >= 0
34-
const isUrlencoded =
35-
contentType.indexOf('application/x-www-form-urlencoded') >= 0
27+
const isMultipart = contentType.includes('multipart')
28+
const isUrlencoded = contentType.includes('application/x-www-form-urlencoded')
3629

3730
// try to transform body to json
3831
let json
@@ -45,12 +38,12 @@ module.exports = function matchBody(spec, body) {
4538
if (json !== undefined) {
4639
body = json
4740
} else if (isUrlencoded) {
48-
body = qs.parse(body, { allowDots: true })
41+
body = querystring.parse(body)
4942
}
5043
}
5144

5245
if (typeof spec === 'function') {
53-
return spec.call(this, body)
46+
return spec.call(options, body)
5447
}
5548

5649
// strip line endings from both so that we get a match no matter what OS we are running on
@@ -63,6 +56,8 @@ module.exports = function matchBody(spec, body) {
6356
spec = spec.replace(/\r?\n|\r/g, '')
6457
}
6558

59+
// Because the nature of URL encoding, all the values in the body have been cast to strings.
60+
// dataEqual does strict checking so we we have to cast the non-regexp values in the spec too.
6661
if (isUrlencoded) {
6762
spec = mapValuesDeep(spec, function(val) {
6863
if (_.isRegExp(val)) {
@@ -72,7 +67,7 @@ module.exports = function matchBody(spec, body) {
7267
})
7368
}
7469

75-
return deepEqualExtended(spec, body)
70+
return common.dataEqual(spec, body)
7671
}
7772

7873
/**
@@ -92,27 +87,3 @@ function mapValuesDeep(obj, cb) {
9287
}
9388
return cb(obj)
9489
}
95-
96-
function deepEqualExtended(spec, body) {
97-
if (spec && spec.constructor === RegExp) {
98-
return spec.test(body)
99-
}
100-
if (
101-
spec &&
102-
(spec.constructor === Object || spec.constructor === Array) &&
103-
body
104-
) {
105-
const keys = Object.keys(spec)
106-
const bodyKeys = Object.keys(body)
107-
if (keys.length !== bodyKeys.length) {
108-
return false
109-
}
110-
for (let i = 0; i < keys.length; i++) {
111-
if (!deepEqualExtended(spec[keys[i]], body[keys[i]])) {
112-
return false
113-
}
114-
}
115-
return true
116-
}
117-
return deepEqual(spec, body, { strict: true })
118-
}

‎package-lock.json‎

Lines changed: 2 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)