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

tls,https: respect address family when connecting by bnoordhuis · Pull Request #6654 · nodejs/node · GitHub

/ node Public

tls,https: respect address family when connecting - #6654

Merged
bnoordhuis merged 2 commits into
nodejs:masterfrom
bnoordhuis:fix6440
May 28, 2016
ConversationCommits (2)ChecksFiles changed
Merged

tls,https: respect address family when connecting#6654
bnoordhuis merged 2 commits into
nodejs:masterfrom
bnoordhuis:fix6440

Conversation

bnoordhuis commented May 9, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

Respect the { family: 6 } address family property when connecting to
a remote peer over TLS.

Fixes: #6440

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

bnoordhuis added tls Issues and PRs related to the tls subsystem. https Issues or PRs related to the https subsystem. lts-watch-v4.x labels May 9, 2016
bnoordhuis mentioned this pull request May 9, 2016
Closed

jasnell commented May 9, 2016

Copy link
Copy Markdown
Member

CI failed to start. https://ci.nodejs.org/job/node-test-pull-request/2549/console

LGTM if CI comes up green

bnoordhuis commented May 10, 2016

Copy link
Copy Markdown
Member Author

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

bnoordhuis commented May 10, 2016

Copy link
Copy Markdown
Member Author

Added a fix-up for the CI hosts where localhost doesn't have an IPv6 address, PTAL.

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

bnoordhuis commented May 10, 2016

Copy link
Copy Markdown
Member Author

That only made it worse... @nodejs/build Can this be fixed on the buildbots?

jbergstroem commented May 10, 2016

Copy link
Copy Markdown
Member

@bnoordhuis to be clear you want ::1 localhost into /etc/hosts rite?

jbergstroem commented May 10, 2016

Copy link
Copy Markdown
Member

..or is it just whatever ::1 entry? debian8 on do has this: ::1 ip6-localhost ip6-loopback

bnoordhuis commented May 11, 2016

Copy link
Copy Markdown
Member Author

@jbergstroem Ideally, I'd like AAAA queries for localhost to resolve to ::1 everywhere.

We currently have this list but trying them in order still doesn't make the tests pass on all machines, see e.g. debian8-x86 (but not debian8-64, oddly enough.)

jbergstroem commented May 11, 2016

Copy link
Copy Markdown
Member

@bnoordhuis the reason they differ is because one is from DO and the other from softlayer. providers usually populate differently.

I'll look at adding ::1 localhost to all hosts via the ansible-playbooks; but it might take a small while.

jbergstroem mentioned this pull request May 11, 2016
Closed
bnoordhuis mentioned this pull request May 27, 2016
Closed

bnoordhuis commented May 27, 2016

Copy link
Copy Markdown
Member Author

I'm marking the tests as flaky on Linux for now, pending resolution of nodejs/build#415.

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

cjihrig reviewed May 27, 2016
View reviewed changes
Comment thread test/parallel/test-https-connect-address-family.js Outdated

cjihrig May 27, 2016

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

Can you switch to the new common.skip().

cjihrig commented May 27, 2016

Copy link
Copy Markdown
Contributor

LGTM with a couple comments.

bnoordhuis commented May 28, 2016 •
edited
Loading

Copy link
Copy Markdown
Member Author

Good points. Updated with feedback. New CI: https://ci.nodejs.org/job/node-test-pull-request/2838/

EDIT: Make that https://ci.nodejs.org/job/node-test-pull-request/2839/ - I just landed #7037 to get the CI in a good state again.

cjihrig commented May 28, 2016

Copy link
Copy Markdown
Contributor

Still EADDRINUSE errors in https://ci.nodejs.org/job/node-test-commit-arm/3507/nodes=armv7-ubuntu1404/console. I guess the port could have still been in use from older CI runs.

bnoordhuis commented May 28, 2016

Copy link
Copy Markdown
Member Author

Should be better now. New CI: https://ci.nodejs.org/job/node-test-pull-request/2840/

cjihrig commented May 28, 2016

Copy link
Copy Markdown
Contributor

All green now.

bnoordhuis added 2 commits May 28, 2016 22:49
tls,https: respect address family when connecting
fb19b80 
Respect the `{ family: 6 }` address family property when connecting to
a remote peer over TLS.

Fixes: nodejs#4139
Fixes: nodejs#6440
PR-URL: nodejs#6654
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
test: use strictEqual consistently in agent test
ead6c2d 
Update parallel/test-http-agent-getname to use assert.strictEqual()
consistently and const-ify variables while we're here.

PR-URL: nodejs#6654
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
bnoordhuis closed this May 28, 2016
bnoordhuis deleted the fix6440 branch May 28, 2016 20:50
bnoordhuis merged commit ead6c2d into nodejs:master May 28, 2016
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 30, 2016
tls,https: respect address family when connecting
2ab7139 
Respect the `{ family: 6 }` address family property when connecting to
a remote peer over TLS.

Fixes: nodejs#4139
Fixes: nodejs#6440
PR-URL: nodejs#6654
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 30, 2016
test: use strictEqual consistently in agent test
0230fe3 
Update parallel/test-http-agent-getname to use assert.strictEqual()
consistently and const-ify variables while we're here.

PR-URL: nodejs#6654
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
rvagg pushed a commit that referenced this pull request Jun 2, 2016
tls,https: respect address family when connecting
257e54b 
Respect the `{ family: 6 }` address family property when connecting to
a remote peer over TLS.

Fixes: #4139
Fixes: #6440
PR-URL: #6654
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
rvagg pushed a commit that referenced this pull request Jun 2, 2016
test: use strictEqual consistently in agent test
8c289df 
Update parallel/test-http-agent-getname to use assert.strictEqual()
consistently and const-ify variables while we're here.

PR-URL: #6654
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
gibfahn mentioned this pull request Jun 13, 2016
Closed
MylesBorins pushed a commit that referenced this pull request Jun 29, 2016
tls,https: respect address family when connecting
c7c5a78 
Respect the `{ family: 6 }` address family property when connecting to
a remote peer over TLS.

Fixes: #4139
Fixes: #6440
PR-URL: #6654
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 29, 2016
test: use strictEqual consistently in agent test
8cbcea0 
Update parallel/test-http-agent-getname to use assert.strictEqual()
consistently and const-ify variables while we're here.

PR-URL: #6654
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
tls,https: respect address family when connecting
c807287 
Respect the `{ family: 6 }` address family property when connecting to
a remote peer over TLS.

Fixes: #4139
Fixes: #6440
PR-URL: #6654
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
test: use strictEqual consistently in agent test
e3f9bc8 
Update parallel/test-http-agent-getname to use assert.strictEqual()
consistently and const-ify variables while we're here.

PR-URL: #6654
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins mentioned this pull request Jul 12, 2016
Merged
kegsay added a commit to matrix-org/matrix-appservice-irc that referenced this pull request Oct 31, 2016
Supported on Node v4.5+
340b8b9 
Was previously 4.x for ES6 goodies. Now v4.5 to fix issues with TLS and IPv6
not playing nicely together, which meant that you couldn't connected to IRC
networks over TLS with IPv6.

See:
 - nodejs/node#6654
DoobleD mentioned this pull request Sep 10, 2019
Closed
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

No reviews

Assignees

No one assigned

Labels

https Issues or PRs related to the https subsystem. 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.

5 participants

Back | FazBrowse Home | New Git URL