From 702331be906fe58e0ef66c7b31c7d2aeb3af3421 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Mon, 27 May 2019 16:42:03 -0400 Subject: [PATCH] lib: no need to strip BOM or shebang for scripts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/27375 Reviewed-By: Michaël Zasso Reviewed-By: Ujjwal Sharma Reviewed-By: Refael Ackermann Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott --- lib/internal/main/check_syntax.js | 6 +--- lib/internal/modules/cjs/helpers.js | 33 ------------------- lib/internal/modules/cjs/loader.js | 4 --- ...shebang.js => utf8-bom-shebang-shebang.js} | 1 + .../test-internal-modules-strip-shebang.js | 14 -------- test/sequential/test-module-loading.js | 2 +- 6 files changed, 3 insertions(+), 57 deletions(-) rename test/fixtures/{utf8-bom-shebang.js => utf8-bom-shebang-shebang.js} (76%) delete mode 100644 test/parallel/test-internal-modules-strip-shebang.js diff --git a/lib/internal/main/check_syntax.js b/lib/internal/main/check_syntax.js index 1a5287520414cb..92407d067cd675 100644 --- 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 index c3bfc2c35cde19..71abcbf880ebb1 100644 --- 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 index 27f8c8912a9ef1..5cb88da18925cb 100644 --- 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 b/test/fixtures/utf8-bom-shebang-shebang.js similarity index 76% rename from test/fixtures/utf8-bom-shebang.js rename to test/fixtures/utf8-bom-shebang-shebang.js index d0495f04552d41..4cf986dff27156 100644 --- a/test/fixtures/utf8-bom-shebang.js +++ b/test/fixtures/utf8-bom-shebang-shebang.js @@ -1,2 +1,3 @@ #!shebang +#!shebang module.exports = 42; \ No newline at end of file diff --git a/test/parallel/test-internal-modules-strip-shebang.js b/test/parallel/test-internal-modules-strip-shebang.js deleted file mode 100644 index 6c23848a969231..00000000000000 --- a/test/parallel/test-internal-modules-strip-shebang.js +++ /dev/null @@ -1,14 +0,0 @@ -// Flags: --expose-internals -'use strict'; -require('../common'); - -const assert = require('assert'); -const stripShebang = require('internal/modules/cjs/helpers').stripShebang; - -[ - ['', ''], - ['aa', 'aa'], - ['#!', ''], - ['#!bin/bash', ''], - ['#!bin/bash\naa', '\naa'], -].forEach((i) => assert.strictEqual(stripShebang(i[0]), i[1])); diff --git a/test/sequential/test-module-loading.js b/test/sequential/test-module-loading.js index 94acdb191b9044..2e55eb29369fcd 100644 --- 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);