Skip to content

Commit 6c34ad6

Browse files
devsnektargos
devsnek
authored and
targos
committed
lib: rework logic of stripping BOM+Shebang from commonjs
Fixes #27767 Backport-PR-URL: #31228 PR-URL: #27768 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent 4e8b8bc commit 6c34ad6

File tree

13 files changed

+100
-85
lines changed

13 files changed

+100
-85
lines changed

‎lib/internal/bootstrap/pre_execution.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ function initializePolicy() {
389389
}
390390

391391
function initializeCJSLoader() {
392-
require('internal/modules/cjs/loader')._initPaths();
392+
require('internal/modules/cjs/loader').Module._initPaths();
393393
}
394394

395395
function initializeESMLoader() {
@@ -438,7 +438,9 @@ function loadPreloadModules() {
438438
const preloadModules = getOptionValue('--require');
439439
if (preloadModules && preloadModules.length > 0) {
440440
const {
441-
_preloadModules
441+
Module: {
442+
_preloadModules
443+
},
442444
} = require('internal/modules/cjs/loader');
443445
_preloadModules(preloadModules);
444446
}

‎lib/internal/main/check_syntax.js‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,15 @@ const {
1313

1414
const { pathToFileURL } = require('url');
1515

16-
const vm = require('vm');
1716
const {
18-
stripShebang, stripBOM
17+
stripShebangOrBOM,
1918
} = require('internal/modules/cjs/helpers');
2019

2120
const {
22-
_resolveFilename: resolveCJSModuleName,
23-
wrap: wrapCJSModule
21+
Module: {
22+
_resolveFilename: resolveCJSModuleName,
23+
},
24+
wrapSafe,
2425
} = require('internal/modules/cjs/loader');
2526

2627
// TODO(joyeecheung): not every one of these are necessary
@@ -49,9 +50,6 @@ if (process.argv[1] && process.argv[1] !== '-') {
4950
}
5051

5152
function checkSyntax(source, filename) {
52-
// Remove Shebang.
53-
source = stripShebang(source);
54-
5553
const { getOptionValue } = require('internal/options');
5654
const experimentalModules = getOptionValue('--experimental-modules');
5755
if (experimentalModules) {
@@ -70,10 +68,5 @@ function checkSyntax(source, filename) {
7068
}
7169
}
7270

73-
// Remove BOM.
74-
source = stripBOM(source);
75-
// Wrap it.
76-
source = wrapCJSModule(source);
77-
// Compile the script, this will throw if it fails.
78-
new vm.Script(source, { displayErrors: true, filename });
71+
wrapSafe(filename, stripShebangOrBOM(source));
7972
}

‎lib/internal/main/run_main_module.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const {
66

77
prepareMainThreadExecution(true);
88

9-
const CJSModule = require('internal/modules/cjs/loader');
9+
const CJSModule = require('internal/modules/cjs/loader').Module;
1010

1111
markBootstrapComplete();
1212

‎lib/internal/modules/cjs/helpers.js‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,17 @@ function stripShebang(content) {
131131
return content;
132132
}
133133

134+
// Strip either the shebang or UTF BOM of a file.
135+
// Note that this only processes one. If both occur in
136+
// either order, the one that comes second is not
137+
// significant.
138+
function stripShebangOrBOM(content) {
139+
if (content.charCodeAt(0) === 0xFEFF) {
140+
return content.slice(1);
141+
}
142+
return stripShebang(content);
143+
}
144+
134145
const builtinLibs = [
135146
'assert', 'async_hooks', 'buffer', 'child_process', 'cluster', 'crypto',
136147
'dgram', 'dns', 'domain', 'events', 'fs', 'http', 'http2', 'https', 'net',
@@ -197,5 +208,6 @@ module.exports = {
197208
makeRequireFunction,
198209
normalizeReferrerURL,
199210
stripBOM,
200-
stripShebang
211+
stripShebang,
212+
stripShebangOrBOM,
201213
};

‎lib/internal/modules/cjs/loader.js‎

Lines changed: 56 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ const {
5151
makeRequireFunction,
5252
normalizeReferrerURL,
5353
stripBOM,
54-
stripShebang,
55-
loadNativeModule
54+
stripShebangOrBOM,
5655
} = require('internal/modules/cjs/helpers');
5756
const { getOptionValue } = require('internal/options');
5857
const enableSourceMaps = getOptionValue('--enable-source-maps');
@@ -73,7 +72,7 @@ const {
7372
const { validateString } = require('internal/validators');
7473
const pendingDeprecation = getOptionValue('--pending-deprecation');
7574

76-
module.exports = Module;
75+
module.exports = { wrapSafe, Module };
7776

7877
let asyncESM, ModuleJob, ModuleWrap, kInstantiated;
7978

@@ -948,26 +947,10 @@ Module.prototype.require = function(id) {
948947
let resolvedArgv;
949948
let hasPausedEntry = false;
950949

951-
// Run the file contents in the correct scope or sandbox. Expose
952-
// the correct helper variables (require, module, exports) to
953-
// the file.
954-
// Returns exception, if any.
955-
Module.prototype._compile = function(content, filename) {
956-
let moduleURL;
957-
let redirects;
958-
if (manifest) {
959-
moduleURL = pathToFileURL(filename);
960-
redirects = manifest.getRedirector(moduleURL);
961-
manifest.assertIntegrity(moduleURL, content);
962-
}
963-
964-
content = stripShebang(content);
965-
maybeCacheSourceMap(filename, content, this);
966-
967-
let compiledWrapper;
950+
function wrapSafe(filename, content) {
968951
if (patched) {
969952
const wrapper = Module.wrap(content);
970-
compiledWrapper = vm.runInThisContext(wrapper, {
953+
return vm.runInThisContext(wrapper, {
971954
filename,
972955
lineOffset: 0,
973956
displayErrors: true,
@@ -976,46 +959,61 @@ Module.prototype._compile = function(content, filename) {
976959
return loader.import(specifier, normalizeReferrerURL(filename));
977960
} : undefined,
978961
});
979-
} else {
980-
let compiled;
981-
try {
982-
compiled = compileFunction(
983-
content,
984-
filename,
985-
0,
986-
0,
987-
undefined,
988-
false,
989-
undefined,
990-
[],
991-
[
992-
'exports',
993-
'require',
994-
'module',
995-
'__filename',
996-
'__dirname',
997-
]
998-
);
999-
} catch (err) {
1000-
if (experimentalModules) {
1001-
enrichCJSError(err);
962+
}
963+
964+
let compiledWrapper;
965+
try {
966+
compiledWrapper = compileFunction(
967+
content,
968+
filename,
969+
0,
970+
0,
971+
undefined,
972+
false,
973+
undefined,
974+
[],
975+
[
976+
'exports',
977+
'require',
978+
'module',
979+
'__filename',
980+
'__dirname',
981+
]
982+
);
983+
} catch (err) {
984+
enrichCJSError(err);
985+
throw err;
986+
}
987+
988+
if (experimentalModules) {
989+
const { callbackMap } = internalBinding('module_wrap');
990+
callbackMap.set(compiledWrapper, {
991+
importModuleDynamically: async (specifier) => {
992+
const loader = await asyncESM.loaderPromise;
993+
return loader.import(specifier, normalizeReferrerURL(filename));
1002994
}
1003-
throw err;
1004-
}
995+
});
996+
}
1005997

1006-
if (experimentalModules) {
1007-
const { callbackMap } = internalBinding('module_wrap');
1008-
callbackMap.set(compiled.cacheKey, {
1009-
importModuleDynamically: async (specifier) => {
1010-
const loader = await asyncESM.loaderPromise;
1011-
return loader.import(specifier, normalizeReferrerURL(filename));
1012-
}
1013-
});
1014-
}
1015-
compiledWrapper = compiled.function;
998+
return compiledWrapper;
999+
}
1000+
1001+
// Run the file contents in the correct scope or sandbox. Expose
1002+
// the correct helper variables (require, module, exports) to
1003+
// the file.
1004+
// Returns exception, if any.
1005+
Module.prototype._compile = function(content, filename) {
1006+
if (manifest) {
1007+
const moduleURL = pathToFileURL(filename);
1008+
manifest.assertIntegrity(moduleURL, content);
10161009
}
10171010

1018-
let inspectorWrapper = null;
1011+
// Strip after manifest integrity check
1012+
content = stripShebangOrBOM(content);
1013+
1014+
const compiledWrapper = wrapSafe(filename, content);
1015+
1016+
var inspectorWrapper = null;
10191017
if (getOptionValue('--inspect-brk') && process._eval == null) {
10201018
if (!resolvedArgv) {
10211019
// We enter the repl if we're not given a filename argument.
@@ -1079,7 +1077,7 @@ Module._extensions['.js'] = function(module, filename) {
10791077
}
10801078
}
10811079
const content = fs.readFileSync(filename, 'utf8');
1082-
module._compile(stripBOM(content), filename);
1080+
module._compile(content, filename);
10831081
};
10841082

10851083

‎lib/internal/modules/esm/translators.js‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,9 @@ const {
1212
const { Buffer } = require('buffer');
1313

1414
const {
15-
stripShebang,
16-
stripBOM,
17-
loadNativeModule
15+
stripBOM
1816
} = require('internal/modules/cjs/helpers');
19-
const CJSModule = require('internal/modules/cjs/loader');
17+
const CJSModule = require('internal/modules/cjs/loader').Module;
2018
const internalURLModule = require('internal/url');
2119
const createDynamicModule = require(
2220
'internal/modules/esm/create_dynamic_module');
@@ -80,8 +78,9 @@ translators.set('module', async function moduleStrategy(url) {
8078
const source = `${await getSource(url)}`;
8179
maybeCacheSourceMap(url, source);
8280
debug(`Translating StandardModule ${url}`);
83-
const module = new ModuleWrap(stripShebang(source), url);
84-
moduleWrap.callbackMap.set(module, {
81+
const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
82+
const module = new ModuleWrap(source, url);
83+
callbackMap.set(module, {
8584
initializeImportMeta,
8685
importModuleDynamically,
8786
});

‎lib/internal/process/execution.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ function evalModule(source, print) {
5353
}
5454

5555
function evalScript(name, body, breakFirstLine, print) {
56-
const CJSModule = require('internal/modules/cjs/loader');
56+
const CJSModule = require('internal/modules/cjs/loader').Module;
5757
const { kVmBreakFirstLineSymbol } = require('internal/util');
5858

5959
const cwd = tryGetCwd();

‎lib/internal/util/inspector.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ function sendInspectorCommand(cb, onError) {
2424
function installConsoleExtensions(commandLineApi) {
2525
if (commandLineApi.require) { return; }
2626
const { tryGetCwd } = require('internal/process/execution');
27-
const CJSModule = require('internal/modules/cjs/loader');
27+
const CJSModule = require('internal/modules/cjs/loader').Module;
2828
const { makeRequireFunction } = require('internal/modules/cjs/helpers');
2929
const consoleAPIModule = new CJSModule('<inspector console>');
3030
const cwd = tryGetCwd();

‎lib/module.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
'use strict';
22

3-
module.exports = require('internal/modules/cjs/loader');
3+
module.exports = require('internal/modules/cjs/loader').Module;

‎lib/repl.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ const path = require('path');
6565
const fs = require('fs');
6666
const { Interface } = require('readline');
6767
const { Console } = require('console');
68-
const CJSModule = require('internal/modules/cjs/loader');
68+
const CJSModule = require('internal/modules/cjs/loader').Module;
6969
const domain = require('domain');
7070
const debug = require('internal/util/debuglog').debuglog('repl');
7171
const {

0 commit comments

Comments
 (0)