fix(Headers Iterator): make spec compliant (nodejs#1431)

KhafraDev · Jun 23, 2022 · 4445f55 · 4445f55
1 parent e6e8278
commit 4445f55
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 7 deletions.
diff --git a/lib/fetch/headers.js b/lib/fetch/headers.js
@@ -77,19 +77,30 @@ function fill (headers, object) {
   }
 }
 
+// https://tc39.es/ecma262/#sec-%25iteratorprototype%25-object
+const esIteratorPrototype = Object.getPrototypeOf(Object.getPrototypeOf([][Symbol.iterator]()))
+
+// https://webidl.spec.whatwg.org/#dfn-iterator-prototype-object
 function makeHeadersIterator (iterator) {
   const i = {
-    next: () => {
-      // TODO(@KhafraDev): brand check
+    next () {
+      if (Object.getPrototypeOf(this) !== i) {
+        throw new TypeError(
+          '\'next\' called on an object that does not implement interface Headers Iterator.'
+        )
+      }
+
       return iterator.next()
     },
-    [Symbol.iterator]: () => {
-      // TODO(@KhafraDev): brand check
-      return makeHeadersIterator(iterator)
-    },
+    // The class string of an iterator prototype object for a given interface is the
+    // result of concatenating the identifier of the interface and the string " Iterator".
     [Symbol.toStringTag]: 'Headers Iterator'
   }
 
+  // The [[Prototype]] internal slot of an iterator prototype object must be %IteratorPrototype%.
+  Object.setPrototypeOf(i, esIteratorPrototype)
+  // esIteratorPrototype needs to be the prototype of i
+  // which is the prototype of an empty object. Yes, it's confusing.
   return Object.setPrototypeOf({}, i)
 }
 

diff --git a/test/fetch/headers-iterator.js b/test/fetch/headers-iterator.js
@@ -9,15 +9,22 @@ test('Implements "Headers Iterator" properly', (t) => {
 
     for (const iterable of ['keys', 'values', 'entries', Symbol.iterator]) {
       const gen = headers[iterable]()
+      // https://tc39.es/ecma262/#sec-%25iteratorprototype%25-object
+      const IteratorPrototype = Object.getPrototypeOf(Object.getPrototypeOf([][Symbol.iterator]()))
+      const iteratorProto = Object.getPrototypeOf(gen)
 
       t.ok(gen.constructor === Object)
       t.ok(gen.prototype === undefined)
       // eslint-disable-next-line no-proto
       t.equal(gen.__proto__[Symbol.toStringTag], 'Headers Iterator')
       // https://github.com/node-fetch/node-fetch/issues/1119#issuecomment-100222049
-      t.notOk(gen instanceof function * () {}.constructor)
+      t.notOk(Headers.prototype[iterable] instanceof function * () {}.constructor)
       // eslint-disable-next-line no-proto
       t.ok(gen.__proto__.next.__proto__ === Function.prototype)
+      // https://webidl.spec.whatwg.org/#dfn-iterator-prototype-object
+      // "The [[Prototype]] internal slot of an iterator prototype object must be %IteratorPrototype%."
+      t.equal(gen[Symbol.iterator], IteratorPrototype[Symbol.iterator])
+      t.equal(Object.getPrototypeOf(iteratorProto), IteratorPrototype)
     }
 
     t.end()
@@ -42,5 +49,23 @@ test('Implements "Headers Iterator" properly', (t) => {
     t.end()
   })
 
+  t.test('Symbol.iterator', (t) => {
+    const values = new Headers([['a', 'b']]).entries()[Symbol.iterator]()
+
+    t.same(Array.from(values), [['a', 'b']])
+    t.end()
+  })
+
+  t.test('brand check', (t) => {
+    const gen = new Headers().entries()
+
+    t.throws(() => {
+      // eslint-disable-next-line no-proto
+      gen.__proto__.next()
+    }, TypeError)
+
+    t.end()
+  })
+
   t.end()
 })