Merge pull request #1650 from acelaya-forks/feature/handle-malformed-body

Feature/handle malformed body

CHANGELOG.md

@@ -18,7 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 * *Nothing*
 
 ### Fixed
-* *Nothing*
+* [#1639](https://github.com/shlinkio/shlink/issues/1639) Fixed 500 error returned when request body is not valid JSON, instead of a proper descriptive error.
 
 
 ## [3.4.0] - 2022-12-16

module/CLI/src/Exception/GeolocationDbUpdateFailedException.php

@@ -15,7 +15,7 @@ class GeolocationDbUpdateFailedException extends RuntimeException implements Exc
 
     private function __construct(string $message, ?Throwable $previous = null)
     {
-        parent::__construct($message, 0, $previous);
+        parent::__construct($message, previous: $previous);
     }
 
     public static function withOlderDb(?Throwable $prev = null): self

module/Core/src/Exception/MalformedBodyException.php

@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Exception;
+
+use Fig\Http\Message\StatusCodeInterface;
+use JsonException;
+use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait;
+use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface;
+
+use function Shlinkio\Shlink\Core\toProblemDetailsType;
+
+class MalformedBodyException extends InvalidArgumentException implements ProblemDetailsExceptionInterface
+{
+    use CommonProblemDetailsExceptionTrait;
+
+    public static function forInvalidJson(JsonException $prev): self
+    {
+        $e = new self('Provided request does not contain a valid JSON body.', previous: $prev);
+
+        $e->detail = $e->getMessage();
+        $e->title = 'Malformed request body';
+        $e->type = toProblemDetailsType('malformed-request-body');
+        $e->status = StatusCodeInterface::STATUS_BAD_REQUEST;
+
+        return $e;
+    }
+}

module/Core/test/Exception/MalformedBodyExceptionTest.php

@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Core\Exception;
+
+use Fig\Http\Message\StatusCodeInterface;
+use JsonException;
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Core\Exception\MalformedBodyException;
+
+class MalformedBodyExceptionTest extends TestCase
+{
+    /** @test */
+    public function createsExpectedException(): void
+    {
+        $prev = new JsonException();
+        $e = MalformedBodyException::forInvalidJson($prev);
+
+        self::assertEquals($prev, $e->getPrevious());
+        self::assertEquals('Provided request does not contain a valid JSON body.', $e->getMessage());
+        self::assertEquals('Provided request does not contain a valid JSON body.', $e->getDetail());
+        self::assertEquals('Malformed request body', $e->getTitle());
+        self::assertEquals('https://shlink.io/api/error/malformed-request-body', $e->getType());
+        self::assertEquals(StatusCodeInterface::STATUS_BAD_REQUEST, $e->getStatus());
+    }
+}

module/Rest/src/Middleware/BodyParserMiddleware.php

@@ -5,10 +5,12 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Rest\Middleware;
 
 use Fig\Http\Message\RequestMethodInterface;
+use JsonException;
 use Psr\Http\Message\ResponseInterface as Response;
 use Psr\Http\Message\ServerRequestInterface as Request;
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
+use Shlinkio\Shlink\Core\Exception\MalformedBodyException;
 
 use function Functional\contains;
 use function Shlinkio\Shlink\Common\json_decode;
@@ -42,7 +44,11 @@ class BodyParserMiddleware implements MiddlewareInterface, RequestMethodInterfac
             return $request;
         }
 
-        $parsedJson = json_decode($rawBody);
-        return $request->withParsedBody($parsedJson);
+        try {
+            $parsedJson = json_decode($rawBody);
+            return $request->withParsedBody($parsedJson);
+        } catch (JsonException $e) {
+            throw MalformedBodyException::forInvalidJson($e);
+        }
     }
 }

module/Rest/test-api/Middleware/BodyParserTest.php

@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioApiTest\Shlink\Rest\Middleware;
+
+use GuzzleHttp\RequestOptions;
+use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
+
+class BodyParserTest extends ApiTestCase
+{
+    /** @test */
+    public function returnsErrorWhenRequestBodyIsInvalidJson(): void
+    {
+        $resp = $this->callApiWithKey(self::METHOD_POST, '/short-urls', [
+            RequestOptions::HEADERS => ['content-type' => 'application/json'],
+            RequestOptions::BODY => '{"foo',
+        ]);
+        $payload = $this->getJsonResponsePayload($resp);
+
+        self::assertEquals(400, $resp->getStatusCode());
+        self::assertEquals(400, $payload['status']);
+        self::assertEquals('Provided request does not contain a valid JSON body.', $payload['detail']);
+        self::assertEquals('Malformed request body', $payload['title']);
+        self::assertEquals('https://shlink.io/api/error/malformed-request-body', $payload['type']);
+    }
+}

module/Rest/test/Middleware/BodyParserMiddlewareTest.php

@@ -7,10 +7,12 @@ namespace ShlinkioTest\Shlink\Rest\Middleware;
 use Laminas\Diactoros\Response;
 use Laminas\Diactoros\ServerRequest;
 use Laminas\Diactoros\Stream;
+use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\RequestHandlerInterface;
+use Shlinkio\Shlink\Core\Exception\MalformedBodyException;
 use Shlinkio\Shlink\Rest\Middleware\BodyParserMiddleware;
 
 class BodyParserMiddlewareTest extends TestCase
@@ -65,7 +67,6 @@ class BodyParserMiddlewareTest extends TestCase
     /** @test */
     public function jsonRequestsAreJsonDecoded(): void
     {
-        $test = $this;
         $body = new Stream('php://temp', 'wr');
         $body->write('{"foo": "bar", "bar": ["one", 5]}');
         $request = (new ServerRequest())->withMethod('PUT')
@@ -73,16 +74,31 @@ class BodyParserMiddlewareTest extends TestCase
         $handler = $this->createMock(RequestHandlerInterface::class);
         $handler->expects($this->once())->method('handle')->with(
             $this->isInstanceOf(ServerRequestInterface::class),
-        )->willReturnCallback(
-            function (ServerRequestInterface $req) use ($test) {
-                $test->assertEquals([
-                    'foo' => 'bar',
-                    'bar' => ['one', 5],
-                ], $req->getParsedBody());
+        )->willReturnCallback(function (ServerRequestInterface $req) {
+            Assert::assertEquals([
+                'foo' => 'bar',
+                'bar' => ['one', 5],
+            ], $req->getParsedBody());
 
-                return new Response();
-            },
-        );
+            return new Response();
+        });
+
+        $this->middleware->process($request, $handler);
+    }
+
+    /** @test */
+    public function invalidBodyResultsInException(): void
+    {
+        $body = new Stream('php://temp', 'wr');
+        $body->write('{"foo": "bar", "bar": ["one');
+        $request = (new ServerRequest())->withMethod('PUT')
+                                        ->withBody($body);
+
+        $handler = $this->createMock(RequestHandlerInterface::class);
+        $handler->expects($this->never())->method('handle');
+
+        $this->expectException(MalformedBodyException::class);
+        $this->expectExceptionMessage('Provided request does not contain a valid JSON body.');
 
         $this->middleware->process($request, $handler);
     }