[HttpKernel] Fix UriSigner::check when _hash is not at the end of the uri #12574
Conversation
| $params = array(); | ||
| } | ||
|
|
||
| if (!isset($params['_hash']) || !$params['_hash']) { |
There was a problem hiding this comment.
if (empty($params['_hash'])) { will do the same in a more readable way
| $path = isset($url['path']) ? $url['path'] : ''; | ||
| $query = isset($url['query']) && $url['query'] ? '?' . $url['query'] : ''; | ||
| $fragment = isset($url['fragment']) ? '#' . $url['fragment'] : ''; | ||
| $testUrl = $scheme.$user.$pass.$host.$port.$path.$query.$fragment; |
There was a problem hiding this comment.
You could use http_build_url to generate the URL
There was a problem hiding this comment.
its only available as an extension
|
@GromNaN I'm running PHP 5.3.3 on the server I'm testing it and the function http_build_url is not defined for this version. |
|
@nyroDev This change will not help when varnish rewrites the query params. The hash is currently also done over the query string (without the hash param). So the order is important and if the order changes, the hash is not valid anymore. So this would also require to change the hashing logic to be independent of the query param order. |
…to create the same hash in every situation
|
i looked at the rfc and didn't found any information about how to normalize the param order. amazon aws for example use this strategy for url signing: Sort the encoded parameter names by character code (that is, in strict ASCII order). For example, a parameter name that begins with the uppercase letter F (ASCII code 70) precedes a parameter name that begins with a lowercase letter b (ASCII code 98). |
|
Thank you @nyroDev. |
… 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 #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
| @@ -33,5 +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->sign('http://example.com/foo?foo=bar&bar=foo') === $signer->sign('http://example.com/foo?bar=foo&foo=bar')); |
…_build_query (Jakub Simon) This PR was squashed before being merged into the 2.6 branch (closes #13944). Discussion ---------- [HttpKernel] UriSigner::buildUrl - default params for http_build_query | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - UriSigner fail to verify hash when custom ini setting arg_separator.output is used. It was introduced in #12574 Commits ------- 3d6933f [HttpKernel] UriSigner::buildUrl - default params for http_build_query
…ric) This PR was merged into the 2.7 branch. Discussion ---------- [HttpKernel] Remove obsolete PHPDoc from UriSigner | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | N/A | License | MIT | Doc PR | N/A `_hash` query param does not have to be at the end of the URL ever since #12574 so this PHPDoc is confusing. I've actually lost couple of hours of work rewriting my URLs to place it at the end before I realized that `UriSigner` doesn't really care. Commits ------- 45ac192 Remove obsolete PHPDoc from UriSigner
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.