bootstrap: make Buffer and process non-enumerable

This makes sure these two properties are non-enumerable. This aligns them with all other globals that are not enumerable by spec. Refs: #20565 PR-URL: #24874 Refs: #20565 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Anto Aravinth <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
nodejs · Dec 13, 2018 · c992639 · c992639
1 parent a3801e9
commit c992639
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 4 deletions.
diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -408,7 +408,12 @@ function setupGlobalVariables() {
     enumerable: false,
     configurable: true
   });
-  global.process = process;
+  Object.defineProperty(global, 'process', {
+    value: process,
+    enumerable: false,
+    writable: true,
+    configurable: true
+  });
   const util = NativeModule.require('util');
 
   function makeGetter(name) {
@@ -445,7 +450,12 @@ function setupGlobalVariables() {
   // and exposes it on `internal/buffer`.
   NativeModule.require('internal/buffer');
 
-  global.Buffer = NativeModule.require('buffer').Buffer;
+  Object.defineProperty(global, 'Buffer', {
+    value: NativeModule.require('buffer').Buffer,
+    enumerable: false,
+    writable: true,
+    configurable: true
+  });
   process.domain = null;
   process._exiting = false;
 }

diff --git a/test/common/index.js b/test/common/index.js
@@ -217,12 +217,10 @@ function platformTimeout(ms) {
 }
 
 let knownGlobals = [
-  Buffer,
   clearImmediate,
   clearInterval,
   clearTimeout,
   global,
-  process,
   setImmediate,
   setInterval,
   setTimeout

diff --git a/test/parallel/test-global.js b/test/parallel/test-global.js
@@ -27,6 +27,41 @@ const common = require('../common');
 const fixtures = require('../common/fixtures');
 
 const assert = require('assert');
+const { builtinModules } = require('module');
+
+// Load all modules to actually cover most code parts.
+builtinModules.forEach((moduleName) => {
+  if (!moduleName.includes('/')) {
+    try {
+      // This could throw for e.g., crypto if the binary is not compiled
+      // accordingly.
+      require(moduleName);
+    } catch {}
+  }
+});
+
+{
+  const expected = [
+    'global',
+    'clearImmediate',
+    'clearInterval',
+    'clearTimeout',
+    'setImmediate',
+    'setInterval',
+    'setTimeout'
+  ];
+  if (global.DTRACE_HTTP_SERVER_RESPONSE) {
+    expected.unshift(
+      'DTRACE_HTTP_SERVER_RESPONSE',
+      'DTRACE_HTTP_SERVER_REQUEST',
+      'DTRACE_HTTP_CLIENT_RESPONSE',
+      'DTRACE_HTTP_CLIENT_REQUEST',
+      'DTRACE_NET_STREAM_END',
+      'DTRACE_NET_SERVER_CONNECTION'
+    );
+  }
+  assert.deepStrictEqual(new Set(Object.keys(global)), new Set(expected));
+}
 
 common.allowGlobals('bar', 'foo');