[TASK] Stop JsonView from setting headers 78/66078/5
authorAlexander Schnitzler <git@alexanderschnitzler.de>
Thu, 8 Oct 2020 05:28:47 +0000 (07:28 +0200)
committerBenni Mack <benni@typo3.org>
Wed, 28 Oct 2020 23:00:31 +0000 (00:00 +0100)
JsonView did set content type headers in its render method which
is a problem since the view should only be responsible for rendering
the content of the to be returned response.

The current approach, setting those headers in the action controller,
is just a temporary solution until Extbase is capable of handling
PSR-7 responses which then makes it superfluous to set headers
magically. Instead people will need to set headers manually or use
a JsonResponse object instead and let the core handle setting headers
further down in the request/response handling stack.

Releases: master
Resolves: #92692
Change-Id: Idfa417cfd25d7f692dea2e6d6822f1d3a44cb38d
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/66078
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Oliver Bartsch <bo@cedev.de>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Oliver Bartsch <bo@cedev.de>
Reviewed-by: Benni Mack <benni@typo3.org>
typo3/sysext/extbase/Classes/Mvc/Controller/ActionController.php
typo3/sysext/extbase/Classes/Mvc/View/JsonView.php
typo3/sysext/extbase/Tests/Unit/Mvc/View/JsonViewTest.php

index ea9f0cb..8661dac 100644 (file)
@@ -33,6 +33,7 @@ use TYPO3\CMS\Extbase\Mvc\RequestInterface;
 use TYPO3\CMS\Extbase\Mvc\Response;
 use TYPO3\CMS\Extbase\Mvc\ResponseInterface;
 use TYPO3\CMS\Extbase\Mvc\View\GenericViewResolver;
+use TYPO3\CMS\Extbase\Mvc\View\JsonView;
 use TYPO3\CMS\Extbase\Mvc\View\NotFoundView;
 use TYPO3\CMS\Extbase\Mvc\View\ViewInterface;
 use TYPO3\CMS\Extbase\Mvc\View\ViewResolverInterface;
@@ -49,6 +50,7 @@ use TYPO3\CMS\Extbase\Validation\Validator\ConjunctionValidator;
 use TYPO3\CMS\Extbase\Validation\Validator\ValidatorInterface;
 use TYPO3\CMS\Extbase\Validation\ValidatorResolver;
 use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
+use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
 use TYPO3Fluid\Fluid\View\TemplateView;
 
 /**
@@ -513,6 +515,28 @@ class ActionController implements ControllerInterface
         }
 
         if ($actionResult === null && $this->view instanceof ViewInterface) {
+            if ($this->view instanceof JsonView) {
+                // this is just a temporary solution until Extbase uses PSR-7 responses and users are forced to return a
+                // response object in their controller actions.
+
+                if (!empty($GLOBALS['TSFE']) && $GLOBALS['TSFE'] instanceof TypoScriptFrontendController) {
+                    /** @var TypoScriptFrontendController $typoScriptFrontendController */
+                    $typoScriptFrontendController = $GLOBALS['TSFE'];
+                    if (empty($typoScriptFrontendController->config['config']['disableCharsetHeader'])) {
+                        // If the charset header is *not* disabled in configuration,
+                        // TypoScriptFrontendController will send the header later with the Content-Type which we set here.
+                        $typoScriptFrontendController->setContentType('application/json');
+                    } else {
+                        // Although the charset header is disabled in configuration, we *must* send a Content-Type header here.
+                        // Content-Type headers optionally carry charset information at the same time.
+                        // Since we have the information about the charset, there is no reason to not include the charset information although disabled in TypoScript.
+                        $this->response->setHeader('Content-Type', 'application/json; charset=' . trim($typoScriptFrontendController->metaCharset));
+                    }
+                } else {
+                    $this->response->setHeader('Content-Type', 'application/json');
+                }
+            }
+
             $this->response->appendContent($this->view->render());
         } elseif (is_string($actionResult) && $actionResult !== '') {
             $this->response->appendContent($actionResult);
index 8923352..22d93a0 100644 (file)
@@ -19,8 +19,6 @@ namespace TYPO3\CMS\Extbase\Mvc\View;
 
 use TYPO3\CMS\Extbase\Persistence\PersistenceManagerInterface;
 use TYPO3\CMS\Extbase\Reflection\ObjectAccess;
-use TYPO3\CMS\Extbase\Reflection\ReflectionService;
-use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
 
 /**
  * A JSON view
@@ -43,16 +41,6 @@ class JsonView extends AbstractView
      */
     const EXPOSE_CLASSNAME_UNQUALIFIED = 2;
 
-    /**
-     * @var ReflectionService
-     */
-    protected $reflectionService;
-
-    /**
-     * @var \TYPO3\CMS\Extbase\Mvc\Controller\ControllerContext
-     */
-    protected $controllerContext;
-
     /**
      * Only variables whose name is contained in this array will be rendered
      *
@@ -174,15 +162,6 @@ class JsonView extends AbstractView
         $this->persistenceManager = $persistenceManager;
     }
 
-    /**
-     * @param ReflectionService $reflectionService
-     * @internal
-     */
-    public function injectReflectionService(ReflectionService $reflectionService): void
-    {
-        $this->reflectionService = $reflectionService;
-    }
-
     /**
      * Specifies which variables this JsonView should render
      * By default only the variable 'value' will be rendered
@@ -211,24 +190,6 @@ class JsonView extends AbstractView
      */
     public function render(): string
     {
-        $response = $this->controllerContext->getResponse();
-        // @todo Ticket: #63643 This should be solved differently once request/response model is available for TSFE.
-        if (!empty($GLOBALS['TSFE']) && $GLOBALS['TSFE'] instanceof TypoScriptFrontendController) {
-            /** @var TypoScriptFrontendController $typoScriptFrontendController */
-            $typoScriptFrontendController = $GLOBALS['TSFE'];
-            if (empty($typoScriptFrontendController->config['config']['disableCharsetHeader'])) {
-                // If the charset header is *not* disabled in configuration,
-                // TypoScriptFrontendController will send the header later with the Content-Type which we set here.
-                $typoScriptFrontendController->setContentType('application/json');
-            } else {
-                // Although the charset header is disabled in configuration, we *must* send a Content-Type header here.
-                // Content-Type headers optionally carry charset information at the same time.
-                // Since we have the information about the charset, there is no reason to not include the charset information although disabled in TypoScript.
-                $response->setHeader('Content-Type', 'application/json; charset=' . trim($typoScriptFrontendController->metaCharset));
-            }
-        } else {
-            $response->setHeader('Content-Type', 'application/json');
-        }
         $propertiesToRender = $this->renderArray();
         return json_encode($propertiesToRender, JSON_UNESCAPED_UNICODE);
     }
index 1b5fd4f..56fdf7b 100644 (file)
@@ -18,11 +18,8 @@ declare(strict_types=1);
 namespace TYPO3\CMS\Extbase\Tests\Unit\Mvc\View;
 
 use TYPO3\CMS\Core\Utility\StringUtility;
-use TYPO3\CMS\Extbase\Mvc\Controller\ControllerContext;
-use TYPO3\CMS\Extbase\Mvc\Response;
 use TYPO3\CMS\Extbase\Mvc\View\JsonView;
 use TYPO3\CMS\Extbase\Persistence\Generic\PersistenceManager;
-use TYPO3\CMS\Extbase\Reflection\ReflectionService;
 use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 
 /**
@@ -37,16 +34,6 @@ class JsonViewTest extends UnitTestCase
      */
     protected $view;
 
-    /**
-     * @var ControllerContext
-     */
-    protected $controllerContext;
-
-    /**
-     * @var Response
-     */
-    protected $response;
-
     /**
      * Sets up this test case
      */
@@ -56,10 +43,6 @@ class JsonViewTest extends UnitTestCase
         $this->view = $this->getMockBuilder(JsonView::class)
             ->setMethods(['loadConfigurationFromYamlFile'])
             ->getMock();
-        $this->controllerContext = $this->createMock(ControllerContext::class);
-        $this->response = $this->createMock(Response::class);
-        $this->controllerContext->expects(self::any())->method('getResponse')->willReturn($this->response);
-        $this->view->setControllerContext($this->controllerContext);
     }
 
     /**
@@ -311,29 +294,12 @@ class JsonViewTest extends UnitTestCase
                 ],
             ],
         ];
-        $reflectionService = $this->getMockBuilder(ReflectionService::class)
-            ->setMethods([ 'getClassNameByObject' ])
-            ->getMock();
-        $reflectionService->expects(self::any())->method('getClassNameByObject')->willReturnCallback(function ($object) {
-            return get_class($object);
-        });
 
         $jsonView = $this->getAccessibleMock(JsonView::class, ['dummy'], [], '', false);
-        $jsonView->injectReflectionService($reflectionService);
         $actual = $jsonView->_call('transformValue', $object, $configuration);
         self::assertSame($expected, $actual);
     }
 
-    /**
-     * @test
-     */
-    public function renderSetsContentTypeHeader(): void
-    {
-        $this->response->expects(self::once())->method('setHeader')->with('Content-Type', 'application/json');
-
-        $this->view->render();
-    }
-
     /**
      * @test
      */