bug #29679 [HttpKernel] Correctly Render Signed URIs Containing Fragm…

…ents (zanbaldwin)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel] Correctly Render Signed URIs Containing Fragments

| Q             | A
| ------------- | ---
| Branch?       | `3.4`
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no?
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

- Rebuild the URL with the computed hash instead of appending it onto the end of the fragment.
- Update unit tests, and add new unit test to cover URIs that include fragments.

Commits
-------

b9ece6b [HttpKernel] Correctly Render Signed URIs Containing Fragments

src/Symfony/Component/HttpKernel/Tests/Fragment/EsiFragmentRendererTest.php

@@ -72,7 +72,7 @@ public function testRenderControllerReference()
         $altReference = new ControllerReference('alt_controller', array(), array());
 
         $this->assertEquals(
-            '<esi:include src="/_fragment?_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dmain_controller&_hash=Jz1P8NErmhKTeI6onI1EdAXTB85359MY3RIk5mSJ60w%3D" alt="/_fragment?_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dalt_controller&_hash=iPJEdRoUpGrM1ztqByiorpfMPtiW%2FOWwdH1DBUXHhEc%3D" />',
+            '<esi:include src="/_fragment?_hash=Jz1P8NErmhKTeI6onI1EdAXTB85359MY3RIk5mSJ60w%3D&_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dmain_controller" alt="/_fragment?_hash=iPJEdRoUpGrM1ztqByiorpfMPtiW%2FOWwdH1DBUXHhEc%3D&_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dalt_controller" />',
             $strategy->render($reference, $request, array('alt' => $altReference))->getContent()
         );
     }

src/Symfony/Component/HttpKernel/Tests/Fragment/HIncludeFragmentRendererTest.php

@@ -32,7 +32,7 @@ public function testRenderWithControllerAndSigner()
     {
         $strategy = new HIncludeFragmentRenderer(null, new UriSigner('foo'));
 
-        $this->assertEquals('<hx:include src="/_fragment?_path=_format%3Dhtml%26_locale%3Den%26_controller%3Dmain_controller&amp;_hash=BP%2BOzCD5MRUI%2BHJpgPDOmoju00FnzLhP3TGcSHbbBLs%3D"></hx:include>', $strategy->render(new ControllerReference('main_controller', array(), array()), Request::create('/'))->getContent());
+        $this->assertEquals('<hx:include src="/_fragment?_hash=BP%2BOzCD5MRUI%2BHJpgPDOmoju00FnzLhP3TGcSHbbBLs%3D&amp;_path=_format%3Dhtml%26_locale%3Den%26_controller%3Dmain_controller"></hx:include>', $strategy->render(new ControllerReference('main_controller', array(), array()), Request::create('/'))->getContent());
     }
 
     public function testRenderWithUri()

src/Symfony/Component/HttpKernel/Tests/Fragment/SsiFragmentRendererTest.php

@@ -51,7 +51,7 @@ public function testRenderControllerReference()
         $altReference = new ControllerReference('alt_controller', array(), array());
 
         $this->assertEquals(
-            '<!--#include virtual="/_fragment?_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dmain_controller&_hash=Jz1P8NErmhKTeI6onI1EdAXTB85359MY3RIk5mSJ60w%3D" -->',
+            '<!--#include virtual="/_fragment?_hash=Jz1P8NErmhKTeI6onI1EdAXTB85359MY3RIk5mSJ60w%3D&_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dmain_controller" -->',
             $strategy->render($reference, $request, array('alt' => $altReference))->getContent()
         );
     }

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

@@ -21,7 +21,8 @@ public function testSign()
         $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 @@ public function testCheckWithDifferentArgSeparator()
         $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 @@ public function testCheckWithDifferentParameter()
         );
         $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')));
+    }
 }

src/Symfony/Component/HttpKernel/UriSigner.php

@@ -51,8 +51,9 @@ public function sign($uri)
         }
 
         $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,15 +76,15 @@ public function check($uri)
             return false;
         }
 
-        $hash = urlencode($params[$this->parameter]);
+        $hash = $params[$this->parameter];
         unset($params[$this->parameter]);
 
         return $this->computeHash($this->buildUrl($url, $params)) === $hash;
     }
 
     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())