Skip to content

Commit

Permalink
bug #20602 [HttpKernel] Revert BC breaking change of Request::isMetho…
Browse files Browse the repository at this point in the history
…dSafe() (nicolas-grekas)

This PR was merged into the 2.7 branch.

Discussion
----------

[HttpKernel] Revert BC breaking change of Request::isMethodSafe()

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | yes (reverting a previous BC break)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #20562
| License       | MIT
| Doc PR        | -

As spotted in #20562, we should not have broken a minor version. Instead, we should have deprecated the bad behavior. This is done in #20603.

Commits
-------

0c3b7d7 [HttpKernel] Revert BC breaking change of Request::isMethodSafe()
  • Loading branch information
fabpot committed Nov 24, 2016
2 parents 78d713c + 0c3b7d7 commit 34b9fd6
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 7 deletions.
Expand Up @@ -190,7 +190,7 @@ public function prepare(Request $request)

if (!$this->headers->has('Accept-Ranges')) {
// Only accept ranges on safe HTTP methods
$this->headers->set('Accept-Ranges', $request->isMethodSafe() ? 'bytes' : 'none');
$this->headers->set('Accept-Ranges', $request->isMethodSafe(false) ? 'bytes' : 'none');
}

if (!$this->headers->has('Content-Type')) {
Expand Down
6 changes: 4 additions & 2 deletions src/Symfony/Component/HttpFoundation/Request.php
Expand Up @@ -1466,11 +1466,13 @@ public function isMethod($method)
/**
* Checks whether the method is safe or not.
*
* @param bool $andCacheable Adds the additional condition that the method should be cacheable. True by default.
*
* @return bool
*/
public function isMethodSafe()
public function isMethodSafe(/* $andCacheable = true */)
{
return in_array($this->getMethod(), array('GET', 'HEAD', 'OPTIONS', 'TRACE'));
return in_array($this->getMethod(), 0 < func_num_args() && !func_get_arg(0) ? array('GET', 'HEAD', 'OPTIONS', 'TRACE') : array('GET', 'HEAD'));
}

/**
Expand Down
9 changes: 8 additions & 1 deletion src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
Expand Up @@ -1929,7 +1929,7 @@ public function testMethodSafe($method, $safe)
{
$request = new Request();
$request->setMethod($method);
$this->assertEquals($safe, $request->isMethodSafe());
$this->assertEquals($safe, $request->isMethodSafe(false));
}

public function methodSafeProvider()
Expand All @@ -1948,6 +1948,13 @@ public function methodSafeProvider()
);
}

public function testMethodSafeChecksCacheable()
{
$request = new Request();
$request->setMethod('OPTION');
$this->assertFalse($request->isMethodSafe());
}

/**
* @dataProvider methodCacheableProvider
*/
Expand Down
Expand Up @@ -81,7 +81,7 @@ public function onKernelRequest(GetResponseEvent $event)
protected function validateRequest(Request $request)
{
// is the Request safe?
if (!$request->isMethodSafe()) {
if (!$request->isMethodSafe(false)) {
throw new AccessDeniedHttpException();
}

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php
Expand Up @@ -202,7 +202,7 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ
}
$this->traces[$request->getMethod().' '.$path] = array();

if (!$request->isMethodSafe()) {
if (!$request->isMethodSafe(false)) {
$response = $this->invalidate($request, $catch);
} elseif ($request->headers->has('expect') || !$request->isMethodCacheable()) {
$response = $this->pass($request, $catch);
Expand Down
Expand Up @@ -209,7 +209,7 @@ private function startAuthentication(Request $request, AuthenticationException $
protected function setTargetPath(Request $request)
{
// session isn't required when using HTTP basic authentication mechanism for example
if ($request->hasSession() && $request->isMethodSafe() && !$request->isXmlHttpRequest()) {
if ($request->hasSession() && $request->isMethodSafe(false) && !$request->isXmlHttpRequest()) {
$request->getSession()->set('_security.'.$this->providerKey.'.target_path', $request->getUri());
}
}
Expand Down

0 comments on commit 34b9fd6

Please sign in to comment.