Skip to content

crypto: simplify assertions in Safe*Print - #49709

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
davidben:ext-method
Apr 20, 2024
Merged

crypto: simplify assertions in Safe*Print#49709
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
davidben:ext-method

Conversation

@davidben

@davidbendavidben commented Sep 18, 2023

Copy link
Copy Markdown
Contributor

Checking the X509V3_EXT_METHOD doesn't do anything. X509V3_EXT_get is already implemented by calling X509V3_EXT_get_nid on the extension's NID. We may as well just check the NID directly.

@nodejs-github-bot

nodejs-github-bot commented Sep 18, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-botnodejs-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 Sep 18, 2023
himself65
himself65 reviewed Sep 20, 2023
View reviewed changes

@himself65himself65 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use CHECK_EQ instead?

@davidben

davidben commented Sep 20, 2023

Copy link
Copy Markdown
ContributorAuthor

use CHECK_EQ instead?

Done

@tniessentniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 20, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 20, 2023
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
This was referenced Sep 21, 2023
Open
Open
Open
Open
@tniessentniessen added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Apr 2, 2024
@github-actionsgithub-actionsBot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Apr 2, 2024
@github-actions

This comment was marked as off-topic.

Sign in to view
@panvapanva added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Apr 3, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 3, 2024
@nodejs-github-bot

nodejs-github-bot commented Apr 3, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Apr 3, 2024

Copy link
Copy Markdown
Collaborator

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

@davidben

davidben commented Apr 19, 2024

Copy link
Copy Markdown
ContributorAuthor

What's the status of this? Anything else needed before it can be merged?

@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 19, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 19, 2024
@nodejs-github-bot

nodejs-github-bot commented Apr 19, 2024

Copy link
Copy Markdown
Collaborator

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

@richardlau

richardlau commented Apr 19, 2024

Copy link
Copy Markdown
Member

The https://ci.nodejs.org/job/node-test-commit-freebsd/54284/ failure is because this PR is old enough that the branch gets detected as being for Node.js 21 instead of 22 (we stopped running the freebsd CI for Node.js 22). A rebase would fix that.

@davidben
crypto: simplify assertions in Safe*Print
48ba7b2
Checking the X509V3_EXT_METHOD doesn't do anything. X509V3_EXT_get is
already implemented by calling X509V3_EXT_get_nid on the extension's
NID. We may as well just check the NID directly.
@davidben

davidben commented Apr 19, 2024

Copy link
Copy Markdown
ContributorAuthor

Rebased.

@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 19, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 19, 2024
@nodejs-github-bot

nodejs-github-bot commented Apr 19, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Apr 20, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Apr 20, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Apr 20, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Apr 20, 2024

Copy link
Copy Markdown
Collaborator

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

@richardlaurichardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 20, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 20, 2024
@nodejs-github-bot
nodejs-github-bot merged commit 9790ddf into nodejs:mainApr 20, 2024
@nodejs-github-bot

nodejs-github-bot commented Apr 20, 2024

Copy link
Copy Markdown
Collaborator

Landed in 9790ddf

@davidben
davidben deleted the ext-method branch April 20, 2024 13:56
aduh95 pushed a commit that referenced this pull request Apr 29, 2024
@davidben@aduh95
crypto: simplify assertions in Safe*Print
910533f
Checking the X509V3_EXT_METHOD doesn't do anything. X509V3_EXT_get is
already implemented by calling X509V3_EXT_get_nid on the extension's
NID. We may as well just check the NID directly.
PR-URL: #49709
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <rlau@redhat.com>
@aduh95aduh95 mentioned this pull request Apr 30, 2024
Merged
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
@davidben@marco-ippolito
crypto: simplify assertions in Safe*Print
2ec949a
Checking the X509V3_EXT_METHOD doesn't do anything. X509V3_EXT_get is
already implemented by calling X509V3_EXT_get_nid on the extension's
NID. We may as well just check the NID directly.
PR-URL: #49709
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <rlau@redhat.com>
@marco-ippolitomarco-ippolito mentioned this pull request May 2, 2024
Merged
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
@davidben@marco-ippolito
crypto: simplify assertions in Safe*Print
a4847c4
Checking the X509V3_EXT_METHOD doesn't do anything. X509V3_EXT_get is
already implemented by calling X509V3_EXT_get_nid on the extension's
NID. We may as well just check the NID directly.
PR-URL: #49709
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <rlau@redhat.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@himself65himself65himself65 left review comments
@tniessentniessentniessen approved these changes
@richardlaurichardlaurichardlau approved these changes

Assignees

No one assigned

Labels

author readyPRs 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++.cryptoIssues and PRs related to the crypto subsystem.needs-ciPRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@davidben@nodejs-github-bot@richardlau@tniessen@himself65@panva