diff --git a/src/node.cc b/src/node.cc index cd9de349bc70e9..953ac31eef5e6e 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2460,7 +2460,7 @@ void ProcessArgv(std::vector* args, bool is_env) { // Parse a few arguments which are specific to Node. std::vector v8_args; - std::string error; + std::vector errors{}; { // TODO(addaleax): The mutex here should ideally be held during the @@ -2472,11 +2472,13 @@ void ProcessArgv(std::vector* args, &v8_args, per_process_opts.get(), is_env ? kAllowedInEnvironment : kDisallowedInEnvironment, - &error); + &errors); } - if (!error.empty()) { - fprintf(stderr, "%s: %s\n", args->at(0).c_str(), error.c_str()); + if (!errors.empty()) { + for (auto const& error : errors) { + fprintf(stderr, "%s: %s\n", args->at(0).c_str(), error.c_str()); + } exit(9); } @@ -2493,30 +2495,7 @@ void ProcessArgv(std::vector* args, for (const std::string& cve : per_process_opts->security_reverts) Revert(cve.c_str()); - // TODO(addaleax): Move this validation to the option parsers. auto env_opts = per_process_opts->per_isolate->per_env; - if (!env_opts->userland_loader.empty() && - !env_opts->experimental_modules) { - fprintf(stderr, "%s: --loader requires --experimental-modules be enabled\n", - args->at(0).c_str()); - exit(9); - } - - if (env_opts->syntax_check_only && env_opts->has_eval_string) { - fprintf(stderr, "%s: either --check or --eval can be used, not both\n", - args->at(0).c_str()); - exit(9); - } - -#if HAVE_OPENSSL - if (per_process_opts->use_openssl_ca && per_process_opts->use_bundled_ca) { - fprintf(stderr, "%s: either --use-openssl-ca or --use-bundled-ca can be " - "used, not both\n", - args->at(0).c_str()); - exit(9); - } -#endif - if (std::find(v8_args.begin(), v8_args.end(), "--abort-on-uncaught-exception") != v8_args.end() || std::find(v8_args.begin(), v8_args.end(), diff --git a/src/node_options-inl.h b/src/node_options-inl.h index ba18e7c19b03c7..1d1eda54751a8f 100644 --- a/src/node_options-inl.h +++ b/src/node_options-inl.h @@ -270,7 +270,7 @@ void OptionsParser::Parse( std::vector* const v8_args, Options* const options, OptionEnvvarSettings required_env_settings, - std::string* const error) { + std::vector* const errors) { ArgsInfo args(orig_args, exec_args); // The first entry is the process name. Make sure it ends up in the V8 argv, @@ -279,7 +279,7 @@ void OptionsParser::Parse( if (v8_args->empty()) v8_args->push_back(args.program_name()); - while (!args.empty() && error->empty()) { + while (!args.empty() && errors->empty()) { if (args.first().size() <= 1 || args.first()[0] != '-') break; // We know that we're either going to consume this @@ -288,7 +288,7 @@ void OptionsParser::Parse( if (arg == "--") { if (required_env_settings == kAllowedInEnvironment) - *error = NotAllowedInEnvErr("--"); + errors->push_back(NotAllowedInEnvErr("--")); break; } @@ -357,7 +357,7 @@ void OptionsParser::Parse( if ((it == options_.end() || it->second.env_setting == kDisallowedInEnvironment) && required_env_settings == kAllowedInEnvironment) { - *error = NotAllowedInEnvErr(original_name); + errors->push_back(NotAllowedInEnvErr(original_name)); break; } @@ -379,7 +379,7 @@ void OptionsParser::Parse( value = arg.substr(equals_index + 1); if (value.empty()) { missing_argument: - *error = RequiresArgumentErr(original_name); + errors->push_back(RequiresArgumentErr(original_name)); break; } } else { @@ -413,7 +413,7 @@ void OptionsParser::Parse( break; case kHostPort: Lookup(info.field, options) - ->Update(SplitHostPort(value, error)); + ->Update(SplitHostPort(value, errors)); break; case kNoOp: break; @@ -424,6 +424,7 @@ void OptionsParser::Parse( UNREACHABLE(); } } + options->CheckOptions(errors); } } // namespace options_parser diff --git a/src/node_options.cc b/src/node_options.cc index 1a614d0e771a8a..156ea11fe91ef0 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -16,6 +16,32 @@ using v8::Undefined; using v8::Value; namespace node { + +void PerProcessOptions::CheckOptions(std::vector* errors) { +#if HAVE_OPENSSL + if (use_openssl_ca && use_bundled_ca) { + errors->push_back("either --use-openssl-ca or --use-bundled-ca can be " + "used, not both"); + } +#endif + per_isolate->CheckOptions(errors); +} + +void PerIsolateOptions::CheckOptions(std::vector* errors) { + per_env->CheckOptions(errors); +} + +void EnvironmentOptions::CheckOptions(std::vector* errors) { + if (!userland_loader.empty() && !experimental_modules) { + errors->push_back("--loader requires --experimental-modules be enabled"); + } + + if (syntax_check_only && has_eval_string) { + errors->push_back("either --check or --eval can be used, not both"); + } + debug_options->CheckOptions(errors); +} + namespace options_parser { // XXX: If you add an option here, please also add it to doc/node.1 and @@ -305,18 +331,20 @@ inline std::string RemoveBrackets(const std::string& host) { return host; } -inline int ParseAndValidatePort(const std::string& port, std::string* error) { +inline int ParseAndValidatePort(const std::string& port, + std::vector* errors) { char* endptr; errno = 0; const long result = strtol(port.c_str(), &endptr, 10); // NOLINT(runtime/int) if (errno != 0 || *endptr != '\0'|| (result != 0 && result < 1024) || result > 65535) { - *error = "Port must be 0 or in range 1024 to 65535."; + errors->push_back(" must be 0 or in range 1024 to 65535."); } return static_cast(result); } -HostPort SplitHostPort(const std::string& arg, std::string* error) { +HostPort SplitHostPort(const std::string& arg, + std::vector* errors) { // remove_brackets only works if no port is specified // so if it has an effect only an IPv6 address was specified. std::string host = RemoveBrackets(arg); @@ -332,11 +360,11 @@ HostPort SplitHostPort(const std::string& arg, std::string* error) { return HostPort { arg, -1 }; } } - return HostPort { "", ParseAndValidatePort(arg, error) }; + return HostPort { "", ParseAndValidatePort(arg, errors) }; } // Host and port found: return HostPort { RemoveBrackets(arg.substr(0, colon)), - ParseAndValidatePort(arg.substr(colon + 1), error) }; + ParseAndValidatePort(arg.substr(colon + 1), errors) }; } // Usage: Either: diff --git a/src/node_options.h b/src/node_options.h index 4b9ee000367405..506db25dec2efa 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -21,11 +21,16 @@ struct HostPort { } }; +class Options { + public: + virtual void CheckOptions(std::vector* errors) {} +}; + // These options are currently essentially per-Environment, but it can be nice // to keep them separate since they are a group of options applying to a very // specific part of Node. It might also make more sense for them to be // per-Isolate, rather than per-Environment. -class DebugOptions { +class DebugOptions : public Options { public: bool inspector_enabled = false; bool deprecated_debug = false; @@ -57,7 +62,7 @@ class DebugOptions { } }; -class EnvironmentOptions { +class EnvironmentOptions : public Options { public: std::shared_ptr debug_options { new DebugOptions() }; bool abort_on_uncaught_exception = false; @@ -91,17 +96,19 @@ class EnvironmentOptions { std::vector user_argv; inline DebugOptions* get_debug_options(); + void CheckOptions(std::vector* errors); }; -class PerIsolateOptions { +class PerIsolateOptions : public Options { public: std::shared_ptr per_env { new EnvironmentOptions() }; bool track_heap_objects = false; inline EnvironmentOptions* get_per_env_options(); + void CheckOptions(std::vector* errors); }; -class PerProcessOptions { +class PerProcessOptions : public Options { public: std::shared_ptr per_isolate { new PerIsolateOptions() }; @@ -139,13 +146,15 @@ class PerProcessOptions { #endif inline PerIsolateOptions* get_per_isolate_options(); + void CheckOptions(std::vector* errors); }; // The actual options parser, as opposed to the structs containing them: namespace options_parser { -HostPort SplitHostPort(const std::string& arg, std::string* error); +HostPort SplitHostPort(const std::string& arg, + std::vector* errors); void GetOptions(const v8::FunctionCallbackInfo& args); enum OptionEnvvarSettings { @@ -255,7 +264,7 @@ class OptionsParser { std::vector* const v8_args, Options* const options, OptionEnvvarSettings required_env_settings, - std::string* const error); + std::vector* const errors); private: // We support the wide variety of different option types by remembering