bug #17287 [HttpKernel] Forcing string comparison on query parameters sort in UriSigner (Tim van Densen)

fabpot · fabpot · commit 5d63c554e886 · 2016-01-07T14:44:10.000+01:00
This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #17287). Discussion ---------- [HttpKernel] Forcing string comparison on query parameters sort in UriSigner | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | The signing of an url fails when using query parameters with integers as keys. The ksort function in the ```UriSigner``` class changes the order of the query params and causes to generate a different hash which results in a failed check. In this PR we force a string comparison for ksort which keeps the correct order of parameters. Commits ------- 2040139 Added sort order SORT_STRING for params in UriSigner
diff --git a/src/Symfony/Component/HttpKernel/Tests/UriSignerTest.php b/src/Symfony/Component/HttpKernel/Tests/UriSignerTest.php
@@ -33,6 +33,7 @@ public function testCheck()
 
         $this->assertTrue($signer->check($signer->sign('http://example.com/foo')));
         $this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar')));
+        $this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&0=integer')));
 
         $this->assertTrue($signer->sign('http://example.com/foo?foo=bar&bar=foo') === $signer->sign('http://example.com/foo?bar=foo&foo=bar'));
     }
diff --git a/src/Symfony/Component/HttpKernel/UriSigner.php b/src/Symfony/Component/HttpKernel/UriSigner.php
@@ -91,8 +91,8 @@ private function computeHash($uri)
 
     private function buildUrl(array $url, array $params = array())
     {
-        ksort($params);
-        $url['query'] = http_build_query($params);
+        ksort($params, SORT_STRING);
+        $url['query'] = http_build_query($params, '', '&');
 
         $scheme = isset($url['scheme']) ? $url['scheme'].'://' : '';
         $host = isset($url['host']) ? $url['host'] : '';

Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,7 @@ public function testCheck()`
`33`	`33`
`34`	`34`	`$this->assertTrue($signer->check($signer->sign('http://example.com/foo')));`
`35`	`35`	`$this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar')));`
	`36`	`+ $this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&0=integer')));`
`36`	`37`
`37`	`38`	`$this->assertTrue($signer->sign('http://example.com/foo?foo=bar&bar=foo') === $signer->sign('http://example.com/foo?bar=foo&foo=bar'));`
`38`	`39`	`}`
Original file line number	Diff line number	Diff line change
`@@ -91,8 +91,8 @@ private function computeHash($uri)`
`91`	`91`
`92`	`92`	`private function buildUrl(array $url, array $params = array())`
`93`	`93`	`{`
`94`		`- ksort($params);`
`95`		`- $url['query'] = http_build_query($params);`
	`94`	`+ ksort($params, SORT_STRING);`
	`95`	`+ $url['query'] = http_build_query($params, '', '&');`
`96`	`96`
`97`	`97`	`$scheme = isset($url['scheme']) ? $url['scheme'].'://' : '';`
`98`	`98`	`$host = isset($url['host']) ? $url['host'] : '';`