From 4a773f5f4879df13a4285127c27935ea9107c5d5 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Fri, 21 Apr 2017 20:34:54 -0700 Subject: [PATCH] inspector: document bad usage for --inspect-port Document --inspect-port, and fix the reporting for when it is misused. The option requires an argument, but when the argument was omitted, the error message incorrectly reported --inspect-port as being bad, as if was not supported at all: % node --inspect-port node: bad option: --inspect-port % node --none-such node: bad option: --none-such It is now correctly reported as requiring an argument: % ./node --inspect-port ./node: --inspect-port requires an argument --- doc/api/cli.md | 12 ++++++++++++ doc/node.1 | 4 ++++ src/node.cc | 4 +++- src/node_debug_options.cc | 21 ++++++++++++++------- src/node_debug_options.h | 2 +- test/parallel/test-cli-bad-options.js | 23 +++++++++++++++++++++++ 6 files changed, 57 insertions(+), 9 deletions(-) create mode 100644 test/parallel/test-cli-bad-options.js diff --git a/doc/api/cli.md b/doc/api/cli.md index d53052dbdc8e4f..856542e0598251 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -112,6 +112,18 @@ added: v7.6.0 --> Activate inspector on host:port and break at start of user script. +Default host:port is 127.0.0.1:9229. + + +### `--inspect-port=[host:]port` + + +Set the host:port to be used when the inspector is activated. +Useful when activating the inspector by sending the `SIGUSR1` signal. + +Default host is 127.0.0.1. ### `--no-deprecation` diff --git a/doc/node.1 b/doc/node.1 index 46771b2faa3992..8e20dbeb64753d 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -103,6 +103,10 @@ instances for debugging and profiling. It uses the Chrome Debugging Protocol. .BR \-\-inspect-brk \fI[=[host:]port]\fR Activate inspector on host:port and break at start of user script. +.TP +.BR \-\-inspect-port \fI=[host:]port\fR +Set the host:port to be used when the inspector is activated. + .TP .BR \-\-no\-deprecation Silence deprecation warnings. diff --git a/src/node.cc b/src/node.cc index fb98fcebc2a4a5..1c9251982eb4d1 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3604,6 +3604,8 @@ static void PrintHelp() { " --inspect-brk[=[host:]port]\n" " activate inspector on host:port\n" " and break at start of user script\n" + " --inspect-port=[host:]port\n" + " set host:port for inspector\n" #endif " --no-deprecation silence deprecation warnings\n" " --trace-deprecation show stack traces on deprecations\n" @@ -3790,7 +3792,7 @@ static void ParseArgs(int* argc, CheckIfAllowedInEnv(argv[0], is_env, arg); - if (debug_options.ParseOption(arg)) { + if (debug_options.ParseOption(argv[0], arg)) { // Done, consumed by DebugOptions::ParseOption(). } else if (strcmp(arg, "--version") == 0 || strcmp(arg, "-v") == 0) { printf("%s\n", NODE_VERSION); diff --git a/src/node_debug_options.cc b/src/node_debug_options.cc index 5033beb8d79ce1..2e8f601401cba0 100644 --- a/src/node_debug_options.cc +++ b/src/node_debug_options.cc @@ -62,7 +62,7 @@ DebugOptions::DebugOptions() : wait_connect_(false), http_enabled_(false), host_name_("127.0.0.1"), port_(-1) { } -bool DebugOptions::ParseOption(const std::string& option) { +bool DebugOptions::ParseOption(const char* argv0, const std::string& option) { bool enable_inspector = false; bool has_argument = false; std::string option_name; @@ -72,9 +72,13 @@ bool DebugOptions::ParseOption(const std::string& option) { if (pos == std::string::npos) { option_name = option; } else { - has_argument = true; option_name = option.substr(0, pos); argument = option.substr(pos + 1); + + if (argument.length() > 0) + has_argument = true; + else + argument.clear(); } if (option_name == "--inspect") { @@ -82,11 +86,14 @@ bool DebugOptions::ParseOption(const std::string& option) { } else if (option_name == "--inspect-brk") { enable_inspector = true; wait_connect_ = true; - } else if ((option_name != "--debug-port" && - option_name != "--inspect-port") || - !has_argument) { - // only other valid possibility is --inspect-port, - // which requires an argument + } else if (option_name == "--debug-port" || + option_name == "--inspect-port") { + if (!has_argument) { + fprintf(stderr, "%s: %s requires an argument\n", + argv0, option.c_str()); + exit(9); + } + } else { return false; } diff --git a/src/node_debug_options.h b/src/node_debug_options.h index ae54bf9d23d93a..fb86060f438dfc 100644 --- a/src/node_debug_options.h +++ b/src/node_debug_options.h @@ -9,7 +9,7 @@ namespace node { class DebugOptions { public: DebugOptions(); - bool ParseOption(const std::string& option); + bool ParseOption(const char* argv0, const std::string& option); bool inspector_enabled() const { #if HAVE_INSPECTOR return inspector_enabled_; diff --git a/test/parallel/test-cli-bad-options.js b/test/parallel/test-cli-bad-options.js new file mode 100644 index 00000000000000..8283ab078fe7f6 --- /dev/null +++ b/test/parallel/test-cli-bad-options.js @@ -0,0 +1,23 @@ +'use strict'; +require('../common'); + +// Tests that node exits consistently on bad option syntax. + +const assert = require('assert'); +const spawn = require('child_process').spawnSync; + +requiresArgument('--inspect-port'); +requiresArgument('--inspect-port='); +requiresArgument('--debug-port'); +requiresArgument('--debug-port='); +requiresArgument('--eval'); + +function requiresArgument(option) { + const r = spawn(process.execPath, [option], {encoding: 'utf8'}); + + assert.strictEqual(r.status, 9); + + const msg = r.stderr.split(/\r?\n/)[0]; + assert.strictEqual(msg, process.execPath + ': ' + option + + ' requires an argument'); +}