diff --git a/CHANGELOG.md b/CHANGELOG.md index ae49563..f747d6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 1.2.2 under development -- no changes in this release. +- Enh #43: Refactor `RequestBodyParser` middleware fallback handling (@olegbaturin) ## 1.2.1 December 21, 2025 diff --git a/README.md b/README.md index e55bb4e..5dd5fec 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,52 @@ the middleware: $requestBodyParser = $requestBodyParser->withParser('application/myformat', MyFormatParser::class); ``` +## Errors handling + +To handle parsing errors, pass an instance of `BadRequestHandlerInterface` to the `RequestBodyParser` middleware's constructor. +The [Yii Request Body Parser](yiisoft/request-body-parser) package provides the `BadRequestHandler` error handler by default. + +To use it, pass it to the `RequestBodyParser` middleware constructor. + +```php +use Psr\Http\Message\ResponseFactoryInterface; +use Yiisoft\Request\Body\RequestBodyParser; +use Yiisoft\Request\Body\BadRequestHandler; + +$responseFactory = $container->get(ResponseFactoryInterface::class); +$BadRequestHandler = new BadRequestHandler($responseFactory); + +$middleware = new RequestBodyParser( + $container, + $badRequestHandler // pass the handler here +); +``` + +or define the `BadRequestHandlerInterface` configuration in the [DI container](https://github.com/yiisoft/di): + +```php +// config/web/di/request-body-parser.php + +use Yiisoft\Request\Body\BadRequestHandler; +use Yiisoft\Request\Body\BadRequestHandlerInterface; + +return [ + BadRequestHandlerInterface::class => BadRequestHandler::class, +]; +``` + +> In case [Yii Request Body Parser](yiisoft/request-body-parser) package is used along with [Yii Config](https://github.com/yiisoft/config) plugin, the package is [configured](./config/di-web.php) +automatically to use `BadRequestHandler`. + +To disable parsing errors handling pass `null` as the `badRequestHandler` parameter of the `RequestBodyParser` middleware constructor: + +```php +$middleware = new RequestBodyParser( + $container, + null // disables parsing errors handling +); +``` + ## Documentation - [Internals](docs/internals.md) diff --git a/composer.json b/composer.json index 9a378e2..fe86109 100644 --- a/composer.json +++ b/composer.json @@ -47,6 +47,7 @@ "roave/infection-static-analysis-plugin": "^1.35", "spatie/phpunit-watcher": "^1.24", "vimeo/psalm": "^5.26.1 || ^6.13.1", + "yiisoft/di": "^1.4", "yiisoft/test-support": "^3.0.2" }, "autoload": { diff --git a/config/di-web.php b/config/di-web.php new file mode 100644 index 0000000..7d49d67 --- /dev/null +++ b/config/di-web.php @@ -0,0 +1,10 @@ + BadRequestHandler::class, +]; diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 9163a2f..305b416 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -25,6 +25,7 @@ + ./config ./src diff --git a/psalm.xml b/psalm.xml index 77a366b..9f55f16 100644 --- a/psalm.xml +++ b/psalm.xml @@ -3,8 +3,6 @@ errorLevel="1" findUnusedBaselineEntry="true" findUnusedCode="false" - ensureOverrideAttribute="false" - strictBinaryOperands="false" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="https://getpsalm.org/schema/config" xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" @@ -17,5 +15,7 @@ + + diff --git a/src/BadRequestHandler.php b/src/BadRequestHandler.php index 0ab2302..6982615 100644 --- a/src/BadRequestHandler.php +++ b/src/BadRequestHandler.php @@ -14,33 +14,24 @@ */ final class BadRequestHandler implements BadRequestHandlerInterface { - private ?ParserException $parserException = null; - public function __construct( - private readonly ResponseFactoryInterface $responseFactory, + private readonly ResponseFactoryInterface $responseFactory ) { } - public function handle(ServerRequestInterface $request): ResponseInterface + public function handle(ServerRequestInterface $request, ?ParserException $e = null): ResponseInterface { $response = $this->responseFactory->createResponse(Status::BAD_REQUEST); $response ->getBody() ->write(Status::TEXTS[Status::BAD_REQUEST]); - if ($this->parserException !== null) { + if ($e !== null) { $response ->getBody() - ->write("\n" . $this->parserException->getMessage()); + ->write("\n" . $e->getMessage()); } return $response; } - - public function withParserException(ParserException $e): BadRequestHandlerInterface - { - $new = clone $this; - $new->parserException = $e; - return $new; - } } diff --git a/src/BadRequestHandlerInterface.php b/src/BadRequestHandlerInterface.php index 9bb472c..f1e0215 100644 --- a/src/BadRequestHandlerInterface.php +++ b/src/BadRequestHandlerInterface.php @@ -4,19 +4,18 @@ namespace Yiisoft\Request\Body; -use Psr\Http\Server\RequestHandlerInterface; +use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ServerRequestInterface; /** * Forms response when there is an error parsing request. */ -interface BadRequestHandlerInterface extends RequestHandlerInterface +interface BadRequestHandlerInterface { /** - * Creates new instance of handler with parser exception set. + * Handles a `ParserException` object and produces a response. * * @param ParserException $e Exception occurred during parsing request. - * - * @return self */ - public function withParserException(ParserException $e): self; + public function handle(ServerRequestInterface $request, ?ParserException $e = null): ResponseInterface; } diff --git a/src/ParserException.php b/src/ParserException.php index 8da0802..3be47e6 100644 --- a/src/ParserException.php +++ b/src/ParserException.php @@ -9,6 +9,6 @@ * * @psalm-suppress ClassMustBeFinal We want to allow extending this exception. */ -class ParserException extends \RuntimeException +final class ParserException extends \RuntimeException { } diff --git a/src/RequestBodyParser.php b/src/RequestBodyParser.php index 1376448..1b80122 100644 --- a/src/RequestBodyParser.php +++ b/src/RequestBodyParser.php @@ -6,7 +6,6 @@ use InvalidArgumentException; use Psr\Container\ContainerInterface; -use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; @@ -16,8 +15,13 @@ use Yiisoft\Request\Body\Parser\JsonParser; use function array_key_exists; +use function count; +use function explode; use function is_array; use function is_object; +use function str_contains; +use function strtolower; +use function trim; /** * The package is a PSR-15 middleware that allows parsing PSR-7 server request body selecting the parser according @@ -28,8 +32,6 @@ */ final class RequestBodyParser implements MiddlewareInterface { - private BadRequestHandlerInterface $badRequestHandler; - /** * @var string[] * @psalm-var array @@ -37,14 +39,40 @@ final class RequestBodyParser implements MiddlewareInterface private array $parsers = [ 'application/json' => JsonParser::class, ]; - private bool $ignoreBadRequestBody = false; + /** + * @param ContainerInterface $container PSR-11 container to create parsers. + * @param BadRequestHandlerInterface|null $badRequestHandler Handler for request parsing errors. + * If set to `null`, the request will continue processing unaltered. + */ public function __construct( - ResponseFactoryInterface $responseFactory, private readonly ContainerInterface $container, - BadRequestHandlerInterface|null $badRequestHandler = null + private readonly ?BadRequestHandlerInterface $badRequestHandler ) { - $this->badRequestHandler = $badRequestHandler ?? new BadRequestHandler($responseFactory); + } + + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + $parser = $this->getParser($this->getContentType($request)); + if ($parser !== null) { + try { + /** @var mixed $parsed */ + $parsed = $parser->parse((string)$request->getBody()); + if ($parsed !== null && !is_object($parsed) && !is_array($parsed)) { + $parserClass = $parser::class; + throw new RuntimeException( + "$parserClass::parse() return value must be an array, an object, or null." + ); + } + $request = $request->withParsedBody($parsed); + } catch (ParserException $e) { + if ($this->badRequestHandler !== null) { + return $this->badRequestHandler->handle($request, $e); + } + } + } + + return $handler->handle($request); } /** @@ -92,44 +120,6 @@ public function withoutParsers(string ...$mimeTypes): self return $new; } - /** - * Makes the middleware to simple skip requests it cannot parse. - * - * @return self - */ - public function ignoreBadRequestBody(): self - { - $new = clone $this; - $new->ignoreBadRequestBody = true; - return $new; - } - - public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface - { - $parser = $this->getParser($this->getContentType($request)); - if ($parser !== null) { - try { - /** @var mixed $parsed */ - $parsed = $parser->parse((string)$request->getBody()); - if ($parsed !== null && !is_object($parsed) && !is_array($parsed)) { - $parserClass = $parser::class; - throw new RuntimeException( - "$parserClass::parse() return value must be an array, an object, or null." - ); - } - $request = $request->withParsedBody($parsed); - } catch (ParserException $e) { - if (!$this->ignoreBadRequestBody) { - return $this->badRequestHandler - ->withParserException($e) - ->handle($request); - } - } - } - - return $handler->handle($request); - } - /** * @psalm-suppress MixedInferredReturnType, MixedReturnStatement */ diff --git a/tests/BadRequestHandlerTest.php b/tests/BadRequestHandlerTest.php index 347824a..8ddf352 100644 --- a/tests/BadRequestHandlerTest.php +++ b/tests/BadRequestHandlerTest.php @@ -10,23 +10,29 @@ use Psr\Http\Message\ServerRequestInterface; use Yiisoft\Http\Status; use Yiisoft\Request\Body\BadRequestHandler; +use Yiisoft\Request\Body\ParserException; final class BadRequestHandlerTest extends TestCase { - public function testShouldReturnCode400(): void + public function testShouldReturnCode400AndCorrectErrorInBody(): void { $response = $this ->createHandler() ->handle($this->createRequest()); + $this->assertEquals(Status::BAD_REQUEST, $response->getStatusCode()); + $this->assertEquals(Status::TEXTS[Status::BAD_REQUEST], (string)$response->getBody()); } - public function testShouldReturnCorrectErrorInBody(): void + public function testShouldReturnCode400AndExceptionMessageInBody(): void { + $e = new ParserException('JGURDA'); $response = $this ->createHandler() - ->handle($this->createRequest()); - $this->assertEquals(Status::TEXTS[Status::BAD_REQUEST], (string)$response->getBody()); + ->handle($this->createRequest(), $e); + + $this->assertEquals(Status::BAD_REQUEST, $response->getStatusCode()); + $this->assertEquals(Status::TEXTS[Status::BAD_REQUEST] . "\n" . $e->getMessage(), (string)$response->getBody()); } private function createHandler(): BadRequestHandler diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php new file mode 100644 index 0000000..62e0d0f --- /dev/null +++ b/tests/ConfigTest.php @@ -0,0 +1,41 @@ +createContainer(); + + $requestHandler = $container->get(BadRequestHandlerInterface::class); + $this->assertInstanceOf(BadRequestHandler::class, $requestHandler); + } + + private function createContainer(): Container + { + return new Container( + ContainerConfig::create()->withDefinitions([ + ResponseFactoryInterface::class => ResponseFactory::class, + ...$this->getContainerDefinitions(), + ]) + ); + } + + private function getContainerDefinitions(): array + { + return require dirname(__DIR__) . '/config/di-web.php'; + } +} diff --git a/tests/RequestBodyParsersTest.php b/tests/RequestBodyParsersTest.php index e591c59..d5acb7a 100644 --- a/tests/RequestBodyParsersTest.php +++ b/tests/RequestBodyParsersTest.php @@ -13,8 +13,10 @@ use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Server\RequestHandlerInterface; use Yiisoft\Http\Header; use Yiisoft\Http\Status; +use Yiisoft\Request\Body\BadRequestHandler; use Yiisoft\Request\Body\BadRequestHandlerInterface; use Yiisoft\Request\Body\Parser\JsonParser; use Yiisoft\Request\Body\ParserException; @@ -32,14 +34,14 @@ public function testWithParser(): void $container = $this->getContainerWithParser($containerId, $expectedOutput); $mimeType = 'test/test'; - $bodyParser = $this - ->getRequestBodyParser($container) + $bodyParser = $this->getRequestBodyParser($container) ->withParser($mimeType, $containerId); + $catcher = new RequestCatcher(); - $requestHandler = $this->createHandler(); - $bodyParser->process($this->createMockRequest($mimeType), $requestHandler); + $bodyParser->process($this->createMockRequest($mimeType), $catcher); + $request = $catcher->getRequest(); - $this->assertSame($expectedOutput, $requestHandler->getRequestParsedBody()); + $this->assertSame($expectedOutput, $request->getParsedBody()); } public function testWithParserWithEmptyParserClass(): void @@ -49,53 +51,36 @@ public function testWithParserWithEmptyParserClass(): void $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('The parser class cannot be an empty string.'); - $this - ->getRequestBodyParser($container) + $this->getRequestBodyParser($container) ->withParser('content/future', ''); } public function testWithoutParsers(): void { - $container = $this->getContainerWithResponseFactory(); - $bodyParser = $this - ->getRequestBodyParser($container) + $bodyParser = $this->getRequestBodyParser($this->getContainer()) ->withoutParsers(); $rawBody = '{"test":"value"}'; + $catcher = new RequestCatcher(); - $requestHandler = $this->createHandler(); - $bodyParser->process($this->createMockRequest('application/json', $rawBody), $requestHandler); + $bodyParser->process($this->createMockRequest('application/json', $rawBody), $catcher); + $request = $catcher->getRequest(); - $this->assertNull($requestHandler->getRequestParsedBody()); + $this->assertNull($request->getParsedBody()); } public function testWithoutParser(): void { - $container = $this->getContainerWithResponseFactory(); - $bodyParser = $this - ->getRequestBodyParser($container) + $bodyParser = $this->getRequestBodyParser($this->getContainer()) ->withoutParsers('application/json'); $rawBody = '{"test":"value"}'; + $catcher = new RequestCatcher(); - $requestHandler = $this->createHandler(); - $bodyParser->process($this->createMockRequest('application/json', $rawBody), $requestHandler); - - $this->assertNull($requestHandler->getRequestParsedBody()); - } - - public function testWithBadRequestResponse(): void - { - $container = $this->getContainerWithResponseFactory(); - $bodyParser = $this->getRequestBodyParser($container); - - $rawBody = '{"test": invalid json}'; - - $requestHandler = $this->createHandler(); - $response = $bodyParser->process($this->createMockRequest('application/json', $rawBody), $requestHandler); + $bodyParser->process($this->createMockRequest('application/json', $rawBody), $catcher); + $request = $catcher->getRequest(); - $this->assertSame(Status::BAD_REQUEST, $response->getStatusCode()); - $this->assertSame(Status::TEXTS[Status::BAD_REQUEST] . "\nInvalid JSON data in request body: Syntax error", (string) $response->getBody()); + $this->assertNull($request->getParsedBody()); } public function testWithoutBadRequestResponse(): void @@ -103,29 +88,40 @@ public function testWithoutBadRequestResponse(): void $containerId = 'test'; $container = $this->getContainerWithParser($containerId, throwException: true); $mimeType = 'test/test'; - $bodyParser = $this - ->getRequestBodyParser($container) - ->withParser($mimeType, $containerId) - ->ignoreBadRequestBody(); + $bodyParser = $this->getRequestBodyParser($container) + ->withParser($mimeType, $containerId); + $catcher = new RequestCatcher(); - $requestHandler = $this->createHandler(); - $response = $bodyParser->process($this->createMockRequest($mimeType), $requestHandler); + $response = $bodyParser->process($this->createMockRequest($mimeType), $catcher); $this->assertSame(Status::OK, $response->getStatusCode()); } - public function testWithCustomBadRequestResponse(): void + public function testWithDefaultBadRequestResponse(): void { - $container = $this->getContainerWithResponseFactory(); + $badResponseHandler = $this->createDefaultBadResponseHandler(); + $bodyParser = $this->getRequestBodyParser($this->getContainer(), $badResponseHandler); + + $rawBody = '{"test": invalid json}'; + $catcher = new RequestCatcher(); + + $response = $bodyParser->process($this->createMockRequest('application/json', $rawBody), $catcher); + + $this->assertSame(Status::BAD_REQUEST, $response->getStatusCode()); + $this->assertSame(Status::TEXTS[Status::BAD_REQUEST] . "\n" . 'Invalid JSON data in request body: Syntax error', (string) $response->getBody()); + } + public function testWithCustomBadRequestResponse(): void + { $customBody = 'custom response'; $badResponseHandler = $this->createCustomBadResponseHandler($customBody); - $bodyParser = $this->getRequestBodyParser($container, $badResponseHandler); + $bodyParser = $this->getRequestBodyParser($this->getContainer(), $badResponseHandler); $rawBody = '{"test": invalid json}'; - $requestHandler = $this->createHandler(); - $response = $bodyParser->process($this->createMockRequest('application/json', $rawBody), $requestHandler); + $catcher = new RequestCatcher(); + + $response = $bodyParser->process($this->createMockRequest('application/json', $rawBody), $catcher); $this->assertSame(Status::BAD_REQUEST, $response->getStatusCode()); $this->assertSame($customBody, (string) $response->getBody()); @@ -136,8 +132,7 @@ public function testThrownExceptionWithNotExistsParser(): void $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('The parser "invalidParser" cannot be found.'); - $this - ->getRequestBodyParser($this->getContainerWithResponseFactory()) + $this->getRequestBodyParser($this->getContainer()) ->withParser('test/test', 'invalidParser'); } @@ -149,8 +144,7 @@ public function testThrownExceptionWithInvalidMimeType(): void $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Invalid mime type.'); - $this - ->getRequestBodyParser($container) + $this->getRequestBodyParser($container) ->withParser('invalid mimeType', $containerId); } @@ -163,19 +157,12 @@ public function testThrownExceptionWithInvalidMimeType(): void #[TestWith(['myapp/json', 'MYAPP/JSON'])] public function testMimeTypeNormalization(string $parserType, string $contentType): void { - $middleware = (new RequestBodyParser( - new ResponseFactory(), - new SimpleContainer([ - JsonParser::class => new JsonParser(), - ]) - ))->withParser($parserType, JsonParser::class); - $request = new ServerRequest( - headers: [Header::CONTENT_TYPE => $contentType], - body: (new StreamFactory())->createStream('{"test":"value"}'), - ); + $middleware = $this->getRequestBodyParser($this->getContainer()) + ->withParser($parserType, JsonParser::class); + $rawBody = '{"test":"value"}'; $catcher = new RequestCatcher(); - $middleware->process($request, $catcher); + $middleware->process($this->createMockRequest($contentType, $rawBody), $catcher); $request = $catcher->getRequest(); $this->assertSame(['test' => 'value'], $request->getParsedBody()); @@ -183,10 +170,7 @@ public function testMimeTypeNormalization(string $parserType, string $contentTyp public function testRequestWithoutContentType(): void { - $middleware = new RequestBodyParser( - new ResponseFactory(), - new SimpleContainer([JsonParser::class => new JsonParser()]), - ); + $middleware = $this->getRequestBodyParser($this->getContainer()); $request = new ServerRequest( body: (new StreamFactory())->createStream('{"test":"value"}'), ); @@ -201,25 +185,16 @@ public function testRequestWithoutContentType(): void public function testImmutability(): void { - $middleware = new RequestBodyParser( - new ResponseFactory(), - new SimpleContainer([JsonParser::class => new JsonParser()]), - ); - $this->assertNotSame($middleware, $middleware->ignoreBadRequestBody()); + $middleware = $this->getRequestBodyParser($this->getContainer()); $this->assertNotSame($middleware, $middleware->withoutParsers()); $this->assertNotSame($middleware, $middleware->withParser('myapp/json', JsonParser::class)); } - private function getContainerWithResponseFactory(): SimpleContainer + private function getContainer(): SimpleContainer { - return new SimpleContainer( - [ - ResponseFactoryInterface::class => static function () { - return new ResponseFactory(); - }, - JsonParser::class => new JsonParser(), - ] - ); + return new SimpleContainer([ + JsonParser::class => new JsonParser(), + ]); } private function getContainerWithParser( @@ -229,7 +204,6 @@ private function getContainerWithParser( ): SimpleContainer { return new SimpleContainer( [ - ResponseFactoryInterface::class => $this->createMock(ResponseFactoryInterface::class), $id => new MockParser($expectedOutput, $throwException), ] ); @@ -245,14 +219,14 @@ private function createMockRequest(string $contentType, string|null $rawBody = n ); } - private function createHandler(): BadRequestHandlerInterface + private function createHandler(): RequestHandlerInterface { $mockResponse = $this->createMock(ResponseInterface::class); $mockResponse ->method('getStatusCode') ->willReturn(Status::OK); - return new class ($mockResponse) implements BadRequestHandlerInterface { + return new class ($mockResponse) implements RequestHandlerInterface { private array|object|null $requestParsedBody = null; public function __construct( @@ -270,12 +244,6 @@ public function getRequestParsedBody(): array|object|null { return $this->requestParsedBody; } - - public function withParserException(ParserException $e): BadRequestHandlerInterface - { - // do nothing - return $this; - } }; } @@ -283,7 +251,12 @@ private function getRequestBodyParser( SimpleContainer $container, BadRequestHandlerInterface|null $badRequestHandler = null ): RequestBodyParser { - return new RequestBodyParser(new ResponseFactory(), $container, $badRequestHandler); + return new RequestBodyParser($container, $badRequestHandler); + } + + private function createDefaultBadResponseHandler(): BadRequestHandler + { + return new BadRequestHandler(new ResponseFactory()); } private function createCustomBadResponseHandler(string $body): BadRequestHandlerInterface @@ -295,18 +268,12 @@ public function __construct( ) { } - public function handle(ServerRequestInterface $request): ResponseInterface + public function handle(ServerRequestInterface $request, ?ParserException $e = null): ResponseInterface { $response = $this->responseFactory->createResponse(Status::BAD_REQUEST); $response->getBody()->write($this->body); return $response; } - - public function withParserException(ParserException $e): BadRequestHandlerInterface - { - // do nothing - return $this; - } }; } }