8000 feature #16076 [HttpFoundation] change precedence of parameters in Re… · symfony/symfony@88e2d70 · GitHub
[go: up one dir, main page]

Skip to content

Commit 88e2d70

Browse files
committed
feature #16076 [HttpFoundation] change precedence of parameters in Request::get (Tobion)
This PR was merged into the 3.0-dev branch. Discussion ---------- [HttpFoundation] change precedence of parameters in Request::get | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Allowing the request attributes to be overwritten via GET parameters is risky and made #8966 even worse. It is even more risky because it skips the requirements checks as configured in routing. So people that set requirements for routing placeholders like `\d+` or `html|json` can be sure it is validated when using the routing variables. But if developers use `$request->get()` to retrieve them, anybody from outside can set any value for those. Commits ------- e8d6764 [HttpFoundation] change precedence of parameters in Request::get
2 parents 38c059f + e8d6764 commit 88e2d70

File tree

3 files changed

+31
-13
lines changed

3 files changed

+31
-13
lines changed

src/Symfony/Component/HttpFoundation/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
3.0.0
5+
-----
6+
7+
* The precedence of parameters returned from `Request::get()` changed from "GET, PATH, BODY" to "PATH, GET, BODY"
8+
49
2.8.0
510
-----
611

src/Symfony/Component/HttpFoundation/Request.php

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -700,19 +700,13 @@ public static function getHttpMethodParameterOverride()
700700
}
701701

702702
/**
703-
* Gets a "parameter" value.
703+
* Gets a "parameter" value from any bag.
704704
*
705-
* This method is mainly useful for libraries that want to pro 10000 vide some flexibility.
705+
* This method is mainly useful for libraries that want to provide some flexibility. If you don't need the
706+
* flexibility in controllers, it is better to explicitly get request parameters from the appropriate
707+
* public property instead (attributes, query, request).
706708
*
707-
* Order of precedence: GET, PATH, POST
708-
*
709-
* Avoid using this method in controllers:
710-
*
711-
* * slow
712-
* * prefer to get from a "named" source
713-
*
714-
* It is better to explicitly get request parameters from the appropriate
715-
* public property instead (query, attributes, request).
709+
* Order of precedence: PATH (routing placeholders or custom attributes), GET, BODY
716710
*
717711
* @param string $key the key
718712
* @param mixed $default the default value
@@ -721,11 +715,11 @@ public static function getHttpMethodParameterOverride()
721715
*/
722716
public function get($key, $default = null)
723717
{
724-
if ($this !== $result = $this->query->get($key, $this)) {
718+
if ($this !== $result = $this->attributes->get($key, $this)) {
725719
return $result;
726720
}
727721

728-
if ($this !== $result = $this->attributes->get($key, $this)) {
722+
if ($this !== $result = $this->query->get($key, $this)) {
729723
return $result;
730724
}
731725

src/Symfony/Component/HttpFoundation/Tests/RequestTest.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,6 +1221,25 @@ public function testGetPathInfo()
12211221
$this->assertEquals('/path%20test/info', $request->getPathInfo());
12221222
}
12231223

1224+
public function testGetParameterPrecedence()
1225+
{
1226+
$request = new Request();
1227+
$request->attributes->set('foo', 'attr');
1228+
$request->query->set('foo', 'query');
1229+
$request->request->set('foo', 'body');
1230+
1231+
$this->assertSame('attr', $request->get('foo'));
1232+
1233+
$request->attributes->remove('foo');
1234+
$this->assertSame('query', $request->get('foo'));
1235+
1236+
$request->query->remove('foo');
1237+
$this->assertSame('body', $request->get('foo'));
1238+
1239+
$request->request->remove('foo');
1240+
$this->assertNull($request->get('foo'));
1241+
}
1242+
12241243
public function testGetPreferredLanguage()
12251244
{
12261245
$request = new Request();

0 commit comments

Comments
 (0)
0