Skip to content

Commit 5da4b58

Browse files
fabpotfabpot
committed
Merge branch '3.x' into 4.x
* 3.x: Check reserved names in TempNameExpression
2 parents 98f8d8a + 448efb8 commit 5da4b58

File tree

7 files changed

+171
-41
lines changed

7 files changed

+171
-41
lines changed

‎src/ExpressionParser.php

Lines changed: 65 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ public function getFunctionNode($name, $line)
505505
return $node;
506506
}
507507

508-
$args = $this->parseArguments();
508+
$args = $this->parseOnlyArguments();
509509
$function = $this->getFunction($name, $line);
510510

511511
if ($function->getParserCallable()) {
@@ -615,7 +615,7 @@ public function parseFilterExpressionRaw($node)
615615
if (!$this->parser->getStream()->test(Token::PUNCTUATION_TYPE, '(')) {
616616
$arguments = new EmptyNode();
617617
} else {
618-
$arguments = $this->parseArguments();
618+
$arguments = $this->parseOnlyArguments();
619619
}
620620

621621
$filter = $this->getFilter($token->getValue(), $token->getLine());
@@ -635,14 +635,26 @@ public function parseFilterExpressionRaw($node)
635635
/**
636636
* Parses arguments.
637637
*
638-
* @param bool $definition Whether we are parsing arguments for a function (or macro) definition
639-
*
640638
* @return Node
641639
*
642640
* @throws SyntaxError
643641
*/
644-
public function parseArguments($namedArguments = true, $definition = false)
642+
public function parseArguments()
645643
{
644+
$namedArguments = false;
645+
$definition = false;
646+
if (func_num_args() > 2) {
647+
trigger_deprecation('twig/twig', '3.15', 'Passing a third argument ($allowArrow) to "%s()" is deprecated.', __METHOD__);
648+
}
649+
if (func_num_args() > 1) {
650+
trigger_deprecation('twig/twig', '3.15', 'Passing a second argument ($definition) to "%s()" is deprecated.', __METHOD__);
651+
$definition = func_get_arg(1);
652+
}
653+
if (func_num_args() > 0) {
654+
trigger_deprecation('twig/twig', '3.15', 'Passing a first argument ($namedArguments) to "%s()" is deprecated.', __METHOD__);
655+
$namedArguments = func_get_arg(0);
656+
}
657+
646658
$args = [];
647659
$stream = $this->parser->getStream();
648660

@@ -757,7 +769,7 @@ private function parseTestExpression(Node $node): TestExpression
757769

758770
$arguments = null;
759771
if ($stream->test(Token::PUNCTUATION_TYPE, '(')) {
760-
$arguments = $this->parseArguments();
772+
$arguments = $this->parseOnlyArguments();
761773
} elseif ($test->hasOneMandatoryArgument()) {
762774
$arguments = new Nodes([0 => $this->getPrimary()]);
763775
}
@@ -846,6 +858,7 @@ private function getFilter(string $name, int $line): TwigFilter
846858
}
847859

848860
// checks that the node only contains "constant" elements
861+
// to be removed in 4.0
849862
private function checkConstantExpression(Node $node): bool
850863
{
851864
if (!($node instanceof ConstantExpression || $node instanceof ArrayExpression
@@ -874,10 +887,55 @@ private function setDeprecationCheck(bool $deprecationCheck): bool
874887
private function createArguments(int $line): ArrayExpression
875888
{
876889
$arguments = new ArrayExpression([], $line);
877-
foreach ($this->parseArguments() as $k => $n) {
890+
foreach ($this->parseOnlyArguments() as $k => $n) {
878891
$arguments->addElement($n, new TempNameExpression($k, $line));
879892
}
880893

881894
return $arguments;
882895
}
896+
897+
public function parseOnlyArguments()
898+
{
899+
$args = [];
900+
$stream = $this->parser->getStream();
901+
$stream->expect(Token::PUNCTUATION_TYPE, '(', 'A list of arguments must begin with an opening parenthesis');
902+
$hasSpread = false;
903+
while (!$stream->test(Token::PUNCTUATION_TYPE, ')')) {
904+
if ($args) {
905+
$stream->expect(Token::PUNCTUATION_TYPE, ',', 'Arguments must be separated by a comma');
906+
907+
// if the comma above was a trailing comma, early exit the argument parse loop
908+
if ($stream->test(Token::PUNCTUATION_TYPE, ')')) {
909+
break;
910+
}
911+
}
912+
913+
if ($stream->nextIf(Token::SPREAD_TYPE)) {
914+
$hasSpread = true;
915+
$value = new SpreadUnary($this->parseExpression(), $stream->getCurrent()->getLine());
916+
} elseif ($hasSpread) {
917+
throw new SyntaxError('Normal arguments must be placed before argument unpacking.', $stream->getCurrent()->getLine(), $stream->getSourceContext());
918+
} else {
919+
$value = $this->parseExpression();
920+
}
921+
922+
$name = null;
923+
if (($token = $stream->nextIf(Token::OPERATOR_TYPE, '=')) || ($token = $stream->nextIf(Token::PUNCTUATION_TYPE, ':'))) {
924+
if (!$value instanceof NameExpression) {
925+
throw new SyntaxError(\sprintf('A parameter name must be a string, "%s" given.', \get_class($value)), $token->getLine(), $stream->getSourceContext());
926+
}
927+
$name = $value->getAttribute('name');
928+
$value = $this->parseExpression();
929+
}
930+
931+
if (null === $name) {
932+
$args[] = $value;
933+
} else {
934+
$args[$name] = $value;
935+
}
936+
}
937+
$stream->expect(Token::PUNCTUATION_TYPE, ')', 'A list of arguments must be closed by a parenthesis');
938+
939+
return new Nodes($args);
940+
}
883941
}

‎src/Node/Expression/TempNameExpression.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,19 @@
1212
namespace Twig\Node\Expression;
1313

1414
use Twig\Compiler;
15+
use Twig\Error\SyntaxError;
1516

1617
class TempNameExpression extends AbstractExpression
1718
{
1819
public const RESERVED_NAMES = ['varargs', 'context', 'macros', 'blocks', 'this'];
1920

2021
public function __construct(string|int $name, int $lineno)
2122
{
23+
// All names supported by ExpressionParser::parsePrimaryExpression() should be excluded
24+
if (\in_array(strtolower($name), ['true', 'false', 'none', 'null'])) {
25+
throw new SyntaxError(\sprintf('You cannot assign a value to "%s".', $name), $lineno);
26+
}
27+
2228
if (is_int($name) || ctype_digit($name)) {
2329
$name = (int) $name;
2430
} elseif (in_array($name, self::RESERVED_NAMES)) {

‎src/Node/MacroNode.php

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Twig\Attribute\YieldReady;
1515
use Twig\Compiler;
1616
use Twig\Error\SyntaxError;
17+
use Twig\Node\Expression\ArrayExpression;
1718
use Twig\Node\Expression\TempNameExpression;
1819

1920
/**
@@ -26,15 +27,11 @@ class MacroNode extends Node
2627
{
2728
public const VARARGS_NAME = 'varargs';
2829

29-
public function __construct(string $name, BodyNode $body, Node $arguments, int $lineno)
30+
public function __construct(string $name, BodyNode $body, ArrayExpression $arguments, int $lineno)
3031
{
31-
foreach ($arguments as $argumentName => $argument) {
32-
if (self::VARARGS_NAME === $argumentName) {
33-
throw new SyntaxError(\sprintf('The argument "%s" in macro "%s" cannot be defined because the variable "%s" is reserved for arbitrary arguments.', self::VARARGS_NAME, $name, self::VARARGS_NAME), $argument->getTemplateLine(), $argument->getSourceContext());
34-
}
35-
if (in_array($argumentName, TempNameExpression::RESERVED_NAMES)) {
36-
$arguments->setNode('_'.$argumentName.'_', $argument);
37-
$arguments->removeNode($argumentName);
32+
foreach ($arguments->getKeyValuePairs() as $pair) {
33+
if ('_'.self::VARARGS_NAME.'_' === $pair['key']->getAttribute('name')) {
34+
throw new SyntaxError(\sprintf('The argument "%s" in macro "%s" cannot be defined because the variable "%s" is reserved for arbitrary arguments.', self::VARARGS_NAME, $name, self::VARARGS_NAME), $pair['value']->getTemplateLine(), $pair['value']->getSourceContext());
3835
}
3936
}
4037

@@ -48,21 +45,15 @@ public function compile(Compiler $compiler): void
4845
->write(\sprintf('public function macro_%s(', $this->getAttribute('name')))
4946
;
5047

51-
$count = \count($this->getNode('arguments'));
52-
$pos = 0;
53-
foreach ($this->getNode('arguments') as $name => $default) {
48+
foreach ($this->getNode('arguments')->getKeyValuePairs() as $pair) {
49+
$name = $pair['key'];
50+
$default = $pair['value'];
5451
$compiler
55-
->raw('$'.$name.' = ')
52+
->subcompile($name)
53+
->raw(' = ')
5654
->subcompile($default)
55+
->raw(', ')
5756
;
58-
59-
if (++$pos < $count) {
60-
$compiler->raw(', ');
61-
}
62-
}
63-
64-
if ($count) {
65-
$compiler->raw(', ');
6657
}
6758

6859
$compiler
@@ -75,11 +66,13 @@ public function compile(Compiler $compiler): void
7566
->indent()
7667
;
7768

78-
foreach ($this->getNode('arguments') as $name => $default) {
69+
foreach ($this->getNode('arguments')->getKeyValuePairs() as $pair) {
70+
$name = $pair['key'];
7971
$compiler
8072
->write('')
81-
->string(trim($name, '_'))
82-
->raw(' => $'.$name)
73+
->string(trim($name->getAttribute('name'), '_'))
74+
->raw(' => ')
75+
->subcompile($name)
8376
->raw(",\n")
8477
;
8578
}

‎src/TokenParser/MacroTokenParser.php

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,15 @@
1414
use Twig\Error\SyntaxError;
1515
use Twig\Node\BodyNode;
1616
use Twig\Node\EmptyNode;
17+
use Twig\Node\Expression\ArrayExpression;
18+
use Twig\Node\Expression\ConstantExpression;
19+
use Twig\Node\Expression\NameExpression;
20+
use Twig\Node\Expression\TempNameExpression;
21+
use Twig\Node\Expression\Unary\NegUnary;
22+
use Twig\Node\Expression\Unary\PosUnary;
1723
use Twig\Node\MacroNode;
1824
use Twig\Node\Node;
25+
use Twig\Node\Nodes;
1926
use Twig\Token;
2027

2128
/**
@@ -34,8 +41,7 @@ public function parse(Token $token): Node
3441
$lineno = $token->getLine();
3542
$stream = $this->parser->getStream();
3643
$name = $stream->expect(Token::NAME_TYPE)->getValue();
37-
38-
$arguments = $this->parser->getExpressionParser()->parseArguments(true, true);
44+
$arguments = $this->parseDefinition();
3945

4046
$stream->expect(Token::BLOCK_END_TYPE);
4147
$this->parser->pushLocalScope();
@@ -64,4 +70,56 @@ public function getTag(): string
6470
{
6571
return 'macro';
6672
}
73+
74+
private function parseDefinition(): ArrayExpression
75+
{
76+
$arguments = new ArrayExpression([], $this->parser->getCurrentToken()->getLine());
77+
$stream = $this->parser->getStream();
78+
$stream->expect(Token::PUNCTUATION_TYPE, '(', 'A list of arguments must begin with an opening parenthesis');
79+
while (!$stream->test(Token::PUNCTUATION_TYPE, ')')) {
80+
if (count($arguments)) {
81+
$stream->expect(Token::PUNCTUATION_TYPE, ',', 'Arguments must be separated by a comma');
82+
83+
// if the comma above was a trailing comma, early exit the argument parse loop
84+
if ($stream->test(Token::PUNCTUATION_TYPE, ')')) {
85+
break;
86+
}
87+
}
88+
89+
$token = $stream->expect(Token::NAME_TYPE, null, 'An argument must be a name');
90+
$name = new TempNameExpression($token->getValue(), $this->parser->getCurrentToken()->getLine());
91+
if ($token = $stream->nextIf(Token::OPERATOR_TYPE, '=')) {
92+
$default = $this->parser->getExpressionParser()->parseExpression();
93+
} else {
94+
$default = new ConstantExpression(null, $this->parser->getCurrentToken()->getLine());
95+
$default->setAttribute('is_implicit', true);
96+
}
97+
98+
if (!$this->checkConstantExpression($default)) {
99+
throw new SyntaxError('A default value for an argument must be a constant (a boolean, a string, a number, a sequence, or a mapping).', $token->getLine(), $stream->getSourceContext());
100+
}
101+
$arguments->addElement($default, $name);
102+
}
103+
$stream->expect(Token::PUNCTUATION_TYPE, ')', 'A list of arguments must be closed by a parenthesis');
104+
105+
return $arguments;
106+
}
107+
108+
// checks that the node only contains "constant" elements
109+
private function checkConstantExpression(Node $node): bool
110+
{
111+
if (!($node instanceof ConstantExpression || $node instanceof ArrayExpression
112+
|| $node instanceof NegUnary || $node instanceof PosUnary
113+
)) {
114+
return false;
115+
}
116+
117+
foreach ($node as $n) {
118+
if (!$this->checkConstantExpression($n)) {
119+
return false;
120+
}
121+
}
122+
123+
return true;
124+
}
67125
}

‎tests/Fixtures/tags/macro/argument_reserved_names.test

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
--TEST--
2+
"macro" tag
3+
--TEMPLATE--
4+
{% import _self as macros %}
5+
6+
{% macro input(true, false, null) %}
7+
{{ true }}
8+
{% endmacro %}
9+
--DATA--
10+
return []
11+
--EXCEPTION--
12+
Twig\Error\SyntaxError: You cannot assign a value to "true" in "index.twig" at line 4.

‎tests/Node/MacroTest.php

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@
1414
use Twig\Environment;
1515
use Twig\Loader\ArrayLoader;
1616
use Twig\Node\BodyNode;
17+
use Twig\Node\Expression\ArrayExpression;
1718
use Twig\Node\Expression\ConstantExpression;
1819
use Twig\Node\Expression\NameExpression;
20+
use Twig\Node\Expression\TempNameExpression;
1921
use Twig\Node\MacroNode;
2022
use Twig\Node\Nodes;
2123
use Twig\Node\TextNode;
@@ -26,7 +28,7 @@ class MacroTest extends NodeTestCase
2628
public function testConstructor()
2729
{
2830
$body = new BodyNode([new TextNode('foo', 1)]);
29-
$arguments = new Nodes([new NameExpression('foo', 1)], 1);
31+
$arguments = new ArrayExpression([new NameExpression('foo', 1), new ConstantExpression(null, 1)], 1);
3032
$node = new MacroNode('foo', $body, $arguments, 1);
3133

3234
$this->assertEquals($body, $node->getNode('body'));
@@ -36,9 +38,11 @@ public function testConstructor()
3638

3739
public static function provideTests(): iterable
3840
{
39-
$arguments = new Nodes([
40-
'foo' => new ConstantExpression(null, 1),
41-
'bar' => new ConstantExpression('Foo', 1),
41+
$arguments = new ArrayExpression([
42+
new TempNameExpression('foo', 1),
43+
new ConstantExpression(null, 1),
44+
new TempNameExpression('bar', 1),
45+
new ConstantExpression('Foo', 1),
4246
], 1);
4347

4448
$body = new BodyNode([new TextNode('foo', 1)]);

‎tests/ParserTest.php

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,12 +185,11 @@ public function testImplicitMacroArgumentDefaultValues()
185185
->getNode('arguments')
186186
;
187187

188-
$this->assertTrue($argumentNodes->getNode('po')->hasAttribute('is_implicit'));
189-
$this->assertTrue($argumentNodes->getNode('po')->getAttribute('is_implicit'));
190-
$this->assertNull($argumentNodes->getNode('po')->getAttribute('value'));
188+
$this->assertTrue($argumentNodes->getNode(1)->hasAttribute('is_implicit'));
189+
$this->assertNull($argumentNodes->getNode(1)->getAttribute('value'));
191190

192-
$this->assertFalse($argumentNodes->getNode('lo')->hasAttribute('is_implicit'));
193-
$this->assertTrue($argumentNodes->getNode('lo')->getAttribute('value'));
191+
$this->assertFalse($argumentNodes->getNode(3)->hasAttribute('is_implicit'));
192+
$this->assertTrue($argumentNodes->getNode(3)->getAttribute('value'));
194193
}
195194

196195
protected function getParser()

0 commit comments

Comments
 (0)