Skip to content

Commit

Permalink
Throw if headersFieldNamesToLowerCase is not provided an object.
Browse files Browse the repository at this point in the history
Adds a test to cover the exception case.

Going through the git history for this function, all the way back to
when it was added with 4048734, I can't find a case where a non-object
input was valid.

Once PR # 1564 is merged in, this utility will only be called in two
places:
 - Creating an Interceptor if the Scope was created with `options`
 - When http.request is called with an `options` object

In both places, providing defined, but non-object values causes chaos.
If the value is a truthy, non-iterable then the header was ignored
during matching. For example, the following will match Interceptors as
if `reqheaders` had not been provided.
```js
nock('http://example.com', { reqheaders: 1 }).get('/').reply
```

However, if the value was provided as a non-plain object iterable, such
as an array or string, the header matching logic would attempt to match
header field names with numerical keys, rendering the whole Scope
useless.
  • Loading branch information
mastermatt committed May 29, 2019
1 parent 78ceee0 commit bafe6e7
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 18 deletions.
25 changes: 13 additions & 12 deletions lib/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,24 +202,25 @@ function isJSONContent(headers) {
return contentType === 'application/json'
}

/**
* Return a new object with all field names of the headers lower-cased.
*
* Duplicates throw an error.
*/
const headersFieldNamesToLowerCase = function(headers) {
if (!_.isObject(headers)) {
// TODO-coverage: Add a test to cover the missing condition, or remove if
// not reachable.
return headers
if (!_.isPlainObject(headers)) {
throw Error('Headers must be provided as an object')
}

// For each key in the headers, delete its value and reinsert it with lower-case key.
// Keys represent headers field names.
const lowerCaseHeaders = {}
_.forOwn(headers, function(fieldVal, fieldName) {
const lowerCaseFieldName = fieldName.toLowerCase()
if (!_.isUndefined(lowerCaseHeaders[lowerCaseFieldName])) {
throw new Error(
`Failed to convert header keys to lower case due to field name conflict: ${lowerCaseFieldName}`
Object.entries(headers).forEach(([fieldName, fieldValue]) => {
const key = fieldName.toLowerCase()
if (lowerCaseHeaders[key] !== undefined) {
throw Error(
`Failed to convert header keys to lower case due to field name conflict: ${key}`
)
}
lowerCaseHeaders[lowerCaseFieldName] = fieldVal
lowerCaseHeaders[key] = fieldValue
})

return lowerCaseHeaders
Expand Down
21 changes: 15 additions & 6 deletions lib/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ function startScope(basePath, options) {
return new Scope(basePath, options)
}

/**
* @param {string|RegExp} basePath
* @param {Object} options
* @param {boolean} options.allowUnmocked
* @param {string[]} options.badheaders
* @param {function} options.conditionally
* @param {boolean} options.encodedQueryParams
* @param {function} options.filteringScope
* @param {Object} options.reqheaders
* @constructor
*/
function Scope(basePath, options) {
EventEmitter.apply(this)
this.keyedInterceptors = {}
Expand Down Expand Up @@ -361,12 +372,10 @@ function define(nockDefs) {
.reply(status, response, rawHeaders)
} else {
nock = startScope(nscope, options)
// If request headers were specified filter by them.
if (_.size(reqheaders) > 0) {
for (const k in reqheaders) {
nock.matchHeader(k, reqheaders[k])
}
}
// If request headers were specified filter by them.
Object.entries(reqheaders).forEach(([fieldName, value]) => {
nock.matchHeader(fieldName, value)
})
const acceptableFilters = ['filteringRequestBody', 'filteringPath']
acceptableFilters.forEach(filter => {
if (nockDef[filter]) {
Expand Down
11 changes: 11 additions & 0 deletions tests/test_header_matching.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,17 @@ test('matches request header with regular expression', t => {
)
})

test('reqheaders throw if they are not an object', async t => {
const options = {
reqheaders: 'Content-Type: text/plain',
}

t.throws(
() => nock('http://example.test', options).get('/'),
Error('Headers must be provided as an object')
)
})

test('request header satisfies the header function', t => {
nock('http://example.test', {
reqheaders: {
Expand Down

0 comments on commit bafe6e7

Please sign in to comment.