src: Only allow --debug-brk w/ --inspect

Jan Krems · Jan Krems · commit 50ecfe448af8 · 2017-05-04T14:55:48.000+02:00
diff --git a/src/node.cc b/src/node.cc
@@ -3955,6 +3955,12 @@ static void ParseArgs(int* argc,
     exit(9);
   }
 
+  if (debug_options.debug_brk_used() && !debug_options.inspector_enabled()) {
+    fprintf(stderr,
+            "%s: Using --debug-brk without --inspect is no longer supported\n", argv[0]);
+    exit(9);
+  }
+
   // Copy remaining arguments.
   const unsigned int args_left = nargs - index;
 
diff --git a/src/node_debug_options.cc b/src/node_debug_options.cc
@@ -59,6 +59,7 @@ DebugOptions::DebugOptions() :
 #if HAVE_INSPECTOR
                                inspector_enabled_(false),
 #endif  // HAVE_INSPECTOR
+                               debug_brk_used_(false),
                                wait_connect_(false), http_enabled_(false),
                                host_name_("127.0.0.1"), port_(-1) { }
 
@@ -79,10 +80,12 @@ bool DebugOptions::ParseOption(const std::string& option) {
 
   if (option_name == "--inspect") {
     enable_inspector = true;
-  } else if (option_name == "--inspect-brk" || option_name == "--debug-brk") {
-    debugger_enabled_ = true;
+  } else if (option_name == "--inspect-brk") {
     enable_inspector = true;
     wait_connect_ = true;
+  } else if (option_name == "--debug-brk") {
+    debug_brk_used_ = true;
+    wait_connect_ = true;
   } else if (option_name == "--inspect-port" || option_name == "--debug-port") {
     if (!has_argument) return false;
   } else {
diff --git a/src/node_debug_options.h b/src/node_debug_options.h
@@ -17,6 +17,9 @@ class DebugOptions {
     return false;
 #endif  // HAVE_INSPECTOR
   }
+  bool debug_brk_used() const {
+    return debug_brk_used_;
+  }
   bool ToolsServerEnabled();
   bool wait_for_connect() const { return wait_connect_; }
   std::string host_name() const { return host_name_; }
@@ -26,6 +29,7 @@ class DebugOptions {
  private:
 #if HAVE_INSPECTOR
   bool inspector_enabled_;
+  bool debug_brk_used_;
 #endif  // HAVE_INSPECTOR
   bool wait_connect_;  // --inspect-brk
   bool http_enabled_;
diff --git a/test/inspector/inspector-helper.js b/test/inspector/inspector-helper.js
@@ -442,9 +442,9 @@ Harness.prototype.kill = function() {
 };
 
 exports.startNodeForInspectorTest = function(callback,
-                                             inspectorFlag = '--inspect-brk',
+                                             inspectorFlags = ['--inspect-brk'],
                                              opt_script_contents) {
-  const args = [inspectorFlag];
+  const args = [].concat(inspectorFlags);
   if (opt_script_contents) {
     args.push('-e', opt_script_contents);
   } else {
diff --git a/test/inspector/test-inspector-debug-brk-only.js b/test/inspector/test-inspector-debug-brk-only.js
@@ -0,0 +1,13 @@
+'use strict';
+const assert = require('assert');
+const execFile = require('child_process').execFile;
+const path = require('path');
+
+const common = require('../common');
+
+const mainScript = path.join(common.fixturesDir, 'loop.js');
+
+execFile(process.execPath, [ '--debug-brk', mainScript ], common.mustCall((error, stdout, stderr) => {
+  assert.equal(error.code, 9);
+  assert.notEqual(stderr.indexOf('Using --debug-brk without --inspect is no longer supported', -1));
+}));
diff --git a/test/inspector/test-inspector-debug-brk.js b/test/inspector/test-inspector-debug-brk.js
@@ -0,0 +1,57 @@
+'use strict';
+const common = require('../common');
+common.skipIfInspectorDisabled();
+const assert = require('assert');
+const helper = require('./inspector-helper.js');
+
+function setupExpectBreakOnLine(line, url, session, scopeIdCallback) {
+  return function(message) {
+    if ('Debugger.paused' === message['method']) {
+      const callFrame = message['params']['callFrames'][0];
+      const location = callFrame['location'];
+      assert.strictEqual(url, session.scriptUrlForId(location['scriptId']));
+      assert.strictEqual(line, location['lineNumber']);
+      scopeIdCallback &&
+          scopeIdCallback(callFrame['scopeChain'][0]['object']['objectId']);
+      return true;
+    }
+  };
+}
+
+function testBreakpointOnStart(session) {
+  const commands = [
+    { 'method': 'Runtime.enable' },
+    { 'method': 'Debugger.enable' },
+    { 'method': 'Debugger.setPauseOnExceptions',
+      'params': {'state': 'none'} },
+    { 'method': 'Debugger.setAsyncCallStackDepth',
+      'params': {'maxDepth': 0} },
+    { 'method': 'Profiler.enable' },
+    { 'method': 'Profiler.setSamplingInterval',
+      'params': {'interval': 100} },
+    { 'method': 'Debugger.setBlackboxPatterns',
+      'params': {'patterns': []} },
+    { 'method': 'Runtime.runIfWaitingForDebugger' }
+  ];
+
+  session
+    .sendInspectorCommands(commands)
+    .expectMessages(setupExpectBreakOnLine(0, session.mainScriptPath, session));
+}
+
+function testWaitsForFrontendDisconnect(session, harness) {
+  console.log('[test]', 'Verify node waits for the frontend to disconnect');
+  session.sendInspectorCommands({ 'method': 'Debugger.resume'})
+    .expectStderrOutput('Waiting for the debugger to disconnect...')
+    .disconnect(true);
+}
+
+function runTests(harness) {
+  harness
+    .runFrontendSession([
+      testBreakpointOnStart,
+      testWaitsForFrontendDisconnect
+    ]).expectShutDown(55);
+}
+
+helper.startNodeForInspectorTest(runTests, ['--inspect', '--debug-brk']);
