incorporate review changes

nodejs · Ethan-Arrowood · Jun 10, 2021 · Jun 10, 2021 · Jun 15, 2021 · Jun 21, 2021
commit 3b902a1db23e41953b75ea7e4d64cf0719096de2
diff --git a/lib/internal/fetch/headers.js b/lib/internal/fetch/headers.js
@@ -13,6 +13,7 @@ const {
   ArrayIsArray,
   ArrayPrototypeSlice,
   ArrayPrototypeSplice,
+  FunctionPrototypeCall,
   MathFloor,
   ObjectDefineProperties,
   ObjectEntries,
@@ -25,13 +26,15 @@ const {
 
 const { validateObject } = require('internal/validators');
 const { isBoxedPrimitive } = require('internal/util/types');
+const {
+  customInspectSymbol,
+  kEnumerableProperty,
+} = require('internal/util')
 
 const { validateHeaderName, validateHeaderValue } = require('_http_outgoing');
 
 const { Buffer } = require('buffer');
 
-const { inspect } = require('internal/util/inspect');
-
 const kHeadersList = Symbol('headers list');
 
 /**
@@ -132,12 +135,6 @@ function fill(headers, object) {
 
 class Headers {
   constructor(init = {}) {
-    // Fail silently on primitives
-    const typeofInit = typeof init;
-    if (typeofInit === 'number' ||
-     typeofInit === 'string' ||
-     typeofInit === 'boolean' ||
-     typeofInit === 'function') return;
     validateObject(init, 'init', { allowArray: true });
     this[kHeadersList] = [];
     fill(this, init);
@@ -256,7 +253,8 @@ class Headers {
     }
 
     for (let index = 0; index < this[kHeadersList].length; index += 2) {
-      callback.call(
+      FunctionPrototypeCall(
+        callback,
         thisArg,
         this[kHeadersList][index + 1],
         this[kHeadersList][index],
@@ -265,23 +263,23 @@ class Headers {
     }
   }
 
-  [inspect.custom] () {
+  [customInspectSymbol] () {
     return this[kHeadersList]
   }
 }
 
 Headers.prototype[SymbolIterator] = Headers.prototype.entries;
 
 ObjectDefineProperties(Headers.prototype, {
-  append: { enumerable: true },
-  delete: { enumerable: true },
-  get: { enumerable: true },
-  has: { enumerable: true },
-  set: { enumerable: true },
-  keys: { enumerable: true },
-  values: { enumerable: true },
-  entries: { enumerable: true },
-  forEach: { enumerable: true },
+  append: kEnumerableProperty,
+  delete: kEnumerableProperty,
+  get: kEnumerableProperty,
+  has: kEnumerableProperty,
+  set: kEnumerableProperty,
+  keys: kEnumerableProperty,
+  values: kEnumerableProperty,
+  entries: kEnumerableProperty,
+  forEach: kEnumerableProperty,
 });
 
 module.exports = {

diff --git a/lib/internal/util.js b/lib/internal/util.js
@@ -449,6 +449,9 @@ const lazyDOMException = hideStackFrames((message, name) => {
   return new DOMException(message, name);
 });
 
+const kEnumerableProperty = ObjectCreate(null);
+kEnumerableProperty.enumerable = true;
+
 module.exports = {
   assertCrypto,
   cachedResult,
@@ -483,5 +486,6 @@ module.exports = {
   // Used by the buffer module to capture an internal reference to the
   // default isEncoding implementation, just in case userland overrides it.
   kIsEncodingSymbol: Symbol('kIsEncodingSymbol'),
-  kVmBreakFirstLineSymbol: Symbol('kVmBreakFirstLineSymbol')
+  kVmBreakFirstLineSymbol: Symbol('kVmBreakFirstLineSymbol'),
+  kEnumerableProperty,
 };
diff --git a/lib/internal/webstreams/compression.js b/lib/internal/webstreams/compression.js
@@ -16,13 +16,11 @@ const {
   newReadableWritablePairFromDuplex,
 } = require('internal/webstreams/adapters');
 
-const {
-  customInspect,
-  kEnumerableProperty,
-} = require('internal/webstreams/util');
+const { customInspect } = require('internal/webstreams/util');
 
 const {
   customInspectSymbol: kInspect,
+  kEnumerableProperty,
 } = require('internal/util');
 
 let zlib;

diff --git a/lib/internal/webstreams/encoding.js b/lib/internal/webstreams/encoding.js
@@ -14,10 +14,9 @@ const {
   TransformStream,
 } = require('internal/webstreams/transformstream');
 
-const {
-  customInspect,
-  kEnumerableProperty,
-} = require('internal/webstreams/util');
+const { customInspect } = require('internal/webstreams/util');
+
+const { kEnumerableProperty } = require('internal/util');
 
 const {
   codes: {

diff --git a/lib/internal/webstreams/queuingstrategies.js b/lib/internal/webstreams/queuingstrategies.js
@@ -14,14 +14,14 @@ const {
 
 const {
   customInspectSymbol: kInspect,
+  kEnumerableProperty,
 } = require('internal/util');
 
 const {
   customInspect,
   isBrandCheck,
   kType,
   kState,
-  kEnumerableProperty,
 } = require('internal/webstreams/util');
 
 const {

diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js
@@ -50,6 +50,7 @@ const {
 const {
   createDeferredPromise,
   customInspectSymbol: kInspect,
+  kEnumerableProperty,
 } = require('internal/util');
 
 const {
@@ -103,7 +104,6 @@ const {
   nonOpStart,
   kType,
   kState,
-  kEnumerableProperty,
 } = require('internal/webstreams/util');
 
 const {

diff --git a/lib/internal/webstreams/transformstream.js b/lib/internal/webstreams/transformstream.js
@@ -27,6 +27,7 @@ const {
 const {
   createDeferredPromise,
   customInspectSymbol: kInspect,
+  kEnumerableProperty,
 } = require('internal/util');
 
 const {
@@ -45,7 +46,6 @@ const {
   nonOpFlush,
   kType,
   kState,
-  kEnumerableProperty,
 } = require('internal/webstreams/util');
 
 const {

diff --git a/lib/internal/webstreams/util.js b/lib/internal/webstreams/util.js
@@ -207,9 +207,6 @@ function lazyTransfer() {
   return transfer;
 }
 
-const kEnumerableProperty = ObjectCreate(null);
-kEnumerableProperty.enumerable = true;
-
 module.exports = {
   ArrayBufferViewGetBuffer,
   ArrayBufferViewGetByteLength,
@@ -237,5 +234,4 @@ module.exports = {
   nonOpWrite,
   kType,
   kState,
-  kEnumerableProperty,
 };
diff --git a/lib/internal/webstreams/writablestream.js b/lib/internal/webstreams/writablestream.js
@@ -33,6 +33,7 @@ const {
 const {
   createDeferredPromise,
   customInspectSymbol: kInspect,
+  kEnumerableProperty,
 } = require('internal/util');
 
 const {
@@ -64,7 +65,6 @@ const {
   nonOpWrite,
   kType,
   kState,
-  kEnumerableProperty,
 } = require('internal/webstreams/util');
 
 const {

diff --git a/test/parallel/test-headers.js b/test/parallel/test-headers.js
@@ -6,9 +6,9 @@ const assert = require('assert');
 // Flags: --expose-internals
 
 const {
-  Headers,
+  binarySearch,
   kHeadersList,
-  binarySearch
+  Headers,
 } = require('internal/fetch/headers');
 
 {
@@ -82,26 +82,6 @@ const {
   assert.strictEqual(headers[kHeadersList].length, 4);
 }
 
-{
-  // Init fails silently when initialized with BoxedPrimitives
-  [
-    new Number(),
-    new Boolean(),
-    new String(),
-  ].forEach((arg) => new Headers(arg));
-}
-
-{
-  // Init fails silently if function or primitive is passed
-  [
-    new Function(),
-    function() {},
-    1,
-    'test',
-    true,
-  ].forEach((arg) => new Headers(arg));
-}
-
 {
   // append
   const headers = new Headers();