Skip to content

https: do not automatically use invalid servername#28209

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
Closed
sam-github wants to merge 1 commit into from
Closed

https: do not automatically use invalid servername #28209

sam-github wants to merge 1 commit into from

Conversation

@sam-github
Copy link
Contributor

@sam-githubsam-github commented Jun 13, 2019

Stop automatically setting servername in https.request() if the target
host is specified with an IP address. Doing so is invalid, and triggers
a deprecation warning. It is still possible to send an IP address as a
servername if its required, but it needs to be explicity configured, it
won't happen automatically.

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
Copy link
Collaborator

nodejs-github-bot commented Jun 13, 2019•
edited
Loading

Sadly, an error occurred when I tried to trigger a build. :(
CI: https://ci.nodejs.org/job/node-test-pull-request/23869/
CI: https://ci.nodejs.org/job/node-test-pull-request/23885/

@nodejs-github-botnodejs-github-bot added the http Issues or PRs related to the http subsystem. label Jun 13, 2019
addaleax
addaleax approved these changes Jun 13, 2019
View reviewed changes
bnoordhuis
bnoordhuis reviewed Jun 13, 2019
View reviewed changes
@sam-github
https: do not automatically use invalid servername
9bbc7d9
Stop automatically setting servername in https.request() if the target host is specified with an IP address. Doing so is invalid, and triggers a deprecation warning. It is still possible to send an IP address as a servername if its required, but it needs to be explicity configured, it won't happen automatically.
@sam-githubsam-githubforce-pushed the fix-invalid-auto-https-servername branch from b467b57 to 9bbc7d9CompareJune 14, 2019 19:04
@awwright
Copy link
Contributor

awwright commented Jun 15, 2019

For reference, this is specified in RFC 6066:

Literal IPv4 and IPv6 addresses are not permitted in "HostName".

HostName here referring to the field in the SNI extension.

@ZYSzysZYSzys added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 16, 2019
@joyeecheungjoyeecheung mentioned this pull request Jun 16, 2019
Closed
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jun 17, 2019
@sam-github@BridgeAR
https: do not automatically use invalid servername
574985c
Stop automatically setting servername in https.request() if the target host is specified with an IP address. Doing so is invalid, and triggers a deprecation warning. It is still possible to send an IP address as a servername if its required, but it needs to be explicity configured, it won't happen automatically. PR-URL: nodejs#28209 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
@BridgeAR
Copy link
Member

BridgeAR commented Jun 17, 2019

Landed in 574985c 🎉

BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
@sam-github@BridgeAR
https: do not automatically use invalid servername
53cc16c
Stop automatically setting servername in https.request() if the target host is specified with an IP address. Doing so is invalid, and triggers a deprecation warning. It is still possible to send an IP address as a servername if its required, but it needs to be explicity configured, it won't happen automatically. PR-URL: #28209 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
@BridgeARBridgeAR mentioned this pull request Jun 17, 2019
Merged
targos pushed a commit that referenced this pull request Jun 18, 2019
@sam-github@targos
https: do not automatically use invalid servername
faeed80
Stop automatically setting servername in https.request() if the target host is specified with an IP address. Doing so is invalid, and triggers a deprecation warning. It is still possible to send an IP address as a servername if its required, but it needs to be explicity configured, it won't happen automatically. PR-URL: #28209 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
@sam-githubsam-github deleted the fix-invalid-auto-https-servername branch November 28, 2019 22:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnoordhuisbnoordhuisbnoordhuis approved these changes

@jasnelljasnelljasnell approved these changes

@TrottTrottTrott approved these changes

@addaleaxaddaleaxaddaleax approved these changes

@BridgeARBridgeARBridgeAR approved these changes

@ZYSzysZYSzysZYSzys 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.httpIssues or PRs related to the http subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@sam-github@nodejs-github-bot@awwright@BridgeAR@bnoordhuis@jasnell@Trott@addaleax@ZYSzys