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 1741a4f9..6dada0f6 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,5 +1,5 @@ - + null|callable @@ -114,10 +114,6 @@ $requestTarget - - $this->uri->getPort() - $uri->getPort() - @@ -170,11 +166,6 @@ $hasContentType - - - json_encode - - gettype($uri) @@ -234,21 +225,12 @@ - + getHeaderLine getServerParams getUri - withHost - withPort - withScheme withUri - - $proxyCIDRList - - - list<non-empty-string> - diff --git a/psalm.xml.dist b/psalm.xml.dist index 0a854e54..3cf45c00 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -5,6 +5,8 @@ xmlns="https://getpsalm.org/schema/config" xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" errorBaseline="psalm-baseline.xml" + findUnusedCode="false" + findUnusedPsalmSuppress="true" > @@ -14,6 +16,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 d0ce13af..ec7ccb9d 100644 --- a/src/UriFactory.php +++ b/src/UriFactory.php @@ -12,7 +12,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; @@ -228,16 +227,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 1b72fb31..e7460c8f 100644 --- a/test/StreamTest.php +++ b/test/StreamTest.php @@ -613,7 +613,7 @@ public function testRaisesExceptionOnConstructionForNonStreamResources(): void $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('stream'); - /** @psalm-suppress ImplicitToStringCast, PossiblyInvalidArgument */ + /** @psalm-suppress PossiblyInvalidArgument */ new Stream($resource); } @@ -632,7 +632,7 @@ public function testRaisesExceptionOnAttachForNonStreamResources(): void $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('stream'); - /** @psalm-suppress ImplicitToStringCast, PossiblyInvalidArgument */ + /** @psalm-suppress 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());