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.

Sign up for GitHub

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

Jump to bottom

child_process: set shell to false in fork() #15352

Merged
merged 2 commits into from
Sep 14, 2017
Merged

child_process: set shell to false in fork() #15352

merged 2 commits into from
Sep 14, 2017

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Sep 12, 2017

In this PR I picked up the lib/ and doc/ changes from the abandoned #15299. I also added a test for the changes.

Refs: #15299
Fixes: #13983

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)

child_process, test

@nodejs-github-bot nodejs-github-bot added the child_process Issues and PRs related to the child_process subsystem. label Sep 12, 2017
@cjihrig cjihrig mentioned this pull request Sep 12, 2017
Closed
@cjihrig
Copy link
Contributor Author

cjihrig commented Sep 12, 2017

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

Alex Gresnel and others added 2 commits September 13, 2017 21:03
@cjihrig
child_process: set shell to false in fork()
ed2f347 
This commit ensures that spawn()'s shell option is unconditionally
set to false when fork() is called.

Refs: nodejs#15299
Fixes: nodejs#13983
PR-URL: nodejs#15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@cjihrig
test: add test for fork() + shell
688765a 
This commit verifies that the child_process fork() method does
not honor the shell option.

Refs: nodejs#15299
PR-URL: nodejs#15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@cjihrig cjihrig merged commit 688765a into nodejs:master Sep 14, 2017
@cjihrig cjihrig deleted the fork branch September 14, 2017 01:07
addaleax pushed a commit to addaleax/ayo that referenced this pull request Sep 17, 2017
@addaleax
child_process: set shell to false in fork()
9bd1044 
This commit ensures that spawn()'s shell option is unconditionally
set to false when fork() is called.

Refs: nodejs/node#15299
Fixes: nodejs/node#13983
PR-URL: nodejs/node#15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Sep 17, 2017
@cjihrig @addaleax
test: add test for fork() + shell
3a618bf 
This commit verifies that the child_process fork() method does
not honor the shell option.

Refs: nodejs/node#15299
PR-URL: nodejs/node#15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
jasnell pushed a commit that referenced this pull request Sep 20, 2017
@cjihrig @jasnell
test: add test for fork() + shell
427bf80 
This commit verifies that the child_process fork() method does
not honor the shell option.

Refs: #15299
PR-URL: #15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
jasnell pushed a commit that referenced this pull request Sep 20, 2017
@jasnell
child_process: set shell to false in fork()
607832d 
This commit ensures that spawn()'s shell option is unconditionally
set to false when fork() is called.

Refs: #15299
Fixes: #13983
PR-URL: #15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@jasnell jasnell mentioned this pull request Sep 20, 2017
Merged
Qard pushed a commit to Qard/ayo that referenced this pull request Sep 21, 2017
child_process: set shell to false in fork()
a6720b8 
This commit ensures that spawn()'s shell option is unconditionally
set to false when fork() is called.

Refs: nodejs/node#15299
Fixes: nodejs/node#13983
PR-URL: nodejs/node#15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Qard pushed a commit to Qard/ayo that referenced this pull request Sep 21, 2017
@cjihrig
test: add test for fork() + shell
7a18111 
This commit verifies that the child_process fork() method does
not honor the shell option.

Refs: nodejs/node#15299
PR-URL: nodejs/node#15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@MylesBorins
Copy link
Contributor

@cjihrig does this make sense for v6.x?

@cjihrig
Copy link
Contributor Author

cjihrig commented Oct 17, 2017

Yes, I think so.

MylesBorins pushed a commit that referenced this pull request Nov 14, 2017
@MylesBorins
child_process: set shell to false in fork()
3e6a7e1 
This commit ensures that spawn()'s shell option is unconditionally
set to false when fork() is called.

Refs: #15299
Fixes: #13983
PR-URL: #15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 14, 2017
@cjihrig @MylesBorins
test: add test for fork() + shell
c69e5b5 
This commit verifies that the child_process fork() method does
not honor the shell option.

Refs: #15299
PR-URL: #15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
@MylesBorins
child_process: set shell to false in fork()
15ced00 
This commit ensures that spawn()'s shell option is unconditionally
set to false when fork() is called.

Refs: #15299
Fixes: #13983
PR-URL: #15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
@cjihrig @MylesBorins
test: add test for fork() + shell
6b05937 
This commit verifies that the child_process fork() method does
not honor the shell option.

Refs: #15299
PR-URL: #15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
@MylesBorins
child_process: set shell to false in fork()
8813867 
This commit ensures that spawn()'s shell option is unconditionally
set to false when fork() is called.

Refs: #15299
Fixes: #13983
PR-URL: #15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
@cjihrig @MylesBorins
test: add test for fork() + shell
2c6aa17 
This commit verifies that the child_process fork() method does
not honor the shell option.

Refs: #15299
PR-URL: #15352
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
child_process Issues and PRs related to the child_process subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@cjihrig @MylesBorins @jasnell @lpinca @richardlau @nodejs-github-bot