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

src: gracefully handle errors in GetX509NameObject #41490

Conversation

tniessen
Copy link
Member

When V8 reports an exception, fail gracefully instead of terminating the process in GetX509NameObject.
A huge thanks to @addaleax for pointing this out.

While this function was added as part of a security release (a336444), this patch does not affect security. Exceptions can realistically only occur due to custom setters on the Array prototype.

Co-authored-by: Anna Henningsen <anna@addaleax.net>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Jan 12, 2022
@tniessen tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 12, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 12, 2022
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

@VoltrexKeyva VoltrexKeyva added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels Jan 14, 2022
tniessen added a commit that referenced this pull request Jan 14, 2022
Co-authored-by: Anna Henningsen <anna@addaleax.net>

PR-URL: #41490
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@tniessen
Copy link
Member Author

tniessen commented Jan 14, 2022

Landed in 4dd1f42, thanks for reviewing.

@tniessen tniessen closed this Jan 14, 2022
tniessen added a commit that referenced this pull request Jan 14, 2022
PR-URL: #41490
Co-authored-by: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Jan 14, 2022
PR-URL: #41490
Co-authored-by: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@tniessen tniessen deleted the src-gracefully-handle-errors-in-getx509nameobject branch January 15, 2022 16:10
mawaregetsuka pushed a commit to mawaregetsuka/node that referenced this pull request Jan 17, 2022
PR-URL: nodejs#41490
Co-authored-by: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
thedull pushed a commit to thedull/node that referenced this pull request Jan 18, 2022
PR-URL: nodejs#41490
Co-authored-by: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Linkgoron pushed a commit to Linkgoron/node that referenced this pull request Jan 31, 2022
PR-URL: nodejs#41490
Co-authored-by: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
PR-URL: #41490
Co-authored-by: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@danielleadams danielleadams mentioned this pull request Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants