Skip to content

Commit 278b203

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '6.3' into 6.4
* 6.3: [TwigBridge] Add integration tests on twig code helpers [TwigBridge] Ensure CodeExtension's filters properly escape their input do not emit an error if an issue suppression handler was not used [Webhook] Remove user-submitted type from HTTP response [Security] Fix possible session fixation when only the *token* changes [HttpClient] fix missing dep Update VERSION for 4.4.50 Update CHANGELOG for 4.4.50
2 parents 5aaa534 + 7610bc2 commit 278b203

File tree

6 files changed

+154
-32
lines changed

6 files changed

+154
-32
lines changed

‎psalm.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
errorBaseline=".github/psalm/psalm.baseline.xml"
1010
findUnusedBaselineEntry="false"
1111
findUnusedCode="false"
12+
findUnusedIssueHandlerSuppression="false"
1213
>
1314
<projectFiles>
1415
<directory name="src" />

‎src/Symfony/Bridge/Twig/Extension/CodeExtension.php‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ public function __construct(string|FileLinkFormatter $fileLinkFormat, string $pr
4141
public function getFilters(): array
4242
{
4343
return [
44-
new TwigFilter('abbr_class', $this->abbrClass(...), ['is_safe' => ['html']]),
45-
new TwigFilter('abbr_method', $this->abbrMethod(...), ['is_safe' => ['html']]),
44+
new TwigFilter('abbr_class', $this->abbrClass(...), ['is_safe' => ['html'], 'pre_escape' => 'html']),
45+
new TwigFilter('abbr_method', $this->abbrMethod(...), ['is_safe' => ['html'], 'pre_escape' => 'html']),
4646
new TwigFilter('format_args', $this->formatArgs(...), ['is_safe' => ['html']]),
4747
new TwigFilter('format_args_as_text', $this->formatArgsAsText(...)),
4848
new TwigFilter('file_excerpt', $this->fileExcerpt(...), ['is_safe' => ['html']]),
@@ -84,22 +84,23 @@ public function formatArgs(array $args): string
8484
$result = [];
8585
foreach ($args as $key => $item) {
8686
if ('object' === $item[0]) {
87+
$item[1] = htmlspecialchars($item[1], \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset);
8788
$parts = explode('\\', $item[1]);
8889
$short = array_pop($parts);
8990
$formattedValue = sprintf('<em>object</em>(<abbr title="%s">%s</abbr>)', $item[1], $short);
9091
} elseif ('array' === $item[0]) {
91-
$formattedValue = sprintf('<em>array</em>(%s)', \is_array($item[1]) ? $this->formatArgs($item[1]) : $item[1]);
92+
$formattedValue = sprintf('<em>array</em>(%s)', \is_array($item[1]) ? $this->formatArgs($item[1]) : htmlspecialchars(var_export($item[1], true), \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset));
9293
} elseif ('null' === $item[0]) {
9394
$formattedValue = '<em>null</em>';
9495
} elseif ('boolean' === $item[0]) {
95-
$formattedValue = '<em>'.strtolower(var_export($item[1], true)).'</em>';
96+
$formattedValue = '<em>'.strtolower(htmlspecialchars(var_export($item[1], true), \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset)).'</em>';
9697
} elseif ('resource' === $item[0]) {
9798
$formattedValue = '<em>resource</em>';
9899
} else {
99100
$formattedValue = str_replace("\n", '', htmlspecialchars(var_export($item[1], true), \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset));
100101
}
101102

102-
$result[] = \is_int($key) ? $formattedValue : sprintf("'%s' => %s", $key, $formattedValue);
103+
$result[] = \is_int($key) ? $formattedValue : sprintf("'%s' => %s", htmlspecialchars($key, \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset), $formattedValue);
103104
}
104105

105106
return implode(', ', $result);
@@ -161,11 +162,14 @@ public function formatFile(string $file, int $line, string $text = null): string
161162
$file = trim($file);
162163

163164
if (null === $text) {
164-
$text = $file;
165-
if (null !== $rel = $this->getFileRelative($text)) {
166-
$rel = explode('/', $rel, 2);
167-
$text = sprintf('<abbr title="%s%2$s">%s</abbr>%s', $this->projectDir, $rel[0], '/'.($rel[1] ?? ''));
165+
if (null !== $rel = $this->getFileRelative($file)) {
166+
$rel = explode('/', htmlspecialchars($rel, \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset), 2);
167+
$text = sprintf('<abbr title="%s%2$s">%s</abbr>%s', htmlspecialchars($this->projectDir, \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset), $rel[0], '/'.($rel[1] ?? ''));
168+
} else {
169+
$text = htmlspecialchars($file, \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset);
168170
}
171+
} else {
172+
$text = htmlspecialchars($text, \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset);
169173
}
170174

171175
if (0 < $line) {

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

Lines changed: 117 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Bridge\Twig\Extension\CodeExtension;
1616
use Symfony\Component\ErrorHandler\ErrorRenderer\FileLinkFormatter;
17+
use Twig\Environment;
18+
use Twig\Loader\ArrayLoader;
1719

1820
class CodeExtensionTest extends TestCase
1921
{
@@ -28,42 +30,136 @@ public function testFileRelative()
2830
$this->assertEquals('file.txt', $this->getExtension()->getFileRelative(\DIRECTORY_SEPARATOR.'project'.\DIRECTORY_SEPARATOR.'file.txt'));
2931
}
3032

31-
/**
32-
* @dataProvider getClassNameProvider
33-
*/
34-
public function testGettingClassAbbreviation($class, $abbr)
33+
public function testClassAbbreviationIntegration()
3534
{
36-
$this->assertEquals($this->getExtension()->abbrClass($class), $abbr);
35+
$data = [
36+
'fqcn' => 'F\Q\N\Foo',
37+
'xss' => '<script>',
38+
];
39+
40+
$template = <<<'TWIG'
41+
{{ 'Bare'|abbr_class }}
42+
{{ fqcn|abbr_class }}
43+
{{ xss|abbr_class }}
44+
TWIG;
45+
46+
$expected = <<<'HTML'
47+
<abbr title="Bare">Bare</abbr>
48+
<abbr title="F\Q\N\Foo">Foo</abbr>
49+
<abbr title="&lt;script&gt;">&lt;script&gt;</abbr>
50+
HTML;
51+
52+
$this->assertEquals($expected, $this->render($template, $data));
3753
}
3854

39-
/**
40-
* @dataProvider getMethodNameProvider
41-
*/
42-
public function testGettingMethodAbbreviation($method, $abbr)
55+
public function testMethodAbbreviationIntegration()
4356
{
44-
$this->assertEquals($this->getExtension()->abbrMethod($method), $abbr);
57+
$data = [
58+
'fqcn' => 'F\Q\N\Foo::Method',
59+
'xss' => '<script>',
60+
];
61+
62+
$template = <<<'TWIG'
63+
{{ 'Bare::Method'|abbr_method }}
64+
{{ fqcn|abbr_method }}
65+
{{ 'Closure'|abbr_method }}
66+
{{ 'Method'|abbr_method }}
67+
{{ xss|abbr_method }}
68+
TWIG;
69+
70+
$expected = <<<'HTML'
71+
<abbr title="Bare">Bare</abbr>::Method()
72+
<abbr title="F\Q\N\Foo">Foo</abbr>::Method()
73+
<abbr title="Closure">Closure</abbr>
74+
<abbr title="Method">Method</abbr>()
75+
<abbr title="&lt;script&gt;">&lt;script&gt;</abbr>()
76+
HTML;
77+
78+
$this->assertEquals($expected, $this->render($template, $data));
4579
}
4680

47-
public static function getClassNameProvider(): array
81+
public function testFormatArgsIntegration()
4882
{
49-
return [
50-
['F\Q\N\Foo', '<abbr title="F\Q\N\Foo">Foo</abbr>'],
51-
['Bare', '<abbr title="Bare">Bare</abbr>'],
83+
$data = [
84+
'args' => [
85+
['object', 'Foo'],
86+
['array', [['string', 'foo'], ['null']]],
87+
['resource'],
88+
['string', 'bar'],
89+
['int', 123],
90+
['bool', true],
91+
],
92+
'xss' => [
93+
['object', '<Foo>'],
94+
['array', [['string', '<foo>']]],
95+
['string', '<bar>'],
96+
['int', 123],
97+
['bool', true],
98+
['<xss>', '<script>'],
99+
],
52100
];
101+
102+
$template = <<<'TWIG'
103+
{{ args|format_args }}
104+
{{ xss|format_args }}
105+
{{ args|format_args_as_text }}
106+
{{ xss|format_args_as_text }}
107+
TWIG;
108+
109+
$expected = <<<'HTML'
110+
<em>object</em>(<abbr title="Foo">Foo</abbr>), <em>array</em>('foo', <em>null</em>), <em>resource</em>, 'bar', 123, true
111+
<em>object</em>(<abbr title="&lt;Foo&gt;">&lt;Foo&gt;</abbr>), <em>array</em>('&lt;foo&gt;'), '&lt;bar&gt;', 123, true, '&lt;script&gt;'
112+
object(Foo), array(&#039;foo&#039;, null), resource, &#039;bar&#039;, 123, true
113+
object(&amp;lt;Foo&amp;gt;), array(&#039;&amp;lt;foo&amp;gt;&#039;), &#039;&amp;lt;bar&amp;gt;&#039;, 123, true, &#039;&amp;lt;script&amp;gt;&#039;
114+
HTML;
115+
116+
$this->assertEquals($expected, $this->render($template, $data));
117+
}
118+
119+
120+
public function testFormatFileIntegration()
121+
{
122+
$template = <<<'TWIG'
123+
{{ 'foo/bar/baz.php'|format_file(21) }}
124+
TWIG;
125+
126+
$expected = <<<'HTML'
127+
<a href="proto://foo/bar/baz.php#&amp;line=21" title="Click to open this file" class="file_link">foo/bar/baz.php at line 21</a>
128+
HTML;
129+
130+
$this->assertEquals($expected, $this->render($template));
53131
}
54132

55-
public static function getMethodNameProvider(): array
133+
public function testFormatFileFromTextIntegration()
56134
{
57-
return [
58-
['F\Q\N\Foo::Method', '<abbr title="F\Q\N\Foo">Foo</abbr>::Method()'],
59-
['Bare::Method', '<abbr title="Bare">Bare</abbr>::Method()'],
60-
['Closure', '<abbr title="Closure">Closure</abbr>'],
61-
['Method', '<abbr title="Method">Method</abbr>()'],
62-
];
135+
$template = <<<'TWIG'
136+
{{ 'in "foo/bar/baz.php" at line 21'|format_file_from_text }}
137+
{{ 'in &quot;foo/bar/baz.php&quot; on line 21'|format_file_from_text }}
138+
{{ 'in "<script>" on line 21'|format_file_from_text }}
139+
TWIG;
140+
141+
$expected = <<<'HTML'
142+
in <a href="proto://foo/bar/baz.php#&amp;line=21" title="Click to open this file" class="file_link">foo/bar/baz.php at line 21</a>
143+
in <a href="proto://foo/bar/baz.php#&amp;line=21" title="Click to open this file" class="file_link">foo/bar/baz.php at line 21</a>
144+
in <a href="proto://&lt;script&gt;#&amp;line=21" title="Click to open this file" class="file_link">&lt;script&gt; at line 21</a>
145+
HTML;
146+
147+
$this->assertEquals($expected, $this->render($template));
63148
}
64149

65150
protected function getExtension(): CodeExtension
66151
{
67152
return new CodeExtension(new FileLinkFormatter('proto://%f#&line=%l&'.substr(__FILE__, 0, 5).'>foobar'), \DIRECTORY_SEPARATOR.'project', 'UTF-8');
68153
}
154+
155+
private function render(string $template, array $context = [])
156+
{
157+
$twig = new Environment(
158+
new ArrayLoader(['index' => $template]),
159+
['debug' => true]
160+
);
161+
$twig->addExtension($this->getExtension());
162+
163+
return $twig->render('index', $context);
164+
}
69165
}

‎src/Symfony/Component/Security/Http/EventListener/SessionStrategyListener.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public function onSuccessfulLogin(LoginSuccessEvent $event): void
4747
$user = $token->getUserIdentifier();
4848
$previousUser = $previousToken->getUserIdentifier();
4949

50-
if ('' !== ($user ?? '') && $user === $previousUser) {
50+
if ('' !== ($user ?? '') && $user === $previousUser && \get_class($token) === \get_class($previousToken)) {
5151
return;
5252
}
5353
}

‎src/Symfony/Component/Security/Http/Tests/EventListener/SessionStrategyListenerTest.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\HttpFoundation\Request;
1717
use Symfony\Component\HttpFoundation\Session\SessionInterface;
1818
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
19+
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
1920
use Symfony\Component\Security\Core\User\InMemoryUser;
2021
use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
2122
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
@@ -82,6 +83,26 @@ public function testRequestWithSamePreviousUser()
8283
$this->listener->onSuccessfulLogin($event);
8384
}
8485

86+
public function testRequestWithSamePreviousUserButDifferentTokenType()
87+
{
88+
$this->configurePreviousSession();
89+
90+
$token = $this->createMock(NullToken::class);
91+
$token->expects($this->once())
92+
->method('getUserIdentifier')
93+
->willReturn('test');
94+
$previousToken = $this->createMock(UsernamePasswordToken::class);
95+
$previousToken->expects($this->once())
96+
->method('getUserIdentifier')
97+
->willReturn('test');
98+
99+
$this->sessionAuthenticationStrategy->expects($this->once())->method('onAuthentication')->with($this->request, $token);
100+
101+
$event = new LoginSuccessEvent($this->createMock(AuthenticatorInterface::class), new SelfValidatingPassport(new UserBadge('test', function () {})), $token, $this->request, null, 'main_firewall', $previousToken);
102+
103+
$this->listener->onSuccessfulLogin($event);
104+
}
105+
85106
private function createEvent($firewallName)
86107
{
87108
return new LoginSuccessEvent($this->createMock(AuthenticatorInterface::class), new SelfValidatingPassport(new UserBadge('test', fn ($username) => new InMemoryUser($username, null))), $this->token, $this->request, null, $firewallName);

‎src/Symfony/Component/Webhook/Controller/WebhookController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function __construct(
3636
public function handle(string $type, Request $request): Response
3737
{
3838
if (!isset($this->parsers[$type])) {
39-
return new Response(sprintf('No parser found for webhook of type "%s".', $type), 404);
39+
return new Response('No webhook parser found for the type given in the URL.', 404, ['Content-Type' => 'text/plain']);
4040
}
4141
/** @var RequestParserInterface $parser */
4242
$parser = $this->parsers[$type]['parser'];

0 commit comments

Comments
 (0)