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

Port 80 is included in the 'Host' request field (instead of defaulted) when it isn't with other popular HTTP clients #515

Closed
celer opened this issue Apr 15, 2013 · 5 comments · Fixed by #2904

Comments

@celer
Copy link

celer commented Apr 15, 2013

Hi,

So we encountered an interesting problem today, less so a bug with request but more of a incongruent behavior from request when compared against other browsers and client's we've tested.

When working with cloudfoundry.com we found that their proxy did not consider a request to port 80 the same if the port was explicitly specified in the 'Host' header field.

So a request to http://foo.cloudfoundry.com:80/ and http://foo.cloudfoundry.com/ behaved differently. We narrowed it down to the difference in the Host field. Request included the port when explicitly specifying it, but not when it wasn't.

No big deal right? Well so everyone else strips port 80 because it is the default :(

  • Curl strips port 80 from the Host header
  • wget strips port 80 from the Host header
  • Safari strips port 80 from the Host header
  • Firefox strips port 80 from the Host header
  • Chrome strips port 80 from the Host header

You can test the behavior using "http://helloworld-ng.cloudify.com:80/" and "http://helloworld-ng.cloudify.com/" - notice that both URLs work the same in all of the above listed browsers

In curl we can see the failure case

pair@foo:~pair/$ curl http://helloworld-ng.cloudfoundry.com:80/ -H "Host: helloworld-ng.cloudfoundry.com:80"
<html>
<head><title>404 Not Found</title></head>
<body bgcolor="white">
<center><h1>404 Not Found</h1></center>
<hr><center>nginx</center>
</body>
</html>
pair@foo:~pair/$ curl http://helloworld-ng.cloudfoundry.com:80/ -H "Host: helloworld-ng.cloudfoundry.com"
Hello from Cloud Foundry

Now let's try it with request

request=require('request');

request.get("http://helloworld-ng.cloudfoundry.com:80/",function(err,req,body){
  console.log("this will fail with port 80",body);
});

request.get("http://helloworld-ng.cloudfoundry.com/",function(err,req,body){
  console.log("this will not fail without port 80",body);
});

(We've asked cloudfoundry to fix this issue, but to be clear this is incongruent with every other client and browser we've tested)

So then the question becomes - should request do the same? Sadly it seems like this shouldn't be an issue but it is an unexpected behavior and no one else does it - hence why I think request should strip port 80 from the Header field if it is specified.

On that note node-http-proxy suffers form the same problem and a nice bit of logic that it uses some places to address it:

  var host=host+(port-80===0?"":":"+port)
@mikeal
Copy link
Member

mikeal commented Apr 15, 2013

hrm.....

do all those applications strip it even when you put in the full url with :80?

@celer
Copy link
Author

celer commented Apr 16, 2013

Hi Mikeal,

Yes, every client I have strips port 80 even when specified, see below for packet captures (using ngrep).

Chrome request to http://helloworld-ng.cloudfoundry.com:80/

T 192.168.2.3:55280 -> 173.243.49.35:80 [AP]
  GET / HTTP/1.1..Host: helloworld-ng.cloudfoundry.com..Connection: keep-alive..Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8..User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_3) AppleW
  ebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.65 Safari/537.31..Accept-Encoding: gzip,deflate,sdch..Accept-Language: en-US,en;q=0.8..Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3..Cookie: __qca=P0-2133402613-136
  6064815021....

Firefox request to http://helloworld-ng.cloudfoundry.com:80/

T 192.168.2.3:55274 -> 173.243.49.35:80 [AP]
  GET / HTTP/1.1..Host: helloworld-ng.cloudfoundry.com..User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20100101 Firefox/20.0..Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q
  =0.8..Accept-Language: en-US,en;q=0.5..Accept-Encoding: gzip, deflate..Connection: keep-alive....

Safari request to http://helloworld-ng.cloudfoundry.com:80/

T 192.168.2.3:55289 -> 173.243.49.35:80 [AP]
  GET / HTTP/1.1..Host: helloworld-ng.cloudfoundry.com..User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_3) AppleWebKit/536.28.10 (KHTML, like Gecko) Version/6.0.3 Safari/536.28.10..Accept: text/html,application
  /xhtml+xml,application/xml;q=0.9,*/*;q=0.8..Accept-Language: en-us..Accept-Encoding: gzip, deflate..Connection: keep-alive....

Curl request to http://helloworld-ng.cloudfoundry.com:80/

T 192.168.2.3:55297 -> 173.243.49.35:80 [AP]
  GET / HTTP/1.1..User-Agent: curl/7.30.0..Host: helloworld-ng.cloudfoundry.com..Accept: */*....

wget request to http://hellloworld-ng.cloudfoundry.com:80/

T 192.168.2.3:55303 -> 173.243.49.35:80 [AP]
  GET / HTTP/1.1..User-Agent: Wget/1.14 (darwin12.3.0)..Accept: */*..Host: helloworld-ng.cloudfoundry.com..Connection: Keep-Alive....

Here is the response we got from cloudfoundry regarding this issue:

Hi, you're are correct and obviously what should happen is the port on the header should be stripped. 
However I am not totally sure that if you were to request http://helloworld-ng.cloudfoundry.com:80 
from a browser, the port would be included in the host header. I have also tested this on a more 
recent version of Cloud Foundry and it's not an issue, so this should change when the new version 
of .com is released.

Regardless of the response from Cloudfoundry, this is still a bug / incongruence in request.

Thanks,

celer

@celer
Copy link
Author

celer commented Apr 17, 2013

I also verified the behavior with HTTPS when using port 443. All the clients strip 443 from the host field when using https.

I used a simple nodejs server and the various development tools with the browsers to verify the behavior.

var fs = require('fs');
var http = require('https');
var privateKey  = fs.readFileSync('privatekey.pem').toString();
var certificate = fs.readFileSync('certificate.pem').toString();

var credentials = {key: privateKey, cert: certificate};
var express = require('express');
var server = express();

var app = express();

app.use(express.bodyParser());
app.use(express.methodOverride());
app.use(app.router);

app.get("/",function(req,res){
  console.log(req.headers);
  console.log("Got /");
  res.send("hello");
});

var https = http.createServer(credentials, app);

https.listen(443);

@mikeal
Copy link
Member

mikeal commented Apr 17, 2013

right, but the pull request you sent doesn't handle https.

@joshenders
Copy link

This doesn't appear to be a violation of the RFC:

http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.23

@mikeal mikeal closed this as completed Aug 28, 2014
splattael pushed a commit to neopoly/rack-reverse-proxy that referenced this issue Jun 9, 2015
Setting default port in Host header break some web servers
like "Apache Coyote".

Most clients don't set this default port too:
* request/request#515 (comment)
* https://github.com/ruby/ruby/blob/d46336e71731aa64d71d4573b2741b7de43ec340/lib/net/http/generic_request.rb#L18
waterlink pushed a commit to waterlink/rack-reverse-proxy that referenced this issue Jul 3, 2015
Setting default port in Host header break some web servers
like "Apache Coyote".

Most clients don't set this default port too:
* request/request#515 (comment)
* https://github.com/ruby/ruby/blob/d46336e71731aa64d71d4573b2741b7de43ec340/lib/net/http/generic_request.rb#L18
nbr added a commit to nbr/rack-cors that referenced this issue Apr 4, 2018
When `http://EXAMPLE:80` is an allowed origin, requests are not allowed from
`http://EXAMPLE`. Since port 80 is the default port for HTTP, browsers
will strip it and thus rack-cors never receives a request from `http://EXAMPLE`.

A similar problem is discussed here: request/request#515
mikeal pushed a commit that referenced this issue Jul 18, 2018
…wn. (#2904)

* Strip port suffix from Host header if the protocol is known.

This partially revert ff6d6c6, and still works for IPv6 addresses as well.

* Port is a string out of url.parse().

See https://nodejs.org/api/url.html#url_url_port

* Port is a string out of url.parse().

See https://nodejs.org/api/url.html#url_url_port

* Add tests for the new Host header changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants