Skip to content

Commit

Permalink
Merge a03480b into ad34222
Browse files Browse the repository at this point in the history
  • Loading branch information
@mastermatt
mastermatt committed Jul 7, 2019
2 parents ad34222 + a03480b commit 9abd0c5
Show file tree
Hide file tree
Showing 6 changed files with 161 additions and 91 deletions.
82 changes: 68 additions & 14 deletions lib/common.js
View file
Expand Up @@ -2,6 +2,7 @@

const _ = require('lodash')
const debug = require('debug')('nock.common')
const url = require('url')

/**
* Normalizes the request options so that it always has `host` property.
Expand Down Expand Up @@ -94,23 +95,16 @@ const overrideRequests = function(newRequest) {
get: overriddenGet,
}

module.request = function(options, callback) {
// debug('request options:', options);
return newRequest(
proto,
overriddenRequest.bind(module),
module.request = function(input, options, callback) {
return newRequest(proto, overriddenRequest.bind(module), [
input,
options,
callback
)
callback,
])
}

module.get = function(options, callback) {
const req = newRequest(
proto,
overriddenRequest.bind(module),
options,
callback
)
module.get = function(input, options, callback) {
const req = module.request(input, options, callback)
req.end()
return req
}
Expand Down Expand Up @@ -490,6 +484,66 @@ function isStream(obj) {
)
}

/**
* Converts the arguments from the various signatures of http[s].request into a standard
* options object and an optional callback function.
*
* https://nodejs.org/api/http.html#http_http_request_url_options_callback
*
* Taken from the beginning of the native `ClientRequest`.
* https://github.com/nodejs/node/blob/908292cf1f551c614a733d858528ffb13fb3a524/lib/_http_client.js#L68
*/
function normalizeClientRequestArgs(input, options, cb) {
if (typeof input === 'string') {
input = urlToOptions(new url.URL(input))
} else if (input instanceof url.URL) {
input = urlToOptions(input)
} else {
cb = options
options = input
input = null
}

if (typeof options === 'function') {
cb = options
options = input || {}
} else {
options = Object.assign(input || {}, options)
}

return { options, callback: cb }
}

/**
* Utility function that converts a URL object into an ordinary
* options object as expected by the http.request and https.request APIs.
*
* This was copied from Node's source
* https://github.com/nodejs/node/blob/908292cf1f551c614a733d858528ffb13fb3a524/lib/internal/url.js#L1257
*/
function urlToOptions(url) {
const options = {
protocol: url.protocol,
hostname:
typeof url.hostname === 'string' && url.hostname.startsWith('[')
? url.hostname.slice(1, -1)
: url.hostname,
hash: url.hash,
search: url.search,
pathname: url.pathname,
path: `${url.pathname}${url.search || ''}`,
href: url.href,
}
if (url.port !== '') {
options.port = Number(url.port)
}
if (url.username || url.password) {
options.auth = `${url.username}:${url.password}`
}
return options
}

exports.normalizeClientRequestArgs = normalizeClientRequestArgs
exports.normalizeRequestOptions = normalizeRequestOptions
exports.isUtf8Representable = isUtf8Representable
exports.overrideRequests = overrideRequests
Expand Down
60 changes: 23 additions & 37 deletions lib/intercept.js
View file
Expand Up @@ -9,8 +9,6 @@ const common = require('./common')
const { inherits } = require('util')
const Interceptor = require('./interceptor')
const http = require('http')
const { parse: urlParse } = require('url')
const { URL } = require('url')
const _ = require('lodash')
const debug = require('debug')('nock.intercept')
const globalEmitter = require('./global_emitter')
Expand Down Expand Up @@ -147,7 +145,7 @@ function interceptorsFor(options) {

// First try to use filteringScope if any of the interceptors has it defined.
let matchingInterceptor
_.each(allInterceptors, function(interceptor, k) {
_.each(allInterceptors, function(interceptor) {
_.each(interceptor.scopes, function(scope) {
const { filteringScope } = scope.__nock_scopeOptions

Expand Down Expand Up @@ -238,7 +236,7 @@ function ErroringClientRequest(error) {
inherits(ErroringClientRequest, http.ClientRequest)

function overrideClientRequest() {
// Here's some background discussion about overridding ClientRequest:
// Here's some background discussion about overriding ClientRequest:
// - https://github.com/nodejitsu/mock-request/issues/4
// - https://github.com/nock/nock/issues/26
// It would be good to add a comment that explains this more clearly.
Expand All @@ -247,8 +245,10 @@ function overrideClientRequest() {
// ----- Extending http.ClientRequest

// Define the overriding client request that nock uses internally.
function OverriddenClientRequest(options, cb) {
if (!options) {
function OverriddenClientRequest(...args) {
const { options, callback } = common.normalizeClientRequestArgs(...args)

if (_.isEmpty(options)) {
// As weird as it is, it's possible to call `http.request` without
// options, and it makes a request to localhost or somesuch. We should
// support it too, for parity. However it doesn't work today, and fixing
Expand All @@ -271,14 +271,12 @@ function overrideClientRequest() {
debug('using', interceptors.length, 'interceptors')

// Use filtered interceptors to intercept requests.
const overrider = RequestOverrider(
this,
options,
interceptors,
remove,
cb
)
const overrider = RequestOverrider(this, options, interceptors, remove)
Object.assign(this, overrider)

if (callback) {
this.once('response', callback)
}
} else {
debug('falling back to original ClientRequest')

Expand Down Expand Up @@ -373,20 +371,12 @@ function activate() {

// ----- Overriding http.request and https.request:

common.overrideRequests(function(
proto,
overriddenRequest,
options,
callback
) {
common.overrideRequests(function(proto, overriddenRequest, args) {
// NOTE: overriddenRequest is already bound to its module.
let req, res

if (typeof options === 'string') {
options = urlParse(options)
} else if (options instanceof URL) {
options = urlParse(options.toString())
} else if (!options) {
const { options, callback } = common.normalizeClientRequestArgs(...args)

if (_.isEmpty(options)) {
// As weird as it is, it's possible to call `http.request` without
// options, and it makes a request to localhost or somesuch. We should
// support it too, for parity. However it doesn't work today, and fixing
Expand All @@ -399,23 +389,25 @@ function activate() {
'Making a request with empty `options` is not supported in Nock'
)
}

// 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.
// Either way, `proto` is used as the source of truth from here on out.
options.proto = proto

const interceptors = interceptorsFor(options)

if (isOn() && interceptors) {
let matches = false
let allowUnmocked = false

matches = !!_.find(interceptors, function(interceptor) {
const matches = !!_.find(interceptors, function(interceptor) {
return interceptor.matchAddress(options)
})

allowUnmocked = !!_.find(interceptors, function(interceptor) {
const allowUnmocked = !!_.find(interceptors, function(interceptor) {
return interceptor.options.allowUnmocked
})

if (!matches && allowUnmocked) {
let req
if (proto === 'https') {
const { ClientRequest } = http
http.ClientRequest = originalClientRequest
Expand All @@ -430,13 +422,7 @@ function activate() {

// NOTE: Since we already overrode the http.ClientRequest we are in fact constructing
// our own OverriddenClientRequest.
req = new http.ClientRequest(options)

res = RequestOverrider(req, options, interceptors, remove)
if (callback) {
res.on('response', callback)
}
return req
return new http.ClientRequest(options, callback)
} else {
globalEmitter.emit('no match', options)
if (isOff() || isEnabledForNetConnect(options)) {
Expand Down
35 changes: 6 additions & 29 deletions lib/recorder.js
View file
@@ -1,13 +1,12 @@
'use strict'

const { inspect } = require('util')
const { parse: urlParse } = require('url')
const common = require('./common')
const intercept = require('./intercept')
const debug = require('debug')('nock.recorder')
const _ = require('lodash')
const URL = require('url')
const qs = require('qs')
const { inspect } = require('util')

const common = require('./common')
const intercept = require('./intercept')

const SEPARATOR = '\n<<<<<<-- cut here -->>>>>>\n'
let recordingInProgress = false
Expand Down Expand Up @@ -235,26 +234,10 @@ function record(recOptions) {
intercept.restoreOverriddenClientRequest()

// We override the requests so that we can save information on them before executing.
common.overrideRequests(function(
proto,
overriddenRequest,
options,
callback
) {
common.overrideRequests(function(proto, overriddenRequest, rawArgs) {
const { options, callback } = common.normalizeClientRequestArgs(...rawArgs)
const bodyChunks = []

if (typeof options === 'string') {
// TODO-coverage: Investigate why this was added. Add a test if
// possible. If we can't figure it out, remove it.
const url = URL.parse(options)
options = {
hostname: url.hostname,
method: 'GET',
port: url.port,
path: url.path,
}
}

// Node 0.11 https.request calls http.request -- don't want to record things
// twice.
if (options._recording) {
Expand All @@ -265,12 +248,6 @@ function record(recOptions) {
const req = overriddenRequest(options, function(res) {
debug(thisRecordingId, 'intercepting', proto, 'request to record')

// TODO-coverage: Investigate why this was added. Add a test if
// possible. If we can't figure it out, remove it.
if (typeof options === 'string') {
options = urlParse(options)
}

// We put our 'end' listener to the front of the listener array.
res.once('end', function() {
debug(thisRecordingId, proto, 'intercepted request ended')
Expand Down
16 changes: 5 additions & 11 deletions lib/request_overrider.js
View file
Expand Up @@ -57,7 +57,7 @@ function setRequestHeaders(req, options, interceptor) {
}
}

function RequestOverrider(req, options, interceptors, remove, cb) {
function RequestOverrider(req, options, interceptors, remove) {
const response = new IncomingMessage(new EventEmitter())

const requestBodyBuffers = []
Expand Down Expand Up @@ -144,7 +144,7 @@ function RequestOverrider(req, options, interceptors, remove, cb) {
if (typeof callback === 'function') {
callback()
}
end(cb)
end()
req.emit('finish')
req.emit('end')
})
Expand All @@ -157,7 +157,7 @@ function RequestOverrider(req, options, interceptors, remove, cb) {
req.flushHeaders = function() {
debug('req.flushHeaders')
if (!req.aborted && !ended) {
end(cb)
end()
}
if (req.aborted) {
emitError(new Error('Request aborted'))
Expand Down Expand Up @@ -214,7 +214,7 @@ function RequestOverrider(req, options, interceptors, remove, cb) {
})
}

const end = function(cb) {
const end = function() {
debug('ending')
ended = true
let requestBody, responseBody, responseBuffers, interceptor
Expand Down Expand Up @@ -257,7 +257,7 @@ function RequestOverrider(req, options, interceptors, remove, cb) {
})
if (interceptor && req instanceof ClientRequest) {
if (interceptor.options.allowUnmocked) {
const newReq = new ClientRequest(options, cb)
const newReq = new ClientRequest(options)
propagate(newReq, req)
// We send the raw buffer as we received it, not as we interpreted it.
newReq.end(requestBodyBuffer)
Expand Down Expand Up @@ -529,12 +529,6 @@ function RequestOverrider(req, options, interceptors, remove, cb) {
}

debug('emitting response')

if (typeof cb === 'function') {
debug('callback with response')
cb(response)
}

req.emit('response', response)

if (common.isStream(responseBody)) {
Expand Down
25 changes: 25 additions & 0 deletions tests/test_common.js
View file
Expand Up @@ -453,3 +453,28 @@ test('percentEncode encodes extra reserved characters', t => {
t.equal(common.percentEncode('foo+(*)!'), 'foo%2B%28%2A%29%21')
t.done()
})

test('normalizeClientRequestArgs throws for invalid URL', async t => {
// no schema
t.throws(() => common.normalizeClientRequestArgs('example.test'), {
input: 'example.test',
name: /TypeError/,
})
})

test('normalizeClientRequestArgs can include auth info', async t => {
const { options } = common.normalizeClientRequestArgs(
'https://user:pw@example.test'
)

t.equal(options.auth, 'user:pw')
})

test('normalizeClientRequestArgs with a single callback', async t => {
const cb = () => {}

const { options, callback } = common.normalizeClientRequestArgs(cb)

t.deepEqual(options, {})
t.is(callback, cb)
})

0 comments on commit 9abd0c5

Please sign in to comment.