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

Jump to bottom

header field "host" includes port 80 #2894

Closed
zhaotian2470 opened this issue Mar 13, 2018 · 2 comments
Closed

header field "host" includes port 80 #2894

zhaotian2470 opened this issue Mar 13, 2018 · 2 comments

Comments

@zhaotian2470
Copy link

As the following code:

request/request.js

Line 294 in bbb3a0b

self.setHeader(hostHeaderName, self.uri.host)

If URI is "http://myhost:80/path", header field "host" will be set as "myhost:80".
Is it correct?

Note: before commit ff6d6c6, "host" header field will be set as "myhost"
@paambaati
Copy link
Contributor

paambaati commented Apr 3, 2018 •
edited

Recently, most clients (including curl & wget) and browsers have started to drop the :80 & :443 port suffixes in the Host header, and include it only when it is neither of the two. Dropping the port prefix for known services is still RFC-compliant; see http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.23

Examples of other projects stripping the port suffix —

  1. Ruby - https://github.com/ruby/ruby/blob/d46336e71731aa64d71d4573b2741b7de43ec340/lib/net/http/generic_request.rb#L18
  2. AsyncHttpClient - Netty: Don't set Host header port when it's the scheme's default one AsyncHttpClient/async-http-client#899
  3. Omit port in Host header for default ports waterlink/rack-reverse-proxy#3
  4. Use request.hostname, not request.headers.host twilio/twilio-node#151
  5. accept implicit port 80 on whitelist bokeh/bokeh#3513
  6. Exclude port 443 from host http header websocket-client/websocket-client#248
  7. https://stackoverflow.com/a/19169664

Related request issue — #515

We discovered this was an issue while working with HAProxy, when backends use host-based routing. When the Host header includes the :80 or :443 suffix, the rules would not match and we'd get a 503 response.

@mikeal Can I send a PR for this? I've sent a PR here → #2904

@paambaati paambaati mentioned this issue Apr 4, 2018
Merged
3 tasks
mikeal pushed a commit that referenced this issue Jul 18, 2018
@paambaati @mikeal
#515, #2894 Strip port suffix from Host header if the protocol is kno…
a92e138 
…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.
@paambaati
Copy link
Contributor

@zhaotian2470 This can be closed now, as v2.88.1 fixes this.

@paambaati paambaati mentioned this issue Nov 12, 2018
Closed
This was referenced Mar 16, 2021
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@paambaati @zhaotian2470