Merge pull request #25 from BigZ/feat/enable-error-body-only-in-debug…

…-mode

feature: Obfuscate exceptions that are not http exceptions

.travis.yml

@@ -3,6 +3,7 @@ language: php
 php:
   - 7.1
   - 7.2
+  - 7.3
 
 install:
   - composer install

Controller/JsonControllerTrait.php

@@ -2,6 +2,7 @@
 
 namespace Wizards\RestBundle\Controller;
 
+use InvalidArgumentException;
 use Symfony\Component\Form\FormInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Wizards\RestBundle\Exception\MultiPartHttpException;
@@ -27,11 +28,11 @@ protected function handleJsonForm(FormInterface $form, Request $request)
         $body = $this->decode($request, $form);
 
         if (empty($body)) {
-            throw new \InvalidArgumentException('invalid, empty or not json/jsonapi body provided');
+            throw new InvalidArgumentException('invalid, empty or not json/jsonapi body provided');
         }
 
         if (empty($body[$form->getName()])) {
-            throw new \InvalidArgumentException(sprintf('json should contain a %s key', $form->getName()));
+            throw new InvalidArgumentException(sprintf('json should contain a %s key', $form->getName()));
         }
 
         $form->submit($body[$form->getName()], $request->getMethod() !== 'PATCH');
@@ -40,7 +41,7 @@ protected function handleJsonForm(FormInterface $form, Request $request)
     private function decode(Request $request, FormInterface $form): array
     {
         if ('application/json' === $request->headers->get('Content-Type')) {
-            return json_decode($request->getContent(), true);
+            return \json_decode($request->getContent(), true);
         }
 
         if ('application/vnd.api+json' === $request->headers->get('Content-Type')) {
@@ -52,14 +53,14 @@ private function decode(Request $request, FormInterface $form): array
 
     private function decodeJsonApi(string $content, FormInterface $form): array
     {
-        $jsonApi = json_decode($content, true);
+        $jsonApi = \json_decode($content, true);
 
         if (empty($jsonApi)) {
             return [];
         }
 
         $fields = isset($jsonApi['data']['id'])
-            ? array_merge(['id' => $jsonApi['data']['id']], $jsonApi['data']['attributes'])
+            ? \array_merge(['id' => $jsonApi['data']['id']], $jsonApi['data']['attributes'])
             : $jsonApi['data']['attributes'];
 
         if (isset($jsonApi['relationships']) && is_array($jsonApi['relationships'])) {
@@ -90,7 +91,7 @@ private function convertFormErrorsToArray(FormInterface $form)
             $childrenErrors = $this->convertFormErrorsToArray($child);
 
             foreach ($childrenErrors as $childrenError) {
-                $errors[] = sprintf('%s: %s', $key, $childrenError);
+                $errors[] = \sprintf('%s: %s', $key, $childrenError);
             }
         }
 

Resources/config/services.yml

@@ -57,6 +57,7 @@ services:
             - { name: kernel.event_subscriber }
         arguments:
             - '@logger'
+            - '@wizards_rest.format_options_getter'
 
     wizards_rest.psr7_param_converter:
         class: Wizards\RestBundle\ParamConverter\Psr7ParamConverter

Subscriber/ExceptionSubscriber.php

@@ -8,11 +8,14 @@
 use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
+use Throwable;
 use Wizards\RestBundle\Exception\MultiPartHttpException;
+use Wizards\RestBundle\Services\FormatOptions;
 use WizardsRest\Exception\HttpException;
+use WizardsRest\Serializer;
 
 /**
- * Serializes a controller output to a configured format response.
+ * When an exception happen, this subscriber helps to serialize it the rest way.
  */
 class ExceptionSubscriber implements EventSubscriberInterface
 {
@@ -21,60 +24,87 @@ class ExceptionSubscriber implements EventSubscriberInterface
      */
     private $logger;
 
-    public function __construct(LoggerInterface $logger)
+    /**
+     * @var FormatOptions
+     */
+    private $formatOptions;
+
+    public function __construct(LoggerInterface $logger, FormatOptions $formatOptions)
     {
         $this->logger = $logger;
+        $this->formatOptions = $formatOptions;
     }
 
-    /**
-     * @return array
-     */
     public static function getSubscribedEvents(): array
     {
         return [
             KernelEvents::EXCEPTION => 'onKernelException'
         ];
     }
 
-    /**
-     * @param GetResponseForExceptionEvent $event
-     */
     public function onKernelException(GetResponseForExceptionEvent $event)
     {
         $exception = $event->getException();
 
         $this->logger->log('error', $exception->getMessage());
 
         $response = new Response();
-        $response->setContent(json_encode(['errors' => $this->getErrorBody($exception)]));
         $response->setStatusCode(Response::HTTP_INTERNAL_SERVER_ERROR);
+        $response->setContent('Internal Server Error');
 
         if ($exception instanceof HttpExceptionInterface || $exception instanceof HttpException) {
+            $content = $this->getErrorResponseContent($exception);
+
             $response->setStatusCode($exception->getStatusCode());
-            $response->headers->replace(['content-type' => 'application/vnd.api+json']);
+
+            if ($content !== null) {
+                $response->setContent($content);
+            }
         }
 
+        $response->headers->replace($this->formatOptions->getFormatSpecificHeaders());
+
         $event->setResponse($response);
     }
 
     /**
-     * Formats the error body.
-     *
-     * @param \Exception $exception
+     * @param HttpExceptionInterface|HttpException $exception
      *
-     * @return array
+     * @return null|string
      */
-    private function getErrorBody($exception)
+    private function getErrorResponseContent($exception): ?string
     {
-        if ($exception instanceof MultiPartHttpException) {
-            return array_map(
+        $errorMessages = $this->getErrorMessages($exception);
+
+        // If the error has no specific text, use the common text for this code
+        if (!$errorMessages[0] && isset(Response::$statusTexts[$exception->getStatusCode()])) {
+            $errorMessages = [Response::$statusTexts[$exception->getStatusCode()]];
+        }
+
+        if (Serializer::SPEC_JSONAPI === $this->formatOptions->getSpecification()) {
+            $errorMessages = \array_map(
                 function ($error) {
                     return ['detail' => $error];
                 },
-                $exception->getMessageList()
+                $errorMessages
             );
         }
 
-        return [['detail' => $exception->getMessage()]];
+        $encodedContent = \json_encode(['errors' => $errorMessages]);
+
+        if ($encodedContent === false) {
+            return null;
+        }
+
+        return $encodedContent;
+    }
+
+    private function getErrorMessages($exception): array
+    {
+        if ($exception instanceof MultiPartHttpException) {
+            return $exception->getMessageList();
+        }
+
+        return [$exception->getMessage()];
     }
 }

composer.json

@@ -25,7 +25,8 @@
     "squizlabs/php_codesniffer": "^3.4",
     "phpmd/phpmd": "^2.6",
     "phpunit/phpunit": "^7.5",
-    "symfony/phpunit-bridge": "^4.2"
+    "symfony/phpunit-bridge": "^4.2",
+    "symfony/var-dumper": "^4.3"
   },
   "autoload": {
     "psr-0": { "Wizards\\RestBundle": "" }

tests/Subsriber/ExceptionSubscriberTest.php

@@ -0,0 +1,75 @@
+<?php
+
+namespace WizardsTest\ObjectManager;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
+use Wizards\RestBundle\Exception\MultiPartHttpException;
+use Wizards\RestBundle\Services\FormatOptions;
+use Wizards\RestBundle\Subscriber\ExceptionSubscriber;
+use Psr\Log\LoggerInterface;
+use WizardsRest\Exception\HttpException;
+
+class ExceptionSubscriberTest extends TestCase
+{
+    public function testFormatJsonApi404()
+    {
+        $logger = $this->createMock(LoggerInterface::class);
+        $logger->expects($this->once())->method('log');
+        $formatOptions = new FormatOptions('jsonapi');
+        $event = $this->createMock(GetResponseForExceptionEvent::class);
+        $exception = $this->createMock(HttpException::class);
+
+        $exception->method('getStatusCode')->willReturn(404);
+        $event->method('getException')->willReturn($exception);
+        $response = new Response(
+            '{"errors":[{"detail":"Not Found"}]}',
+            404,
+            $formatOptions->getFormatSpecificHeaders()
+        );
+        $event->expects($this->once())->method('setResponse')->with($response);
+
+        $subscriber = new ExceptionSubscriber($logger, $formatOptions);
+        $subscriber->onKernelException($event);
+    }
+
+    public function testFormatJsonApi400()
+    {
+        $logger = $this->createMock(LoggerInterface::class);
+        $logger->expects($this->once())->method('log');
+        $formatOptions = new FormatOptions('jsonapi');
+        $event = $this->createMock(GetResponseForExceptionEvent::class);
+        $exception = new MultiPartHttpException(400, ['one', 'two']);
+        $event->method('getException')->willReturn($exception);
+        $response = new Response(
+            '{"errors":[{"detail":"one"},{"detail":"two"}]}',
+            400,
+            $formatOptions->getFormatSpecificHeaders()
+        );
+        $event->expects($this->once())->method('setResponse')->with($response);
+
+        $subscriber = new ExceptionSubscriber($logger, $formatOptions);
+        $subscriber->onKernelException($event);
+    }
+
+    public function testObfuscateError()
+    {
+        $logger = $this->createMock(LoggerInterface::class);
+        $formatOptions = new FormatOptions('jsonapi');
+        $event = $this->createMock(GetResponseForExceptionEvent::class);
+
+        $exception = new \RuntimeException('internal problems');
+
+        $event->method('getException')->willReturn($exception);
+        $response = new Response(
+            'Internal Server Error',
+            500,
+            $formatOptions->getFormatSpecificHeaders()
+        );
+        $event->expects($this->once())->method('setResponse')->with($response);
+
+        $subscriber = new ExceptionSubscriber($logger, $formatOptions);
+        $subscriber->onKernelException($event);
+    }
+}