Skip to content

Commit

Permalink
feature #31658 [HTTP Foundation] Deprecate passing argument to method…
Browse files Browse the repository at this point in the history
… Request::isMethodSafe() (dFayet)

This PR was squashed before being merged into the 4.4 branch (closes #31658).

Discussion
----------

[HTTP Foundation] Deprecate passing argument to method Request::isMethodSafe()

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #31323    <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        |  <!-- required for new features -->

Passing argument to `Request::isMethodSafe()` should have been deprecated in 4.1. As mentionned there:  https://github.com/symfony/http-foundation/blob/master/Request.php#L1435-L1452

We also remove Exceptions throwed when you call `Request::isMethodSafe()`  or `Request::isMethodSafe(true)`

Commits
-------

59fa1bd [HTTP Foundation] Deprecate passing argument to method Request::isMethodSafe()
  • Loading branch information
fabpot committed Jun 5, 2019
2 parents 926ded8 + 59fa1bd commit 5c8fb7b
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 22 deletions.
Expand Up @@ -204,7 +204,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(false) ? 'bytes' : 'none');
$this->headers->set('Accept-Ranges', $request->isMethodSafe() ? 'bytes' : 'none');
}

if (self::$trustXSendfileTypeHeader && $request->headers->has('X-Sendfile-Type')) {
Expand Down
5 changes: 5 additions & 0 deletions src/Symfony/Component/HttpFoundation/CHANGELOG.md
@@ -1,6 +1,11 @@
CHANGELOG
=========

4.4.0
-----

* passing arguments to `Request::isMethodSafe()` is deprecated.

4.3.0
-----

Expand Down
9 changes: 3 additions & 6 deletions src/Symfony/Component/HttpFoundation/Request.php
Expand Up @@ -1437,15 +1437,12 @@ public function isMethod($method)
*
* @see https://tools.ietf.org/html/rfc7231#section-4.2.1
*
* @param bool $andCacheable Adds the additional condition that the method should be cacheable. True by default.
*
* @return bool
*/
public function isMethodSafe(/* $andCacheable = true */)
public function isMethodSafe()
{
if (!\func_num_args() || func_get_arg(0)) {
// setting $andCacheable to false should be deprecated in 4.1
throw new \BadMethodCallException('Checking only for cacheable HTTP methods with Symfony\Component\HttpFoundation\Request::isMethodSafe() is not supported.');
if (\func_num_args() > 0) {
@trigger_error(sprintf('Passing arguments to "%s()" has been deprecated since Symfony 4.4; use "%s::isMethodCacheable() to check if the method is cacheable instead."', __METHOD__, __CLASS__), E_USER_DEPRECATED);
}

return \in_array($this->getMethod(), ['GET', 'HEAD', 'OPTIONS', 'TRACE']);
Expand Down
12 changes: 1 addition & 11 deletions src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
Expand Up @@ -2115,7 +2115,7 @@ public function testMethodSafe($method, $safe)
{
$request = new Request();
$request->setMethod($method);
$this->assertEquals($safe, $request->isMethodSafe(false));
$this->assertEquals($safe, $request->isMethodSafe());
}

public function methodSafeProvider()
Expand All @@ -2134,16 +2134,6 @@ public function methodSafeProvider()
];
}

/**
* @expectedException \BadMethodCallException
*/
public function testMethodSafeChecksCacheable()
{
$request = new Request();
$request->setMethod('OPTIONS');
$request->isMethodSafe();
}

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

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

$this->traces[$this->getTraceKey($request)] = [];

if (!$request->isMethodSafe(false)) {
if (!$request->isMethodSafe()) {
$response = $this->invalidate($request, $catch);
} elseif ($request->headers->has('expect') || !$request->isMethodCacheable()) {
$response = $this->pass($request, $catch);
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpKernel/composer.json
Expand Up @@ -18,7 +18,7 @@
"require": {
"php": "^7.1.3",
"symfony/event-dispatcher": "^4.3",
"symfony/http-foundation": "^4.1.1|^5.0",
"symfony/http-foundation": "^4.4|^5.0",
"symfony/debug": "^3.4|^4.0|^5.0",
"symfony/polyfill-ctype": "^1.8",
"symfony/polyfill-php73": "^1.9",
Expand Down
Expand Up @@ -208,7 +208,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(false) && !$request->isXmlHttpRequest()) {
if ($request->hasSession() && $request->isMethodSafe() && !$request->isXmlHttpRequest()) {
$this->saveTargetPath($request->getSession(), $this->providerKey, $request->getUri());
}
}
Expand Down

0 comments on commit 5c8fb7b

Please sign in to comment.