feat: Throw error if request headers are not an object (#1574)

mastermatt · web-flow · commit 8ba0fc71cbc9 · 2019-06-08T09:27:55.000-06:00
Throw if headersFieldNamesToLowerCase is not provided an object. 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.
diff --git a/lib/common.js b/lib/common.js
@@ -201,24 +201,25 @@ function isJSONContent(headers) {
   return contentType.startsWith('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
diff --git a/lib/scope.js b/lib/scope.js
@@ -26,6 +26,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 = {}
diff --git a/tests/test_header_matching.js b/tests/test_header_matching.js
@@ -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: {
