[v12 backport] lib: no need to strip BOM or shebang for scripts

lib/internal/bootstrap/pre_execution.js

@@ -389,7 +389,7 @@ function initializePolicy() {
 }
 
 function initializeCJSLoader() {
-  require('internal/modules/cjs/loader')._initPaths();
+  require('internal/modules/cjs/loader').Module._initPaths();
 }
 
 function initializeESMLoader() {
@@ -438,7 +438,9 @@ function loadPreloadModules() {
   const preloadModules = getOptionValue('--require');
   if (preloadModules && preloadModules.length > 0) {
     const {
-      _preloadModules
+      Module: {
+        _preloadModules
+      },
     } = require('internal/modules/cjs/loader');
     _preloadModules(preloadModules);
   }

lib/internal/main/check_syntax.js

@@ -13,14 +13,11 @@ const {
 
 const { pathToFileURL } = require('url');
 
-const vm = require('vm');
 const {
-  stripShebang, stripBOM
-} = require('internal/modules/cjs/helpers');
-
-const {
-  _resolveFilename: resolveCJSModuleName,
-  wrap: wrapCJSModule
+  Module: {
+    _resolveFilename: resolveCJSModuleName,
+  },
+  wrapSafe,
 } = require('internal/modules/cjs/loader');
 
 // TODO(joyeecheung): not every one of these are necessary
@@ -49,9 +46,6 @@ if (process.argv[1] && process.argv[1] !== '-') {
 }
 
 function checkSyntax(source, filename) {
-  // Remove Shebang.
-  source = stripShebang(source);
-
   const { getOptionValue } = require('internal/options');
   const experimentalModules = getOptionValue('--experimental-modules');
   if (experimentalModules) {
@@ -70,10 +64,5 @@ function checkSyntax(source, filename) {
     }
   }
 
-  // Remove BOM.
-  source = stripBOM(source);
-  // Wrap it.
-  source = wrapCJSModule(source);
-  // Compile the script, this will throw if it fails.
-  new vm.Script(source, { displayErrors: true, filename });
+  wrapSafe(filename, source);
 }

lib/internal/main/run_main_module.js

@@ -6,7 +6,7 @@ const {
 
 prepareMainThreadExecution(true);
 
-const CJSModule = require('internal/modules/cjs/loader');
+const CJSModule = require('internal/modules/cjs/loader').Module;
 
 markBootstrapComplete();
 

lib/internal/modules/cjs/helpers.js

@@ -111,26 +111,6 @@ function stripBOM(content) {
   return content;
 }
 
-/**
- * Find end of shebang line and slice it off
- */
-function stripShebang(content) {
-  // Remove shebang
-  if (content.charAt(0) === '#' && content.charAt(1) === '!') {
-    // Find end of shebang line and slice it off
-    let index = content.indexOf('\n', 2);
-    if (index === -1)
-      return '';
-    if (content.charAt(index - 1) === '\r')
-      index--;
-    // Note that this actually includes the newline character(s) in the
-    // new output. This duplicates the behavior of the regular expression
-    // that was previously used to replace the shebang line.
-    content = content.slice(index);
-  }
-  return content;
-}
-
 const builtinLibs = [
   'assert', 'async_hooks', 'buffer', 'child_process', 'cluster', 'crypto',
   'dgram', 'dns', 'domain', 'events', 'fs', 'http', 'http2', 'https', 'net',
@@ -197,5 +177,4 @@ module.exports = {
   makeRequireFunction,
   normalizeReferrerURL,
   stripBOM,
-  stripShebang
 };

lib/internal/modules/cjs/loader.js

@@ -51,7 +51,6 @@ const {
   makeRequireFunction,
   normalizeReferrerURL,
   stripBOM,
-  stripShebang,
   loadNativeModule
 } = require('internal/modules/cjs/helpers');
 const { getOptionValue } = require('internal/options');
@@ -73,7 +72,7 @@ const { validateString } = require('internal/validators');
 const pendingDeprecation = getOptionValue('--pending-deprecation');
 const experimentalExports = getOptionValue('--experimental-exports');
 
-module.exports = Module;
+module.exports = { wrapSafe, Module };
 
 let asyncESM, ModuleJob, ModuleWrap, kInstantiated;
 
@@ -857,26 +856,10 @@ Module.prototype.require = function(id) {
 let resolvedArgv;
 let hasPausedEntry = false;
 
-// Run the file contents in the correct scope or sandbox. Expose
-// the correct helper variables (require, module, exports) to
-// the file.
-// Returns exception, if any.
-Module.prototype._compile = function(content, filename) {
-  let moduleURL;
-  let redirects;
-  if (manifest) {
-    moduleURL = pathToFileURL(filename);
-    redirects = manifest.getRedirector(moduleURL);
-    manifest.assertIntegrity(moduleURL, content);
-  }
-
-  content = stripShebang(content);
-  maybeCacheSourceMap(filename, content, this);
-
-  let compiledWrapper;
+function wrapSafe(filename, content) {
   if (patched) {
     const wrapper = Module.wrap(content);
-    compiledWrapper = vm.runInThisContext(wrapper, {
+    return vm.runInThisContext(wrapper, {
       filename,
       lineOffset: 0,
       displayErrors: true,
@@ -885,46 +868,64 @@ Module.prototype._compile = function(content, filename) {
         return loader.import(specifier, normalizeReferrerURL(filename));
       } : undefined,
     });
-  } else {
-    let compiled;
-    try {
-      compiled = compileFunction(
-        content,
-        filename,
-        0,
-        0,
-        undefined,
-        false,
-        undefined,
-        [],
-        [
-          'exports',
-          'require',
-          'module',
-          '__filename',
-          '__dirname',
-        ]
-      );
-    } catch (err) {
-      if (experimentalModules) {
-        enrichCJSError(err);
-      }
-      throw err;
-    }
+  }
 
+  let compiled;
+  try {
+    compiled = compileFunction(
+      content,
+      filename,
+      0,
+      0,
+      undefined,
+      false,
+      undefined,
+      [],
+      [
+        'exports',
+        'require',
+        'module',
+        '__filename',
+        '__dirname',
+      ]
+    );
+  } catch (err) {
     if (experimentalModules) {
-      const { callbackMap } = internalBinding('module_wrap');
-      callbackMap.set(compiled.cacheKey, {
-        importModuleDynamically: async (specifier) => {
-          const loader = await asyncESM.loaderPromise;
-          return loader.import(specifier, normalizeReferrerURL(filename));
-        }
-      });
+      enrichCJSError(err);
     }
-    compiledWrapper = compiled.function;
+    throw err;
+  }
+
+  if (experimentalModules) {
+    const { callbackMap } = internalBinding('module_wrap');
+    callbackMap.set(compiled.cacheKey, {
+      importModuleDynamically: async (specifier) => {
+        const loader = await asyncESM.loaderPromise;
+        return loader.import(specifier, normalizeReferrerURL(filename));
+      }
+    });
   }
 
-  let inspectorWrapper = null;
+  return compiled.function;
+}
+
+// Run the file contents in the correct scope or sandbox. Expose
+// the correct helper variables (require, module, exports) to
+// the file.
+// Returns exception, if any.
+Module.prototype._compile = function(content, filename) {
+  let moduleURL;
+  let redirects;
+  if (manifest) {
+    moduleURL = pathToFileURL(filename);
+    redirects = manifest.getRedirector(moduleURL);
+    manifest.assertIntegrity(moduleURL, content);
+  }
+
+  maybeCacheSourceMap(filename, content, this);
+  const compiledWrapper = wrapSafe(filename, content);
+
+  var inspectorWrapper = null;
   if (getOptionValue('--inspect-brk') && process._eval == null) {
     if (!resolvedArgv) {
       // We enter the repl if we're not given a filename argument.
@@ -988,7 +989,7 @@ Module._extensions['.js'] = function(module, filename) {
     }
   }
   const content = fs.readFileSync(filename, 'utf8');
-  module._compile(stripBOM(content), filename);
+  module._compile(content, filename);
 };
 
 

lib/internal/modules/esm/translators.js

@@ -12,11 +12,10 @@ const {
 const { Buffer } = require('buffer');
 
 const {
-  stripShebang,
   stripBOM,
   loadNativeModule
 } = require('internal/modules/cjs/helpers');
-const CJSModule = require('internal/modules/cjs/loader');
+const CJSModule = require('internal/modules/cjs/loader').Module;
 const internalURLModule = require('internal/url');
 const createDynamicModule = require(
   'internal/modules/esm/create_dynamic_module');
@@ -80,7 +79,7 @@ translators.set('module', async function moduleStrategy(url) {
   const source = `${await getSource(url)}`;
   maybeCacheSourceMap(url, source);
   debug(`Translating StandardModule ${url}`);
-  const module = new ModuleWrap(stripShebang(source), url);
+  const module = new ModuleWrap(source, url);
   moduleWrap.callbackMap.set(module, {
     initializeImportMeta,
     importModuleDynamically,

lib/internal/process/execution.js

@@ -53,7 +53,7 @@ function evalModule(source, print) {
 }
 
 function evalScript(name, body, breakFirstLine, print) {
-  const CJSModule = require('internal/modules/cjs/loader');
+  const CJSModule = require('internal/modules/cjs/loader').Module;
   const { kVmBreakFirstLineSymbol } = require('internal/util');
 
   const cwd = tryGetCwd();

lib/internal/util/inspector.js

@@ -24,7 +24,7 @@ function sendInspectorCommand(cb, onError) {
 function installConsoleExtensions(commandLineApi) {
   if (commandLineApi.require) { return; }
   const { tryGetCwd } = require('internal/process/execution');
-  const CJSModule = require('internal/modules/cjs/loader');
+  const CJSModule = require('internal/modules/cjs/loader').Module;
   const { makeRequireFunction } = require('internal/modules/cjs/helpers');
   const consoleAPIModule = new CJSModule('<inspector console>');
   const cwd = tryGetCwd();

lib/module.js

@@ -1,3 +1,3 @@
 'use strict';
 
-module.exports = require('internal/modules/cjs/loader');
+module.exports = require('internal/modules/cjs/loader').Module;

lib/repl.js

@@ -65,7 +65,7 @@ const path = require('path');
 const fs = require('fs');
 const { Interface } = require('readline');
 const { Console } = require('console');
-const CJSModule = require('internal/modules/cjs/loader');
+const CJSModule = require('internal/modules/cjs/loader').Module;
 const domain = require('domain');
 const debug = require('internal/util/debuglog').debuglog('repl');
 const {

test/fixtures/utf8-bom-shebang-shebang.js

@@ -0,0 +1,3 @@
+#!shebang
+#!shebang
+module.exports = 42;

test/fixtures/utf8-bom-shebang.js

@@ -0,0 +1,2 @@
+#!shebang
+module.exports = 42;

test/fixtures/utf8-shebang-bom.js

@@ -0,0 +1,2 @@
+#!shebang
+module.exports = 42;

test/sequential/test-module-loading.js

@@ -352,6 +352,13 @@ process.on('exit', function() {
 assert.strictEqual(require('../fixtures/utf8-bom.js'), 42);
 assert.strictEqual(require('../fixtures/utf8-bom.json'), 42);
 
+// Loading files with BOM + shebang.
+// See https://github.com/nodejs/node/issues/27767
+assert.throws(() => {
+  require('../fixtures/utf8-bom-shebang-shebang.js');
+}, { name: 'SyntaxError' });
+assert.strictEqual(require('../fixtures/utf8-shebang-bom.js'), 42);
+
 // Error on the first line of a module should
 // have the correct line number
 assert.throws(function() {