Skip to content

Commit

Permalink
Fix for issue #21299. HEAD requests map to GET request interface and …
Browse files Browse the repository at this point in the history
…are sent without body attached.
  • Loading branch information
mattijv committed Mar 5, 2019
1 parent 7d0efc5 commit d8eb160
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 3 deletions.
2 changes: 1 addition & 1 deletion app/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1739,7 +1739,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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

/**
* Marker for actions processing HEAD requests.
*
* @deprecated Both GET and HEAD requests map to HttpGetActionInterface
*/
interface HttpHeadActionInterface extends ActionInterface
{
Expand Down
14 changes: 14 additions & 0 deletions lib/internal/Magento/Framework/App/Http.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,26 @@ 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()
{
$contentLength = strlen($this->_response->getContent());
$this->_response->clearBody();
$this->_response->setHeader('Content-Length', $contentLength);
}

/**
* {@inheritdoc}
*/
Expand Down
31 changes: 30 additions & 1 deletion lib/internal/Magento/Framework/App/Test/Unit/HttpTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -155,6 +155,7 @@ private function setUpLaunch()
public function testLaunchSuccess()
{
$this->setUpLaunch();
$this->requestMock->expects($this->once())->method('isHead')->will($this->returnValue(false));
$this->eventManagerMock->expects($this->once())
->method('dispatch')
->with(
Expand All @@ -181,6 +182,34 @@ function () {
$this->http->launch();
}

public function testLaunchHeadRequest()
{
$body = "<html><head></head><body>Test</body></html>";
$contentLength = strlen($body);
$this->setUpLaunch();
$this->requestMock->expects($this->once())->method('isHead')->will($this->returnValue(true));
$this->responseMock->expects($this->once())
->method('getHttpResponseCode')
->will($this->returnValue(200));
$this->responseMock->expects($this->once())
->method('getContent')
->will($this->returnValue($body));
$this->responseMock->expects($this->once())
->method('clearBody')
->will($this->returnValue($this->responseMock));
$this->responseMock->expects($this->once())
->method('setHeader')
->with('Content-Length', $contentLength)
->will($this->returnValue($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());
}

public function testHandleDeveloperModeNotInstalled()
{
$dir = $this->getMockForAbstractClass(\Magento\Framework\Filesystem\Directory\ReadInterface::class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@

class HttpTest extends \PHPUnit\Framework\TestCase
{
/**
* @var \Magento\Framework\App\Request\Http|\PHPUnit_Framework_MockObject_MockObject
*/
private $request;

/**
* @var \Magento\Framework\HTTP\PhpEnvironment\Response|\PHPUnit_Framework_MockObject_MockObject
*/
Expand All @@ -29,12 +34,16 @@ class HttpTest extends \PHPUnit\Framework\TestCase
*/
protected function setUp()
{
$this->request = $this->createPartialMock(
\Magento\Framework\App\Request\Http::class,
['isHead']
);
$this->response = $this->createPartialMock(
\Magento\Framework\HTTP\PhpEnvironment\Response::class,
['setHeader', 'sendHeaders', 'setHeaders']
);
$this->mime = $this->createMock(\Magento\Framework\File\Mime::class);
$this->object = new Http($this->response, $this->mime);
$this->object = new Http($this->request, $this->response, $this->mime);
}

/**
Expand All @@ -57,6 +66,9 @@ public function testSend(): void
->method('getMimeType')
->with($file)
->will($this->returnValue($contentType));
$this->request->expects($this->once())
->method('isHead')
->will($this->returnValue(false));
$this->expectOutputString(file_get_contents($file));

$this->object->send($file);
Expand All @@ -83,6 +95,9 @@ public function testSendWithOptions(): void
->method('getMimeType')
->with($file)
->will($this->returnValue($contentType));
$this->request->expects($this->once())
->method('isHead')
->will($this->returnValue(false));
$this->expectOutputString(file_get_contents($file));

$this->object->send(['filepath' => $file, 'headers' => $headers]);
Expand All @@ -106,4 +121,32 @@ public function testSendNoFileExistException(): void
{
$this->object->send('nonexistent.file');
}

/**
* @return void
*/
public function testSendHeadRequest(): void
{
$file = __DIR__ . '/../../_files/javascript.js';
$contentType = 'content/type';

$this->response->expects($this->at(0))
->method('setHeader')
->with('Content-length', filesize($file));
$this->response->expects($this->at(1))
->method('setHeader')
->with('Content-Type', $contentType);
$this->response->expects($this->once())
->method('sendHeaders');
$this->mime->expects($this->once())
->method('getMimeType')
->with($file)
->will($this->returnValue($contentType));
$this->request->expects($this->once())
->method('isHead')
->will($this->returnValue(true));

$this->object->send($file);
$this->assertEquals(false, $this->hasOutput());
}
}
13 changes: 13 additions & 0 deletions lib/internal/Magento/Framework/File/Transfer/Adapter/Http.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@

class Http
{
/**
* @var \Magento\Framework\App\Request\Http
*/
private $request;

/**
* @var \Magento\Framework\HTTP\PhpEnvironment\Response
*/
Expand All @@ -19,13 +24,16 @@ class Http
private $mime;

/**
* @param \Magento\Framework\App\Request\Http $request
* @param \Magento\Framework\App\Response\Http $response
* @param \Magento\Framework\File\Mime $mime
*/
public function __construct(
\Magento\Framework\App\Request\Http $request,
\Magento\Framework\HTTP\PhpEnvironment\Response $response,
\Magento\Framework\File\Mime $mime
) {
$this->request = $request;
$this->response = $response;
$this->mime = $mime;
}
Expand Down Expand Up @@ -55,6 +63,11 @@ public function send($options = null)

$this->response->sendHeaders();

if ($this->request->isHead()) {
// Do not send the body on HEAD requests.
return;
}

$handle = fopen($filepath, 'r');
if ($handle) {
while (($buffer = fgets($handle, 4096)) !== false) {
Expand Down

0 comments on commit d8eb160

Please sign in to comment.