8000 feature #59022 [HttpFoundation] Generate url-safe hashes for signed u… · symfony/symfony@dd061aa · GitHub
[go: up one dir, main page]
Skip to content

Commit dd061aa

Browse files
fabpotfabpot
committed
feature #59022 [HttpFoundation] Generate url-safe hashes for signed urls (valtzu)
This PR was merged into the 7.3 branch. Discussion ---------- [HttpFoundation] Generate url-safe hashes for signed urls | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | Fix #52822 | License | MIT Generate url-safe signed urls since some clients can't handle parameter encoding correctly and are sending `%2B` as `+` which PHP reads as ` ` (space), making the signature invalid. Backwards-compatibility is included, and planned to be removed in 8.0 – which will mean all signed urls generated with <7.3 are rendered invalid. Commits ------- a533750 Generate url-safe signatures
2 parents 7d6b9ad + a533750 commit dd061aa

File tree

10 files changed

+27
-20
lines changed

10 files changed

+27
-20
lines changed

‎src/Symfony/Bridge/Twig/Tests/Extension/HttpKernelExtensionTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public function testGenerateFragmentUri()
8080
]);
8181
$twig->addRuntimeLoader($loader);
8282

83-
$this->assertSame('/_fragment?_hash=XCg0hX8QzSwik8Xuu9aMXhoCeI4oJOob7lUVacyOtyY%3D&amp;_path=template%3Dfoo.html.twig%26_format%3Dhtml%26_locale%3Den%26_controller%3DSymfony%255CBundle%255CFrameworkBundle%255CController%255CTemplateController%253A%253AtemplateAction', $twig->render('index'));
83+
$this->assertSame('/_fragment?_hash=XCg0hX8QzSwik8Xuu9aMXhoCeI4oJOob7lUVacyOtyY&amp;_path=template%3Dfoo.html.twig%26_format%3Dhtml%26_locale%3Den%26_controller%3DSymfony%255CBundle%255CFrameworkBundle%255CController%255CTemplateController%253A%253AtemplateAction', $twig->render('index'));
8484
}
8585

8686
protected function getFragmentHandler($returnOrException): FragmentHandler

‎src/Symfony/Bridge/Twig/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
"symfony/finder": "^6.4|^7.0",
3333
"symfony/form": "^6.4|^7.0",
3434
"symfony/html-sanitizer": "^6.4|^7.0",
35-
"symfony/http-foundation": "^6.4|^7.0",
35+
"symfony/http-foundation": "^7.3",
3636
"symfony/http-kernel": "^6.4|^7.0",
3737
"symfony/intl": "^6.4|^7.0",
3838
"symfony/mime": "^6.4|^7.0",

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/FragmentTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,6 @@ public function testGenerateFragmentUri()
5050
$client = self::createClient(['test_case' => 'Fragment', 'root_config' => 'config.yml', 'debug' => true]);
5151
$client->request('GET', '/fragment_uri');
5252

53-
$this->assertSame('/_fragment?_hash=CCRGN2D%2FoAJbeGz%2F%2FdoH3bNSPwLCrmwC1zAYCGIKJ0E%3D&_path=_format%3Dhtml%26_locale%3Den%26_controller%3DSymfony%255CBundle%255CFrameworkBundle%255CTests%255CFunctional%255CBundle%255CTestBundle%255CController%255CFragmentController%253A%253AindexAction', $client->getResponse()->getContent());
53+
$this->assertSame('/_fragment?_hash=CCRGN2D_oAJbeGz__doH3bNSPwLCrmwC1zAYCGIKJ0E&_path=_format%3Dhtml%26_locale%3Den%26_controller%3DSymfony%255CBundle%255CFrameworkBundle%255CTests%255CFunctional%255CBundle%255CTestBundle%255CController%255CFragmentController%253A%253AindexAction', $client->getRespons 628C e()->getContent());
5454
}
5555
}

‎src/Symfony/Bundle/FrameworkBundle/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
"symfony/deprecation-contracts": "^2.5|^3",
2626
"symfony/error-handler": "^6.4|^7.0",
2727
"symfony/event-dispatcher": "^6.4|^7.0",
28-
"symfony/http-foundation": "^6.4|^7.0",
28+
"symfony/http-foundation": "^7.3",
2929
"symfony/http-kernel": "^7.2",
3030
"symfony/polyfill-mbstring": "~1.0",
3131
"symfony/filesystem": "^7.1",

‎src/Symfony/Component/HttpFoundation/Tests/UriSignerTest.php

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,13 @@ public function testCheckWithDifferentArgSeparator()
7070
$signer = new UriSigner('foobar');
7171

7272
$this->assertSame(
73-
'http://example.com/foo?_hash=rIOcC%2FF3DoEGo%2FvnESjSp7uU9zA9S%2F%2BOLhxgMexoPUM%3D&baz=bay&foo=bar',
73+
'http://example.com/foo?_hash=rIOcC_F3DoEGo_vnESjSp7uU9zA9S_-OLhxgMexoPUM&baz=bay&foo=bar',
7474
$signer->sign('http://example.com/foo?foo=bar&baz=bay')
7575
);
7676
$this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&baz=bay')));
7777

7878
$this->assertSame(
79-
'http://example.com/foo?_expiration=2145916800&_hash=xLhnPMzV3KqqHaaUffBUJvtRDAZ4%2FZ9Y8Sw%2BgmS%2B82Q%3D&baz=bay&foo=bar',
79+
'http://example.com/foo?_expiration=2145916800&_hash=xLhnPMzV3KqqHaaUffBUJvtRDAZ4_Z9Y8Sw-gmS-82Q&baz=bay&foo=bar',
8080
$signer->sign('http://example.com/foo?foo=bar&baz=bay', new \DateTimeImmutable('2038-01-01 00:00:00', new \DateTimeZone('UTC')))
8181
);
8282
$this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&baz=bay', new \DateTimeImmutable('2099-01-01 00:00:00'))));
@@ -103,13 +103,13 @@ public function testCheckWithDifferentParameter()
103103
$signer = new UriSigner('foobar', 'qux', 'abc');
104104

105105
$this->assertSame(
106-
'http://example.com/foo?baz=bay&foo=bar&qux=rIOcC%2FF3DoEGo%2FvnESjSp7uU9zA9S%2F%2BOLhxgMexoPUM%3D',
106+
'http://example.com/foo?baz=bay&foo=bar&qux=rIOcC_F3DoEGo_vnESjSp7uU9zA9S_-OLhxgMexoPUM',
107107
$signer->sign('http://example.com/foo?foo=bar&baz=bay')
108108
);
109109
$this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&baz=bay')));
110110

111111
$this->assertSame(
112-
'http://example.com/foo?abc=2145916800&baz=bay&foo=bar&qux=kE4rK2MzeiwrYAKy%2B%2FGKvKA6bnzqCbACBdpC3yGnPVU%3D',
112+
'http://example.com/foo?abc=2145916800&baz=bay&foo=bar&qux=kE4rK2MzeiwrYAKy-_GKvKA6bnzqCbACBdpC3yGnPVU',
113113
$signer->sign('http://example.com/foo?foo=bar&baz=bay', new \DateTimeImmutable('2038-01-01 00:00:00', new \DateTimeZone('UTC')))
114114
);
115115
$this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&baz=bay', new \DateTimeImmutable('2099-01-01 00:00:00'))));
@@ -120,14 +120,14 @@ public function testSignerWorksWithFragments()
120120
$signer = new UriSigner('foobar');
121121

122122
$this->assertSame(
123-
'http://example.com/foo?_hash=EhpAUyEobiM3QTrKxoLOtQq5IsWyWedoXDPqIjzNj5o%3D&bar=foo&foo=bar#foobar',
123+
'http://example.com/foo?_hash=EhpAUyEobiM3QTrKxoLOtQq5IsWyWedoXDPqIjzNj5o&bar=foo&foo=bar#foobar',
124124
$signer->sign('http://example.com/foo?bar=foo&foo=bar#foobar')
125125
);
126126

127127
$this->assertTrue($signer->check($signer->sign('http://example.com/foo?bar=foo&foo=bar#foobar')));
128128

129129
$this->assertSame(
130-
'http://example.com/foo?_expiration=2145916800&_hash=jTdrIE9MJSorNpQmkX6tmOtocxXtHDzIJawcAW4IFYo%3D&bar=foo&foo=bar#foobar',
130+
'http://example.com/foo?_expiration=2145916800&_hash=jTdrIE9MJSorNpQmkX6tmOtocxXtHDzIJawcAW4IFYo&bar=foo&foo=bar#foobar',
131131
$signer->sign('http://example.com/foo?bar=foo&foo=bar#foobar', new \DateTimeImmutable('2038-01-01 00:00:00', new \DateTimeZone('UTC')))
132132
);
133133

@@ -198,4 +198,10 @@ public function testCheckWithUriExpiration()
198198
$this->assertFalse($signer->check($relativeUriFromNow2));
199199
$this->assertFalse($signer->check($relativeUriFromNow3));
200200
}
201+
202+
public function testNonUrlSafeBase64()
203+
{
204+
$signer = new UriSigner('foobar');
205+
$this->assertTrue($signer->check('http://example.com/foo?_hash=rIOcC%2FF3DoEGo%2FvnESjSp7uU9zA9S%2F%2BOLhxgMexoPUM%3D&baz=bay&foo=bar'));
206+
}
201207
}

‎src/Symfony/Component/HttpFoundation/UriSigner.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public function __construct(
4646
*
4747
* The expiration is added as a query string parameter.
4848
*/
49-
public function sign(string $uri/*, \DateTimeInterface|\DateInterval|int|null $expiration = null*/): string
49+
public function sign(string $uri/* , \DateTimeInterface|\DateInterval|int|null $expiration = null */): string
5050
{
5151
$expiration = null;
5252

@@ -55,7 +55,7 @@ public function sign(string $uri/*, \DateTimeInterface|\DateInterval|int|null $e
5555
}
5656

5757
if (null !== $expiration && !$expiration instanceof \DateTimeInterface && !$expiration instanceof \DateInterval && !\is_int($expiration)) {
58-
throw new \TypeError(\sprintf('The second argument of %s() must be an instance of %s or %s, an integer or null (%s given).', __METHOD__, \DateTimeInterface::class, \DateInterval::class, get_debug_type($expiration)));
58+
throw new \TypeError(\sprintf('The second argument of "%s()" must be an instance of "%s" or "%s", an integer or null (%s given).', __METHOD__, \DateTimeInterface::class, \DateInterval::class, get_debug_type($expiration)));
5959
}
6060

6161
$url = parse_url($uri);
@@ -103,7 +103,8 @@ public function check(string $uri): bool
103103
$hash = $params[$this->hashParameter];
104104
unset($params[$this->hashParameter]);
105105

106-
if (!hash_equals($this->computeHash($this->buildUrl($url, $params)), $hash)) {
106+
// In 8.0, remove support for non-url-safe tokens
107+
if (!hash_equals($this->computeHash($this->buildUrl($url, $params)), strtr(rtrim($hash, '='), ['/' => '_', '+' => '-']))) {
107108
return false;
108109
}
109110

@@ -124,7 +125,7 @@ public function checkRequest(Request $request): bool
124125

125126
private function computeHash(string $uri): string
126127
{
127-
return base64_encode(hash_hmac('sha256', $uri, $this->secret, true));
128+
return strtr(rtrim(base64_encode(hash_hmac('sha256', $uri, $this->secret, true)), '='), ['/' => '_', '+' => '-']);
128129
}
129130

130131
private function buildUrl(array $url, array $params = []): string

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public function testRenderControllerReference()
6161
$altReference = new ControllerReference('alt_controller', [], []);
6262

6363
$this->assertEquals(
64-
'<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" />',
64+
'<esi:include src="/_fragment?_hash=Jz1P8NErmhKTeI6onI1EdAXTB85359MY3RIk5mSJ60w&_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dmain_controller" alt="/_fragment?_hash=iPJEdRoUpGrM1ztqByiorpfMPtiW_OWwdH1DBUXHhEc&_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dalt_controller" />',
6565
$strategy->render($reference, $request, ['alt' => $altReference])->getContent()
6666
);
6767
}
@@ -79,7 +79,7 @@ public function testRenderControllerReferenceWithAbsoluteUri()
7979
$altReference = new ControllerReference('alt_controller', [], []);
8080

8181
$this->assertSame(
82-
'<esi:include src="http://localhost/_fragment?_hash=Jz1P8NErmhKTeI6onI1EdAXTB85359MY3RIk5mSJ60w%3D&_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dmain_controller" alt="http://localhost/_fragment?_hash=iPJEdRoUpGrM1ztqByiorpfMPtiW%2FOWwdH1DBUXHhEc%3D&_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dalt_controller" />',
82+
'<esi:include src="http://localhost/_fragment?_hash=Jz1P8NErmhKTeI6onI1EdAXTB85359MY3RIk5mSJ60w&_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dmain_controller" alt="http://localhost/_fragment?_hash=iPJEdRoUpGrM1ztqByiorpfMPtiW_OWwdH1DBUXHhEc&_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dalt_controller" />',
8383
$strategy->render($reference, $request, ['alt' => $altReference, 'absolute_uri' => true])->getContent()
8484
);
8585
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public function testRenderWithControllerAndSigner()
3232
{
3333
$strategy = new HIncludeFragmentRenderer(null, new UriSigner('foo'));
3434

35-
$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', [], []), Request::create('/'))->getContent());
35+
$this->assertEquals('<hx:include src="/_fragment?_hash=BP-OzCD5MRUI-HJpgPDOmoju00FnzLhP3TGcSHbbBLs&amp;_path=_format%3Dhtml%26_locale%3Den%26_controller%3Dmain_controller"></hx:include>', $strategy->render(new ControllerReference('main_controller', [], []), Request::create('/'))->getContent());
3636
}
3737

3838
public function testRenderWithUri()

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public function testRenderControllerReference()
5252
$altReference = new ControllerReference('alt_controller', [], []);
5353

5454
$this->assertEquals(
55-
'<!--#include virtual="/_fragment?_hash=Jz1P8NErmhKTeI6onI1EdAXTB85359MY3RIk5mSJ60w%3D&_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dmain_controller" -->',
55+
'<!--#include virtual="/_fragment?_hash=Jz1P8NErmhKTeI6onI1EdAXTB85359MY3RIk5mSJ60w&_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dmain_controller" -->',
5656
$strategy->render($reference, $request, ['alt' => $altReference])->getContent()
5757
);
5858
}
@@ -70,7 +70,7 @@ public function testRenderControllerReferenceWithAbsoluteUri()
7070
$altReference = new ControllerReference('alt_controller', [], []);
7171

7272
$this->assertSame(
73-
'<!--#include virtual="http://localhost/_fragment?_hash=Jz1P8NErmhKTeI6onI1EdAXTB85359MY3RIk5mSJ60w%3D&_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dmain_controller" -->',
73+
'<!--#include virtual="http://localhost/_fragment?_hash=Jz1P8NErmhKTeI6onI1EdAXTB85359MY3RIk5mSJ60w&_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dmain_controller" -->',
7474
$strategy->render($reference, $request, ['alt' => $altReference, 'absolute_uri' => true])->getContent()
7575
);
7676
}

‎src/Symfony/Component/HttpKernel/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"symfony/deprecation-contracts": "^2.5|^3",
2121
"symfony/error-handler": "^6.4|^7.0",
2222
"symfony/event-dispatcher": "^6.4|^7.0",
23-
"symfony/http-foundation": "^6.4|^7.0",
23+
"symfony/http-foundation": "^7.3",
2424
"symfony/polyfill-ctype": "^1.8",
2525
"psr/log": "^1|^2|^3"
2626
},

0 commit comments

Comments
 (0)
0