merged branch Tobion/query (PR #4735)

Commits
-------

c40a4e5 [HttpFoundation] fix query string normalization
f9ec2ea refactored test method
0880174 [HttpFoundation] added failing tests for query string normalization

Discussion
----------

[HttpFoundation] fix query string normalization

This fixes the query string normalization. There were several problems in it (see test cases that I added).
The main issue, that first catched my eye, was that the query string was urldecoded before it was exploded by `=`. See old code: `explode('=', rawurldecode($segment), 2);`. This means an encoded `=` (`%3D`) would falsely be considered a separator and thus lead to complete different parameters. The fixed test case is at `pa%3Dram=foo%26bar%3Dbaz&test=test`.

---------------------------------------------------------------------------

by Tobion at 2012-07-04T02:21:25Z

cc @simensen considering your PR 4711
This commit is contained in:
Fabien Potencier 2012-07-04 07:27:44 +02:00
commit b9f005eab3
2 changed files with 61 additions and 30 deletions

View File

@ -463,32 +463,41 @@ class Request
/** /**
* Normalizes a query string. * Normalizes a query string.
* *
* It builds a normalized query string, where keys/value pairs are alphabetized * It builds a normalized query string, where keys/value pairs are alphabetized,
* and have consistent escaping. * have consistent escaping and unneeded delimiters are removed.
* *
* @param string $qs Query string * @param string $qs Query string
* *
* @return string|null A normalized query string for the Request * @return string A normalized query string for the Request
*/ */
static public function normalizeQueryString($qs = null) static public function normalizeQueryString($qs)
{ {
if (!$qs) { if ('' == $qs) {
return null; return '';
} }
$parts = array(); $parts = array();
$order = array(); $order = array();
foreach (explode('&', $qs) as $segment) { foreach (explode('&', $qs) as $param) {
if (false === strpos($segment, '=')) { if ('' === $param || '=' === $param[0]) {
$parts[] = $segment; // Ignore useless delimiters, e.g. "x=y&".
$order[] = $segment; // Also ignore pairs with empty key, even if there was a value, e.g. "=value", as such nameless values cannot be retrieved anyway.
} else { // PHP also does not include them when building _GET.
$tmp = explode('=', rawurldecode($segment), 2); continue;
$parts[] = rawurlencode($tmp[0]).'='.rawurlencode($tmp[1]);
$order[] = $tmp[0];
} }
$keyValuePair = explode('=', $param, 2);
// GET parameters, that are submitted from a HTML form, encode spaces as "+" by default (as defined in enctype application/x-www-form-urlencoded).
// PHP also converts "+" to spaces when filling the global _GET or when using the function parse_str. This is why we use urldecode and then normalize to
// RFC 3986 with rawurlencode.
$parts[] = isset($keyValuePair[1]) ?
rawurlencode(urldecode($keyValuePair[0])).'='.rawurlencode(urldecode($keyValuePair[1])) :
rawurlencode(urldecode($keyValuePair[0]));
$order[] = urldecode($keyValuePair[0]);
} }
array_multisort($order, SORT_ASC, $parts); array_multisort($order, SORT_ASC, $parts);
return implode('&', $parts); return implode('&', $parts);
@ -838,7 +847,8 @@ class Request
*/ */
public function getQueryString() public function getQueryString()
{ {
return static::normalizeQueryString($this->server->get('QUERY_STRING')); $qs = static::normalizeQueryString($this->server->get('QUERY_STRING'));
return '' === $qs ? null : $qs;
} }
/** /**

View File

@ -505,31 +505,52 @@ class RequestTest extends \PHPUnit_Framework_TestCase
/** /**
* @covers Symfony\Component\HttpFoundation\Request::getQueryString * @covers Symfony\Component\HttpFoundation\Request::getQueryString
* @covers Symfony\Component\HttpFoundation\Request::normalizeQueryString
* @dataProvider getQueryStringNormalizationData
*/ */
public function testGetQueryString() public function testGetQueryString($query, $expectedQuery, $msg)
{ {
$request = new Request(); $request = new Request();
$request->server->set('QUERY_STRING', 'foo'); $request->server->set('QUERY_STRING', $query);
$this->assertEquals('foo', $request->getQueryString(), '->getQueryString() works with valueless parameters'); $this->assertSame($expectedQuery, $request->getQueryString(), $msg);
}
$request->server->set('QUERY_STRING', 'foo='); public function getQueryStringNormalizationData()
$this->assertEquals('foo=', $request->getQueryString(), '->getQueryString() includes a dangling equal sign'); {
return array(
array('foo', 'foo', 'works with valueless parameters'),
array('foo=', 'foo=', 'includes a dangling equal sign'),
array('bar=&foo=bar', 'bar=&foo=bar', '->works with empty parameters'),
array('foo=bar&bar=', 'bar=&foo=bar', 'sorts keys alphabetically'),
$request->server->set('QUERY_STRING', 'bar=&foo=bar'); // GET parameters, that are submitted from a HTML form, encode spaces as "+" by default (as defined in enctype application/x-www-form-urlencoded).
$this->assertEquals('bar=&foo=bar', $request->getQueryString(), '->getQueryString() works when empty parameters'); // PHP also converts "+" to spaces when filling the global _GET or when using the function parse_str.
array('him=John%20Doe&her=Jane+Doe', 'her=Jane%20Doe&him=John%20Doe', 'normalizes spaces in both encodings "%20" and "+"'),
$request->server->set('QUERY_STRING', 'foo=bar&bar='); array('foo[]=1&foo[]=2', 'foo%5B%5D=1&foo%5B%5D=2', 'allows array notation'),
$this->assertEquals('bar=&foo=bar', $request->getQueryString(), '->getQueryString() sorts keys alphabetically'); array('foo=1&foo=2', 'foo=1&foo=2', 'allows repeated parameters'),
array('pa%3Dram=foo%26bar%3Dbaz&test=test', 'pa%3Dram=foo%26bar%3Dbaz&test=test', 'works with encoded delimiters'),
array('0', '0', 'allows "0"'),
array('Jane Doe&John%20Doe', 'Jane%20Doe&John%20Doe', 'normalizes encoding in keys'),
array('her=Jane Doe&him=John%20Doe', 'her=Jane%20Doe&him=John%20Doe', 'normalizes encoding in values'),
array('foo=bar&&&test&&', 'foo=bar&test', 'removes unneeded delimiters'),
array('formula=e=m*c^2', 'formula=e%3Dm%2Ac%5E2', 'correctly treats only the first "=" as delimiter and the next as value'),
$request->server->set('QUERY_STRING', 'him=John%20Doe&her=Jane+Doe'); // Ignore pairs with empty key, even if there was a value, e.g. "=value", as such nameless values cannot be retrieved anyway.
$this->assertEquals('her=Jane%2BDoe&him=John%20Doe', $request->getQueryString(), '->getQueryString() normalizes encoding'); // PHP also does not include them when building _GET.
array('foo=bar&=a=b&=x=y', 'foo=bar', 'removes params with empty key'),
);
}
$request->server->set('QUERY_STRING', 'foo[]=1&foo[]=2'); public function testGetQueryStringReturnsNull()
$this->assertEquals('foo%5B%5D=1&foo%5B%5D=2', $request->getQueryString(), '->getQueryString() allows array notation'); {
$request = new Request();
$request->server->set('QUERY_STRING', 'foo=1&foo=2'); $this->assertNull($request->getQueryString(), '->getQueryString() returns null for non-existent query string');
$this->assertEquals('foo=1&foo=2', $request->getQueryString(), '->getQueryString() allows repeated parameters');
$request->server->set('QUERY_STRING', '');
$this->assertNull($request->getQueryString(), '->getQueryString() returns null for empty query string');
} }
/** /**