★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/28406Nouvelle recherche | Portail wanayoo
Skip to content

/node

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

http2: remove square brackets from parsed hostname #28406

Closed
wants to merge 3 commits into from
+39 −2

Conversation

@lpinca
Copy link
Member

commented Jun 24, 2019

Make http2.connect() work when using URLs with literal IPv6
addresses.

Fixes: #28216

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
@lpinca
http2: remove square brackets from parsed hostname 
Make `http2.connect()` work when using URLs with literal IPv6
addresses.

Fixes: #28216
Loading status checks…
44daa79

@nodejs-github-bot nodejs-github-bot added the http2 label Jun 24, 2019

@addaleax
addaleax approved these changes Jun 24, 2019
View changes
@cjihrig
cjihrig approved these changes Jun 24, 2019
View changes
@Trott
Trott approved these changes Jun 25, 2019
View changes
@trivikr
trivikr approved these changes Jun 25, 2019
View changes
@jasnell
jasnell approved these changes Jun 25, 2019
View changes
@gengjiawen
gengjiawen reviewed Jun 25, 2019
View changes
lib/internal/http2/core.js Outdated Show resolved Hide resolved
@lpinca
fixup! http2: remove square brackets from parsed hostname
Loading status checks…
9111e8a
@nodejs-github-bot

This comment has been minimized.

Sign in to view
Copy link

commented Jun 25, 2019

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

@lpinca lpinca added the author ready label Jun 25, 2019

@gengjiawen
gengjiawen reviewed Jun 26, 2019
View changes
lib/internal/http2/core.js Outdated
@@ -2774,7 +2774,18 @@ function connect(authority, options, listener) {
const protocol = authority.protocol || options.protocol || 'https:';
const port = '' + (authority.port !== '' ?
authority.port : (authority.protocol === 'http:' ? 80 : 443));
const host = authority.hostname || authority.host || 'localhost';
let host = '';

This comment has been minimized.

Sign in to view
Copy link
@gengjiawen

gengjiawen Jun 26, 2019

Member

Looks like host can be default to localhost and remove the last else ?
@lpinca
fixup! fixup! http2: remove square brackets from parsed hostname
Loading status checks…
7d6584d
@gengjiawen
gengjiawen approved these changes Jun 26, 2019
View changes
@nodejs-github-bot

This comment has been minimized.

Sign in to view
Copy link

commented Jun 29, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/24166/
@nodejs-github-bot

This comment has been minimized.

Sign in to view
Copy link

commented Jun 29, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/24167/
@lpinca

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Jul 1, 2019

Landed in 26b048e.

@lpinca lpinca closed this Jul 1, 2019

@lpinca lpinca deleted the lpinca:gh-28216 branch Jul 1, 2019

lpinca added a commit that referenced this pull request Jul 1, 2019
@lpinca
http2: remove square brackets from parsed hostname 
Make `http2.connect()` work when using URLs with literal IPv6
addresses.

Fixes: #28216

PR-URL: #28406
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Loading status checks…
26b048e
targos added a commit that referenced this pull request Jul 2, 2019
@lpinca @targos
http2: remove square brackets from parsed hostname 
Make `http2.connect()` work when using URLs with literal IPv6
addresses.

Fixes: #28216

PR-URL: #28406
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
8ed57bf
targos added a commit that referenced this pull request Jul 2, 2019
@lpinca @targos
http2: remove square brackets from parsed hostname 
Make `http2.connect()` work when using URLs with literal IPv6
addresses.

Fixes: #28216

PR-URL: #28406
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
fd4d1e2
@targos targos referenced this pull request Jul 2, 2019
Merged
This was referenced Jul 3, 2019
Closed
Closed
Closed
@murgatroid99

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Aug 23, 2019

Is there any chance of this getting backported to node 10?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell

@Trott Trott

@addaleax addaleax

@cjihrig cjihrig

@gengjiawen gengjiawen

@trivikr trivikr

Assignees
No one assigned
Labels
author ready http2
Projects
None yet
Milestone
No milestone
9 participants
@lpinca @nodejs-github-bot @murgatroid99 @jasnell @Trott @addaleax @cjihrig @gengjiawen @trivikr
You can’t perform that action at this time.