Backport of MAGETWO-71545 for Magento 2.1: Added 'application/json' C…

[hostep]

…ontent-Type to Ajax responses in the Magento_UI module. #10521

(cherry picked from commit ddfc01e)

Description

This is a backport of #10521 for Magento 2.1

Fixed Issues (if relevant)

1. Set correct Content-Type header for ajax responses in the Magento_UI module. #10521: Set correct Content-Type header for ajax responses in the Magento_UI module.
2. https://github.com/magento/up-for-grabs/issues/1: Newrelic Ignore Transaction

Contribution checklist

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[hostep]

I'll have a look at fixing the tests later today probably.

[hostep]

I've fixed the static test failure, but I failed at fixing the unit tests, it's because createMock doesn't exists in PHPUnit 4.x. I tried by replacing it with getMock or getMockForAbstractClass and some variations on that, but then I get another error which I don't know how to solve:

PHP Fatal error:  Uncaught TypeError: Argument 1 passed to Mock_UiComponentTypeResolver_c45134a1::resolve() must implement interface Magento\Framework\View\Element\UiComponent\ContextInterface, null given, called in app/code/Magento/Ui/Controller/Adminhtml/Index/Render.php on line 62 and defined in vendor/phpunit/phpunit-mock-objects/src/Framework/MockObject/Generator.php(290) : eval()'d code:11
Stack trace:
#0 app/code/Magento/Ui/Controller/Adminhtml/Index/Render.php(62): Mock_UiComponentTypeResolver_c45134a1->resolve(NULL)
#1 app/code/Magento/Ui/Controller/Adminhtml/AbstractAction.php(60): Magento\Ui\Controller\Adminhtml\Index\Render->execute()
#2 app/code/Magento/Ui/Test/Unit/Controller/Adminhtml/Index/RenderTest.php(214): Magento\Ui\Controller\Adminhtml\AbstractAction->executeA in vendor/phpunit/phpunit-mock-objects/src/Framework/MockObject/Generator.php(290) : eval()'d code on line 11

Any help with fixing the unit tests would be appreciated! My PHPUnit skills are almost non-existing, so yeah ... :)

[duckchip]

Try something like:

$this->someParam = $this
             ->getMockBuilder(SomeObject::class)
             ->disableOriginalConstructor()
             ->setMethods(['create'])
             ->getMock();

[hostep]

Unit tests should be fixed now (on my local machine at least).

Would be great if someone can review if they are fixed correctly, see my second commit.
I had to add some extra mocking in the testExecuteAjaxRequestWithoutPermissions method to make the unit tests work, no idea why this is necessary for PHPUnit 4 and not for PHPUnit 6...

Thanks!

[dmanners]

Hi @hostep would you be able to look through the conflicts in this PR so we can process this?

Thanks

[hostep]

Hi @dmanners, the conflicts originate from changes introduced in Magento 2.1.12, which are also introduced in 2.2.3 (I find very little information in the commit messages as to why the changes were added: "MAGETWO-84847: Prepare codebase for 2.1.12" doesn't give me a lot of useful information to be honest).
The problem is that both the security fixes in 2.1.12 and 2.2.3 and this PR kind of do the same thing, but not exactly the same and in a mostly different way.

I'm going to hold of on fixing this, because I want to see how these security related changes are going to be forward ported to 2.3-develop it might give me a better idea about how to fix these merge conflicts. Any idea when these security fixes will be introduced in 2.3-develop?

Just FYI, I started fixing the conflicts, and fixed the conflict in Magento\Ui\Controller\Adminhtml\Index\Render as follows, but I'm struggling a lot with the tests, I have the feeling they have to be completely re-evaluated again, I'd like to wait to see how this is happening in 2.3-develop

diff --git a/app/code/Magento/Ui/Controller/Adminhtml/Index/Render.php b/app/code/Magento/Ui/Controller/Adminhtml/Index/Render.php
index e16e3e43bdf..0523a5c40ed 100644
--- a/app/code/Magento/Ui/Controller/Adminhtml/Index/Render.php
+++ b/app/code/Magento/Ui/Controller/Adminhtml/Index/Render.php
@@ -9,12 +9,32 @@ use Magento\Backend\App\Action\Context;
 use Magento\Ui\Controller\Adminhtml\AbstractAction;
 use Magento\Framework\View\Element\UiComponentFactory;
 use Magento\Framework\View\Element\UiComponentInterface;
+use Magento\Ui\Model\UiComponentTypeResolver;

 /**
  * Class Render
  */
 class Render extends AbstractAction
 {
+    /**
+     * @var \Magento\Ui\Model\UiComponentTypeResolver
+     */
+    private $contentTypeResolver;
+
+    /**
+     * @param Context $context
+     * @param UiComponentFactory $factory
+     * @param UiComponentTypeResolver $contentTypeResolver
+     */
+    public function __construct(
+        Context $context,
+        UiComponentFactory $factory,
+        UiComponentTypeResolver $contentTypeResolver
+    ) {
+        parent::__construct($context, $factory);
+        $this->contentTypeResolver = $contentTypeResolver;
+    }
+
     /**
      * Action for AJAX request
      *
@@ -27,7 +47,7 @@ class Render extends AbstractAction
             return;
         }

-        $component = $this->factory->create($this->_request->getParam('namespace'));
+        $component = $this->factory->create($this->getRequest()->getParam('namespace'));

         $aclResource = $component->getData('acl');

@@ -38,11 +58,10 @@ class Render extends AbstractAction

         $this->prepareComponent($component);

-        if ($component->getContext()->getAcceptType() === 'json') {
-            $this->_response->setHeader('Content-Type', 'application/json');
-        }
+        $this->getResponse()->appendBody((string) $component->render());

-        $this->_response->appendBody((string) $component->render());
+        $contentType = $this->contentTypeResolver->resolve($component->getContext());
+        $this->getResponse()->setHeader('Content-Type', $contentType, true);
     }

     /**

[dmanners]

Hey @hostep sorry for the late response, I am not sure when this will make it's way into 2.3-develop.

[hostep]

Ok, let's see what happens.

The original issue should be fixed in the security release from 2.1.12 and 2.2.3, so it's not really necessary to backport the PR. The only difference is that the PR added support for both the application/xml and application/json mime types, whereas the security update only added support for the application/json. I'm not sure if there is anything in core Magento which returns some xml data using ajax, I doubt it, so it's probably not very urgent to get this also supported in 2.1.x and 2.2.x

The only reason I see now to backport the PR, is to have more consistent code in between 2.1, 2.2 & 2.3

[VladimirZaets]

Hi, @hostep, thank you for collaboration.
I will close this PR since the issue is already fixed in current Magento 2.1 branch.

The only reason I see now to backport the PR, is to have more consistent code in between 2.1, 2.2 & 2.3

• In current mainline, we have the stable code, and I think, in this case, needn't to merging code that potentially can introduce the new bug.

[hostep]

Ok makes sense to close, since original issue should indeed be fixed by changes in 2.1.12 & 2.2.3.

And it was also already "fixed" (more a workaround since the bug was in Magento and not in New Relic) in New Relic's PHP agent some time ago: https://docs.newrelic.com/docs/release-notes/agent-release-notes/php-release-notes/php-agent-760201:

In some cases, Magento 2's UI Component JSON data sources return with an (incorrect) Content-Type of text/html. This resulted in auto instrumentation insertions into JSON data that contained a string. We now identify these specific JSON data sources and ensure the auto instrumentation is not inserted into this JSON data.