Skip to content

Commit

Permalink
Merge pull request #2576 from lordrhodos/2386-fix-for-head-requests
Browse files Browse the repository at this point in the history
Fix for #2386 on Slim 3
  • Loading branch information
l0gicgate committed Feb 8, 2019
2 parents 715d130 + 25470cb commit e97d122
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 14 deletions.
25 changes: 21 additions & 4 deletions Slim/App.php
Expand Up @@ -11,7 +11,6 @@
use Exception;
use Psr\Http\Message\UriInterface;
use Slim\Exception\InvalidMethodException;
use Slim\Http\Response;
use Throwable;
use Closure;
use InvalidArgumentException;
Expand All @@ -27,7 +26,6 @@
use Slim\Http\Headers;
use Slim\Http\Body;
use Slim\Http\Request;
use Slim\Interfaces\Http\EnvironmentInterface;
use Slim\Interfaces\RouteGroupInterface;
use Slim\Interfaces\RouteInterface;
use Slim\Interfaces\RouterInterface;
Expand Down Expand Up @@ -445,7 +443,8 @@ public function respond(ResponseInterface $response)
}

// Body
if (!$this->isEmptyResponse($response)) {
$request = $this->container->get('request');
if (!$this->isEmptyResponse($response) && !$this->isHeadRequest($request)) {
$body = $response->getBody();
if ($body->isSeekable()) {
$body->rewind();
Expand Down Expand Up @@ -614,7 +613,8 @@ protected function finalize(ResponseInterface $response)
// stop PHP sending a Content-Type automatically
ini_set('default_mimetype', '');

if ($this->isEmptyResponse($response)) {
$request = $this->container->get('request');
if ($this->isEmptyResponse($response) && !$this->isHeadRequest($request)) {
return $response->withoutHeader('Content-Type')->withoutHeader('Content-Length');
}

Expand All @@ -631,6 +631,11 @@ protected function finalize(ResponseInterface $response)
}
}

// clear the body if this is a HEAD request
if ($this->isHeadRequest($request)) {
return $response->withBody(new Body(fopen('php://temp', 'r+')));
}

return $response;
}

Expand All @@ -652,6 +657,18 @@ protected function isEmptyResponse(ResponseInterface $response)
return in_array($response->getStatusCode(), [204, 205, 304]);
}

/**
* Helper method to check if the current request is a HEAD request
*
* @param RequestInterface $request
*
* @return bool
*/
protected function isHeadRequest(RequestInterface $request)
{
return strtoupper($request->getMethod()) === 'HEAD';
}

/**
* Call relevant handler from the Container if needed. If it doesn't exist,
* then just re-throw.
Expand Down
85 changes: 75 additions & 10 deletions tests/AppTest.php
Expand Up @@ -9,6 +9,7 @@

namespace Slim\Tests;

use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\UriInterface;
use Slim\App;
Expand All @@ -24,6 +25,7 @@
use Slim\Http\Request;
use Slim\Http\RequestBody;
use Slim\Http\Response;
use Slim\Http\StatusCode;
use Slim\Http\Uri;
use Slim\Router;
use Slim\Tests\Assets\HeaderStack;
Expand Down Expand Up @@ -1515,37 +1517,72 @@ public function testInvokeSubRequestUsesResponseObject()

// TODO: Test run()
public function testRun()
{
$app = $this->getAppForTestingRunMethod();

ob_start();
$app->run();
$resOut = ob_get_clean();

$this->assertEquals('bar', (string)$resOut);
}

public function testRunReturnsEmptyResponseBodyWithHeadRequestMethod()
{
$app = $this->getAppForTestingRunMethod('HEAD');

ob_start();
$app->run();
$resOut = ob_get_clean();

$this->assertEquals('', (string)$resOut);
}

public function testRunReturnsEmptyResponseBodyWithGetRequestMethodInSilentMode()
{
$app = $this->getAppForTestingRunMethod();
$response = $app->run(true);

$this->assertEquals('bar', $response->getBody()->__toString());
}

public function testRunReturnsEmptyResponseBodyWithHeadRequestMethodInSilentMode()
{
$app = $this->getAppForTestingRunMethod('HEAD');
$response = $app->run(true);

$this->assertEquals('', $response->getBody()->__toString());
}

private function getAppForTestingRunMethod($method = 'GET')
{
$app = new App();

// Prepare request and response objects
$env = Environment::mock([
'SCRIPT_NAME' => '/index.php',
'REQUEST_URI' => '/foo',
'REQUEST_METHOD' => 'GET',
'REQUEST_METHOD' => $method,
]);
$uri = Uri::createFromEnvironment($env);
$headers = Headers::createFromEnvironment($env);
$cookies = [];
$serverParams = $env->all();
$body = new Body(fopen('php://temp', 'r+'));
$req = new Request('GET', $uri, $headers, $cookies, $serverParams, $body);
$res = new Response();
$req = new Request($method, $uri, $headers, $cookies, $serverParams, $body);
$res = new Response(StatusCode::HTTP_OK, null, $body);
$app->getContainer()['request'] = $req;
$app->getContainer()['response'] = $res;

$app->get('/foo', function ($req, $res) {
echo 'bar';
});
$res->getBody()->write('bar');

ob_start();
$app->run();
$resOut = ob_get_clean();
return $res;
});

$this->assertEquals('bar', (string)$resOut);
return $app;
}


public function testRespond()
{
$app = new App();
Expand Down Expand Up @@ -2326,6 +2363,34 @@ public function testIsEmptyResponseWithoutEmptyMethod()
$this->assertTrue($result);
}

public function testIsHeadRequestWithGetRequest()
{
$method = new \ReflectionMethod('Slim\App', 'isHeadRequest');
$method->setAccessible(true);

/** @var Request $request */
$request = $this->getMockBuilder(RequestInterface::class)->getMock();
$request->method('getMethod')
->willReturn('get');

$result = $method->invoke(new App(), $request);
$this->assertFalse($result);
}

public function testIsHeadRequestWithHeadRequest()
{
$method = new \ReflectionMethod('Slim\App', 'isHeadRequest');
$method->setAccessible(true);

/** @var Request $request */
$request = $this->getMockBuilder(RequestInterface::class)->getMock();
$request->method('getMethod')
->willReturn('head');

$result = $method->invoke(new App(), $request);
$this->assertTrue($result);
}

public function testHandlePhpError()
{
$this->skipIfPhp70();
Expand Down

0 comments on commit e97d122

Please sign in to comment.