Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

child_process: don't fork bomb ourselves from -e #3575

Merged
merged 1 commit into from
Oct 29, 2015

Conversation

bnoordhuis
Copy link
Member

Remove the -e argument from process.execArgv in child_process.fork()
to keep node -e 'require("child_process").fork("empty.js")' from
spawning itself recursively.

Fixes: #3574

R=@Trott

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

@bnoordhuis bnoordhuis added the child_process Issues and PRs related to the child_process subsystem. label Oct 28, 2015
@cjihrig
Copy link
Contributor

cjihrig commented Oct 28, 2015

LGTM

@Trott
Copy link
Member

Trott commented Oct 28, 2015

LGTM :shipit:

Remove the `-e` argument from process.execArgv in child_process.fork()
to keep `node -e 'require("child_process").fork("empty.js")'` from
spawning itself recursively.

Fixes: nodejs#3574
PR-URL: nodejs#3575
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@bnoordhuis bnoordhuis closed this Oct 29, 2015
@bnoordhuis bnoordhuis deleted the fix3574 branch October 29, 2015 12:10
@bnoordhuis bnoordhuis merged commit 57bce60 into nodejs:master Oct 29, 2015
@rvagg
Copy link
Member

rvagg commented Oct 30, 2015

suggesting this goes into v4.x-staging after having done some time in v5.0.0, @jasnell what do you think?

@jasnell
Copy link
Member

jasnell commented Oct 30, 2015

Agreed but likely better to land it post v4.2.2

@bnoordhuis
Copy link
Member Author

It's not a regression in any way (it's a buglet that apparently goes all the way back to v0.6) so it would IMO be perfectly acceptable to leave it out of LTS.

@jasnell
Copy link
Member

jasnell commented Oct 30, 2015

@bnoordhuis .. noted! 👍

On Fri, Oct 30, 2015 at 9:55 AM, Ben Noordhuis notifications@github.com
wrote:

It's not a regression in any way (it's a buglet that apparently goes all
the way back to v0.6) so it would IMO be perfectly acceptable to leave it
out of LTS.


Reply to this email directly or view it on GitHub
#3575 (comment).

bnoordhuis added a commit that referenced this pull request Nov 7, 2015
Remove the `-e` argument from process.execArgv in child_process.fork()
to keep `node -e 'require("child_process").fork("empty.js")'` from
spawning itself recursively.

Fixes: #3574
PR-URL: #3575
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This was referenced Nov 10, 2015
@MylesBorins
Copy link
Contributor

@jasnell @bnoordhuis @rvagg has this spent enough time in 5.1.0?

@jasnell
Copy link
Member

jasnell commented Nov 30, 2015

@thealphanerd ... I'd say yes.

bnoordhuis added a commit that referenced this pull request Nov 30, 2015
Remove the `-e` argument from process.execArgv in child_process.fork()
to keep `node -e 'require("child_process").fork("empty.js")'` from
spawning itself recursively.

Fixes: #3574
PR-URL: #3575
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@bnoordhuis
Copy link
Member Author

Yes, I think so.

EDIT: I see you landed it a few minutes ago. :-)

bnoordhuis added a commit that referenced this pull request Dec 4, 2015
Remove the `-e` argument from process.execArgv in child_process.fork()
to keep `node -e 'require("child_process").fork("empty.js")'` from
spawning itself recursively.

Fixes: #3574
PR-URL: #3575
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@jasnell jasnell mentioned this pull request Dec 17, 2015
bnoordhuis added a commit that referenced this pull request Dec 17, 2015
Remove the `-e` argument from process.execArgv in child_process.fork()
to keep `node -e 'require("child_process").fork("empty.js")'` from
spawning itself recursively.

Fixes: #3574
PR-URL: #3575
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
bnoordhuis added a commit that referenced this pull request Dec 23, 2015
Remove the `-e` argument from process.execArgv in child_process.fork()
to keep `node -e 'require("child_process").fork("empty.js")'` from
spawning itself recursively.

Fixes: #3574
PR-URL: #3575
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants