Repository: nock/nock
Pull request #2049 (2020-07-18 17:29:09)

fix: data comparison with undefined object values
Updates the `dataEqual` utility and underlying `deepEqual` recursive functions to treat explicit and implicit `undefined` object values as equal.
Also ensures that `expand` is only called on top-level data if they're plain objects, thus avoiding accidentally equating an object and any array if the object's keys happen to be integers.

Fixes: #2048

MichaelBuhler: Commenting here to confirm here that this PR (6d654a2) fixes the production use case which caused me to file #2048.
MichaelBuhler: @mastermatt any update on this?
mastermatt: I think it's good to go, but needs an approval from one of the other @nock/maintainers 

Patch:
diff --git a/lib/common.js b/lib/common.js
index 904ce4c..fa6cf8c 100644
--- a/lib/common.js
+++ b/lib/common.js
@@ -548,8 +548,15 @@ function urlToOptions(url) {
  *  - The expected data can use regexp to compare values
  *  - JSON path notation and nested objects are considered equal
  */
-const dataEqual = (expected, actual) =>
-  deepEqual(expand(expected), expand(actual))
+const dataEqual = (expected, actual) => {
+  if (isPlainObject(expected)) {
+    expected = expand(expected)
+  }
+  if (isPlainObject(actual)) {
+    actual = expand(actual)
+  }
+  return deepEqual(expected, actual)
+}
 
 /**
  * Converts flat objects whose keys use JSON path notation to nested objects.
@@ -573,17 +580,20 @@ function deepEqual(expected, actual) {
     return expected.test(actual)
   }
 
-  if (Array.isArray(expected) || isPlainObject(expected)) {
-    if (actual === undefined) {
+  if (Array.isArray(expected) && Array.isArray(actual)) {
+    if (expected.length !== actual.length) {
       return false
     }
 
-    const expKeys = Object.keys(expected)
-    if (expKeys.length !== Object.keys(actual).length) {
-      return false
-    }
+    return expected.every((expVal, idx) => deepEqual(expVal, actual[idx]))
+  }
+
+  if (isPlainObject(expected) && isPlainObject(actual)) {
+    const allKeys = Array.from(
+      new Set(Object.keys(expected).concat(Object.keys(actual)))
+    )
 
-    return expKeys.every(key => deepEqual(expected[key], actual[key]))
+    return allKeys.every(key => deepEqual(expected[key], actual[key]))
   }
 
   return expected === actual


Test patch:
diff --git a/tests/test_common.js b/tests/test_common.js
index 04cfeab..e768556 100644
--- a/tests/test_common.js
+++ b/tests/test_common.js
@@ -500,6 +500,28 @@ describe('`normalizeClientRequestArgs()`', () => {
   })
 })
 
+describe('`dataEqual()`', () => {
+  it('treats explicit and implicit undefined object values as equal', () => {
+    const result = common.dataEqual({ a: 'a', b: undefined }, { a: 'a' })
+    expect(result).to.equal(true)
+  })
+  it('does not conflate object and array keys', () => {
+    const result = common.dataEqual(['a', 'b'], { '0': 'a', '1': 'b' })
+    expect(result).to.equal(false)
+  })
+  it('treats JSON path notated and nested objects as equal', () => {
+    const result = common.dataEqual(
+      { 'foo[bar][0]': 'baz' },
+      { foo: { bar: ['baz'] } }
+    )
+    expect(result).to.equal(true)
+  })
+  it('does not equate arrays of different length', () => {
+    const result = common.dataEqual(['a'], ['a', 'b'])
+    expect(result).to.equal(false)
+  })
+})
+
 it('testing timers are deleted correctly', done => {
   const timeoutSpy = sinon.spy()
   const intervalSpy = sinon.spy()