From 4025612e49a6831bbe70cc1eb50dea7a5863afcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Nison?= Date: Sat, 3 Nov 2018 10:52:44 -0700 Subject: [PATCH 1/5] src: allows escaping NODE_OPTIONS with backslashes The characters specified within NODE_OPTIONS can now be escaped, which is handy especially in conjunction with `--require` (where the file path might happen to contain spaces that shouldn't cause the option to be split into two). Fixes: https://github.com/nodejs/node/issues/12971 --- doc/api/cli.md | 4 +++ src/node.cc | 37 +++++++++++++++++++++++--- test/fixtures/print A.js | 1 + test/parallel/test-cli-node-options.js | 4 +++ 4 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/print A.js diff --git a/doc/api/cli.md b/doc/api/cli.md index 79f2dcd2ba4596..d9b5127fd082f7 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -774,6 +774,10 @@ if they had been specified on the command line before the actual command line (so they can be overridden). Node.js will exit with an error if an option that is not allowed in the environment is used, such as `-p` or a script file. +In case an option happens to contain a space or a backslash (for example within +the path passed to `--require`), they must be escaped using an additional +backslash. + Node.js options that are allowed are: - `--diagnostic-report-directory` - `--diagnostic-report-filename` diff --git a/src/node.cc b/src/node.cc index 53a9ae397bf20b..6b6ae04b68a250 100644 --- a/src/node.cc +++ b/src/node.cc @@ -671,11 +671,40 @@ int InitializeNodeWithArgs(std::vector* argv, #if !defined(NODE_WITHOUT_NODE_OPTIONS) std::string node_options; + if (credentials::SafeGetenv("NODE_OPTIONS", &node_options)) { - // [0] is expected to be the program name, fill it in from the real argv - // and use 'x' as a placeholder while parsing. - std::vector env_argv = SplitString("x " + node_options, ' '); - env_argv[0] = argv->at(0); + std::vector env_argv; + // [0] is expected to be the program name, fill it in from the real argv. + env_argv.push_back(argv->at(0)); + + bool will_start_new_arg = true; + for (std::string::size_type index = 0; + index < node_options.size(); + ++index) { + char c = node_options.at(index); + + // Backslashes escape the following character + if (c == '\\') { + if (index + 1 == node_options.size()) { + fprintf(stderr, + "%s: invalid escaping for NODE_OPTIONS\n", + argv->at(0).c_str()); + exit(9); + } else { + c = node_options.at(++index); + } + } else if (c == ' ') { + will_start_new_arg = true; + continue; + } + + if (will_start_new_arg) { + env_argv.push_back(std::string(1, c)); + will_start_new_arg = false; + } else { + env_argv.back() += c; + } + } const int exit_code = ProcessGlobalArgs(&env_argv, nullptr, errors, true); if (exit_code != 0) return exit_code; diff --git a/test/fixtures/print A.js b/test/fixtures/print A.js new file mode 100644 index 00000000000000..52f6164a0f8f45 --- /dev/null +++ b/test/fixtures/print A.js @@ -0,0 +1 @@ +console.log('A'); diff --git a/test/parallel/test-cli-node-options.js b/test/parallel/test-cli-node-options.js index d3d1938bf8a096..1281ad3f42147c 100644 --- a/test/parallel/test-cli-node-options.js +++ b/test/parallel/test-cli-node-options.js @@ -12,7 +12,11 @@ const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); const printA = require.resolve('../fixtures/printA.js'); +const printSpaceA = require.resolve('../fixtures/print A.js'); + expect(`-r ${printA}`, 'A\nB\n'); +expect(`-r ${printA.replace(/[a-z]/gi, '\\$&')}`, 'A\nB\n'); +expect(`-r ${printSpaceA.replace(/[\\ ]/g, '\\$&')}`, 'A\nB\n'); expect(`-r ${printA} -r ${printA}`, 'A\nB\n'); expect(` -r ${printA} -r ${printA}`, 'A\nB\n'); expect(` --require ${printA} --require ${printA}`, 'A\nB\n'); From 1d3d14968e6d7e1b0f829056261012be3bf4d30a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Nison?= Date: Sat, 9 Mar 2019 16:31:12 +0000 Subject: [PATCH 2/5] Switches to a basic json-like parsing --- src/node.cc | 8 ++++++-- test/fixtures/print A.js | 2 +- test/parallel/test-cli-node-options.js | 5 +++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/node.cc b/src/node.cc index 6b6ae04b68a250..067993631549d7 100644 --- a/src/node.cc +++ b/src/node.cc @@ -677,6 +677,7 @@ int InitializeNodeWithArgs(std::vector* argv, // [0] is expected to be the program name, fill it in from the real argv. env_argv.push_back(argv->at(0)); + bool is_in_string = false; bool will_start_new_arg = true; for (std::string::size_type index = 0; index < node_options.size(); @@ -684,7 +685,7 @@ int InitializeNodeWithArgs(std::vector* argv, char c = node_options.at(index); // Backslashes escape the following character - if (c == '\\') { + if (c == '\\' && is_in_string) { if (index + 1 == node_options.size()) { fprintf(stderr, "%s: invalid escaping for NODE_OPTIONS\n", @@ -693,9 +694,12 @@ int InitializeNodeWithArgs(std::vector* argv, } else { c = node_options.at(++index); } - } else if (c == ' ') { + } else if (c == ' ' && !is_in_string) { will_start_new_arg = true; continue; + } else if (c == '"') { + is_in_string = !is_in_string; + continue; } if (will_start_new_arg) { diff --git a/test/fixtures/print A.js b/test/fixtures/print A.js index 52f6164a0f8f45..30a56425de829c 100644 --- a/test/fixtures/print A.js +++ b/test/fixtures/print A.js @@ -1 +1 @@ -console.log('A'); +console.log('A') diff --git a/test/parallel/test-cli-node-options.js b/test/parallel/test-cli-node-options.js index 1281ad3f42147c..07b69c5b130ea5 100644 --- a/test/parallel/test-cli-node-options.js +++ b/test/parallel/test-cli-node-options.js @@ -14,9 +14,10 @@ tmpdir.refresh(); const printA = require.resolve('../fixtures/printA.js'); const printSpaceA = require.resolve('../fixtures/print A.js'); +expect(` -r ${printA} `, 'A\nB\n'); expect(`-r ${printA}`, 'A\nB\n'); -expect(`-r ${printA.replace(/[a-z]/gi, '\\$&')}`, 'A\nB\n'); -expect(`-r ${printSpaceA.replace(/[\\ ]/g, '\\$&')}`, 'A\nB\n'); +expect(`-r ${JSON.stringify(printA)}`, 'A\nB\n'); +expect(`-r ${JSON.stringify(printSpaceA)}`, 'A\nB\n'); expect(`-r ${printA} -r ${printA}`, 'A\nB\n'); expect(` -r ${printA} -r ${printA}`, 'A\nB\n'); expect(` --require ${printA} --require ${printA}`, 'A\nB\n'); From b659b4624b0f51474a0658ca6927ff56bbe22578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Nison?= Date: Wed, 13 Mar 2019 15:52:43 +0000 Subject: [PATCH 3/5] Fixes error return --- src/node.cc | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/node.cc b/src/node.cc index 067993631549d7..c3b0a50e380ca5 100644 --- a/src/node.cc +++ b/src/node.cc @@ -687,10 +687,9 @@ int InitializeNodeWithArgs(std::vector* argv, // Backslashes escape the following character if (c == '\\' && is_in_string) { if (index + 1 == node_options.size()) { - fprintf(stderr, - "%s: invalid escaping for NODE_OPTIONS\n", - argv->at(0).c_str()); - exit(9); + errors->push_back("invalid value for NODE_OPTIONS " + "(invalid escape)\n"); + return 9; } else { c = node_options.at(++index); } @@ -710,6 +709,12 @@ int InitializeNodeWithArgs(std::vector* argv, } } + if (is_in_string) { + errors->push_back("invalid value for NODE_OPTIONS " + "(unterminated string)\n"); + return 9; + } + const int exit_code = ProcessGlobalArgs(&env_argv, nullptr, errors, true); if (exit_code != 0) return exit_code; } From 51481a313f50a8434429221efe104fa70668390f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Nison?= Date: Wed, 3 Apr 2019 19:24:49 +0100 Subject: [PATCH 4/5] Updates the wording --- doc/api/cli.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index d9b5127fd082f7..1dbffadcf6d461 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -774,9 +774,11 @@ if they had been specified on the command line before the actual command line (so they can be overridden). Node.js will exit with an error if an option that is not allowed in the environment is used, such as `-p` or a script file. -In case an option happens to contain a space or a backslash (for example within -the path passed to `--require`), they must be escaped using an additional -backslash. +In case an option value happens to contain a space (for example a path listed in +`--require`), it must be escaped using double quotes. For example: +``` +--require "./my path/file.js" +``` Node.js options that are allowed are: - `--diagnostic-report-directory` From 25163254666efa33d098ab99fa3c9cd9068a6af7 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 16 Apr 2019 23:40:00 +0200 Subject: [PATCH 5/5] fixup --- doc/api/cli.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 1dbffadcf6d461..a2c92b13341df0 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -776,7 +776,8 @@ that is not allowed in the environment is used, such as `-p` or a script file. In case an option value happens to contain a space (for example a path listed in `--require`), it must be escaped using double quotes. For example: -``` + +```bash --require "./my path/file.js" ```