From f8d6cbbe147266bca1654dd72f475e39f544600c Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Mon, 15 Jul 2019 08:58:06 -0600 Subject: [PATCH] fix: allow unmocked when providing literal search params. (#1614) fixes: #1421 There was inconsistent behavior around how search params were handled when they were provided as part of a URI string to new Interceptors. The issue happen to come to light when `allowUnmocked` was set. The fix normalizes the behavior by refactoring the constructor of Interceptor to look for literal search params. If present, they're stripped from the `path` attribute and set via the `query` method. As part of the work, `Interceptor.query` was modified to accept `URLSearchParams` instances as valid input. --- README.md | 21 +++++++++++++- lib/interceptor.js | 53 ++++++++++++++++++++++++------------ tests/test_allow_unmocked.js | 11 ++++++++ tests/test_query.js | 42 ++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 970553c0d..2c5ab2d4f 100644 --- a/README.md +++ b/README.md @@ -254,7 +254,15 @@ nock('http://www.example.com') ### Specifying request query string -Nock understands query strings. Instead of placing the entire URL, you can specify the query part as an object: +Nock understands query strings. Search parameters can be included as part of the path: + +```js +nock('http://example.com') + .get('/users?foo=bar') + .reply(200) +``` + +Instead of placing the entire URL, you can specify the query part as an object: ```js nock('http://example.com') @@ -278,6 +286,17 @@ nock('http://example.com') .reply(200, { results: [{ id: 'pgte' }] }) ``` +A `URLSearchParams` instance can be provided. + +```js +const params = new URLSearchParams({ foo: 'bar' }) + +nock('http://example.com') + .get('/') + .query(params) + .reply(200) +``` + Nock supports passing a function to query. The function determines if the actual query matches or not. ```js diff --git a/lib/interceptor.js b/lib/interceptor.js index 60429fa32..0afbcc77a 100644 --- a/lib/interceptor.js +++ b/lib/interceptor.js @@ -6,6 +6,7 @@ const _ = require('lodash') const debug = require('debug')('nock.interceptor') const stringify = require('json-stringify-safe') const qs = require('qs') +const url = require('url') let fs try { @@ -16,30 +17,40 @@ try { module.exports = Interceptor +/** + * + * Valid argument types for `uri`: + * - A string used for strict comparisons with pathname. + * The search portion of the URI may also be postfixed, in which case the search params + * are striped and added via the `query` method. + * - A RegExp instance that tests against only the pathname of requests. + * - A synchronous function bound to this Interceptor instance. It's provided the pathname + * of requests and must return a boolean denoting if the request is considered a match. + */ function Interceptor(scope, uri, method, requestBody, interceptorOptions) { + const uriIsStr = typeof uri === 'string' // Check for leading slash. Uri can be either a string or a regexp, but // we only need to check strings. - if (typeof uri === 'string' && /^[^/*]/.test(uri)) { + if (uriIsStr && /^[^/*]/.test(uri)) { throw Error( "Non-wildcard URL path strings must begin with a slash (otherwise they won't match anything)" ) } - this.scope = scope - this.interceptorMatchHeaders = [] - - if (typeof method === 'undefined' || !method) { + if (!method) { throw new Error('The "method" parameter is required for an intercept call.') } + + this.scope = scope + this.interceptorMatchHeaders = [] this.method = method.toUpperCase() this.uri = uri this._key = `${this.method} ${scope.basePath}${scope.basePathname}${ - typeof uri === 'string' ? '' : '/' + uriIsStr ? '' : '/' }${uri}` this.basePath = this.scope.basePath - this.path = typeof uri === 'string' ? scope.basePathname + uri : uri + this.path = uriIsStr ? scope.basePathname + uri : uri - this.baseUri = `${this.method} ${scope.basePath}${scope.basePathname}` this.options = interceptorOptions || {} this.counter = 1 this._requestBody = requestBody @@ -56,6 +67,15 @@ function Interceptor(scope, uri, method, requestBody, interceptorOptions) { this.delayConnectionInMs = 0 this.optional = false + + // strip off literal query parameters if they were provided as part of the URI + if (uriIsStr && uri.includes('?')) { + // localhost is a dummy value because the URL constructor errors for only relative inputs + const parsedURL = new url.URL(this.path, 'http://localhost') + this.path = parsedURL.pathname + this.query(parsedURL.searchParams) + this._key = `${this.method} ${scope.basePath}${this.path}` + } } Interceptor.prototype.optionally = function optionally(value) { @@ -463,18 +483,17 @@ Interceptor.prototype.query = function query(queries) { return this } - let stringFormattingFn + let strFormattingFn if (this.scope.scopeOptions.encodedQueryParams) { - stringFormattingFn = common.percentDecode + strFormattingFn = common.percentDecode } - for (const key in queries) { - if (_.isUndefined(this.queries[key])) { - const formattedPair = common.formatQueryValue( - key, - queries[key], - stringFormattingFn - ) + const entries = + queries instanceof url.URLSearchParams ? queries : Object.entries(queries) + + for (const [key, value] of entries) { + if (this.queries[key] === undefined) { + const formattedPair = common.formatQueryValue(key, value, strFormattingFn) this.queries[formattedPair[0]] = formattedPair[1] } } diff --git a/tests/test_allow_unmocked.js b/tests/test_allow_unmocked.js index 07ead09f4..496d7f3f9 100644 --- a/tests/test_allow_unmocked.js +++ b/tests/test_allow_unmocked.js @@ -259,6 +259,17 @@ test('match multiple paths to domain using regexp with allowUnmocked', async t = scope2.done() }) +test('match domain and path with literal query params and allowUnmocked', async t => { + const scope = nock('http://example.test', { allowUnmocked: true }) + .get('/foo?bar=baz') + .reply() + + const { statusCode } = await got('http://example.test/foo?bar=baz') + + t.is(statusCode, 200) + scope.done() +}) + test('match domain and path using regexp with query params and allowUnmocked', t => { const imgResponse = 'Matched Images Page' const opts = { allowUnmocked: true } diff --git a/tests/test_query.js b/tests/test_query.js index d851c031d..282cadad0 100644 --- a/tests/test_query.js +++ b/tests/test_query.js @@ -2,7 +2,9 @@ const mikealRequest = require('request') const { test } = require('tap') +const url = require('url') const nock = require('..') +const got = require('./got_client') require('./cleanup_after_each')() @@ -32,6 +34,17 @@ test('query() matches multiple query strings of the same name=value', t => { }) }) +test('literal query params have the same behavior as calling query() directly', async t => { + const scope = nock('http://example.test') + .get('/foo?bar=baz') + .reply() + + const { statusCode } = await got('http://example.test/foo?bar=baz') + + t.is(statusCode, 200) + scope.done() +}) + test('query() matches multiple query strings of the same name=value regardless of order', t => { nock('http://example.test') .get('/') @@ -92,6 +105,35 @@ test('query() matches a query string using regexp', t => { }) }) +test('query() accepts URLSearchParams as input', async t => { + const params = new url.URLSearchParams({ + foo: 'bar', + }) + + const scope = nock('http://example.test') + .get('/') + .query(params) + .reply() + + const { statusCode } = await got('http://example.test?foo=bar') + + t.is(statusCode, 200) + scope.done() +}) + +test('multiple set query keys use the first occurrence', async t => { + const scope = nock('http://example.test') + .get('/') + .query({ foo: 'bar' }) + .query({ foo: 'baz' }) + .reply() + + const { statusCode } = await got('http://example.test?foo=bar') + + t.is(statusCode, 200) + scope.done() +}) + test('query() matches a query string that contains special RFC3986 characters', t => { nock('http://example.test') .get('/')