diff --git a/README.md b/README.md index 3049b5c..f42608f 100644 --- a/README.md +++ b/README.md @@ -579,11 +579,16 @@ Slim. Any framework works, as long as the closure returns the pattern and never Three exceptions reach the consumer, and every one of them reports a configuration mistake rather than a runtime condition. -| Exception | Thrown when | -|------------------------|-------------------------------------------------------------------------| -| `MappingNotConfigured` | `build()` runs before a single mapping is registered | -| `HttpStatusOutOfRange` | a rule maps to a status that is not a known HTTP error status | -| `HttpHeaderMalformed` | a rule declares a header whose name or value is not a well-formed field | +| Exception | Thrown when | +|------------------------|-------------------------------------------------------------------------------------------------------| +| `MappingNotConfigured` | `build()` runs before a single mapping is registered | +| `HttpStatusOutOfRange` | a rule maps to a status that is not a known HTTP error status | +| `HttpHeaderMalformed` | a rule declares a header whose name or value is not a well-formed field, or a connection-specific one | + +Connection-specific headers are `Connection`, `Keep-Alive`, `Proxy-Connection`, `Transfer-Encoding`, `Upgrade`, and +`TE` with any value other than `trailers`. HTTP/2 and HTTP/3 treat a response carrying one as malformed, and +`tiny-blocks/http` refuses to set one on a response, so `MappedError` refuses it up front instead of letting the error +response fail while it is being built. A rule closed with `mapsTo` builds its `MappedError` while the table is being declared, so a wrong status or a malformed header fails at boot rather than on the first request that would have returned it. A rule closed with diff --git a/composer.json b/composer.json index f513cad..90d9bac 100644 --- a/composer.json +++ b/composer.json @@ -28,7 +28,7 @@ "psr/http-server-handler": "^1.0", "psr/http-server-middleware": "^1.0", "psr/log": "^3.0", - "tiny-blocks/http": "^7.2", + "tiny-blocks/http": "^8.0", "tiny-blocks/http-correlation-id": "^2.1" }, "require-dev": { diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 7434451..77a8180 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -47,6 +47,9 @@ parameters: # Tag map the fake exception replays; PHPDoc is prohibited on constructors and inside tests/. - identifier: missingType.iterableValue path: tests/Unit/ContextualException.php + # Header maps the data providers feed to the tests; PHPDoc is prohibited inside tests/. + - identifier: missingType.iterableValue + path: tests/Unit/MappedErrorTest.php # The replayed map cannot be narrowed without the constructor tag; root cause is the same as above. - identifier: return.type path: tests/Unit/ContextualException.php diff --git a/src/Exceptions/HttpHeaderMalformed.php b/src/Exceptions/HttpHeaderMalformed.php index f201982..bc28b8c 100644 --- a/src/Exceptions/HttpHeaderMalformed.php +++ b/src/Exceptions/HttpHeaderMalformed.php @@ -7,7 +7,12 @@ use InvalidArgumentException; /** - * Thrown when a header configured on a mapped error is not a well-formed HTTP field. + * Thrown when a header configured on a mapped error is not a well-formed HTTP response field. + * + *

A connection-specific header (Connection, Keep-Alive, + * Proxy-Connection, Transfer-Encoding, Upgrade, or TE with any + * value other than trailers) is not one: HTTP/2 and HTTP/3 treat a response carrying it as + * malformed.

*/ final class HttpHeaderMalformed extends InvalidArgumentException { diff --git a/src/Internal/Response/ResponseHeaders.php b/src/Internal/Response/ResponseHeaders.php index 984ad41..8ef6a5e 100644 --- a/src/Internal/Response/ResponseHeaders.php +++ b/src/Internal/Response/ResponseHeaders.php @@ -6,8 +6,17 @@ final readonly class ResponseHeaders { - private const string NAME = '/^[!#$%&\'*+\-.^_`|~0-9A-Za-z]+$/'; - private const string VALUE = '/^[\x20\x09\x21-\x7E\x80-\xFF]*$/'; + private const string TE = 'te'; + private const string NAME = '/^[!#$%&\'*+\-.^_`|~0-9A-Za-z]+$/D'; + private const string VALUE = '/^[\x20\x09\x21-\x7E\x80-\xFF]*$/D'; + private const string TRAILERS = 'trailers'; + private const array CONNECTION_SPECIFIC = [ + 'upgrade', + 'connection', + 'keep-alive', + 'proxy-connection', + 'transfer-encoding' + ]; public static function malformedIn(array $headers): ?string { @@ -24,10 +33,15 @@ public static function malformedIn(array $headers): ?string private static function isWellFormed(string $name, array $values): bool { + $loweredName = strtolower($name); + return preg_match(self::NAME, $name) === 1 + && !in_array($loweredName, self::CONNECTION_SPECIFIC, true) && array_all( $values, - static fn(mixed $value): bool => is_scalar($value) && preg_match(self::VALUE, (string)$value) === 1 + static fn(mixed $value): bool => is_scalar($value) + && preg_match(self::VALUE, (string)$value) === 1 + && ($loweredName !== self::TE || strtolower(trim((string)$value)) === self::TRAILERS) ); } } diff --git a/tests/Unit/MappedErrorTest.php b/tests/Unit/MappedErrorTest.php index 9570075..d8b6456 100644 --- a/tests/Unit/MappedErrorTest.php +++ b/tests/Unit/MappedErrorTest.php @@ -4,6 +4,7 @@ namespace Test\TinyBlocks\Http\ErrorHandler\Unit; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use TinyBlocks\Http\ErrorHandler\Exceptions\HttpHeaderMalformed; use TinyBlocks\Http\ErrorHandler\Exceptions\HttpStatusOutOfRange; @@ -73,6 +74,18 @@ public function testConstructionWhenStatusIsAtUpperBoundaryThenNoExceptionIsThro self::assertSame(511, $error->status); } + #[DataProvider('trailersOnlyTeHeadersProvider')] + public function testConstructionWhenTeHeaderCarriesOnlyTrailersThenHeaderIsPreserved(array $headers): void + { + /** @Given a TE header whose every value is trailers, the only TE value a response may carry */ + + /** @When constructing a MappedError with that header */ + $error = new MappedError(code: 'ERR', status: 422, message: 'An error occurred.', headers: $headers); + + /** @Then the header should be preserved as provided */ + self::assertSame($headers, $error->headers); + } + public function testConstructionWhenHeaderNameIsMalformedThenHttpHeaderMalformedIsThrown(): void { /** @Given a header whose name carries a character no HTTP field name admits */ @@ -107,6 +120,42 @@ public function testConstructionWhenHeaderValueIsMalformedThenHttpHeaderMalforme new MappedError(code: 'ERR', status: 429, message: 'An error occurred.', headers: $headers); } + public function testConstructionWhenHeaderNameEndsInLineFeedThenHttpHeaderMalformedIsThrown(): void + { + /** @Given a header whose name ends in a line feed, which a dollar anchor alone would let through */ + $headers = ["Retry-After\n" => '60']; + + /** @Then a malformed header exception should be thrown */ + $this->expectException(HttpHeaderMalformed::class); + + /** @When constructing a MappedError with that header */ + new MappedError(code: 'ERR', status: 429, message: 'An error occurred.', headers: $headers); + } + + public function testConstructionWhenHeaderValueEndsInLineFeedThenHttpHeaderMalformedIsThrown(): void + { + /** @Given a well-formed name carrying a value that ends in a line feed */ + $headers = ['Retry-After' => "60\n"]; + + /** @Then a malformed header exception should be thrown */ + $this->expectException(HttpHeaderMalformed::class); + + /** @When constructing a MappedError with that header */ + new MappedError(code: 'ERR', status: 429, message: 'An error occurred.', headers: $headers); + } + + #[DataProvider('connectionSpecificHeadersProvider')] + public function testConstructionWhenHeaderIsConnectionSpecificThenHttpHeaderMalformedIsThrown(array $headers): void + { + /** @Given a connection-specific header, which HTTP/2 and HTTP/3 treat as a malformed response */ + + /** @Then a malformed header exception should be thrown */ + $this->expectException(HttpHeaderMalformed::class); + + /** @When constructing a MappedError with that header */ + new MappedError(code: 'ERR', status: 503, message: 'An error occurred.', headers: $headers); + } + public function testConstructionWhenStatusExceedsUpperBoundaryThenHttpStatusOutOfRangeIsThrown(): void { /** @Then an exception indicating the status is out of range should be thrown */ @@ -150,4 +199,27 @@ public function testConstructionWhenNumericHeaderNameCarriesMalformedValueThenHt /** @When constructing a MappedError with that header */ new MappedError(code: 'ERR', status: 422, message: 'An error occurred.', headers: $headers); } + + public static function trailersOnlyTeHeadersProvider(): array + { + return [ + 'TE with trailers' => ['headers' => ['TE' => 'trailers']], + 'TE with padded capital trailers' => ['headers' => ['TE' => ' Trailers ']], + 'TE with trailers in every value' => ['headers' => ['te' => ['trailers', 'TRAILERS']]] + ]; + } + + public static function connectionSpecificHeadersProvider(): array + { + return [ + 'Connection' => ['headers' => ['Connection' => 'close']], + 'Keep-Alive' => ['headers' => ['Keep-Alive' => 'timeout=5']], + 'Proxy-Connection in upper case' => ['headers' => ['PROXY-CONNECTION' => 'keep-alive']], + 'Transfer-Encoding' => ['headers' => ['Transfer-Encoding' => 'chunked']], + 'Upgrade in lower case' => ['headers' => ['upgrade' => 'h2c']], + 'TE with a coding' => ['headers' => ['TE' => 'gzip']], + 'TE with a numeric value' => ['headers' => ['TE' => 1]], + 'TE with trailers and a coding' => ['headers' => ['te' => ['trailers', 'gzip']]] + ]; + } }