From b9ece6bde7a204b6afe94fc7978181a14eae2a22 Mon Sep 17 00:00:00 2001 From: Zan Baldwin Date: Mon, 24 Dec 2018 12:22:50 +0000 Subject: [PATCH] [HttpKernel] Correctly Render Signed URIs Containing Fragments Rebuild the URL with the computed hash instead of appending it onto the end of the URI, preventing incorrect formatting when dealing with URIs containing fragments. --- .../Tests/Fragment/EsiFragmentRendererTest.php | 2 +- .../Fragment/HIncludeFragmentRendererTest.php | 2 +- .../Tests/Fragment/SsiFragmentRendererTest.php | 2 +- .../Component/HttpKernel/Tests/UriSignerTest.php | 16 ++++++++++++++-- src/Symfony/Component/HttpKernel/UriSigner.php | 7 ++++--- 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/Tests/Fragment/EsiFragmentRendererTest.php b/src/Symfony/Component/HttpKernel/Tests/Fragment/EsiFragmentRendererTest.php index 00d796d520..51e5756203 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Fragment/EsiFragmentRendererTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Fragment/EsiFragmentRendererTest.php @@ -72,7 +72,7 @@ class EsiFragmentRendererTest extends TestCase $altReference = new ControllerReference('alt_controller', array(), array()); $this->assertEquals( - '', + '', $strategy->render($reference, $request, array('alt' => $altReference))->getContent() ); } diff --git a/src/Symfony/Component/HttpKernel/Tests/Fragment/HIncludeFragmentRendererTest.php b/src/Symfony/Component/HttpKernel/Tests/Fragment/HIncludeFragmentRendererTest.php index e3926708c1..68f8ded4e9 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Fragment/HIncludeFragmentRendererTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Fragment/HIncludeFragmentRendererTest.php @@ -32,7 +32,7 @@ class HIncludeFragmentRendererTest extends TestCase { $strategy = new HIncludeFragmentRenderer(null, new UriSigner('foo')); - $this->assertEquals('', $strategy->render(new ControllerReference('main_controller', array(), array()), Request::create('/'))->getContent()); + $this->assertEquals('', $strategy->render(new ControllerReference('main_controller', array(), array()), Request::create('/'))->getContent()); } public function testRenderWithUri() diff --git a/src/Symfony/Component/HttpKernel/Tests/Fragment/SsiFragmentRendererTest.php b/src/Symfony/Component/HttpKernel/Tests/Fragment/SsiFragmentRendererTest.php index f725803118..ff98fd2616 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Fragment/SsiFragmentRendererTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Fragment/SsiFragmentRendererTest.php @@ -51,7 +51,7 @@ class SsiFragmentRendererTest extends TestCase $altReference = new ControllerReference('alt_controller', array(), array()); $this->assertEquals( - '', + '', $strategy->render($reference, $request, array('alt' => $altReference))->getContent() ); } diff --git a/src/Symfony/Component/HttpKernel/Tests/UriSignerTest.php b/src/Symfony/Component/HttpKernel/Tests/UriSignerTest.php index 84ec19f70d..9b7fe08a99 100644 --- a/src/Symfony/Component/HttpKernel/Tests/UriSignerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/UriSignerTest.php @@ -21,7 +21,8 @@ class UriSignerTest extends TestCase $signer = new UriSigner('foobar'); $this->assertContains('?_hash=', $signer->sign('http://example.com/foo')); - $this->assertContains('&_hash=', $signer->sign('http://example.com/foo?foo=bar')); + $this->assertContains('?_hash=', $signer->sign('http://example.com/foo?foo=bar')); + $this->assertContains('&foo=', $signer->sign('http://example.com/foo?foo=bar')); } public function testCheck() @@ -45,7 +46,7 @@ class UriSignerTest extends TestCase $signer = new UriSigner('foobar'); $this->assertSame( - 'http://example.com/foo?baz=bay&foo=bar&_hash=rIOcC%2FF3DoEGo%2FvnESjSp7uU9zA9S%2F%2BOLhxgMexoPUM%3D', + 'http://example.com/foo?_hash=rIOcC%2FF3DoEGo%2FvnESjSp7uU9zA9S%2F%2BOLhxgMexoPUM%3D&baz=bay&foo=bar', $signer->sign('http://example.com/foo?foo=bar&baz=bay') ); $this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&baz=bay'))); @@ -61,4 +62,15 @@ class UriSignerTest extends TestCase ); $this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&baz=bay'))); } + + public function testSignerWorksWithFragments() + { + $signer = new UriSigner('foobar'); + + $this->assertSame( + 'http://example.com/foo?_hash=EhpAUyEobiM3QTrKxoLOtQq5IsWyWedoXDPqIjzNj5o%3D&bar=foo&foo=bar#foobar', + $signer->sign('http://example.com/foo?bar=foo&foo=bar#foobar') + ); + $this->assertTrue($signer->check($signer->sign('http://example.com/foo?bar=foo&foo=bar#foobar'))); + } } diff --git a/src/Symfony/Component/HttpKernel/UriSigner.php b/src/Symfony/Component/HttpKernel/UriSigner.php index 28459b4ecd..63d1a0856f 100644 --- a/src/Symfony/Component/HttpKernel/UriSigner.php +++ b/src/Symfony/Component/HttpKernel/UriSigner.php @@ -51,8 +51,9 @@ class UriSigner } $uri = $this->buildUrl($url, $params); + $params[$this->parameter] = $this->computeHash($uri); - return $uri.(false === strpos($uri, '?') ? '?' : '&').$this->parameter.'='.$this->computeHash($uri); + return $this->buildUrl($url, $params); } /** @@ -75,7 +76,7 @@ class UriSigner return false; } - $hash = urlencode($params[$this->parameter]); + $hash = $params[$this->parameter]; unset($params[$this->parameter]); return $this->computeHash($this->buildUrl($url, $params)) === $hash; @@ -83,7 +84,7 @@ class UriSigner private function computeHash($uri) { - return urlencode(base64_encode(hash_hmac('sha256', $uri, $this->secret, true))); + return base64_encode(hash_hmac('sha256', $uri, $this->secret, true)); } private function buildUrl(array $url, array $params = array())