Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

http.request fails when host contains port #25751

Closed
nfriedly opened this issue Jul 22, 2015 · 7 comments
Closed

http.request fails when host contains port #25751

nfriedly opened this issue Jul 22, 2015 · 7 comments

Comments

@nfriedly
Copy link

host is different from hostname in that it may contain a port number. For example:

require('url').parse('http://localhost:8080')
{ protocol: 'http:',
  slashes: true,
  auth: null,
  host: 'localhost:8080',
  port: '8080',
  hostname: 'localhost',
  hash: null,
  search: null,
  query: null,
  pathname: '/',
  path: '/',
  href: 'http://localhost:8080/' }

However, http.request({options}) treats both fields as hostname and attempts an invalid DNS lookup when given a host with a port. For example:

http.get({host: 'localhost:8080', path: '/'}, console.log)
> Error: getaddrinfo ENOTFOUND localhost:8080
    at errnoException (dns.js:44:10)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:94:26)
@nfriedly
Copy link
Author

Fixed, with a test :)

@nfriedly
Copy link
Author

I also though about possibly adding a normalize() method to the URL library, which would handle this and related things... but that would be a larger change.

@jasnell
Copy link
Member

jasnell commented Jul 28, 2015

This is due to the fact that the object you're passing into get does not have enough information to construct the URL properly. If you pass in the full output from url.parse then it works fine.

var u = require('url').parse('http://localhost:8080');
require('http').get(u, function(err,res) { console.log(res.statusCode); });

Closing as it's not actually a bug in node. Yes, the API could be doing things better, but this is the way it's worked for a while and it's not going to be changing in either v0.10 or v0.12.

@jasnell jasnell closed this as completed Jul 28, 2015
@nfriedly
Copy link
Author

I respectfully disagree. There is enough information to make the request correctly, the patch I provided (https://github.com/joyent/node/pull/25752/files) does exactly that with a simple 3-line change.

This bug was found when writing unit tests for a module that normally connects to a remote server. I change the host that my library uses from "example.com" to "localhost:12345" to match the mock remote server that I set up and then run the tests. This works correctly with my patch, but fails because node passes the wrong value to the dns lookup in the current 0.12 release.

@nfriedly
Copy link
Author

(BTW, I'm OK if the fix doesn't get included in 0.10 and 0.12 - I'd just like to see it included eventually.)

@jasnell
Copy link
Member

jasnell commented Jul 28, 2015

There are a number of issues with the url.parse response and the way url's are reconstructed from it. It's definitely a known issue, but it's certainly not going to be changing here. What I would recommend is moving discussion over to either the nodejs/io.js or nodejs/node repos.

@nfriedly
Copy link
Author

Oh, alright. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants