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

[8.x] src: fix util abort #19224

Closed
wants to merge 1 commit into from
Closed

[8.x] src: fix util abort #19224

wants to merge 1 commit into from

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Mar 8, 2018

This makes sure util.isRegExp and util.isDate will not abort in case
more than one argument is passed to the function.

This got fixed in #18415 in 10.x. So this is a direct fix for 8.x instead of a backport. I also opened #19223 for 9.x.

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

@BridgeAR
src: fix util abort
a7e499c 
This makes sure util.isRegExp and util.isDate will not abort in case
more than one argument is passed to the function.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. util Issues and PRs related to the built-in util module. v8.x labels Mar 8, 2018
@BridgeAR
Copy link
Member Author

BridgeAR commented Mar 8, 2018

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

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 8, 2018
@BridgeAR BridgeAR mentioned this pull request Mar 20, 2018
Closed
4 tasks
@MylesBorins
Copy link
Contributor

This has not yet landed on v9.x, so please hold off until the next release

@MylesBorins MylesBorins force-pushed the v8.x-staging branch 2 times, most recently from 44cb0d3 to 16bf5fe Compare March 30, 2018 03:28
@BridgeAR BridgeAR removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 9, 2018
@gibfahn
Copy link
Member

gibfahn commented Apr 12, 2018

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

@gibfahn
Copy link
Member

gibfahn commented Apr 12, 2018 •
edited
Loading

This got fixed in #18415 in 10.x. So this is a direct fix for 8.x instead of a backport. I also opened #19223 for 9.x.

I'm not following, isn't this the same change as 8181c60, and thus a straight backport?

EDIT: nvm, worked it out.

@gibfahn gibfahn requested a review from addaleax April 13, 2018 01:37
gibfahn pushed a commit that referenced this pull request Apr 13, 2018
@BridgeAR @gibfahn
src: fix util abort
9ac91b1 
This makes sure util.isRegExp and util.isDate will not abort in case
more than one argument is passed to the function.

PR-URL: #19224
Reviewed-By: Gibson Fahnestock <[email protected]>
@gibfahn
Copy link
Member

gibfahn commented Apr 13, 2018

I'm pretty sure this is fine, and thus have landed it, but @addaleax if you could take a quick look just to double-check that would be great for my peace of mind!

Landed in 9ac91b1

@gibfahn gibfahn closed this Apr 13, 2018
@addaleax
Copy link
Member

@gibfahn Yes, this patch LGTM :)

@MylesBorins MylesBorins mentioned this pull request May 2, 2018
Merged
@BridgeAR BridgeAR deleted the v8-fix-util-abort branch April 1, 2019 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gibfahn gibfahn gibfahn approved these changes

@addaleax addaleax Awaiting requested review from addaleax

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. util Issues and PRs related to the built-in util module.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@BridgeAR @MylesBorins @gibfahn @addaleax @nodejs-github-bot