FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

net: fix family autoselection SSL connection handling by ShogunPanda · Pull Request #48189 · nodejs/node · GitHub

/ node Public

net: fix family autoselection SSL connection handling - #48189

Closed
ShogunPanda wants to merge 2 commits into
nodejs:mainfrom
ShogunPanda:fix-family-autoselection-ssl-connect
ConversationCommits (2)ChecksFiles changed
Closed

net: fix family autoselection SSL connection handling#48189
ShogunPanda wants to merge 2 commits into
nodejs:mainfrom
ShogunPanda:fix-family-autoselection-ssl-connect

Conversation

ShogunPanda commented May 26, 2023

Copy link
Copy Markdown
Contributor

This PR fixes SSL connection attempts by not attempting to restore SSL information (like server name) if such informations are not available anymore.

Fixes #48000.

nodejs-github-bot added needs-ci PRs that need a full CI run. tls Issues and PRs related to the tls subsystem. labels May 26, 2023
ShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label May 26, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 26, 2023

nodejs-github-bot commented May 26, 2023

Copy link
Copy Markdown
Collaborator

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

mscdex reviewed May 26, 2023
View reviewed changes
Comment thread test/parallel/test-https-autoselectfamily-slow-timeout.js Outdated

mscdex May 26, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Shouldn't this test go in test/internet/ if it's attempting to reach an internet host?

ShogunPanda May 28, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes, I didn't know about it. Fixed now.

richardlau reviewed May 26, 2023
View reviewed changes
Comment thread test/parallel/test-https-autoselectfamily-slow-timeout.js Outdated

richardlau May 26, 2023

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

If this is reaching out to the internet this should go in internet and not parallel. It should also be using a constant from common/internet to allow people to run the test suites in places that restrict access to some addresses.

ShogunPanda May 28, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks, I didn't know about this. This is fixed now.

This was referenced May 27, 2023
Open
Open
ShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label May 28, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 28, 2023

nodejs-github-bot commented May 28, 2023

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented May 28, 2023

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented May 28, 2023

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented May 28, 2023

Copy link
Copy Markdown
Collaborator

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

github-actions Bot mentioned this pull request May 29, 2023
Open
22 tasks
ShogunPanda force-pushed the fix-family-autoselection-ssl-connect branch from 7425b2d to 5150216 Compare May 29, 2023 11:01
ShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label May 29, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 29, 2023

nodejs-github-bot commented May 29, 2023

Copy link
Copy Markdown
Collaborator

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

github-actions Bot mentioned this pull request May 30, 2023
Open
23 tasks

ShogunPanda commented May 30, 2023

Copy link
Copy Markdown
Contributor Author

@nodejs/build I have a failure in GH suite but I don't think it's related to my changes. Can I ignore or do you first want to address it?

github-actions Bot mentioned this pull request May 31, 2023
Open
22 tasks
This was referenced May 31, 2023
Merged
Merged

ShogunPanda commented May 31, 2023

Copy link
Copy Markdown
Contributor Author

Landed in 26450c5

ShogunPanda added a commit that referenced this pull request May 31, 2023
net: fix family autoselection SSL connection handling
26450c5 
PR-URL: #48189
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>

targos commented May 31, 2023

Copy link
Copy Markdown
Member

The test-internet failure was confirmed to be unrelated and we opened #48262 for it.

ShogunPanda deleted the fix-family-autoselection-ssl-connect branch June 1, 2023 20:50
ShogunPanda mentioned this pull request Jun 1, 2023
Merged
This was referenced Jun 2, 2023
Open
Open
Open
targos pushed a commit that referenced this pull request Jun 4, 2023
net: fix family autoselection SSL connection handling
5ddca72 
PR-URL: #48189
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
targos mentioned this pull request Jun 4, 2023
Merged
mhdawson mentioned this pull request Jul 27, 2023
Merged
mhdawson mentioned this pull request Aug 4, 2023
Closed
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
net: fix family autoselection SSL connection handling
63877ea 
PR-URL: nodejs#48189
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
net: fix family autoselection SSL connection handling
20ecd6c 
PR-URL: nodejs#48189
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
mhdawson mentioned this pull request Aug 15, 2023
Closed
mhdawson pushed a commit to mhdawson/io.js that referenced this pull request Aug 15, 2023
net: fix family autoselection SSL connection handling
009414e 
PR-URL: nodejs#48189
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
ruyadorno pushed a commit that referenced this pull request Aug 17, 2023
net: fix family autoselection SSL connection handling
b5f53d9 
PR-URL: #48189
Backport-PR-URL: #49183
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
ruyadorno mentioned this pull request Aug 17, 2023
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

richardlau richardlau left review comments
jasnell jasnell approved these changes
marco-ippolito marco-ippolito approved these changes
+1 more reviewer
mscdex mscdex left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. tls Issues and PRs related to the tls subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Assertion (wrap->ssl_) != nullptr failed in TLSWrap::GetServername

10 participants

Back | FazBrowse Home | New Git URL