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

http.request option "family: 6" doesn't enable request over IPv6 #6440

Closed
DimitryDushkin opened this issue Apr 28, 2016 · 8 comments
Closed
Labels
confirmed-bug Issues with confirmed bugs. https Issues or PRs related to the https subsystem. tls Issues and PRs related to the tls subsystem.

Comments

@DimitryDushkin
Copy link

DimitryDushkin commented Apr 28, 2016

  • Version: 5.5
  • Platform: Ubuntu 14.04.1
  • Subsystem: http(s)

Seems like family option does nothing with actual DNS resolve to ipv6.
Problem is internal server should only use ipv6. So, instead of

https.get({
    hostname: 'cloud-api.yandex.net',
    path: '/v1/disk/resources?path=/',
    family: 6,
    port: 443,
    protocol: 'https:'
}, (res) => {
    console.log(res);
}).on('error', (e) => {
  console.log(`Got error: ${e.message}`);    // Got error: connect EHOSTUNREACH 213.180.204.127:443
});

i should write

dns.lookup('cloud-api.yandex.net', { family: 6 }, (err, addr, family) => {
    https.get({
        hostname: addr,
        path: '/v1/disk/resources?path=/',
        port: 443,
        protocol: 'https:',
        headers: { host: 'cloud-api.yandex.net' }
    }, (res) => {
        console.log(res);
    }).on('error', (e) => {
      console.log(`Got error: ${e.message}`);
    });
});
@DimitryDushkin DimitryDushkin changed the title http.request option "family" do nothing http.request option "family: 6" do not enable request over IPv6 Apr 28, 2016
@DimitryDushkin DimitryDushkin changed the title http.request option "family: 6" do not enable request over IPv6 http.request option "family: 6" doesn't enable request over IPv6 Apr 28, 2016
@bnoordhuis bnoordhuis added confirmed-bug Issues with confirmed bugs. tls Issues and PRs related to the tls subsystem. https Issues or PRs related to the https subsystem. labels Apr 28, 2016
@bnoordhuis
Copy link
Member

Confirmed. The bug is in tls.connect() with a second-order bug in http.Agent#getName(). This patch should fix it:

diff --git a/lib/_http_agent.js b/lib/_http_agent.js
index fd74daa..3f3d148 100644
--- a/lib/_http_agent.js
+++ b/lib/_http_agent.js
@@ -102,6 +102,10 @@ Agent.prototype.getName = function(options) {
   if (options.localAddress)
     name += options.localAddress;

+  name += ':';
+  if (options.family === 4 || options.family === 6)
+    name += options.family;
+
   return name;
 };

diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
index 029d9ac..d42f855 100644
--- a/lib/_tls_wrap.js
+++ b/lib/_tls_wrap.js
@@ -1021,6 +1021,7 @@ exports.connect = function(/* [port, host], options, cb */) {
       connect_opt = {
         port: options.port,
         host: options.host,
+        family: options.family,
         localAddress: options.localAddress
       };
     }

@JungMinu
Copy link
Member

JungMinu commented Apr 29, 2016

@bnoordhuis would you mind if I make a PR to resolve this issue based on your comment?

@bnoordhuis
Copy link
Member

I will but I haven't had time yet to write a robust regression test.

@bnoordhuis
Copy link
Member

#6654

bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue May 28, 2016
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 issue May 30, 2016
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>
rvagg pushed a commit that referenced this issue Jun 2, 2016
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>
@dzlk
Copy link

dzlk commented Jun 29, 2016

Do you have plans to fix this in v4.x?

@bnoordhuis
Copy link
Member

#6654 is tagged lts-watch-v4.x. It will make its way into a LTS release eventually.

MylesBorins pushed a commit that referenced this issue Jun 29, 2016
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
Copy link
Contributor

I've just backported #6654 to LTS staging. it should be in the next release

@dzlk
Copy link

dzlk commented Jun 29, 2016

Thank you!

MylesBorins pushed a commit that referenced this issue Jul 12, 2016
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. https Issues or PRs related to the https subsystem. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants