From 3209a8ebf3aa2587c68c4b72e6c3b93176090807 Mon Sep 17 00:00:00 2001
From: Teddy Katz <teddy.katz@gmail.com>
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 <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
---
 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 aea83fc331..b4ed16573e 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 d7781eddff..07dd3c4712 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);
+});