diff --git a/.gitattributes b/.gitattributes index fae892b7..ed26f59d 100644 --- a/.gitattributes +++ b/.gitattributes @@ -12,3 +12,4 @@ /psalm.xml.dist export-ignore /renovate.json export-ignore /test/ export-ignore +/psalm/ export-ignore diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 2d5fc326..cab33714 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -7,6 +7,10 @@ $contents + + int + string + @@ -14,6 +18,16 @@ getCode()]]> + + + forInvalidProxyArgument + + + + + __construct + + ! is_string($name) @@ -59,6 +73,11 @@ is_string($version) + + + Module + + $maxLength @@ -82,7 +101,7 @@ $protocolVersion $requestTarget $uri - + self::getValueFromKey($serializedRequest, 'body') $headers @@ -108,10 +127,6 @@ is_string($method) - - uri->getPort()]]> - getPort()]]> - @@ -139,7 +154,7 @@ $protocolVersion $reasonPhrase $statusCode - + self::getValueFromKey($serializedResponse, 'body') $headers @@ -215,7 +230,7 @@ - $requestFilter(new ServerRequest( $server, $files, UriFactory::createFromSapi($server, $headers), @@ -226,10 +241,10 @@ $query ?: $_GET, $body ?: $_POST, marshalProtocolVersionFromSapi($server) - ))]]> + )) - + $headers['cookie'] $headers @@ -247,9 +262,6 @@ getHeaderLine getServerParams getUri - withHost - withPort - withScheme withUri @@ -259,6 +271,11 @@ ]]> + + + __construct + + $resource @@ -266,6 +283,9 @@ resource]]> resource]]> + + $stream + @@ -334,6 +354,9 @@ SensitiveParameter + + parseUri + @@ -342,10 +365,10 @@ - - - - + $spec['error'] + $spec['name'] ?? null + $spec['tmp_name'] + $spec['type'] ?? null @@ -370,8 +393,8 @@ string - - + $server['REQUEST_METHOD'] ?? 'GET' + $server['REQUEST_METHOD'] ?? 'GET' @@ -381,7 +404,7 @@ - + $server['SERVER_PROTOCOL'] @@ -401,14 +424,14 @@ static function (string $name, array $headers, $default = null) { - + $getHeaderFromArray('x-forwarded-proto', $headers, '') $host $host $host $host $port $requestUri - + $server['QUERY_STRING'] $headers[$header] @@ -425,7 +448,7 @@ string - + $server['SERVER_ADDR'] $defaults @@ -433,7 +456,7 @@ $unencodedUrl - + strrpos($host, ':') @@ -444,13 +467,13 @@ - - + $apacheRequestHeaders['Authorization'] + $apacheRequestHeaders['authorization'] $apacheRequestHeaders - - + $server['HTTP_AUTHORIZATION'] + $server['HTTP_AUTHORIZATION'] @@ -478,25 +501,25 @@ $nameTree[$key] ?? null, $typeTree[$key] ?? null ) - $recursiveNormalize( $files['tmp_name'], $files['size'], $files['error'], $files['name'] ?? null, $files['type'] ?? null - )]]> + ) array - $recursiveNormalize( $files['tmp_name'], $files['size'], $files['error'], $files['name'] ?? null, $files['type'] ?? null - )]]> + ) @@ -576,7 +599,7 @@ - + $normalizedFiles['fooFiles'] @@ -584,11 +607,27 @@ $parsedBody + + + RequestInterfaceStaticReturnTypes + + setMethods + + + sampleCallback + sampleStaticCallback + + + + + HeaderStack + + $path @@ -602,26 +641,32 @@ $test + + $query + + + withUserInfo + - - + $normalised['my-form']['details']['avatars'] + $normalised['slide-shows'][0]['slides'] - - - - - - - - - - - - - + $normalised['my-form']['details']['avatar'] + $normalised['my-form']['details']['avatars'] + $normalised['my-form']['details']['avatars'] + $normalised['my-form']['details']['avatars'] + $normalised['my-form']['details']['avatars'] + $normalised['my-form']['details']['avatars'][0] + $normalised['my-form']['details']['avatars'][1] + $normalised['my-form']['details']['avatars'][2] + $normalised['slide-shows'][0]['slides'] + $normalised['slide-shows'][0]['slides'] + $normalised['slide-shows'][0]['slides'] + $normalised['slide-shows'][0]['slides'][0] + $normalised['slide-shows'][0]['slides'][1] getClientFilename @@ -632,14 +677,14 @@ getClientFilename - - - - - - - - + $normalised['my-form'] + $normalised['my-form'] + $normalised['my-form'] + $normalised['my-form'] + $normalised['my-form'] + $normalised['slide-shows'] + $normalised['slide-shows'] + $normalised['slide-shows'] diff --git a/psalm.xml.dist b/psalm.xml.dist index 0a854e54..248eafd9 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -5,6 +5,9 @@ xmlns="https://getpsalm.org/schema/config" xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" errorBaseline="psalm-baseline.xml" + findUnusedCode="true" + findUnusedPsalmSuppress="true" + findUnusedBaselineEntry="true" > @@ -14,6 +17,10 @@ + + + + diff --git a/psalm/http-message-stubs/UriInterface.phpstub b/psalm/http-message-stubs/UriInterface.phpstub new file mode 100644 index 00000000..79540212 --- /dev/null +++ b/psalm/http-message-stubs/UriInterface.phpstub @@ -0,0 +1,18 @@ +withHost($header); + [$host, $port] = UriFactory::marshalHostAndPortFromHeader($header); + $uri = $uri + ->withHost($host); + if ($port !== null) { + $uri = $uri->withPort($port); + } break; case self::HEADER_PORT: $uri = $uri->withPort((int) $header); diff --git a/src/Uri.php b/src/Uri.php index 95c7af93..59f776ee 100644 --- a/src/Uri.php +++ b/src/Uri.php @@ -38,6 +38,8 @@ * might change state are implemented such that they retain the internal * state of the current instance and return a new instance that contains the * changed state. + * + * @psalm-immutable */ class Uri implements UriInterface, Stringable { @@ -67,8 +69,7 @@ class Uri implements UriInterface, Stringable private string $host = ''; - /** @var int|null */ - private $port; + private ?int $port = null; private string $path = ''; @@ -110,6 +111,7 @@ public function __toString(): string return $this->uriString; } + /** @psalm-suppress ImpureMethodCall, InaccessibleProperty */ $this->uriString = static::createUriString( $this->scheme, $this->getAuthority(), @@ -442,6 +444,9 @@ public function withFragment($fragment): UriInterface /** * Parse a URI into its parts, and set the properties + * + * @psalm-suppress InaccessibleProperty Method is only called in {@see Uri::__construct} and thus immutability is + * still given. */ private function parseUri(string $uri): void { @@ -552,8 +557,12 @@ private function filterUserInfoPart(string $part): string { $part = $this->filterInvalidUtf8($part); - // Note the addition of `%` to initial charset; this allows `|` portion - // to match and thus prevent double-encoding. + /** + * @psalm-suppress ImpureFunctionCall Even tho the callback targets this immutable class, + * psalm reports an issue here. + * Note the addition of `%` to initial charset; this allows `|` portion + * to match and thus prevent double-encoding. + */ return preg_replace_callback( '/(?:[^%' . self::CHAR_UNRESERVED . self::CHAR_SUB_DELIMS . ']+|%(?![A-Fa-f0-9]{2}))/u', [$this, 'urlEncodeChar'], @@ -568,6 +577,10 @@ private function filterPath(string $path): string { $path = $this->filterInvalidUtf8($path); + /** + * @psalm-suppress ImpureFunctionCall Even tho the callback targets this immutable class, + * psalm reports an issue here. + */ return preg_replace_callback( '/(?:[^' . self::CHAR_UNRESERVED . ')(:@&=\+\$,\/;%]+|%(?![A-Fa-f0-9]{2}))/u', [$this, 'urlEncodeChar'], @@ -656,6 +669,10 @@ private function filterQueryOrFragment(string $value): string { $value = $this->filterInvalidUtf8($value); + /** + * @psalm-suppress ImpureFunctionCall Even tho the callback targets this immutable class, + * psalm reports an issue here. + */ return preg_replace_callback( '/(?:[^' . self::CHAR_UNRESERVED . self::CHAR_SUB_DELIMS . '%:@\/\?]+|%(?![A-Fa-f0-9]{2}))/u', [$this, 'urlEncodeChar'], diff --git a/src/UriFactory.php b/src/UriFactory.php index ba3bd78e..31f5a1de 100644 --- a/src/UriFactory.php +++ b/src/UriFactory.php @@ -14,7 +14,6 @@ use function explode; use function gettype; use function implode; -use function is_array; use function is_bool; use function is_scalar; use function is_string; @@ -232,16 +231,14 @@ private static function marshalHttpsValue(mixed $https): bool } /** - * @param string|list $host + * @internal + * * @return array{string, int|null} Array of two items, host and port, in that order (can be * passed to a list() operation). + * @psalm-mutation-free */ - private static function marshalHostAndPortFromHeader($host): array + public static function marshalHostAndPortFromHeader(string $host): array { - if (is_array($host)) { - $host = implode(', ', $host); - } - $port = null; // works for regname, IPv4 & IPv6 diff --git a/test/MessageTraitTest.php b/test/MessageTraitTest.php index 92c63f96..4c636926 100644 --- a/test/MessageTraitTest.php +++ b/test/MessageTraitTest.php @@ -382,7 +382,7 @@ public function numericHeaderValuesProvider(): array /** * @dataProvider numericHeaderValuesProvider * @group 99 - * @psalm-suppress InvalidArgument,InvalidScalarArgument this test + * @psalm-suppress InvalidArgument this test * explicitly verifies that pre-type-declaration implicit type * conversion semantics still apply, for BC Compliance */ diff --git a/test/ServerRequestFilter/FilterUsingXForwardedHeadersTest.php b/test/ServerRequestFilter/FilterUsingXForwardedHeadersTest.php index ff677867..e4bbd4a7 100644 --- a/test/ServerRequestFilter/FilterUsingXForwardedHeadersTest.php +++ b/test/ServerRequestFilter/FilterUsingXForwardedHeadersTest.php @@ -368,4 +368,68 @@ public function testOnlyHonorsXForwardedProtoIfValueResolvesToHTTPS( $uri = $filteredRequest->getUri(); $this->assertSame($expectedScheme, $uri->getScheme()); } + + /** + * Caddy server and NGINX seem to strip ports from `X-Forwarded-Host` as it should only contain the `host` which + * was initially requested. Due to Mozilla, the `Host` header is allowed to contain a port. Apache2 does pass the + * `Host` header via `X-Forwarded-Host` and thus, it should be stripped. We should only trust the `X-Forwarded-Port` + * header which is provided by the proxy since the `Host` header could contain port `80` while the initial request + * was still sent to port `443`. + * + * @link https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Host + * @link https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host + * @link https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/ + * @link https://caddyserver.com/docs/caddyfile/directives/reverse_proxy#defaults + * @link https://httpd.apache.org/docs/2.4/en/mod/mod_proxy.html#x-headers + */ + public function testWillFilterXForwardedHostPortWithPreservingForwardedPort(): void + { + $request = new ServerRequest( + ['REMOTE_ADDR' => '192.168.0.1'], + [], + 'http://localhost:80/foo/bar', + 'GET', + 'php://temp', + [ + 'Host' => 'localhost', + 'X-Forwarded-Proto' => 'https', + 'X-Forwarded-Host' => 'example.org:80', + 'X-Forwarded-Port' => '443', + ] + ); + + $filter = FilterUsingXForwardedHeaders::trustAny(); + + $filteredRequest = $filter($request); + $uri = $filteredRequest->getUri(); + self::assertSame('example.org', $uri->getHost()); + self::assertNull( + $uri->getPort(), + 'Port is omitted due to the fact that `https` protocol was used and port 80 is being ignored due' + . ' to the availability of `X-Forwarded-Port' + ); + } + + public function testWillFilterXForwardedHostPort(): void + { + $request = new ServerRequest( + ['REMOTE_ADDR' => '192.168.0.1'], + [], + 'http://localhost:80/foo/bar', + 'GET', + 'php://temp', + [ + 'Host' => 'localhost', + 'X-Forwarded-Proto' => 'https', + 'X-Forwarded-Host' => 'example.org:8080', + ] + ); + + $filter = FilterUsingXForwardedHeaders::trustAny(); + + $filteredRequest = $filter($request); + $uri = $filteredRequest->getUri(); + self::assertSame('example.org', $uri->getHost()); + self::assertSame(8080, $uri->getPort()); + } } diff --git a/test/StreamTest.php b/test/StreamTest.php index b63e7a41..245d7bde 100644 --- a/test/StreamTest.php +++ b/test/StreamTest.php @@ -614,7 +614,6 @@ public function testRaisesExceptionOnConstructionForNonStreamResources(): void $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('stream'); - /** @psalm-suppress ImplicitToStringCast, PossiblyInvalidArgument */ new Stream($resource); } @@ -633,7 +632,6 @@ public function testRaisesExceptionOnAttachForNonStreamResources(): void $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('stream'); - /** @psalm-suppress ImplicitToStringCast, PossiblyInvalidArgument */ $stream->attach($resource); } diff --git a/test/UriTest.php b/test/UriTest.php index 848241af..5c65b192 100644 --- a/test/UriTest.php +++ b/test/UriTest.php @@ -166,7 +166,7 @@ public function testWithPortReturnsNewInstanceWithProvidedPort($port): void public function testWithPortReturnsSameInstanceWithProvidedPortIsSameAsBefore(): void { $uri = new Uri('https://user:pass@local.example.com:3001/foo?bar=baz#quz'); - /** @psalm-suppress PossiblyInvalidArgument,InvalidArgument */ + /** @psalm-suppress InvalidArgument */ $new = $uri->withPort('3001'); $this->assertSame($uri, $new); $this->assertSame(3001, $new->getPort());