Skip to content

Commit 448efb8

Browse files
fabpotfabpot
committed
bug #4397 Check reserved names in TempNameExpression (fabpot)
This PR was merged into the 3.x branch. Discussion ---------- Check reserved names in TempNameExpression Commits ------- 66658a3 Check reserved names in TempNameExpression
2 parents 950ad5a + 66658a3 commit 448efb8

File tree

7 files changed

+176
-43
lines changed

7 files changed

+176
-43
lines changed

‎src/ExpressionParser.php

Lines changed: 60 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ public function getFunctionNode($name, $line)
538538
return $node;
539539
}
540540

541-
$args = $this->parseArguments(true);
541+
$args = $this->parseOnlyArguments();
542542
$function = $this->getFunction($name, $line);
543543

544544
if ($function->getParserCallable()) {
@@ -660,7 +660,7 @@ public function parseFilterExpressionRaw($node)
660660
if (!$this->parser->getStream()->test(Token::PUNCTUATION_TYPE, '(')) {
661661
$arguments = new EmptyNode();
662662
} else {
663-
$arguments = $this->parseArguments(true);
663+
$arguments = $this->parseOnlyArguments();
664664
}
665665

666666
$filter = $this->getFilter($token->getValue(), $token->getLine());
@@ -689,20 +689,24 @@ public function parseFilterExpressionRaw($node)
689689
/**
690690
* Parses arguments.
691691
*
692-
* @param bool $namedArguments Whether to allow named arguments or not
693-
* @param bool $definition Whether we are parsing arguments for a function (or macro) definition
694-
*
695692
* @return Node
696693
*
697694
* @throws SyntaxError
698695
*/
699-
public function parseArguments($namedArguments = false, $definition = false)
696+
public function parseArguments()
700697
{
698+
$namedArguments = false;
699+
$definition = false;
701700
if (func_num_args() > 2) {
702701
trigger_deprecation('twig/twig', '3.15', 'Passing a third argument ($allowArrow) to "%s()" is deprecated.', __METHOD__);
703702
}
704-
if (!$namedArguments) {
705-
trigger_deprecation('twig/twig', '3.15', 'Passing "false" for the first argument ($namedArguments) to "%s()" is deprecated.', __METHOD__);
703+
if (func_num_args() > 1) {
704+
trigger_deprecation('twig/twig', '3.15', 'Passing a second argument ($definition) to "%s()" is deprecated.', __METHOD__);
705+
$definition = func_get_arg(1);
706+
}
707+
if (func_num_args() > 0) {
708+
trigger_deprecation('twig/twig', '3.15', 'Passing a first argument ($namedArguments) to "%s()" is deprecated.', __METHOD__);
709+
$namedArguments = func_get_arg(0);
706710
}
707711

708712
$args = [];
@@ -819,7 +823,7 @@ private function parseTestExpression(Node $node): TestExpression
819823

820824
$arguments = null;
821825
if ($stream->test(Token::PUNCTUATION_TYPE, '(')) {
822-
$arguments = $this->parseArguments(true);
826+
$arguments = $this->parseOnlyArguments();
823827
} elseif ($test->hasOneMandatoryArgument()) {
824828
$arguments = new Nodes([0 => $this->getPrimary()]);
825829
}
@@ -917,6 +921,7 @@ private function getFilter(string $name, int $line): TwigFilter
917921
}
918922

919923
// checks that the node only contains "constant" elements
924+
// to be removed in 4.0
920925
private function checkConstantExpression(Node $node): bool
921926
{
922927
if (!($node instanceof ConstantExpression || $node instanceof ArrayExpression
@@ -945,10 +950,55 @@ private function setDeprecationCheck(bool $deprecationCheck): bool
945950
private function createArguments(int $line): ArrayExpression
946951
{
947952
$arguments = new ArrayExpression([], $line);
948-
foreach ($this->parseArguments(true) as $k => $n) {
953+
foreach ($this->parseOnlyArguments() as $k => $n) {
949954
$arguments->addElement($n, new TempNameExpression($k, $line));
950955
}
951956

952957
return $arguments;
953958
}
959+
960+
public function parseOnlyArguments()
961+
{
962+
$args = [];
963+
$stream = $this->parser->getStream();
964+
$stream->expect(Token::PUNCTUATION_TYPE, '(', 'A list of arguments must begin with an opening parenthesis');
965+
$hasSpread = false;
966+
while (!$stream->test(Token::PUNCTUATION_TYPE, ')')) {
967+
if ($args) {
968+
$stream->expect(Token::PUNCTUATION_TYPE, ',', 'Arguments must be separated by a comma');
969+
970+
// if the comma above was a trailing comma, early exit the argument parse loop
971+
if ($stream->test(Token::PUNCTUATION_TYPE, ')')) {
972+
break;
973+
}
974+
}
975+
976+
if ($stream->nextIf(Token::SPREAD_TYPE)) {
977+
$hasSpread = true;
978+
$value = new SpreadUnary($this->parseExpression(), $stream->getCurrent()->getLine());
979+
} elseif ($hasSpread) {
980+
throw new SyntaxError('Normal arguments must be placed before argument unpacking.', $stream->getCurrent()->getLine(), $stream->getSourceContext());
981+
} else {
982+
$value = $this->parseExpression();
983+
}
984+
985+
$name = null;
986+
if (($token = $stream->nextIf(Token::OPERATOR_TYPE, '=')) || ($token = $stream->nextIf(Token::PUNCTUATION_TYPE, ':'))) {
987+
if (!$value instanceof NameExpression) {
988+
throw new SyntaxError(\sprintf('A parameter name must be a string, "%s" given.', \get_class($value)), $token->getLine(), $stream->getSourceContext());
989+
}
990+
$name = $value->getAttribute('name');
991+
$value = $this->parseExpression();
992+
}
993+
994+
if (null === $name) {
995+
$args[] = $value;
996+
} else {
997+
$args[$name] = $value;
998+
}
999+
}
1000+
$stream->expect(Token::PUNCTUATION_TYPE, ')', 'A list of arguments must be closed by a parenthesis');
1001+
1002+
return new Nodes($args);
1003+
}
9541004
}

‎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: 26 additions & 22 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
/**
@@ -27,21 +28,28 @@ class MacroNode extends Node
2728
public const VARARGS_NAME = 'varargs';
2829

2930
/**
30-
* @param BodyNode $body
31+
* @param BodyNode $body
32+
* @param ArrayExpression $arguments
3133
*/
3234
public function __construct(string $name, Node $body, Node $arguments, int $lineno)
3335
{
3436
if (!$body instanceof BodyNode) {
3537
trigger_deprecation('twig/twig', '3.12', \sprintf('Not passing a "%s" instance as the "body" argument of the "%s" constructor is deprecated ("%s" given).', BodyNode::class, static::class, $body::class));
3638
}
3739

38-
foreach ($arguments as $argumentName => $argument) {
39-
if (self::VARARGS_NAME === $argumentName) {
40-
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());
40+
if (!$arguments instanceof ArrayExpression) {
41+
trigger_deprecation('twig/twig', '3.15', \sprintf('Not passing a "%s" instance as the "arguments" argument of the "%s" constructor is deprecated ("%s" given).', ArrayExpression::class, static::class, $arguments::class));
42+
43+
$args = new ArrayExpression([], $arguments->getTemplateLine());
44+
foreach ($arguments as $name => $default) {
45+
$args->addElement($default, new TempNameExpression($name, $default->getTemplateLine()));
4146
}
42-
if (in_array($argumentName, TempNameExpression::RESERVED_NAMES)) {
43-
$arguments->setNode('_'.$argumentName.'_', $argument);
44-
$arguments->removeNode($argumentName);
47+
$arguments = $args;
48+
}
49+
50+
foreach ($arguments->getKeyValuePairs() as $pair) {
51+
if ('_'.self::VARARGS_NAME.'_' === $pair['key']->getAttribute('name')) {
52+
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());
4553
}
4654
}
4755

@@ -55,21 +63,15 @@ public function compile(Compiler $compiler): void
5563
->write(\sprintf('public function macro_%s(', $this->getAttribute('name')))
5664
;
5765

58-
$count = \count($this->getNode('arguments'));
59-
$pos = 0;
60-
foreach ($this->getNode('arguments') as $name => $default) {
66+
foreach ($this->getNode('arguments')->getKeyValuePairs() as $pair) {
67+
$name = $pair['key'];
68+
$default = $pair['value'];
6169
$compiler
62-
->raw('$'.$name.' = ')
70+
->subcompile($name)
71+
->raw(' = ')
6372
->subcompile($default)
73+
->raw(', ')
6474
;
65-
66-
if (++$pos < $count) {
67-
$compiler->raw(', ');
68-
}
69-
}
70-
71-
if ($count) {
72-
$compiler->raw(', ');
7375
}
7476

7577
$compiler
@@ -82,11 +84,13 @@ public function compile(Compiler $compiler): void
8284
->indent()
8385
;
8486

85-
foreach ($this->getNode('arguments') as $name => $default) {
87+
foreach ($this->getNode('arguments')->getKeyValuePairs() as $pair) {
88+
$name = $pair['key'];
8689
$compiler
8790
->write('')
88-
->string(trim($name, '_'))
89-
->raw(' => $'.$name)
91+
->string(trim($name->getAttribute('name'), '_'))
92+
->raw(' => ')
93+
->subcompile($name)
9094
->raw(",\n")
9195
;
9296
}

‎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
@@ -194,12 +194,11 @@ public function testImplicitMacroArgumentDefaultValues()
194194
->getNode('arguments')
195195
;
196196

197-
$this->assertTrue($argumentNodes->getNode('po')->hasAttribute('is_implicit'));
198-
$this->assertTrue($argumentNodes->getNode('po')->getAttribute('is_implicit'));
199-
$this->assertNull($argumentNodes->getNode('po')->getAttribute('value'));
197+
$this->assertTrue($argumentNodes->getNode(1)->hasAttribute('is_implicit'));
198+
$this->assertNull($argumentNodes->getNode(1)->getAttribute('value'));
200199

201-
$this->assertFalse($argumentNodes->getNode('lo')->hasAttribute('is_implicit'));
202-
$this->assertTrue($argumentNodes->getNode('lo')->getAttribute('value'));
200+
$this->assertFalse($argumentNodes->getNode(3)->hasAttribute('is_implicit'));
201+
$this->assertTrue($argumentNodes->getNode(3)->getAttribute('value'));
203202
}
204203

205204
protected function getParser()

0 commit comments

Comments
 (0)