Merge pull request #978 from jordscream/not_expose_sensitive_information

Do not expose sensitive information

Author: dunglas

src/Action/ExceptionAction.php

@@ -66,9 +66,11 @@ public function __construct(SerializerInterface $serializer, array $errorFormats
     public function __invoke(FlattenException $exception, Request $request): Response
     {
         $exceptionClass = $exception->getClass();
+        $statusCode = $exception->getStatusCode();
+
         foreach ($this->exceptionToStatus as $class => $status) {
             if (is_a($exceptionClass, $class, true)) {
-                $exception->setStatusCode($status);
+                $statusCode = $status;
 
                 break;
             }
@@ -80,6 +82,6 @@ public function __invoke(FlattenException $exception, Request $request): Respons
         $headers['X-Content-Type-Options'] = 'nosniff';
         $headers['X-Frame-Options'] = 'deny';
 
-        return new Response($this->serializer->serialize($exception, $format['key']), $exception->getStatusCode(), $headers);
+        return new Response($this->serializer->serialize($exception, $format['key'], ['statusCode' => $statusCode]), $statusCode, $headers);
     }
 }

src/Hydra/Serializer/ErrorNormalizer.php

@@ -13,6 +13,7 @@
 
 use ApiPlatform\Core\Api\UrlGeneratorInterface;
 use Symfony\Component\Debug\Exception\FlattenException;
+use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
 
 /**
@@ -42,6 +43,11 @@ public function normalize($object, $format = null, array $context = [])
         $message = $object->getMessage();
         if ($this->debug) {
             $trace = $object->getTrace();
+        } elseif ($object instanceof FlattenException) {
+            $statusCode = $context['statusCode'] ?? $object->getStatusCode();
+            if ($statusCode >= 500 && $statusCode < 600) {
+                $message = Response::$statusTexts[$statusCode];
+            }
         }
 
         $data = [

tests/Action/ExceptionActionTest.php

@@ -33,7 +33,7 @@ public function testActionWithCatchableException()
         $flattenException = FlattenException::create($serializerException->reveal());
 
         $serializer = $this->prophesize(SerializerInterface::class);
-        $serializer->serialize($flattenException, 'jsonproblem')->willReturn();
+        $serializer->serialize($flattenException, 'jsonproblem', ['statusCode' => Response::HTTP_BAD_REQUEST])->willReturn();
 
         $exceptionAction = new ExceptionAction($serializer->reveal(), ['jsonproblem' => ['application/problem+json'], 'jsonld' => ['application/ld+json']], [ExceptionInterface::class => Response::HTTP_BAD_REQUEST, InvalidArgumentException::class => Response::HTTP_BAD_REQUEST]);
 
@@ -57,7 +57,7 @@ public function testActionWithUncatchableException()
         $flattenException = FlattenException::create($serializerException->reveal());
 
         $serializer = $this->prophesize(SerializerInterface::class);
-        $serializer->serialize($flattenException, 'jsonproblem')->willReturn();
+        $serializer->serialize($flattenException, 'jsonproblem', ['statusCode' => $flattenException->getStatusCode()])->willReturn();
 
         $exceptionAction = new ExceptionAction($serializer->reveal(), ['jsonproblem' => ['application/problem+json'], 'jsonld' => ['application/ld+json']]);
 

tests/Hydra/Serializer/ErrorNormalizerTest.php

@@ -14,6 +14,7 @@
 use ApiPlatform\Core\Api\UrlGeneratorInterface;
 use ApiPlatform\Core\Hydra\Serializer\ErrorNormalizer;
 use Symfony\Component\Debug\Exception\FlattenException;
+use Symfony\Component\HttpFoundation\Response;
 
 /**
  * @author Kévin Dunglas <[email protected]>
@@ -35,6 +36,47 @@ public function testSupportNormalization()
         $this->assertFalse($normalizer->supportsNormalization(new \stdClass(), ErrorNormalizer::FORMAT));
     }
 
+    /**
+     * @dataProvider providerStatusCode
+     *
+     * @param $status            http status code of the Exception
+     * @param $originalMessage   original message of the Exception
+     * @param $debug             simulates kernel debug variable
+     */
+    public function testErrorServerNormalize($status, $originalMessage, $debug)
+    {
+        $urlGeneratorProphecy = $this->prophesize(UrlGeneratorInterface::class);
+        $urlGeneratorProphecy->generate('api_jsonld_context', ['shortName' => 'Error'])->willReturn('/context/foo')->shouldBeCalled();
+
+        $normalizer = new ErrorNormalizer($urlGeneratorProphecy->reveal(), $debug);
+        $exception = FlattenException::create(new \Exception($originalMessage), $status);
+
+        $expected = [
+            '@context' => '/context/foo',
+            '@type' => 'hydra:Error',
+            'hydra:title' => 'An error occurred',
+            'hydra:description' => ($debug || $status < 500) ? $originalMessage : Response::$statusTexts[$status],
+        ];
+
+        if ($debug) {
+            $expected['trace'] = $exception->getTrace();
+        }
+
+        $this->assertEquals($expected, $normalizer->normalize($exception, null, ['statusCode' => $status]));
+    }
+
+    public function providerStatusCode()
+    {
+        return [
+            [Response::HTTP_INTERNAL_SERVER_ERROR, 'Sensitive SQL error displayed', false],
+            [Response::HTTP_GATEWAY_TIMEOUT, 'Sensitive server error displayed', false],
+            [Response::HTTP_BAD_REQUEST, 'Bad Request Message', false],
+            [Response::HTTP_INTERNAL_SERVER_ERROR, 'Sensitive SQL error displayed', true],
+            [Response::HTTP_GATEWAY_TIMEOUT, 'Sensitive server error displayed', true],
+            [Response::HTTP_BAD_REQUEST, 'Bad Request Message', true],
+        ];
+    }
+
     public function testNormalize()
     {
         $urlGeneratorProphecy = $this->prophesize(UrlGeneratorInterface::class);