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

https: do not automatically use invalid servername #28209

Closed

Conversation

sam-github
Copy link
Contributor

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

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-bot nodejs-github-bot added the http Issues or PRs related to the http subsystem. label Jun 13, 2019
doc/api/https.md Outdated Show resolved Hide resolved
doc/api/https.md Outdated Show resolved Hide resolved
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-github sam-github force-pushed the fix-invalid-auto-https-servername branch from b467b57 to 9bbc7d9 Compare June 14, 2019 19:04
@awwright
Copy link
Contributor

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.

@ZYSzys ZYSzys added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 16, 2019
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jun 17, 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.

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

Landed in 574985c 🎉

@BridgeAR BridgeAR closed this Jun 17, 2019
BridgeAR pushed a commit that referenced this pull request Jun 17, 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.

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]>
@BridgeAR BridgeAR mentioned this pull request Jun 17, 2019
targos pushed a commit that referenced this pull request Jun 18, 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.

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-github sam-github deleted the fix-invalid-auto-https-servername branch November 28, 2019 22:21
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. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants