Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make sure HEAD requests do not return a body #2576

Merged
merged 6 commits into from Feb 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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