Skip to content

Commit

Permalink
Merge pull request #95 from weierophinney/hotfix/host-proto-regressions
Browse files Browse the repository at this point in the history 
Resolve Host header and X-Forwarded-Proto regressions
  • Loading branch information
@weierophinney
weierophinney authored Jun 29, 2022
2 parents 25b11d4 + 52d7cf6 commit 78846cb
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 6 deletions.
9 changes: 8 additions & 1 deletion psalm-baseline.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,13 @@
</file>
<file src="src/ServerRequestFactory.php">
<LessSpecificReturnStatement occurrences="1"/>
<MixedArgument occurrences="1">
<MixedArgument occurrences="2">
<code>$headers['cookie']</code>
<code>$host</code>
</MixedArgument>
<MixedArgumentTypeCoercion occurrences="1">
<code>$headers</code>
</MixedArgumentTypeCoercion>
<MixedAssignment occurrences="3">
<code>$iisUrlRewritten</code>
<code>$requestUri</code>
Expand All @@ -257,6 +261,9 @@
<MixedReturnStatement occurrences="1">
<code>$defaults</code>
</MixedReturnStatement>
<MixedReturnTypeCoercion occurrences="1">
<code>self::marshalHostAndPortFromHeader($host)</code>
</MixedReturnTypeCoercion>
<MoreSpecificReturnType occurrences="1">
<code>ServerRequest</code>
</MoreSpecificReturnType>
Expand Down
59 changes: 55 additions & 4 deletions src/ServerRequestFactory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<string, string|list<string>> $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('');

Expand All @@ -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)) {
Expand Down Expand Up @@ -157,13 +158,19 @@ private static function marshalUriFromSapi(array $server) : Uri
/**
* Marshal the host and port from the PHP environment.
*
* @param array<string, string|list<string>> $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;
}
Expand Down Expand Up @@ -256,4 +263,48 @@ private static function marshalHttpsValue($https) : bool

return 'on' === strtolower($https);
}

/**
* @param string|list<string> $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): array
{
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<string, string|list<string>> $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]);
}
}
3 changes: 2 additions & 1 deletion src/ServerRequestFilter/FilterUsingXForwardedHeaders.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
34 changes: 34 additions & 0 deletions test/ServerRequestFactoryTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
}
37 changes: 37 additions & 0 deletions test/ServerRequestFilter/FilterUsingXForwardedHeadersTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,4 +331,41 @@ public function testTrustReservedSubnetsProducesFilterThatRejectsAddressesNotFro
$filteredRequest = $filter($request);
$this->assertSame($request, $filteredRequest);
}

/** @psalm-return iterable<string, array{0: string, 1: string}> */
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());
}
}

0 comments on commit 78846cb

Please sign in to comment.