From bb62bc2b8f3fa2e4a5c8b2784e88ab959c29fb32 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 29 Jun 2022 07:25:35 -0500 Subject: [PATCH 1/3] fix: ensure x-forwarded-proto ony considers 'https' values This resolves a regression in 2.11.1 whereby FilterUsingXForwardedHeaders was using the X-Forwarded-Proto value without filtering it (which was done in prior releases). Signed-off-by: Matthew Weier O'Phinney --- .../FilterUsingXForwardedHeaders.php | 3 +- .../FilterUsingXForwardedHeadersTest.php | 37 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/ServerRequestFilter/FilterUsingXForwardedHeaders.php b/src/ServerRequestFilter/FilterUsingXForwardedHeaders.php index fcac3753..00b9707b 100644 --- a/src/ServerRequestFilter/FilterUsingXForwardedHeaders.php +++ b/src/ServerRequestFilter/FilterUsingXForwardedHeaders.php @@ -83,7 +83,8 @@ public function __invoke(ServerRequestInterface $request): ServerRequestInterfac $uri = $uri->withPort((int) $header); break; case self::HEADER_PROTO: - $uri = $uri->withScheme($header); + $scheme = strtolower($header) === 'https' ? 'https' : 'http'; + $uri = $uri->withScheme($scheme); break; } } diff --git a/test/ServerRequestFilter/FilterUsingXForwardedHeadersTest.php b/test/ServerRequestFilter/FilterUsingXForwardedHeadersTest.php index 653d91d1..317dc2ca 100644 --- a/test/ServerRequestFilter/FilterUsingXForwardedHeadersTest.php +++ b/test/ServerRequestFilter/FilterUsingXForwardedHeadersTest.php @@ -331,4 +331,41 @@ public function testTrustReservedSubnetsProducesFilterThatRejectsAddressesNotFro $filteredRequest = $filter($request); $this->assertSame($request, $filteredRequest); } + + /** @psalm-return iterable */ + public function xForwardedProtoValues() : iterable + { + yield 'https-lowercase' => ['https', 'https']; + yield 'https-uppercase' => ['HTTPS', 'https']; + yield 'https-mixed-case' => ['hTTpS', 'https']; + yield 'http-lowercase' => ['http', 'http']; + yield 'http-uppercase' => ['HTTP', 'http']; + yield 'http-mixed-case' => ['hTTp', 'http']; + yield 'unknown-value' => ['foo', 'http']; + yield 'empty' => ['', 'http']; + } + + /** @dataProvider xForwardedProtoValues */ + public function testOnlyHonorsXForwardedProtoIfValueResolvesToHTTPS( + string $xForwarededProto, + string $expectedScheme + ): void { + $request = new ServerRequest( + ['REMOTE_ADDR' => '192.168.0.1'], + [], + 'http://localhost:80/foo/bar', + 'GET', + 'php://temp', + [ + 'Host' => 'localhost', + 'X-Forwarded-Proto' => $xForwarededProto, + ] + ); + + $filter = FilterUsingXForwardedHeaders::trustReservedSubnets(); + + $filteredRequest = $filter($request); + $uri = $filteredRequest->getUri(); + $this->assertSame($expectedScheme, $uri->getScheme()); + } } From 8e201614932ff62b2d503c060400c2a16373714a Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 29 Jun 2022 07:40:36 -0500 Subject: [PATCH 2/3] fix: ensure Host header is considered when marshaling URI 2.11.1 introduced a regression whereby the `Host` header was no longer being considered for purposes of generating the URI. This patch adds those facilities to `ServerRequestFactory::fromGlobals()`, as well as a unit test. Signed-off-by: Matthew Weier O'Phinney --- psalm-baseline.xml | 9 ++++- src/ServerRequestFactory.php | 59 ++++++++++++++++++++++++++++--- test/ServerRequestFactoryTest.php | 34 ++++++++++++++++++ 3 files changed, 97 insertions(+), 5 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 21ba7271..1456fb12 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -243,9 +243,13 @@ - + $headers['cookie'] + $host + + $headers + $iisUrlRewritten $requestUri @@ -257,6 +261,9 @@ $defaults + + self::marshalHostAndPortFromHeader($host) + ServerRequest diff --git a/src/ServerRequestFactory.php b/src/ServerRequestFactory.php index 8b9d5fda..30d40d44 100644 --- a/src/ServerRequestFactory.php +++ b/src/ServerRequestFactory.php @@ -75,7 +75,7 @@ public static function fromGlobals( return $requestFilter(new ServerRequest( $server, $files, - self::marshalUriFromSapi($server), + self::marshalUriFromSapi($server, $headers), marshalMethodFromSapi($server), 'php://input', $headers, @@ -105,9 +105,10 @@ public function createServerRequest(string $method, $uri, array $serverParams = /** * Marshal a Uri instance based on the values present in the $_SERVER array and headers. * + * @param array> $headers * @param array $server SAPI parameters */ - private static function marshalUriFromSapi(array $server) : Uri + private static function marshalUriFromSapi(array $server, array $headers) : Uri { $uri = new Uri(''); @@ -122,7 +123,7 @@ private static function marshalUriFromSapi(array $server) : Uri $uri = $uri->withScheme($https ? 'https' : 'http'); // Set the host - [$host, $port] = self::marshalHostAndPort($server); + [$host, $port] = self::marshalHostAndPort($server, $headers); if (! empty($host)) { $uri = $uri->withHost($host); if (! empty($port)) { @@ -157,13 +158,19 @@ private static function marshalUriFromSapi(array $server) : Uri /** * Marshal the host and port from the PHP environment. * + * @param array> $headers * @return array{string, int|null} Array of two items, host and port, * in that order (can be passed to a list() operation). */ - private static function marshalHostAndPort(array $server) : array + private static function marshalHostAndPort(array $server, array $headers) : array { static $defaults = ['', null]; + $host = self::getHeaderFromArray('host', $headers, false); + if ($host !== false) { + return self::marshalHostAndPortFromHeader($host); + } + if (! isset($server['SERVER_NAME'])) { return $defaults; } @@ -256,4 +263,48 @@ private static function marshalHttpsValue($https) : bool return 'on' === strtolower($https); } + + /** + * @param string|list $host + * @return array Array of two items, host and port, in that order (can be + * passed to a list() operation). + */ + private static function marshalHostAndPortFromHeader($host) + { + if (is_array($host)) { + $host = implode(', ', $host); + } + + $port = null; + + // works for regname, IPv4 & IPv6 + if (preg_match('|\:(\d+)$|', $host, $matches)) { + $host = substr($host, 0, -1 * (strlen($matches[1]) + 1)); + $port = (int) $matches[1]; + } + + return [$host, $port]; + } + + /** + * Retrieve a header value from an array of headers using a case-insensitive lookup. + * + * @param array> $headers Key/value header pairs + * @param mixed $default Default value to return if header not found + * @return mixed + */ + private static function getHeaderFromArray(string $name, array $headers, $default = null) + { + $header = strtolower($name); + $headers = array_change_key_case($headers, CASE_LOWER); + if (! array_key_exists($header, $headers)) { + return $default; + } + + if (is_string($headers[$header])) { + return $headers[$header]; + } + + return implode(', ', $headers[$header]); + } } diff --git a/test/ServerRequestFactoryTest.php b/test/ServerRequestFactoryTest.php index fe3beb7a..eeda1682 100644 --- a/test/ServerRequestFactoryTest.php +++ b/test/ServerRequestFactoryTest.php @@ -6,6 +6,7 @@ use Laminas\Diactoros\ServerRequest; use Laminas\Diactoros\ServerRequestFactory; +use Laminas\Diactoros\ServerRequestFilter\DoNotFilter; use Laminas\Diactoros\ServerRequestFilter\FilterServerRequestInterface; use Laminas\Diactoros\UploadedFile; use Laminas\Diactoros\Uri; @@ -752,4 +753,37 @@ public function __invoke(ServerRequestInterface $request): ServerRequestInterfac $this->assertSame($expectedRequest, $request); } + + public function testHonorsHostHeaderOverServerNameWhenMarshalingUrl(): void + { + $server = [ + 'SERVER_NAME' => 'localhost', + 'SERVER_PORT' => '80', + 'SERVER_ADDR' => '172.22.0.4', + 'REMOTE_PORT' => '36852', + 'SERVER_PROTOCOL' => 'HTTP/1.1', + 'DOCUMENT_ROOT' => '/var/www/public', + 'DOCUMENT_URI' => '/index.php', + 'REQUEST_URI' => '/api/messagebox-schema', + 'PATH_TRANSLATED' => '/var/www/public', + 'PATH_INFO' => '', + 'SCRIPT_NAME' => '/index.php', + 'REQUEST_METHOD' => 'GET', + 'SCRIPT_FILENAME' => '/var/www/public/index.php', + // headers + 'HTTP_HOST' => 'example.com', + ]; + + $request = ServerRequestFactory::fromGlobals( + $server, + null, + null, + null, + null, + new DoNotFilter() + ); + + $uri = $request->getUri(); + $this->assertSame('example.com', $uri->getHost()); + } } From 52d7cf62d52b0d4b148efce00a60a046c8a8a629 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 29 Jun 2022 09:03:31 -0500 Subject: [PATCH 3/3] fix: add return type to marshalHostAndPortFromHeader Signed-off-by: Matthew Weier O'Phinney --- src/ServerRequestFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServerRequestFactory.php b/src/ServerRequestFactory.php index 30d40d44..30eb71cb 100644 --- a/src/ServerRequestFactory.php +++ b/src/ServerRequestFactory.php @@ -269,7 +269,7 @@ private static function marshalHttpsValue($https) : bool * @return array Array of two items, host and port, in that order (can be * passed to a list() operation). */ - private static function marshalHostAndPortFromHeader($host) + private static function marshalHostAndPortFromHeader($host): array { if (is_array($host)) { $host = implode(', ', $host);