Skip to content

Commit efc7116

Browse files
fabpotfabpot
committed
feature #35849 [ExpressionLanguage] Added expression language syntax validator (Andrej-in-ua)
This PR was squashed before being merged into the 5.1-dev branch. Discussion ---------- [ExpressionLanguage] Added expression language syntax validator | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | #35700 | License | MIT | Doc PR | N/A <!-- required for new features --> Proposal implementation #35700 The current solution is a compromise between support complexity and cleanliness. I tried different solutions to the issue. A beautiful solution was obtained only with full duplication of the parser code. That is unacceptable because parser complexity is quite high. The main problem in this solution is that nodes instances are created which are then not used. I do not think that linter can be a bottleneck and will greatly affect performance. If this is corrected, the parser code becomes a bunch of if's. JFI: I did not added parsing without variable names, because this breaks caching and potential location for vulnerabilities. Commits ------- a5cd965494 [ExpressionLanguage] Added expression language syntax validator
2 parents 087ec45 + 25af56d commit efc7116

File tree

4 files changed

+154
-7
lines changed

4 files changed

+154
-7
lines changed

‎CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
CHANGELOG
22
=========
33

4+
5.1.0
5+
-----
6+
7+
* added `lint` method to `ExpressionLanguage` class
8+
* added `lint` method to `Parser` class
9+
410
4.0.0
511
-----
612

‎ExpressionLanguage.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,23 @@ public function parse($expression, array $names)
100100
return $parsedExpression;
101101
}
102102

103+
/**
104+
* Validates the syntax of an expression.
105+
*
106+
* @param Expression|string $expression The expression to validate
107+
* @param array|null $names The list of acceptable variable names in the expression, or null to accept any names
108+
*
109+
* @throws SyntaxError When the passed expression is invalid
110+
*/
111+
public function lint($expression, ?array $names): void
112+
{
113+
if ($expression instanceof ParsedExpression) {
114+
return;
115+
}
116+
117+
$this->getParser()->lint($this->getLexer()->tokenize((string) $expression), $names);
118+
}
119+
103120
/**
104121
* Registers a function.
105122
*

‎Parser.php

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ class Parser
3131
private $binaryOperators;
3232
private $functions;
3333
private $names;
34+
private $lint;
3435

3536
public function __construct(array $functions)
3637
{
@@ -90,6 +91,30 @@ public function __construct(array $functions)
9091
* @throws SyntaxError
9192
*/
9293
public function parse(TokenStream $stream, array $names = [])
94+
{
95+
$this->lint = false;
96+
97+
return $this->doParse($stream, $names);
98+
}
99+
100+
/**
101+
* Validates the syntax of an expression.
102+
*
103+
* The syntax of the passed expression will be checked, but not parsed.
104+
* If you want to skip checking dynamic variable names, pass `null` instead of the array.
105+
*
106+
* @throws SyntaxError When the passed expression is invalid
107+
*/
108+
public function lint(TokenStream $stream, ?array $names = []): void
109+
{
110+
$this->lint = true;
111+
$this->doParse($stream, $names);
112+
}
113+
114+
/**
115+
* @throws SyntaxError
116+
*/
117+
private function doParse(TokenStream $stream, ?array $names = []): Node\Node
93118
{
94119
$this->stream = $stream;
95120
$this->names = $names;
@@ -197,13 +222,17 @@ public function parsePrimaryExpression()
197222

198223
$node = new Node\FunctionNode($token->value, $this->parseArguments());
199224
} else {
200-
if (!\in_array($token->value, $this->names, true)) {
201-
throw new SyntaxError(sprintf('Variable "%s" is not valid.', $token->value), $token->cursor, $this->stream->getExpression(), $token->value, $this->names);
202-
}
203-
204-
// is the name used in the compiled code different
205-
// from the name used in the expression?
206-
if (\is_int($name = array_search($token->value, $this->names))) {
225+
if (!$this->lint || \is_array($this->names)) {
226+
if (!\in_array($token->value, $this->names, true)) {
227+
throw new SyntaxError(sprintf('Variable "%s" is not valid.', $token->value), $token->cursor, $this->stream->getExpression(), $token->value, $this->names);
228+
}
229+
230+
// is the name used in the compiled code different
231+
// from the name used in the expression?
232+
if (\is_int($name = array_search($token->value, $this->names))) {
233+
$name = $token->value;
234+
}
235+
} else {
207236
$name = $token->value;
208237
}
209238

‎Tests/ParserTest.php

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\ExpressionLanguage\Lexer;
1616
use Symfony\Component\ExpressionLanguage\Node;
1717
use Symfony\Component\ExpressionLanguage\Parser;
18+
use Symfony\Component\ExpressionLanguage\SyntaxError;
1819

1920
class ParserTest extends TestCase
2021
{
@@ -234,4 +235,98 @@ public function testNameProposal()
234235

235236
$parser->parse($lexer->tokenize('foo > bar'), ['foo', 'baz']);
236237
}
238+
239+
/**
240+
* @dataProvider getLintData
241+
*/
242+
public function testLint($expression, $names, ?string $exception = null)
243+
{
244+
if ($exception) {
245+
$this->expectException(SyntaxError::class);
246+
$this->expectExceptionMessage($exception);
247+
}
248+
249+
$lexer = new Lexer();
250+
$parser = new Parser([]);
251+
$parser->lint($lexer->tokenize($expression), $names);
252+
253+
// Parser does't return anything when the correct expression is passed
254+
$this->expectNotToPerformAssertions();
255+
}
256+
257+
public function getLintData(): array
258+
{
259+
return [
260+
'valid expression' => [
261+
'expression' => 'foo["some_key"].callFunction(a ? b)',
262+
'names' => ['foo', 'a', 'b'],
263+
],
264+
'allow expression without names' => [
265+
'expression' => 'foo.bar',
266+
'names' => null,
267+
],
268+
'disallow expression without names' => [
269+
'expression' => 'foo.bar',
270+
'names' => [],
271+
'exception' => 'Variable "foo" is not valid around position 1 for expression `foo.bar',
272+
],
273+
'operator collisions' => [
274+
'expression' => 'foo.not in [bar]',
275+
'names' => ['foo', 'bar'],
276+
],
277+
'incorrect expression ending' => [
278+
'expression' => 'foo["a"] foo["b"]',
279+
'names' => ['foo'],
280+
'exception' => 'Unexpected token "name" of value "foo" '.
281+
'around position 10 for expression `foo["a"] foo["b"]`.',
282+
],
283+
'incorrect operator' => [
284+
'expression' => 'foo["some_key"] // 2',
285+
'names' => ['foo'],
286+
'exception' => 'Unexpected token "operator" of value "/" '.
287+
'around position 18 for expression `foo["some_key"] // 2`.',
288+
],
289+
'incorrect array' => [
290+
'expression' => '[value1, value2 value3]',
291+
'names' => ['value1', 'value2', 'value3'],
292+
'exception' => 'An array element must be followed by a comma. '.
293+
'Unexpected token "name" of value "value3" ("punctuation" expected with value ",") '.
294+
'around position 17 for expression `[value1, value2 value3]`.',
295+
],
296+
'incorrect array element' => [
297+
'expression' => 'foo["some_key")',
298+
'names' => ['foo'],
299+
'exception' => 'Unclosed "[" around position 3 for expression `foo["some_key")`.',
300+
],
301+
'missed array key' => [
302+
'expression' => 'foo[]',
303+
'names' => ['foo'],
304+
'exception' => 'Unexpected token "punctuation" of value "]" around position 5 for expression `foo[]`.',
305+
],
306+
'missed closing bracket in sub expression' => [
307+
'expression' => 'foo[(bar ? bar : "default"]',
308+
'names' => ['foo', 'bar'],
309+
'exception' => 'Unclosed "(" around position 4 for expression `foo[(bar ? bar : "default"]`.',
310+
],
311+
'incorrect hash following' => [
312+
'expression' => '{key: foo key2: bar}',
313+
'names' => ['foo', 'bar'],
314+
'exception' => 'A hash value must be followed by a comma. '.
315+
'Unexpected token "name" of value "key2" ("punctuation" expected with value ",") '.
316+
'around position 11 for expression `{key: foo key2: bar}`.',
317+
],
318+
'incorrect hash assign' => [
319+
'expression' => '{key => foo}',
320+
'names' => ['foo'],
321+
'exception' => 'Unexpected character "=" around position 5 for expression `{key => foo}`.',
322+
],
323+
'incorrect array as hash using' => [
324+
'expression' => '[foo: foo]',
325+
'names' => ['foo'],
326+
'exception' => 'An array element must be followed by a comma. '.
327+
'Unexpected token "punctuation" of value ":" ("punctuation" expected with value ",") '.
328+
'around position 5 for expression `[foo: foo]`.',
329+
],
330+
];
331+
}
237332
}

0 commit comments

Comments
 (0)