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

tls: use the most recently added matching SecureContext in default SN… #36072

Merged
merged 1 commit into from
Jan 15, 2021
Merged

tls: use the most recently added matching SecureContext in default SN… #36072

merged 1 commit into from
Jan 15, 2021

Conversation

mkrawczuk
Copy link
Contributor

@mkrawczuk mkrawczuk commented Nov 10, 2020 •
edited by Trott
Loading

Sorry, ruined the previous PR (#34638) with a merge.

Fixes: #34110

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

@nodejs-github-bot nodejs-github-bot added the tls Issues and PRs related to the tls subsystem. label Nov 10, 2020
@mkrawczuk mkrawczuk mentioned this pull request Nov 10, 2020
Closed
4 tasks
mscdex
mscdex reviewed Nov 10, 2020
View reviewed changes
test/parallel/test-tls-secure-context-usage-order.js
// connections.
// Connection to subdomain 'a' is made when the 'bad' secure context is
// the only one in use.
if ('a.example.com' === c.servername) {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor nit: we could be more strict here and assert that these are equal depending on the current step, same for the other conditional below

Trott
Trott reviewed Nov 12, 2020
View reviewed changes
doc/api/tls.md Outdated Show resolved Hide resolved
@Trott
Copy link
Member

Trott commented Nov 12, 2020

@nodejs/crypto @addaleax

@Trott Trott requested a review from addaleax November 16, 2020 14:41
@mkrawczuk
Copy link
Contributor Author

Rebased. cc @addaleax

@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 23, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 23, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@mkrawczuk
Copy link
Contributor Author

Did a clear rebase, because the previous one failed. @Trott you might want to request another CI.

@nodejs-github-bot
Copy link
Collaborator

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

@mkrawczuk @Trott
tls: use recently added matching SecureContext in default SNICallback
8cf5281 
PR-URL: nodejs#36072
Fixes: nodejs#34110
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@Trott
Copy link
Member

Trott commented Jan 15, 2021

Landed in 8cf5281

@Trott Trott merged commit 8cf5281 into nodejs:master Jan 15, 2021
ruyadorno pushed a commit that referenced this pull request Jan 22, 2021
@mkrawczuk @ruyadorno
tls: use recently added matching SecureContext in default SNICallback
864b97b 
PR-URL: #36072
Fixes: #34110
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@ruyadorno ruyadorno mentioned this pull request Jan 22, 2021
Merged
@Ethan-Arrowood Ethan-Arrowood mentioned this pull request Aug 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mscdex mscdex mscdex left review comments

@addaleax addaleax addaleax approved these changes

@Trott Trott Trott approved these changes

Assignees
No one assigned
Labels
tls Issues and PRs related to the tls subsystem.
Projects
None yet
Milestone
No milestone
6 participants
@mkrawczuk @Trott @nodejs-github-bot @mscdex @addaleax @targos