lib: no need to strip BOM or shebang for scripts

PR-URL: #27375 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>
nodejs · Jun 1, 2019 · 702331b · 702331b
1 parent abfc8ae
commit 702331b
Show file tree

Hide file tree

Showing 6 changed files with 3 additions and 57 deletions.
diff --git a/lib/internal/main/check_syntax.js b/lib/internal/main/check_syntax.js
@@ -13,10 +13,6 @@ const {
 
 const { pathToFileURL } = require('url');
 
-const {
-  stripShebangOrBOM,
-} = require('internal/modules/cjs/helpers');
-
 const {
   Module: {
     _resolveFilename: resolveCJSModuleName,
@@ -68,5 +64,5 @@ function checkSyntax(source, filename) {
     }
   }
 
-  wrapSafe(filename, stripShebangOrBOM(source));
+  wrapSafe(filename, source);
 }
diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js
@@ -52,37 +52,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;
-}
-
-// Strip either the shebang or UTF BOM of a file.
-// Note that this only processes one. If both occur in
-// either order, the one that comes second is not
-// significant.
-function stripShebangOrBOM(content) {
-  if (content.charCodeAt(0) === 0xFEFF) {
-    return content.slice(1);
-  }
-  return stripShebang(content);
-}
-
 const builtinLibs = [
   'assert', 'async_hooks', 'buffer', 'child_process', 'cluster', 'crypto',
   'dgram', 'dns', 'domain', 'events', 'fs', 'http', 'http2', 'https', 'net',
@@ -148,6 +117,4 @@ module.exports = {
   makeRequireFunction,
   normalizeReferrerURL,
   stripBOM,
-  stripShebang,
-  stripShebangOrBOM,
 };
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -40,7 +40,6 @@ const {
   makeRequireFunction,
   normalizeReferrerURL,
   stripBOM,
-  stripShebangOrBOM,
 } = require('internal/modules/cjs/helpers');
 const { getOptionValue } = require('internal/options');
 const preserveSymlinks = getOptionValue('--preserve-symlinks');
@@ -745,9 +744,6 @@ Module.prototype._compile = function(content, filename) {
     manifest.assertIntegrity(moduleURL, content);
   }
 
-  // Strip after manifest integrity check
-  content = stripShebangOrBOM(content);
-
   const compiledWrapper = wrapSafe(filename, content);
 
   var inspectorWrapper = null;

diff --git a/test/fixtures/utf8-bom-shebang.js → test/fixtures/utf8-bom-shebang-shebang.js b/test/fixtures/utf8-bom-shebang.js → test/fixtures/utf8-bom-shebang-shebang.js
@@ -1,2 +1,3 @@
 #!shebang
+#!shebang
 module.exports = 42;
diff --git a/test/parallel/test-internal-modules-strip-shebang.js b/test/parallel/test-internal-modules-strip-shebang.js
diff --git a/test/sequential/test-module-loading.js b/test/sequential/test-module-loading.js
@@ -355,7 +355,7 @@ 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.js');
+  require('../fixtures/utf8-bom-shebang-shebang.js');
 }, { name: 'SyntaxError' });
 assert.strictEqual(require('../fixtures/utf8-shebang-bom.js'), 42);