From 3209a8ebf3aa2587c68c4b72e6c3b93176090807 Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Sun, 5 Mar 2017 03:05:40 -0500 Subject: [PATCH] lib: ensure --check flag works for piped-in code Previously, the --check CLI flag had no effect when run on code piped from stdin. This commit updates the bootstrap logic to handle the --check flag the same way regardless of whether the code is piped from stdin. PR-URL: https://github.com/nodejs/node/pull/11689 Fixes: https://github.com/nodejs/node/issues/11680 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Richard Lau --- lib/internal/bootstrap_node.js | 30 +++++++++++++++-------- test/parallel/test-cli-syntax.js | 41 +++++++++++++++++++++++++++++--- 2 files changed, 58 insertions(+), 13 deletions(-) diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index aea83fc331e771..b4ed16573e1e07 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -132,18 +132,11 @@ // check if user passed `-c` or `--check` arguments to Node. if (process._syntax_check_only != null) { - const vm = NativeModule.require('vm'); const fs = NativeModule.require('fs'); - const internalModule = NativeModule.require('internal/module'); // read the source const filename = Module._resolveFilename(process.argv[1]); var source = fs.readFileSync(filename, 'utf-8'); - // remove shebang and BOM - source = internalModule.stripBOM(source.replace(/^#!.*/, '')); - // wrap it - source = Module.wrap(source); - // compile the script, this will throw if it fails - new vm.Script(source, {filename: filename, displayErrors: true}); + checkScriptSyntax(source, filename); process.exit(0); } @@ -184,8 +177,12 @@ }); process.stdin.on('end', function() { - process._eval = code; - evalScript('[stdin]'); + if (process._syntax_check_only != null) { + checkScriptSyntax(code, '[stdin]'); + } else { + process._eval = code; + evalScript('[stdin]'); + } }); } } @@ -445,6 +442,19 @@ } } + function checkScriptSyntax(source, filename) { + const Module = NativeModule.require('module'); + const vm = NativeModule.require('vm'); + const internalModule = NativeModule.require('internal/module'); + + // remove shebang and BOM + source = internalModule.stripBOM(source.replace(/^#!.*/, '')); + // wrap it + source = Module.wrap(source); + // compile the script, this will throw if it fails + new vm.Script(source, {displayErrors: true, filename}); + } + // Below you find a minimal module system, which is used to load the node // core modules found in lib/*.js. All core modules are compiled into the // node binary, so they can be loaded faster. diff --git a/test/parallel/test-cli-syntax.js b/test/parallel/test-cli-syntax.js index d7781eddffbb42..07dd3c4712e751 100644 --- a/test/parallel/test-cli-syntax.js +++ b/test/parallel/test-cli-syntax.js @@ -31,7 +31,7 @@ const syntaxArgs = [ // no output should be produced assert.strictEqual(c.stdout, '', 'stdout produced'); assert.strictEqual(c.stderr, '', 'stderr produced'); - assert.strictEqual(c.status, 0, 'code == ' + c.status); + assert.strictEqual(c.status, 0, 'code === ' + c.status); }); }); @@ -52,11 +52,14 @@ const syntaxArgs = [ // no stdout should be produced assert.strictEqual(c.stdout, '', 'stdout produced'); + // stderr should include the filename + assert(c.stderr.startsWith(file), "stderr doesn't start with the filename"); + // stderr should have a syntax error message const match = c.stderr.match(/^SyntaxError: Unexpected identifier$/m); assert(match, 'stderr incorrect'); - assert.strictEqual(c.status, 1, 'code == ' + c.status); + assert.strictEqual(c.status, 1, 'code === ' + c.status); }); }); @@ -79,6 +82,38 @@ const syntaxArgs = [ const match = c.stderr.match(/^Error: Cannot find module/m); assert(match, 'stderr incorrect'); - assert.strictEqual(c.status, 1, 'code == ' + c.status); + assert.strictEqual(c.status, 1, 'code === ' + c.status); }); }); + +// should not execute code piped from stdin with --check +// loop each possible option, `-c` or `--check` +syntaxArgs.forEach(function(args) { + const stdin = 'throw new Error("should not get run");'; + const c = spawnSync(node, args, {encoding: 'utf8', input: stdin}); + + // no stdout or stderr should be produced + assert.strictEqual(c.stdout, '', 'stdout produced'); + assert.strictEqual(c.stderr, '', 'stderr produced'); + + assert.strictEqual(c.status, 0, 'code === ' + c.status); +}); + +// should should throw if code piped from stdin with --check has bad syntax +// loop each possible option, `-c` or `--check` +syntaxArgs.forEach(function(args) { + const stdin = 'var foo bar;'; + const c = spawnSync(node, args, {encoding: 'utf8', input: stdin}); + + // stderr should include '[stdin]' as the filename + assert(c.stderr.startsWith('[stdin]'), "stderr doesn't start with [stdin]"); + + // no stdout or stderr should be produced + assert.strictEqual(c.stdout, '', 'stdout produced'); + + // stderr should have a syntax error message + const match = c.stderr.match(/^SyntaxError: Unexpected identifier$/m); + assert(match, 'stderr incorrect'); + + assert.strictEqual(c.status, 1, 'code === ' + c.status); +});