HomePhabricator

Don't include default port (80, 443) in "Host:" headers from Conduit

Description

Don't include default port (80, 443) in "Host:" headers from Conduit

Summary:
Fixes T11587.

In Q473, a user had lighttpd configured with a nonstandard port (8080), and their configuration did not work because we omitted the port from the Host: header.

There is a good argument that our behavior was in the wrong, and D16464 fixed this, by adding the port to the Host: header.

However, in T11587, this broke a different user who had a configuration which did not expect a port number in a port 80 request.

Use a behavior which is more similar to what browsers do:

  • If the protocol is http and the port is 80, or the protocol is https and the port is 443, omit the port.
  • Otherwise, include the port.

I think there's a good argument that our current behavior is spec-compliant and servers that do not treat Host: domain.com:80 the same as Host: domain.com are in the wrong, but realistically the lifetime support cost of fighting this battle is almost certainly higher than following the "empirical spec" and doing what browsers do.

Test Plan:

  • Added some debugging code to dump the strings.
  • Ran arc list to test non-signed requests.
  • Ran bin/repository reparse --message ... on a cluster device to test signed requests.
  • In both cases, saw proper outputs (for Host: domain.com, domain.com:123; for signing: domain.com:80, domain.com:123).

Reviewers: michaeljs1990, chad

Reviewed By: chad

Maniphest Tasks: T11587

Differential Revision: https://secure.phabricator.com/D16491