RFC - Consistent path comparison

lib/intercept.js

@@ -390,7 +390,7 @@ function activate() {
     }
 
     // The option per the docs is `protocol`. Its unclear if this line is meant to override that and is misspelled or if
-    // the intend is to explicitly keep track of which module was called using a separate name.
+    // the intent is to explicitly keep track of which module was called using a separate name.
     // Either way, `proto` is used as the source of truth from here on out.
     options.proto = proto
 

lib/interceptor.js

@@ -253,7 +253,6 @@ module.exports = class Interceptor {
 
     const method = (options.method || 'GET').toUpperCase()
     let { path = '/' } = options
-    let matches
     let matchKey
     const { proto } = options
 
@@ -268,6 +267,9 @@ module.exports = class Interceptor {
       path = this.scope.transformPathFunction(path)
     }
 
+    // can't rely on pathname or search being in the options, but path has a default
+    const [pathname, search] = path.split('?')
+
     const requestMatchesFilter = ({ name, value: predicate }) => {
       const headerValue = req.getHeader(name)
       if (typeof predicate === 'function') {
@@ -317,8 +319,6 @@ module.exports = class Interceptor {
     if (this.queries === null) {
       this.scope.logger('query matching skipped')
     } else {
-      // can't rely on pathname or search being in the options, but path has a default
-      const [pathname, search] = path.split('?')
       const matchQueries = this.matchQuery({ search })
 
       this.scope.logger(
@@ -328,53 +328,57 @@ module.exports = class Interceptor {
       if (!matchQueries) {
         return false
       }
-
-      // If the query string was explicitly checked then subsequent checks against
-      // the path using a callback or regexp only validate the pathname.
-      path = pathname
     }
 
     // If we have a filtered scope then we use it instead reconstructing the
     // scope from the request options (proto, host and port) as these two won't
-    // necessarily match and we have to remove the scope that was matched (vs.
+    // necessarily match, and we have to remove the scope that was matched (vs.
     // that was defined).
     if (this.__nock_filteredScope) {
       matchKey = this.__nock_filteredScope
     } else {
       matchKey = common.normalizeOrigin(proto, options.host, options.port)
     }
 
+    let pathMatches
     if (typeof this.uri === 'function') {
-      matches =
+      pathMatches =
         common.matchStringOrRegexp(matchKey, this.basePath) &&
         // This is a false positive, as `uri` is not bound to `this`.
         // eslint-disable-next-line no-useless-call
-        this.uri.call(this, path)
+        this.uri.call(this, pathname)
     } else {
-      matches =
+      pathMatches =
         common.matchStringOrRegexp(matchKey, this.basePath) &&
-        common.matchStringOrRegexp(path, this.path)
+        common.matchStringOrRegexp(pathname, this.path)
     }
 
-    this.scope.logger(`matching ${matchKey}${path} to ${this._key}: ${matches}`)
+    this.scope.logger(
+      `matching ${matchKey}${pathname} to ${this._key}: ${pathMatches}`
+    )
+
+    if (!pathMatches) {
+      return false
+    }
 
-    if (matches && this._requestBody !== undefined) {
+    if (this._requestBody !== undefined) {
       if (this.scope.transformRequestBodyFunction) {
         body = this.scope.transformRequestBodyFunction(body, this._requestBody)
       }
 
-      matches = matchBody(options, this._requestBody, body)
-      if (!matches) {
+      const bodyMatches = matchBody(options, this._requestBody, body)
+      if (!bodyMatches) {
         this.scope.logger(
           "bodies don't match: \n",
           this._requestBody,
           '\n',
           body
         )
+        return false
       }
     }
 
-    return matches
+    return true
   }
 
   /**
@@ -387,30 +391,28 @@ module.exports = class Interceptor {
     const isRegexBasePath = this.scope.basePath instanceof RegExp
 
     const method = (options.method || 'GET').toUpperCase()
-    let { path } = options
+    let { path = '/' } = options
     const { proto } = options
 
-    // NOTE: Do not split off the query params as the regex could use them
-    if (!isRegex) {
-      path = path ? path.split('?')[0] : ''
-    }
-
     if (this.scope.transformPathFunction) {
       path = this.scope.transformPathFunction(path)
     }
+
+    // can't rely on pathname or search being in the options, but path has a default
+    const [pathname] = path.split('?')
     const comparisonKey = isPathFn || isRegex ? this.__nock_scopeKey : this._key
-    const matchKey = `${method} ${proto}://${options.host}${path}`
+    const matchKey = `${method} ${proto}://${options.host}${pathname}`
 
     if (isPathFn) {
-      return !!(matchKey.match(comparisonKey) && this.path(path))
+      return !!(matchKey.match(comparisonKey) && this.path(pathname))
     }
 
     if (isRegex && !isRegexBasePath) {
-      return !!matchKey.match(comparisonKey) && this.path.test(path)
+      return !!matchKey.match(comparisonKey) && this.path.test(pathname)
     }
 
     if (isRegexBasePath) {
-      return this.scope.basePath.test(matchKey) && !!path.match(this.path)
+      return this.scope.basePath.test(matchKey) && !!pathname.match(this.path)
     }
 
     return comparisonKey === matchKey

tests/test_allow_unmocked.js

@@ -208,11 +208,11 @@ describe('allowUnmocked option', () => {
     scope.done()
   })
 
-  it('match domain and path using regexp with query params and allowUnmocked', async () => {
+  it('match domain and path using regexp and allowUnmocked, with query params', async () => {
     const imgResponse = 'Matched Images Page'
 
     const scope = nock(/example/, { allowUnmocked: true })
-      .get(/imghp\?hl=en/)
+      .get(/^\/imghp$/)
       .reply(200, imgResponse)
 
     const { body, statusCode } = await got('http://example.test/imghp?hl=en')

tests/test_intercept.js

@@ -818,7 +818,7 @@ describe('Intercept', () => {
     scope2.done()
   })
 
-  it('match domain using intercept callback', async () => {
+  it('match path using intercept callback', async () => {
     const validUrl = ['/cats', '/dogs']
 
     nock('http://example.test')
@@ -1109,4 +1109,48 @@ describe('Intercept', () => {
         done()
       })
   })
+
+  describe('ignoring query params', () => {
+    it('ignores the query string if `query()` is not called and request includes query params', async () => {
+      const scope = nock('http://example.test').get('/foo').reply()
+
+      const { statusCode } = await got('http://example.test/foo?bar=baz')
+
+      expect(statusCode).to.equal(200)
+      scope.done()
+    })
+
+    it('ignores the query string when matching path using intercept callback', async () => {
+      const scope = nock('http://example.test')
+        .get(pathname => pathname === '/foo')
+        .reply()
+
+      const { statusCode } = await got('http://example.test/foo?bar=baz')
+
+      expect(statusCode).to.equal(200)
+      scope.done()
+    })
+
+    it('ignores the query string if regexp is used for the pathname', async () => {
+      const scope = nock('http://example.test')
+        .get(/^\/foo$/)
+        .reply()
+
+      const { statusCode } = await got('http://example.test/foo?bar=baz')
+
+      expect(statusCode).to.equal(200)
+      scope.done()
+    })
+
+    it('ignores the query string if regexp is used for the domain and pathname', async () => {
+      const scope = nock(/example/)
+        .get(/^\/foo$/)
+        .reply()
+
+      const { statusCode } = await got('http://example.test/foo?bar=baz')
+
+      expect(statusCode).to.equal(200)
+      scope.done()
+    })
+  })
 })

tests/test_query.js

@@ -70,6 +70,18 @@ describe('`query()`', () => {
       scope.done()
     })
 
+    it('matches query values if regexp is used on the pathname', async () => {
+      const scope = nock('http://example.test')
+        .get(/^\/foo$/)
+        .query({ bar: 'baz' })
+        .reply()
+
+      const { statusCode } = await got('http://example.test/foo?bar=baz')
+
+      expect(statusCode).to.equal(200)
+      scope.done()
+    })
+
     it("doesn't match query values of requests without query string", async () => {
       const scope1 = nock('http://example.test')
         .get('/')

types/index.d.ts

@@ -35,7 +35,7 @@ declare namespace nock {
   let recorder: Recorder
 
   type InterceptFunction = (
-    uri: string | RegExp | { (uri: string): boolean },
+    pathname: string | RegExp | { (uri: string): boolean },
     requestBody?: RequestBodyMatcher,
     interceptorOptions?: Options
   ) => Interceptor