From 0880174a541fcbf2ae8f47d3327990ec7095db34 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Wed, 4 Jul 2012 02:38:41 +0200 Subject: [PATCH 1/3] [HttpFoundation] added failing tests for query string normalization --- .../HttpFoundation/Tests/RequestTest.php | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php index 9a847554b3..2fc65a7620 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php @@ -510,6 +510,11 @@ class RequestTest extends \PHPUnit_Framework_TestCase { $request = new Request(); + $this->assertNull($request->getQueryString(), '->getQueryString() returns null for non-existent query string'); + + $request->server->set('QUERY_STRING', ''); + $this->assertNull($request->getQueryString(), '->getQueryString() returns null for empty query string'); + $request->server->set('QUERY_STRING', 'foo'); $this->assertEquals('foo', $request->getQueryString(), '->getQueryString() works with valueless parameters'); @@ -523,13 +528,38 @@ class RequestTest extends \PHPUnit_Framework_TestCase $this->assertEquals('bar=&foo=bar', $request->getQueryString(), '->getQueryString() sorts keys alphabetically'); $request->server->set('QUERY_STRING', 'him=John%20Doe&her=Jane+Doe'); - $this->assertEquals('her=Jane%2BDoe&him=John%20Doe', $request->getQueryString(), '->getQueryString() normalizes encoding'); + // 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->assertSame('her=Jane%20Doe&him=John%20Doe', $request->getQueryString(), '->getQueryString() normalizes spaces in both encodings "%20" and "+"'); $request->server->set('QUERY_STRING', 'foo[]=1&foo[]=2'); $this->assertEquals('foo%5B%5D=1&foo%5B%5D=2', $request->getQueryString(), '->getQueryString() allows array notation'); $request->server->set('QUERY_STRING', 'foo=1&foo=2'); $this->assertEquals('foo=1&foo=2', $request->getQueryString(), '->getQueryString() allows repeated parameters'); + + $request->server->set('QUERY_STRING', 'pa%3Dram=foo%26bar%3Dbaz&test=test'); + $this->assertSame('pa%3Dram=foo%26bar%3Dbaz&test=test', $request->getQueryString(), '->getQueryString() works with encoded delimiters'); + + $request->server->set('QUERY_STRING', '0'); + $this->assertSame('0', $request->getQueryString(), '->getQueryString() allows "0"'); + + $request->server->set('QUERY_STRING', 'Jane Doe&John%20Doe'); + $this->assertSame('Jane%20Doe&John%20Doe', $request->getQueryString(), '->getQueryString() normalizes encoding in keys'); + + $request->server->set('QUERY_STRING', 'her=Jane Doe&him=John%20Doe'); + $this->assertSame('her=Jane%20Doe&him=John%20Doe', $request->getQueryString(), '->getQueryString() normalizes encoding in values'); + + $request->server->set('QUERY_STRING', 'foo=bar&&&test&&'); + $this->assertSame('foo=bar&test', $request->getQueryString(), '->getQueryString() removes unneeded delimiters'); + + $request->server->set('QUERY_STRING', 'formula=e=m*c^2'); + $this->assertSame('formula=e%3Dm%2Ac%5E2', $request->getQueryString(), '->getQueryString() correctly treats only the first "=" as delimiter and the next as value'); + + $request->server->set('QUERY_STRING', 'foo=bar&=a=b&=x=y'); + // Ignore pairs with empty key, even if there was a value, e.g. "=value", as such nameless values cannot be retrieved anyway. + // PHP also does not include them when building _GET. + $this->assertSame('foo=bar', $request->getQueryString(), '->getQueryString() removes params with empty key'); } /** From f9ec2ea3be2258959323d762d577903337e0e424 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Wed, 4 Jul 2012 03:05:07 +0200 Subject: [PATCH 2/3] refactored test method --- .../HttpFoundation/Tests/RequestTest.php | 85 +++++++++---------- 1 file changed, 38 insertions(+), 47 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php index 2fc65a7620..14e01aa969 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php @@ -505,8 +505,45 @@ class RequestTest extends \PHPUnit_Framework_TestCase /** * @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->server->set('QUERY_STRING', $query); + $this->assertSame($expectedQuery, $request->getQueryString(), $msg); + } + + public function getQueryStringNormalizationData() + { + 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'), + + // 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. + array('him=John%20Doe&her=Jane+Doe', 'her=Jane%20Doe&him=John%20Doe', 'normalizes spaces in both encodings "%20" and "+"'), + + array('foo[]=1&foo[]=2', 'foo%5B%5D=1&foo%5B%5D=2', 'allows array notation'), + 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'), + + // Ignore pairs with empty key, even if there was a value, e.g. "=value", as such nameless values cannot be retrieved anyway. + // PHP also does not include them when building _GET. + array('foo=bar&=a=b&=x=y', 'foo=bar', 'removes params with empty key'), + ); + } + + public function testGetQueryStringReturnsNull() { $request = new Request(); @@ -514,52 +551,6 @@ class RequestTest extends \PHPUnit_Framework_TestCase $request->server->set('QUERY_STRING', ''); $this->assertNull($request->getQueryString(), '->getQueryString() returns null for empty query string'); - - $request->server->set('QUERY_STRING', 'foo'); - $this->assertEquals('foo', $request->getQueryString(), '->getQueryString() works with valueless parameters'); - - $request->server->set('QUERY_STRING', 'foo='); - $this->assertEquals('foo=', $request->getQueryString(), '->getQueryString() includes a dangling equal sign'); - - $request->server->set('QUERY_STRING', 'bar=&foo=bar'); - $this->assertEquals('bar=&foo=bar', $request->getQueryString(), '->getQueryString() works when empty parameters'); - - $request->server->set('QUERY_STRING', 'foo=bar&bar='); - $this->assertEquals('bar=&foo=bar', $request->getQueryString(), '->getQueryString() sorts keys alphabetically'); - - $request->server->set('QUERY_STRING', 'him=John%20Doe&her=Jane+Doe'); - // 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->assertSame('her=Jane%20Doe&him=John%20Doe', $request->getQueryString(), '->getQueryString() normalizes spaces in both encodings "%20" and "+"'); - - $request->server->set('QUERY_STRING', 'foo[]=1&foo[]=2'); - $this->assertEquals('foo%5B%5D=1&foo%5B%5D=2', $request->getQueryString(), '->getQueryString() allows array notation'); - - $request->server->set('QUERY_STRING', 'foo=1&foo=2'); - $this->assertEquals('foo=1&foo=2', $request->getQueryString(), '->getQueryString() allows repeated parameters'); - - $request->server->set('QUERY_STRING', 'pa%3Dram=foo%26bar%3Dbaz&test=test'); - $this->assertSame('pa%3Dram=foo%26bar%3Dbaz&test=test', $request->getQueryString(), '->getQueryString() works with encoded delimiters'); - - $request->server->set('QUERY_STRING', '0'); - $this->assertSame('0', $request->getQueryString(), '->getQueryString() allows "0"'); - - $request->server->set('QUERY_STRING', 'Jane Doe&John%20Doe'); - $this->assertSame('Jane%20Doe&John%20Doe', $request->getQueryString(), '->getQueryString() normalizes encoding in keys'); - - $request->server->set('QUERY_STRING', 'her=Jane Doe&him=John%20Doe'); - $this->assertSame('her=Jane%20Doe&him=John%20Doe', $request->getQueryString(), '->getQueryString() normalizes encoding in values'); - - $request->server->set('QUERY_STRING', 'foo=bar&&&test&&'); - $this->assertSame('foo=bar&test', $request->getQueryString(), '->getQueryString() removes unneeded delimiters'); - - $request->server->set('QUERY_STRING', 'formula=e=m*c^2'); - $this->assertSame('formula=e%3Dm%2Ac%5E2', $request->getQueryString(), '->getQueryString() correctly treats only the first "=" as delimiter and the next as value'); - - $request->server->set('QUERY_STRING', 'foo=bar&=a=b&=x=y'); - // Ignore pairs with empty key, even if there was a value, e.g. "=value", as such nameless values cannot be retrieved anyway. - // PHP also does not include them when building _GET. - $this->assertSame('foo=bar', $request->getQueryString(), '->getQueryString() removes params with empty key'); } /** From c40a4e50a918cc559fbdb5e91c05beb5b3b0d710 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Wed, 4 Jul 2012 03:10:48 +0200 Subject: [PATCH 3/3] [HttpFoundation] fix query string normalization --- .../Component/HttpFoundation/Request.php | 40 ++++++++++++------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php index afab299767..756f6b7f6c 100644 --- a/src/Symfony/Component/HttpFoundation/Request.php +++ b/src/Symfony/Component/HttpFoundation/Request.php @@ -466,32 +466,41 @@ class Request /** * Normalizes a query string. * - * It builds a normalized query string, where keys/value pairs are alphabetized - * and have consistent escaping. + * It builds a normalized query string, where keys/value pairs are alphabetized, + * have consistent escaping and unneeded delimiters are removed. * * @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) { - return null; + if ('' == $qs) { + return ''; } $parts = array(); $order = array(); - foreach (explode('&', $qs) as $segment) { - if (false === strpos($segment, '=')) { - $parts[] = $segment; - $order[] = $segment; - } else { - $tmp = explode('=', rawurldecode($segment), 2); - $parts[] = rawurlencode($tmp[0]).'='.rawurlencode($tmp[1]); - $order[] = $tmp[0]; + foreach (explode('&', $qs) as $param) { + if ('' === $param || '=' === $param[0]) { + // Ignore useless delimiters, e.g. "x=y&". + // Also ignore pairs with empty key, even if there was a value, e.g. "=value", as such nameless values cannot be retrieved anyway. + // PHP also does not include them when building _GET. + continue; } + + $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); return implode('&', $parts); @@ -843,7 +852,8 @@ class Request */ public function getQueryString() { - return static::normalizeQueryString($this->server->get('QUERY_STRING')); + $qs = static::normalizeQueryString($this->server->get('QUERY_STRING')); + return '' === $qs ? null : $qs; } /**