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

src: CHECK() for argument overflow in Spawn() #16761

Merged
merged 1 commit into from
Nov 7, 2017
Merged

src: CHECK() for argument overflow in Spawn() #16761

merged 1 commit into from
Nov 7, 2017

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Nov 4, 2017 •
edited
Loading

This commit adds checks for overflow of args and env in Spawn(). It seems extremely unlikely that either of these values would overflow from a valid use case.

Fixes: #15622

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. child_process Issues and PRs related to the child_process subsystem. labels Nov 4, 2017
@gireeshpunathil
Copy link
Member

If a program requires 2^31 number of arguments for its function, either it should be uimaginably complex, or buggy. In either case this check is great to have!

@cjihrig
Copy link
Contributor Author

cjihrig commented Nov 7, 2017

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

@cjihrig
src: CHECK() for argument overflow in Spawn()
de1754a 
This commit adds checks for overflow to args and env in Spawn().
It seems extremely unlikely that either of these values would
overflow from a valid use case.

Fixes: nodejs#15622
PR-URL: nodejs#16761
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@cjihrig cjihrig merged commit de1754a into nodejs:master Nov 7, 2017
@cjihrig cjihrig deleted the check-args branch November 7, 2017 20:22
evanlucas pushed a commit that referenced this pull request Nov 13, 2017
@cjihrig @evanlucas
src: CHECK() for argument overflow in Spawn()
bce5db2 
This commit adds checks for overflow to args and env in Spawn().
It seems extremely unlikely that either of these values would
overflow from a valid use case.

Fixes: #15622
PR-URL: #16761
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@evanlucas evanlucas mentioned this pull request Nov 13, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 17, 2017
@cjihrig @MylesBorins
src: CHECK() for argument overflow in Spawn()
7b25eba 
This commit adds checks for overflow to args and env in Spawn().
It seems extremely unlikely that either of these values would
overflow from a valid use case.

Fixes: #15622
PR-URL: #16761
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
MylesBorins pushed a commit that referenced this pull request Nov 17, 2017
@cjihrig @MylesBorins
src: CHECK() for argument overflow in Spawn()
8d31294 
This commit adds checks for overflow to args and env in Spawn().
It seems extremely unlikely that either of these values would
overflow from a valid use case.

Fixes: #15622
PR-URL: #16761
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@gibfahn gibfahn mentioned this pull request Nov 21, 2017
Merged
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
@cjihrig @MylesBorins
src: CHECK() for argument overflow in Spawn()
c49961a 
This commit adds checks for overflow to args and env in Spawn().
It seems extremely unlikely that either of these values would
overflow from a valid use case.

Fixes: #15622
PR-URL: #16761
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
@cjihrig @MylesBorins
src: CHECK() for argument overflow in Spawn()
c79dd9e 
This commit adds checks for overflow to args and env in Spawn().
It seems extremely unlikely that either of these values would
overflow from a valid use case.

Fixes: #15622
PR-URL: #16761
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. 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.
3 participants
@cjihrig @gireeshpunathil @nodejs-github-bot