Skip to content

Commit

Permalink
bug #20205 [HttpCache] fix: do not cache OPTIONS request (dmaicher)
Browse files Browse the repository at this point in the history
This PR was merged into the 2.7 branch.

Discussion
----------

[HttpCache] fix: do not cache OPTIONS request

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #20202
| License       | MIT
| Doc PR        | -

The HttpCache should not cache any OPTIONS request as they are by spec not cacheable (mentioned here #20202 (comment) by @xabbuh).

Commits
-------

c43de7f [HttpCache] fix: do not cache OPTIONS request
  • Loading branch information
fabpot committed Oct 14, 2016
2 parents df138fb + c43de7f commit 55c3f89
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 2 deletions.
10 changes: 10 additions & 0 deletions src/Symfony/Component/HttpFoundation/Request.php
Expand Up @@ -1473,6 +1473,16 @@ public function isMethodSafe()
return in_array($this->getMethod(), array('GET', 'HEAD', 'OPTIONS', 'TRACE'));
}

/**
* Checks whether the method is cachaeble or not.
*
* @return bool
*/
public function isMethodCacheable()
{
return in_array($this->getMethod(), array('GET', 'HEAD'));
}

/**
* Returns the request body content.
*
Expand Down
26 changes: 26 additions & 0 deletions src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
Expand Up @@ -1948,6 +1948,32 @@ public function methodSafeProvider()
array('CONNECT', false),
);
}

/**
* @dataProvider methodCacheableProvider
*/
public function testMethodCacheable($method, $chacheable)
{
$request = new Request();
$request->setMethod($method);
$this->assertEquals($chacheable, $request->isMethodCacheable());
}

public function methodCacheableProvider()
{
return array(
array('HEAD', true),
array('GET', true),
array('POST', false),
array('PUT', false),
array('PATCH', false),
array('DELETE', false),
array('PURGE', false),
array('OPTIONS', false),
array('TRACE', false),
array('CONNECT', false),
);
}
}

class RequestContentProxy extends Request
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php
Expand Up @@ -204,7 +204,7 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ

if (!$request->isMethodSafe()) {
$response = $this->invalidate($request, $catch);
} elseif ($request->headers->has('expect')) {
} elseif ($request->headers->has('expect') || !$request->isMethodCacheable()) {
$response = $this->pass($request, $catch);
} else {
$response = $this->lookup($request, $catch);
Expand Down
15 changes: 15 additions & 0 deletions src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php
Expand Up @@ -1264,6 +1264,21 @@ public function testEsiCacheRemoveValidationHeadersIfEmbeddedResponses()
$this->assertNull($this->response->getETag());
$this->assertNull($this->response->getLastModified());
}

public function testDoesNotCacheOptionsRequest()
{
$this->setNextResponse(200, array('Cache-Control' => 'public, s-maxage=60'), 'get');
$this->request('GET', '/');
$this->assertHttpKernelIsCalled();

$this->setNextResponse(200, array('Cache-Control' => 'public, s-maxage=60'), 'options');
$this->request('OPTIONS', '/');
$this->assertHttpKernelIsCalled();

$this->request('GET', '/');
$this->assertHttpKernelIsNotCalled();
$this->assertSame('get', $this->response->getContent());
}
}

class TestKernel implements HttpKernelInterface
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpKernel/composer.json
Expand Up @@ -18,7 +18,7 @@
"require": {
"php": ">=5.3.9",
"symfony/event-dispatcher": "~2.6,>=2.6.7",
"symfony/http-foundation": "~2.7.15|~2.8.8",
"symfony/http-foundation": "~2.7.20|~2.8.13",
"symfony/debug": "~2.6,>=2.6.2",
"psr/log": "~1.0"
},
Expand Down

0 comments on commit 55c3f89

Please sign in to comment.