ENGCOM-4474: Fix for issue #21299. Change HEAD action mapping to GET action interface and add HEAD request handling #21378

sidolov · web-flow · commit aed3169b2894 · 2019-04-18T15:01:01.000-05:00
diff --git a/app/etc/di.xml b/app/etc/di.xml
@@ -1740,7 +1740,7 @@
             <argument name="map" xsi:type="array">
                 <item name="OPTIONS" xsi:type="string">\Magento\Framework\App\Action\HttpOptionsActionInterface</item>
                 <item name="GET" xsi:type="string">\Magento\Framework\App\Action\HttpGetActionInterface</item>
-                <item name="HEAD" xsi:type="string">\Magento\Framework\App\Action\HttpHeadActionInterface</item>
+                <item name="HEAD" xsi:type="string">\Magento\Framework\App\Action\HttpGetActionInterface</item>
                 <item name="POST" xsi:type="string">\Magento\Framework\App\Action\HttpPostActionInterface</item>
                 <item name="PUT" xsi:type="string">\Magento\Framework\App\Action\HttpPutActionInterface</item>
                 <item name="PATCH" xsi:type="string">\Magento\Framework\App\Action\HttpPatchActionInterface</item>
diff --git a/lib/internal/Magento/Framework/App/Action/HttpHeadActionInterface.php b/lib/internal/Magento/Framework/App/Action/HttpHeadActionInterface.php
@@ -12,6 +12,8 @@
 
 /**
  * Marker for actions processing HEAD requests.
+ *
+ * @deprecated Both GET and HEAD requests map to HttpGetActionInterface
  */
 interface HttpHeadActionInterface extends ActionInterface
 {
diff --git a/lib/internal/Magento/Framework/App/Http.php b/lib/internal/Magento/Framework/App/Http.php
@@ -3,17 +3,18 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
 namespace Magento\Framework\App;
 
 use Magento\Framework\App\Filesystem\DirectoryList;
-use Magento\Framework\Debug;
-use Magento\Framework\ObjectManager\ConfigLoaderInterface;
 use Magento\Framework\App\Request\Http as RequestHttp;
 use Magento\Framework\App\Response\Http as ResponseHttp;
 use Magento\Framework\App\Response\HttpInterface;
 use Magento\Framework\Controller\ResultInterface;
+use Magento\Framework\Debug;
 use Magento\Framework\Event;
 use Magento\Framework\Filesystem;
+use Magento\Framework\ObjectManager\ConfigLoaderInterface;
 
 /**
  * HTTP web application. Called from webroot index.php to serve web requests.
@@ -143,12 +144,31 @@ public function launch()
         } else {
             throw new \InvalidArgumentException('Invalid return type');
         }
+        if ($this->_request->isHead() && $this->_response->getHttpResponseCode() == 200) {
+            $this->handleHeadRequest();
+        }
         // This event gives possibility to launch something before sending output (allow cookie setting)
         $eventParams = ['request' => $this->_request, 'response' => $this->_response];
         $this->_eventManager->dispatch('controller_front_send_response_before', $eventParams);
         return $this->_response;
     }
 
+    /**
+     * Handle HEAD requests by adding the Content-Length header and removing the body from the response.
+     *
+     * @return void
+     */
+    private function handleHeadRequest()
+    {
+        // It is possible that some PHP installations have overloaded strlen to use mb_strlen instead.
+        // This means strlen might return the actual number of characters in a non-ascii string instead
+        // of the number of bytes. Use mb_strlen explicitly with a single byte character encoding to ensure
+        // that the content length is calculated in bytes.
+        $contentLength = mb_strlen($this->_response->getContent(), '8bit');
+        $this->_response->clearBody();
+        $this->_response->setHeader('Content-Length', $contentLength);
+    }
+
     /**
      * @inheritdoc
      */
@@ -248,7 +268,7 @@ private function redirectToSetup(Bootstrap $bootstrap, \Exception $exception)
                 . "because the Magento setup directory cannot be accessed. \n"
                 . 'You can install Magento using either the command line or you must restore access '
                 . 'to the following directory: ' . $setupInfo->getDir($projectRoot) . "\n";
-
+            // phpcs:ignore Magento2.Exceptions.DirectThrow
             throw new \Exception($newMessage, 0, $exception);
         }
     }
@@ -264,6 +284,7 @@ private function handleBootstrapErrors(Bootstrap $bootstrap, \Exception &$except
     {
         $bootstrapCode = $bootstrap->getErrorCode();
         if (Bootstrap::ERR_MAINTENANCE == $bootstrapCode) {
+            // phpcs:ignore Magento2.Security.IncludeFile
             require $this->_filesystem->getDirectoryRead(DirectoryList::PUB)->getAbsolutePath('errors/503.php');
             return true;
         }
@@ -304,6 +325,7 @@ private function handleInitException(\Exception $exception)
     {
         if ($exception instanceof \Magento\Framework\Exception\State\InitException) {
             $this->getLogger()->critical($exception);
+            // phpcs:ignore Magento2.Security.IncludeFile
             require $this->_filesystem->getDirectoryRead(DirectoryList::PUB)->getAbsolutePath('errors/404.php');
             return true;
         }
@@ -335,6 +357,7 @@ private function handleGenericReport(Bootstrap $bootstrap, \Exception $exception
         if (isset($params['SCRIPT_NAME'])) {
             $reportData['script_name'] = $params['SCRIPT_NAME'];
         }
+        // phpcs:ignore Magento2.Security.IncludeFile
         require $this->_filesystem->getDirectoryRead(DirectoryList::PUB)->getAbsolutePath('errors/report.php');
         return true;
     }
diff --git a/lib/internal/Magento/Framework/App/Test/Unit/HttpTest.php b/lib/internal/Magento/Framework/App/Test/Unit/HttpTest.php
@@ -92,7 +92,7 @@ protected function setUp()
                 'pathInfoProcessor' => $pathInfoProcessorMock,
                 'objectManager' => $objectManagerMock
             ])
-            ->setMethods(['getFrontName'])
+            ->setMethods(['getFrontName', 'isHead'])
             ->getMock();
         $this->areaListMock = $this->getMockBuilder(\Magento\Framework\App\AreaList::class)
             ->disableOriginalConstructor()
@@ -135,12 +135,17 @@ private function setUpLaunch()
     {
         $frontName = 'frontName';
         $areaCode = 'areaCode';
-        $this->requestMock->expects($this->once())->method('getFrontName')->will($this->returnValue($frontName));
+        $this->requestMock->expects($this->once())
+            ->method('getFrontName')
+            ->willReturn($frontName);
         $this->areaListMock->expects($this->once())
             ->method('getCodeByFrontName')
-            ->with($frontName)->will($this->returnValue($areaCode));
+            ->with($frontName)
+            ->willReturn($areaCode);
         $this->configLoaderMock->expects($this->once())
-            ->method('load')->with($areaCode)->will($this->returnValue([]));
+            ->method('load')
+            ->with($areaCode)
+            ->willReturn([]);
         $this->objectManagerMock->expects($this->once())->method('configure')->with([]);
         $this->objectManagerMock->expects($this->once())
             ->method('get')
@@ -149,12 +154,15 @@ private function setUpLaunch()
         $this->frontControllerMock->expects($this->once())
             ->method('dispatch')
             ->with($this->requestMock)
-            ->will($this->returnValue($this->responseMock));
+            ->willReturn($this->responseMock);
     }
 
     public function testLaunchSuccess()
     {
         $this->setUpLaunch();
+        $this->requestMock->expects($this->once())
+            ->method('isHead')
+            ->willReturn(false);
         $this->eventManagerMock->expects($this->once())
             ->method('dispatch')
             ->with(
@@ -171,33 +179,101 @@ public function testLaunchSuccess()
     public function testLaunchException()
     {
         $this->setUpLaunch();
-        $this->frontControllerMock->expects($this->once())->method('dispatch')->with($this->requestMock)->will(
-            $this->returnCallback(
-                function () {
-                    throw new \Exception('Message');
-                }
-            )
-        );
+        $this->frontControllerMock->expects($this->once())
+            ->method('dispatch')
+            ->with($this->requestMock)
+            ->willThrowException(
+                new \Exception('Message')
+            );
         $this->http->launch();
     }
 
+    /**
+     * Test that HEAD requests lead to an empty body and a Content-Length header matching the original body size.
+     * @dataProvider dataProviderForTestLaunchHeadRequest
+     * @param string $body
+     * @param int $expectedLength
+     */
+    public function testLaunchHeadRequest($body, $expectedLength)
+    {
+        $this->setUpLaunch();
+        $this->requestMock->expects($this->once())
+            ->method('isHead')
+            ->willReturn(true);
+        $this->responseMock->expects($this->once())
+            ->method('getHttpResponseCode')
+            ->willReturn(200);
+        $this->responseMock->expects($this->once())
+            ->method('getContent')
+            ->willReturn($body);
+        $this->responseMock->expects($this->once())
+            ->method('clearBody')
+            ->willReturn($this->responseMock);
+        $this->responseMock->expects($this->once())
+            ->method('setHeader')
+            ->with('Content-Length', $expectedLength)
+            ->willReturn($this->responseMock);
+        $this->eventManagerMock->expects($this->once())
+            ->method('dispatch')
+            ->with(
+                'controller_front_send_response_before',
+                ['request' => $this->requestMock, 'response' => $this->responseMock]
+            );
+        $this->assertSame($this->responseMock, $this->http->launch());
+    }
+
+    /**
+     * Different test content for responseMock with their expected lengths in bytes.
+     * @return array
+     */
+    public function dataProviderForTestLaunchHeadRequest(): array
+    {
+        return [
+            [
+                "<html><head></head><body>Test</body></html>",                // Ascii text
+                43                                                            // Expected Content-Length
+            ],
+            [
+                "<html><head></head><body>部落格</body></html>",               // Multi-byte characters
+                48                                                            // Expected Content-Length
+            ],
+            [
+                "<html><head></head><body>\0</body></html>",     // Null byte
+                40                                                            // Expected Content-Length
+            ],
+            [
+                "<html><head></head>خرید<body></body></html>",                // LTR text
+                47                                                            // Expected Content-Length
+            ]
+        ];
+    }
+
     public function testHandleDeveloperModeNotInstalled()
     {
         $dir = $this->getMockForAbstractClass(\Magento\Framework\Filesystem\Directory\ReadInterface::class);
-        $dir->expects($this->once())->method('getAbsolutePath')->willReturn(__DIR__);
+        $dir->expects($this->once())
+            ->method('getAbsolutePath')
+            ->willReturn(__DIR__);
         $this->filesystemMock->expects($this->once())
             ->method('getDirectoryRead')
             ->with(DirectoryList::ROOT)
             ->willReturn($dir);
-        $this->responseMock->expects($this->once())->method('setRedirect')->with('/_files/');
-        $this->responseMock->expects($this->once())->method('sendHeaders');
+        $this->responseMock->expects($this->once())
+            ->method('setRedirect')
+            ->with('/_files/');
+        $this->responseMock->expects($this->once())
+            ->method('sendHeaders');
         $bootstrap = $this->getBootstrapNotInstalled();
-        $bootstrap->expects($this->once())->method('getParams')->willReturn([
-            'SCRIPT_NAME' => '/index.php',
-            'DOCUMENT_ROOT' => __DIR__,
-            'SCRIPT_FILENAME' => __DIR__ . '/index.php',
-            SetupInfo::PARAM_NOT_INSTALLED_URL_PATH => '_files',
-        ]);
+        $bootstrap->expects($this->once())
+            ->method('getParams')
+            ->willReturn(
+                [
+                    'SCRIPT_NAME' => '/index.php',
+                    'DOCUMENT_ROOT' => __DIR__,
+                    'SCRIPT_FILENAME' => __DIR__ . '/index.php',
+                    SetupInfo::PARAM_NOT_INSTALLED_URL_PATH => '_files',
+                ]
+            );
         $this->assertTrue($this->http->catchException($bootstrap, new \Exception('Test Message')));
     }
 
@@ -206,24 +282,37 @@ public function testHandleDeveloperMode()
         $this->filesystemMock->expects($this->once())
             ->method('getDirectoryRead')
             ->will($this->throwException(new \Exception('strange error')));
-        $this->responseMock->expects($this->once())->method('setHttpResponseCode')->with(500);
-        $this->responseMock->expects($this->once())->method('setHeader')->with('Content-Type', 'text/plain');
+        $this->responseMock->expects($this->once())
+            ->method('setHttpResponseCode')
+            ->with(500);
+        $this->responseMock->expects($this->once())
+            ->method('setHeader')
+            ->with('Content-Type', 'text/plain');
         $constraint = new \PHPUnit\Framework\Constraint\StringStartsWith('1 exception(s):');
-        $this->responseMock->expects($this->once())->method('setBody')->with($constraint);
-        $this->responseMock->expects($this->once())->method('sendResponse');
+        $this->responseMock->expects($this->once())
+            ->method('setBody')
+            ->with($constraint);
+        $this->responseMock->expects($this->once())
+            ->method('sendResponse');
         $bootstrap = $this->getBootstrapNotInstalled();
-        $bootstrap->expects($this->once())->method('getParams')->willReturn(
-            ['DOCUMENT_ROOT' => 'something', 'SCRIPT_FILENAME' => 'something/else']
-        );
+        $bootstrap->expects($this->once())
+            ->method('getParams')
+            ->willReturn(
+                ['DOCUMENT_ROOT' => 'something', 'SCRIPT_FILENAME' => 'something/else']
+            );
         $this->assertTrue($this->http->catchException($bootstrap, new \Exception('Test')));
     }
 
     public function testCatchExceptionSessionException()
     {
-        $this->responseMock->expects($this->once())->method('setRedirect');
-        $this->responseMock->expects($this->once())->method('sendHeaders');
+        $this->responseMock->expects($this->once())
+            ->method('setRedirect');
+        $this->responseMock->expects($this->once())
+            ->method('sendHeaders');
         $bootstrap = $this->createMock(\Magento\Framework\App\Bootstrap::class);
-        $bootstrap->expects($this->once())->method('isDeveloperMode')->willReturn(false);
+        $bootstrap->expects($this->once())
+            ->method('isDeveloperMode')
+            ->willReturn(false);
         $this->assertTrue($this->http->catchException(
             $bootstrap,
             new \Magento\Framework\Exception\SessionException(new \Magento\Framework\Phrase('Test'))
@@ -238,8 +327,12 @@ public function testCatchExceptionSessionException()
     private function getBootstrapNotInstalled()
     {
         $bootstrap = $this->createMock(\Magento\Framework\App\Bootstrap::class);
-        $bootstrap->expects($this->once())->method('isDeveloperMode')->willReturn(true);
-        $bootstrap->expects($this->once())->method('getErrorCode')->willReturn(Bootstrap::ERR_IS_INSTALLED);
+        $bootstrap->expects($this->once())
+            ->method('isDeveloperMode')
+            ->willReturn(true);
+        $bootstrap->expects($this->once())
+            ->method('getErrorCode')
+            ->willReturn(Bootstrap::ERR_IS_INSTALLED);
         return $bootstrap;
     }
 }
diff --git a/lib/internal/Magento/Framework/File/Test/Unit/Transfer/Adapter/HttpTest.php b/lib/internal/Magento/Framework/File/Test/Unit/Transfer/Adapter/HttpTest.php
diff --git a/lib/internal/Magento/Framework/File/Transfer/Adapter/Http.php b/lib/internal/Magento/Framework/File/Transfer/Adapter/Http.php

Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,8 @@`
`12`	`12`
`13`	`13`	`/**`
`14`	`14`	`* Marker for actions processing HEAD requests.`
	`15`	`+ *`
	`16`	`+ * @deprecated Both GET and HEAD requests map to HttpGetActionInterface`
`15`	`17`	`*/`
`16`	`18`	`interface HttpHeadActionInterface extends ActionInterface`
`17`	`19`	`{`