Resolve Host header and X-Forwarded-Proto regressions

[weierophinney]

Q	A
Documentation	no
Bugfix	yes
BC Break	no
New Feature	no
RFC	no
QA	yes

Description

The patch introduced in 2.11.1 introduced 2 regressions:

• FilterUsingXForwardedHeaders was not filtering the X-Forwarded-Proto value before assigning it to the Uri instance, which could lead to exceptions when the value was not one of "http" or "https".
• ServerRequestFactory::fromGlobals() was no longer considering the Host header when resolving the host and port, which could lead to unexpected values for both items in the generated Uri instance.

This patch introduces tests to ensure the regressions do not occur in future releases, and patches to resolve them.

[ketedrum]

Hi guys,

One curious side-effect from this is that on a host with a port e.g localhost:8000, the port now appears twice in the Host header, i.e the Host header value now becomes localhost:8000:8000.

That's probably because $_SERVER['HTTP_HOST'] already contains the port within it (at least for the server I've observed this with, which is Caddy/v2.5.1). In this case, the $headers result from marshalHeadersFromSapi already has a port within the Host header.

marshalHostAndPortFromHeader correctly marshalls the port (but by this time the host itself has a port), therefore when checking for a port in marshalUriFromSapi and performing $uri->withPort($port) the resulting $uri repeats the port.

Not sure if I'm missing something here or if it's my server confguration, I noticed when ServerUrlHelper was generating two ports for this particular use case..

Many thanks.

[Ocramius]

@collinspamba please report a new issue, and if you can, add a failing test case to it.