Skip to content

Commit 58e8e9b

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '4.4' into 5.4
* 4.4: [TwigBridge] Add integration tests on twig code helpers [TwigBridge] Ensure CodeExtension's filters properly escape their input [HttpClient] fix missing dep Update VERSION for 4.4.50 Update CHANGELOG for 4.4.50
2 parents 7467bd7 + 5d095d5 commit 58e8e9b

File tree

2 files changed

+130
-30
lines changed

2 files changed

+130
-30
lines changed

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

+13-9
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ public function __construct($fileLinkFormat, string $projectDir, string $charset
4242
public function getFilters(): array
4343
{
4444
return [
45-
new TwigFilter('abbr_class', [$this, 'abbrClass'], ['is_safe' => ['html']]),
46-
new TwigFilter('abbr_method', [$this, 'abbrMethod'], ['is_safe' => ['html']]),
45+
new TwigFilter('abbr_class', [$this, 'abbrClass'], ['is_safe' => ['html'], 'pre_escape' => 'html']),
46+
new TwigFilter('abbr_method', [$this, 'abbrMethod'], ['is_safe' => ['html'], 'pre_escape' => 'html']),
4747
new TwigFilter('format_args', [$this, 'formatArgs'], ['is_safe' => ['html']]),
4848
new TwigFilter('format_args_as_text', [$this, 'formatArgsAsText']),
4949
new TwigFilter('file_excerpt', [$this, 'fileExcerpt'], ['is_safe' => ['html']]),
@@ -85,22 +85,23 @@ public function formatArgs(array $args): string
8585
$result = [];
8686
foreach ($args as $key => $item) {
8787
if ('object' === $item[0]) {
88+
$item[1] = htmlspecialchars($item[1], \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset);
8889
$parts = explode('\\', $item[1]);
8990
$short = array_pop($parts);
9091
$formattedValue = sprintf('<em>object</em>(<abbr title="%s">%s</abbr>)', $item[1], $short);
9192
} elseif ('array' === $item[0]) {
92-
$formattedValue = sprintf('<em>array</em>(%s)', \is_array($item[1]) ? $this->formatArgs($item[1]) : $item[1]);
93+
$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));
9394
} elseif ('null' === $item[0]) {
9495
$formattedValue = '<em>null</em>';
9596
} elseif ('boolean' === $item[0]) {
96-
$formattedValue = '<em>'.strtolower(var_export($item[1], true)).'</em>';
97+
$formattedValue = '<em>'.strtolower(htmlspecialchars(var_export($item[1], true), \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset)).'</em>';
9798
} elseif ('resource' === $item[0]) {
9899
$formattedValue = '<em>resource</em>';
99100
} else {
100101
$formattedValue = str_replace("\n", '', htmlspecialchars(var_export($item[1], true), \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset));
101102
}
102103

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

106107
return implode(', ', $result);
@@ -166,11 +167,14 @@ public function formatFile(string $file, int $line, string $text = null): string
166167
$file = trim($file);
167168

168169
if (null === $text) {
169-
$text = $file;
170-
if (null !== $rel = $this->getFileRelative($text)) {
171-
$rel = explode('/', $rel, 2);
172-
$text = sprintf('<abbr title="%s%2$s">%s</abbr>%s', $this->projectDir, $rel[0], '/'.($rel[1] ?? ''));
170+
if (null !== $rel = $this->getFileRelative($file)) {
171+
$rel = explode('/', htmlspecialchars($rel, \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset), 2);
172+
$text = sprintf('<abbr title="%s%2$s">%s</abbr>%s', htmlspecialchars($this->projectDir, \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset), $rel[0], '/'.($rel[1] ?? ''));
173+
} else {
174+
$text = htmlspecialchars($file, \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset);
173175
}
176+
} else {
177+
$text = htmlspecialchars($text, \ENT_COMPAT | \ENT_SUBSTITUTE, $this->charset);
174178
}
175179

176180
if (0 < $line) {

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

+117-21
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\HttpKernel\Debug\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
}

0 commit comments

Comments
 (0)