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

tls: tweak clientCertEngine argument parsing #38900

Closed

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Jun 2, 2021

This PR slightly tweaks the argument parsing within configSecureContext.

BoringSSL defines OPENSSL_NO_ENGINE and jasnell@35274cb changed behavior so that if a bad clientCertEngine argument is passed, ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED() will be thrown before ERR_INVALID_ARG_TYPE.

This made Electron's smoke test of parallel/test-tls-clientcertengine-invalid-arg-type.js fail - this PR makes that test pass once more for us.

@codebytere codebytere requested a review from jasnell June 2, 2021 09:19
@codebytere codebytere added the tls Issues and PRs related to the tls subsystem. label Jun 2, 2021
@github-actions github-actions bot added the needs-ci PRs that need a full CI run. label Jun 2, 2021
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

codebytere added a commit that referenced this pull request Jun 4, 2021
PR-URL: #38900
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@codebytere
Copy link
Member Author

Landed in dc25378

@codebytere codebytere closed this Jun 4, 2021
@codebytere codebytere deleted the tweak-configSecureContext branch June 4, 2021 09:20
@codebytere codebytere 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 Jun 4, 2021
targos pushed a commit that referenced this pull request Jun 11, 2021
PR-URL: #38900
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@danielleadams danielleadams mentioned this pull request Jun 14, 2021
codebytere added a commit to electron/electron that referenced this pull request Jun 24, 2021
codebytere added a commit to electron/electron that referenced this pull request Jun 24, 2021
@richardlau
Copy link
Member

This appears dependent on #38116 which is dont-land-on-v14.x so marking as the same.

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. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants