Skip to content

Commit d2e951b

Browse files
committed
bug symfony#12574 [HttpKernel] Fix UriSigner::check when _hash is not at the end of the uri (nyroDev)
This PR was submitted for the 2.5 branch but it was merged into the 2.3 branch instead (closes symfony#12574). Discussion ---------- [HttpKernel] Fix UriSigner::check when _hash is not at the end of the uri | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - I have a weird server installation behind Varnish that rewrite the signed URL to add the _hash at the end of the url queries. Exemple : URL called: http://exemple.com/page?foo=bar&_hash=123 URL received by PHP: http://exemple.com/page?_hash=123&foo=bar When the _hash is not at the end of the URL, the UriSigner fail to verify it even if the _hash is correct. The fix rewrites the check function to use parse_url and parse_str to analyse the URI and check the signature. Commits ------- 29b217c [HttpKernel] Fix UriSigner::check when _hash is not at the end of the uri
2 parents a7d5624 + 29b217c commit d2e951b

File tree

2 files changed

+41
-2
lines changed

2 files changed

+41
-2
lines changed

src/Symfony/Component/HttpKernel/Tests/UriSignerTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,7 @@ public function testCheck()
3333

3434
$this->assertTrue($signer->check($signer->sign('http://example.com/foo')));
3535
$this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar')));
36+
37+
$this->assertTrue($signer->sign('http://example.com/foo?foo=bar&bar=foo') === $signer->sign('http://example.com/foo?bar=foo&foo=bar'));
3638
}
3739
}

src/Symfony/Component/HttpKernel/UriSigner.php

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,15 @@ public function __construct($secret)
4242
*/
4343
public function sign($uri)
4444
{
45+
$url = parse_url($uri);
46+
if (isset($url['query'])) {
47+
parse_str($url['query'], $params);
48+
} else {
49+
$params = array();
50+
}
51+
52+
$uri = $this->buildUrl($url, $params);
53+
4554
return $uri.(false === (strpos($uri, '?')) ? '?' : '&').'_hash='.$this->computeHash($uri);
4655
}
4756

@@ -58,15 +67,43 @@ public function sign($uri)
5867
*/
5968
public function check($uri)
6069
{
61-
if (!preg_match('/^(.*)(?:\?|&)_hash=(.+?)$/', $uri, $matches)) {
70+
$url = parse_url($uri);
71+
if (isset($url['query'])) {
72+
parse_str($url['query'], $params);
73+
} else {
74+
$params = array();
75+
}
76+
77+
if (empty($params['_hash'])) {
6278
return false;
6379
}
6480

65-
return $this->computeHash($matches[1]) === $matches[2];
81+
$hash = urlencode($params['_hash']);
82+
unset($params['_hash']);
83+
84+
return $this->computeHash($this->buildUrl($url, $params)) === $hash;
6685
}
6786

6887
private function computeHash($uri)
6988
{
7089
return urlencode(base64_encode(hash_hmac('sha1', $uri, $this->secret, true)));
7190
}
91+
92+
private function buildUrl(array $url, array $params = array())
93+
{
94+
ksort($params);
95+
$url['query'] = http_build_query($params);
96+
97+
$scheme = isset($url['scheme']) ? $url['scheme'].'://' : '';
98+
$host = isset($url['host']) ? $url['host'] : '';
99+
$port = isset($url['port']) ? ':'.$url['port'] : '';
100+
$user = isset($url['user']) ? $url['user'] : '';
101+
$pass = isset($url['pass']) ? ':'.$url['pass'] : '';
102+
$pass = ($user || $pass) ? "$pass@" : '';
103+
$path = isset($url['path']) ? $url['path'] : '';
104+
$query = isset($url['query']) && $url['query'] ? '?'.$url['query'] : '';
105+
$fragment = isset($url['fragment']) ? '#'.$url['fragment'] : '';
106+
107+
return $scheme.$user.$pass.$host.$port.$path.$query.$fragment;
108+
}
72109
}

0 commit comments

Comments
 (0)