(v6.x backport) src: allow CLI args in env with NODE_OPTIONS #12677

sam-github · 2017-04-26T19:01:27Z

Not all CLI options are supported, those that are problematic from a
security or implementation point of view are disallowed, as are ones
that are inappropriate (for example, -e, -p, --i), or that only make
sense when changed with code changes (such as options that change the
javascript syntax or add new APIs).

PR-URL: #12028
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Michael Dawson michael_dawson@ca.ibm.com
Reviewed-By: Refael Ackermann refack@gmail.com
Reviewed-By: Gibson Fahnestock gibfahn@gmail.com
Reviewed-By: Bradley Farias bradley.meck@gmail.com

EDIT: also pushed #13002 and #13172 onto this PR, so it combines all the NODE_OPTIONS PRs. Note that they are all released as of 8.0.0.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

gibfahn · 2017-05-10T14:14:04Z

@sam-github needs a rebase (but this probably needs to bake in v7.x first, so maybe leave it)

sam-github · 2017-05-10T15:37:05Z

rebased without conflicts

MylesBorins · 2017-05-16T01:21:41Z

@sam-github SafeGetenv() is now in v6.x-staging

feel free to rework this PR

sam-github · 2017-05-19T17:04:46Z

ci: https://ci.nodejs.org/job/node-test-pull-request/8182/

sam-github · 2017-05-26T17:11:27Z

@nodejs/lts, ci was all green, can I land this?

gibfahn · 2017-05-26T17:31:13Z

cc/ @MylesBorins I can't remember whether we said it was better to wait till the release goes out and v6.x-staging is rebased.

MylesBorins · 2017-05-30T13:01:13Z

@gibfahn with this being a minor it likely shouldn't land until our next minor release

sam-github · 2017-10-11T18:01:47Z

@MylesBorins done

mhdawson

LGTM

The --redirect-warnings command line argument allows process warnings to be written to a specified file rather than printed to stderr. Also adds an equivalent NODE_REDIRECT_WARNINGS environment variable. If the specified file cannot be opened or written to for any reason, the argument is ignored and the warning is printed to stderr. If the file already exists, it will be appended to. Backport-PR-URL: #12677 PR-URL: #10116 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Michal Zasso <targos@protonmail.com> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>

Mutations of the environment can invalidate pointers to environment variables, so make `secure_getenv()` copy them out instead of returning pointers. This is the part of #11051 that applies to be11fb4. This part wasn't backported to 6.x when #11051 was backported because the semver-minor introduction of NODE_REDIRECT_WARNINGS hadn't been backported yet. Now that the env var is backported, this last bit of #11051 is needed. PR-URL: #12677 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

A dynamically allocated array was being used, simplify the memory management by using std::vector. Backport-PR-URL: #12677 PR-URL: #12241 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Not all CLI options are supported, those that are problematic from a security or implementation point of view are disallowed, as are ones that are inappropriate (for example, -e, -p, --i), or that only make sense when changed with code changes (such as options that change the javascript syntax or add new APIs). Backport-PR-URL: #12677 PR-URL: #12028 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com>

Add --debug-*, --napi-modules Remove --prof-process, like -p and -e, it causes node to do something other than run node js scripts. Backport-PR-URL: #12677 PR-URL: #13002 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Backport-PR-URL: #12677 PR-URL: #13172 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

When test-cli-node-options is run it uses the --trace-events-enabled option which generates a file named node_trace.1.log. This commit changes the working directory to the test tmp directory to avoid this file being created in the project root. Backport-PR-URL: #12677 PR-URL: #12660 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

Currently when configure --without-ssl the test will throw the following error: bad option: --use-openssl-ca This commit checks if crypto was enabled and skips the crypto related tests if that is the case. Backport-PR-URL: #12677 PR-URL: #12692 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>/

Allow --abort-on-uncaught-exception in NODE_OPTIONS, its useful to enable for post-mortem debugging. Backport-PR-URL: #12677 PR-URL: #13932 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins · 2017-10-16T20:59:15Z

landed in f35071b...a76c2ae

sam-github · 2017-10-16T22:44:42Z

Lovely, thanks @MylesBorins