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
This commit is contained in:
Fabien Potencier
commit
3d30ff7677
|
|
@ -1,6 +1,12 @@
|
|||
CHANGELOG
|
||||
=========
|
||||
|
||||
5.1.0
|
||||
-----
|
||||
|
||||
* added `lint` method to `ExpressionLanguage` class
|
||||
* added `lint` method to `Parser` class
|
||||
|
||||
4.0.0
|
||||
-----
|
||||
|
||||
|
|
|
|||
|
|
@ -100,6 +100,23 @@ class ExpressionLanguage
|
|||
return $parsedExpression;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates the syntax of an expression.
|
||||
*
|
||||
* @param Expression|string $expression The expression to validate
|
||||
* @param array|null $names The list of acceptable variable names in the expression, or null to accept any names
|
||||
*
|
||||
* @throws SyntaxError When the passed expression is invalid
|
||||
*/
|
||||
public function lint($expression, ?array $names): void
|
||||
{
|
||||
if ($expression instanceof ParsedExpression) {
|
||||
return;
|
||||
}
|
||||
|
||||
$this->getParser()->lint($this->getLexer()->tokenize((string) $expression), $names);
|
||||
}
|
||||
|
||||
/**
|
||||
* Registers a function.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ class Parser
|
|||
private $binaryOperators;
|
||||
private $functions;
|
||||
private $names;
|
||||
private $lint;
|
||||
|
||||
public function __construct(array $functions)
|
||||
{
|
||||
|
|
@ -90,6 +91,30 @@ class Parser
|
|||
* @throws SyntaxError
|
||||
*/
|
||||
public function parse(TokenStream $stream, array $names = [])
|
||||
{
|
||||
$this->lint = false;
|
||||
|
||||
return $this->doParse($stream, $names);
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates the syntax of an expression.
|
||||
*
|
||||
* The syntax of the passed expression will be checked, but not parsed.
|
||||
* If you want to skip checking dynamic variable names, pass `null` instead of the array.
|
||||
*
|
||||
* @throws SyntaxError When the passed expression is invalid
|
||||
*/
|
||||
public function lint(TokenStream $stream, ?array $names = []): void
|
||||
{
|
||||
$this->lint = true;
|
||||
$this->doParse($stream, $names);
|
||||
}
|
||||
|
||||
/**
|
||||
* @throws SyntaxError
|
||||
*/
|
||||
private function doParse(TokenStream $stream, ?array $names = []): Node\Node
|
||||
{
|
||||
$this->stream = $stream;
|
||||
$this->names = $names;
|
||||
|
|
@ -197,13 +222,17 @@ class Parser
|
|||
|
||||
$node = new Node\FunctionNode($token->value, $this->parseArguments());
|
||||
} else {
|
||||
if (!\in_array($token->value, $this->names, true)) {
|
||||
throw new SyntaxError(sprintf('Variable "%s" is not valid.', $token->value), $token->cursor, $this->stream->getExpression(), $token->value, $this->names);
|
||||
}
|
||||
if (!$this->lint || \is_array($this->names)) {
|
||||
if (!\in_array($token->value, $this->names, true)) {
|
||||
throw new SyntaxError(sprintf('Variable "%s" is not valid.', $token->value), $token->cursor, $this->stream->getExpression(), $token->value, $this->names);
|
||||
}
|
||||
|
||||
// is the name used in the compiled code different
|
||||
// from the name used in the expression?
|
||||
if (\is_int($name = array_search($token->value, $this->names))) {
|
||||
// is the name used in the compiled code different
|
||||
// from the name used in the expression?
|
||||
if (\is_int($name = array_search($token->value, $this->names))) {
|
||||
$name = $token->value;
|
||||
}
|
||||
} else {
|
||||
$name = $token->value;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ use PHPUnit\Framework\TestCase;
|
|||
use Symfony\Component\ExpressionLanguage\Lexer;
|
||||
use Symfony\Component\ExpressionLanguage\Node;
|
||||
use Symfony\Component\ExpressionLanguage\Parser;
|
||||
use Symfony\Component\ExpressionLanguage\SyntaxError;
|
||||
|
||||
class ParserTest extends TestCase
|
||||
{
|
||||
|
|
@ -234,4 +235,98 @@ class ParserTest extends TestCase
|
|||
|
||||
$parser->parse($lexer->tokenize('foo > bar'), ['foo', 'baz']);
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider getLintData
|
||||
*/
|
||||
public function testLint($expression, $names, ?string $exception = null)
|
||||
{
|
||||
if ($exception) {
|
||||
$this->expectException(SyntaxError::class);
|
||||
$this->expectExceptionMessage($exception);
|
||||
}
|
||||
|
||||
$lexer = new Lexer();
|
||||
$parser = new Parser([]);
|
||||
$parser->lint($lexer->tokenize($expression), $names);
|
||||
|
||||
// Parser does't return anything when the correct expression is passed
|
||||
$this->expectNotToPerformAssertions();
|
||||
}
|
||||
|
||||
public function getLintData(): array
|
||||
{
|
||||
return [
|
||||
'valid expression' => [
|
||||
'expression' => 'foo["some_key"].callFunction(a ? b)',
|
||||
'names' => ['foo', 'a', 'b'],
|
||||
],
|
||||
'allow expression without names' => [
|
||||
'expression' => 'foo.bar',
|
||||
'names' => null,
|
||||
],
|
||||
'disallow expression without names' => [
|
||||
'expression' => 'foo.bar',
|
||||
'names' => [],
|
||||
'exception' => 'Variable "foo" is not valid around position 1 for expression `foo.bar',
|
||||
],
|
||||
'operator collisions' => [
|
||||
'expression' => 'foo.not in [bar]',
|
||||
'names' => ['foo', 'bar'],
|
||||
],
|
||||
'incorrect expression ending' => [
|
||||
'expression' => 'foo["a"] foo["b"]',
|
||||
'names' => ['foo'],
|
||||
'exception' => 'Unexpected token "name" of value "foo" '.
|
||||
'around position 10 for expression `foo["a"] foo["b"]`.',
|
||||
],
|
||||
'incorrect operator' => [
|
||||
'expression' => 'foo["some_key"] // 2',
|
||||
'names' => ['foo'],
|
||||
'exception' => 'Unexpected token "operator" of value "/" '.
|
||||
'around position 18 for expression `foo["some_key"] // 2`.',
|
||||
],
|
||||
'incorrect array' => [
|
||||
'expression' => '[value1, value2 value3]',
|
||||
'names' => ['value1', 'value2', 'value3'],
|
||||
'exception' => 'An array element must be followed by a comma. '.
|
||||
'Unexpected token "name" of value "value3" ("punctuation" expected with value ",") '.
|
||||
'around position 17 for expression `[value1, value2 value3]`.',
|
||||
],
|
||||
'incorrect array element' => [
|
||||
'expression' => 'foo["some_key")',
|
||||
'names' => ['foo'],
|
||||
'exception' => 'Unclosed "[" around position 3 for expression `foo["some_key")`.',
|
||||
],
|
||||
'missed array key' => [
|
||||
'expression' => 'foo[]',
|
||||
'names' => ['foo'],
|
||||
'exception' => 'Unexpected token "punctuation" of value "]" around position 5 for expression `foo[]`.',
|
||||
],
|
||||
'missed closing bracket in sub expression' => [
|
||||
'expression' => 'foo[(bar ? bar : "default"]',
|
||||
'names' => ['foo', 'bar'],
|
||||
'exception' => 'Unclosed "(" around position 4 for expression `foo[(bar ? bar : "default"]`.',
|
||||
],
|
||||
'incorrect hash following' => [
|
||||
'expression' => '{key: foo key2: bar}',
|
||||
'names' => ['foo', 'bar'],
|
||||
'exception' => 'A hash value must be followed by a comma. '.
|
||||
'Unexpected token "name" of value "key2" ("punctuation" expected with value ",") '.
|
||||
'around position 11 for expression `{key: foo key2: bar}`.',
|
||||
],
|
||||
'incorrect hash assign' => [
|
||||
'expression' => '{key => foo}',
|
||||
'names' => ['foo'],
|
||||
'exception' => 'Unexpected character "=" around position 5 for expression `{key => foo}`.',
|
||||
],
|
||||
'incorrect array as hash using' => [
|
||||
'expression' => '[foo: foo]',
|
||||
'names' => ['foo'],
|
||||
'exception' => 'An array element must be followed by a comma. '.
|
||||
'Unexpected token "punctuation" of value ":" ("punctuation" expected with value ",") '.
|
||||
'around position 5 for expression `[foo: foo]`.',
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ CHANGELOG
|
|||
* allow to define a reusable set of constraints by extending the `Compound` constraint
|
||||
* added `Sequentially` constraint, to sequentially validate a set of constraints (any violation raised will prevent further validation of the nested constraints)
|
||||
* added the `divisibleBy` option to the `Count` constraint
|
||||
* added the `ExpressionLanguageSyntax` constraint
|
||||
|
||||
5.0.0
|
||||
-----
|
||||
|
|
|
|||
|
|
@ -0,0 +1,42 @@
|
|||
<?php
|
||||
|
||||
/*
|
||||
* This file is part of the Symfony package.
|
||||
*
|
||||
* (c) Fabien Potencier <fabien@symfony.com>
|
||||
*
|
||||
* For the full copyright and license information, please view the LICENSE
|
||||
* file that was distributed with this source code.
|
||||
*/
|
||||
|
||||
namespace Symfony\Component\Validator\Constraints;
|
||||
|
||||
use Symfony\Component\Validator\Constraint;
|
||||
|
||||
/**
|
||||
* @Annotation
|
||||
* @Target({"PROPERTY", "METHOD", "ANNOTATION"})
|
||||
*
|
||||
* @author Andrey Sevastianov <mrpkmail@gmail.com>
|
||||
*/
|
||||
class ExpressionLanguageSyntax extends Constraint
|
||||
{
|
||||
const EXPRESSION_LANGUAGE_SYNTAX_ERROR = '1766a3f3-ff03-40eb-b053-ab7aa23d988a';
|
||||
|
||||
protected static $errorNames = [
|
||||
self::EXPRESSION_LANGUAGE_SYNTAX_ERROR => 'EXPRESSION_LANGUAGE_SYNTAX_ERROR',
|
||||
];
|
||||
|
||||
public $message = 'This value should be a valid expression.';
|
||||
public $service;
|
||||
public $validateNames = true;
|
||||
public $names = [];
|
||||
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function validatedBy()
|
||||
{
|
||||
return $this->service;
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,55 @@
|
|||
<?php
|
||||
|
||||
/*
|
||||
* This file is part of the Symfony package.
|
||||
*
|
||||
* (c) Fabien Potencier <fabien@symfony.com>
|
||||
*
|
||||
* For the full copyright and license information, please view the LICENSE
|
||||
* file that was distributed with this source code.
|
||||
*/
|
||||
|
||||
namespace Symfony\Component\Validator\Constraints;
|
||||
|
||||
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
|
||||
use Symfony\Component\ExpressionLanguage\SyntaxError;
|
||||
use Symfony\Component\Validator\Constraint;
|
||||
use Symfony\Component\Validator\ConstraintValidator;
|
||||
use Symfony\Component\Validator\Exception\UnexpectedTypeException;
|
||||
|
||||
/**
|
||||
* @author Andrey Sevastianov <mrpkmail@gmail.com>
|
||||
*/
|
||||
class ExpressionLanguageSyntaxValidator extends ConstraintValidator
|
||||
{
|
||||
private $expressionLanguage;
|
||||
|
||||
public function __construct(ExpressionLanguage $expressionLanguage)
|
||||
{
|
||||
$this->expressionLanguage = $expressionLanguage;
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function validate($expression, Constraint $constraint): void
|
||||
{
|
||||
if (!$constraint instanceof ExpressionLanguageSyntax) {
|
||||
throw new UnexpectedTypeException($constraint, ExpressionLanguageSyntax::class);
|
||||
}
|
||||
|
||||
if (!\is_string($expression)) {
|
||||
throw new UnexpectedTypeException($expression, 'string');
|
||||
}
|
||||
|
||||
try {
|
||||
$this->expressionLanguage->lint($expression, ($constraint->validateNames ? ($constraint->names ?? []) : null));
|
||||
} catch (SyntaxError $exception) {
|
||||
$this->context->buildViolation($constraint->message)
|
||||
->setParameter('{{ syntax_error }}', $this->formatValue($exception->getMessage()))
|
||||
->setInvalidValue((string) $expression)
|
||||
->setCode(ExpressionLanguageSyntax::EXPRESSION_LANGUAGE_SYNTAX_ERROR)
|
||||
->addViolation();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,88 @@
|
|||
<?php
|
||||
|
||||
/*
|
||||
* This file is part of the Symfony package.
|
||||
*
|
||||
* (c) Fabien Potencier <fabien@symfony.com>
|
||||
*
|
||||
* For the full copyright and license information, please view the LICENSE
|
||||
* file that was distributed with this source code.
|
||||
*/
|
||||
|
||||
namespace Symfony\Component\Validator\Tests\Constraints;
|
||||
|
||||
use PHPUnit\Framework\MockObject\MockObject;
|
||||
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
|
||||
use Symfony\Component\ExpressionLanguage\SyntaxError;
|
||||
use Symfony\Component\Validator\Constraints\ExpressionLanguageSyntax;
|
||||
use Symfony\Component\Validator\Constraints\ExpressionLanguageSyntaxValidator;
|
||||
use Symfony\Component\Validator\Test\ConstraintValidatorTestCase;
|
||||
|
||||
class ExpressionLanguageSyntaxTest extends ConstraintValidatorTestCase
|
||||
{
|
||||
/**
|
||||
* @var \PHPUnit\Framework\MockObject\MockObject|ExpressionLanguage
|
||||
*/
|
||||
protected $expressionLanguage;
|
||||
|
||||
protected function createValidator()
|
||||
{
|
||||
return new ExpressionLanguageSyntaxValidator($this->expressionLanguage);
|
||||
}
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
$this->expressionLanguage = $this->createExpressionLanguage();
|
||||
|
||||
parent::setUp();
|
||||
}
|
||||
|
||||
public function testExpressionValid(): void
|
||||
{
|
||||
$this->expressionLanguage->expects($this->once())
|
||||
->method('lint')
|
||||
->with($this->value, []);
|
||||
|
||||
$this->validator->validate($this->value, new ExpressionLanguageSyntax([
|
||||
'message' => 'myMessage',
|
||||
]));
|
||||
|
||||
$this->assertNoViolation();
|
||||
}
|
||||
|
||||
public function testExpressionWithoutNames(): void
|
||||
{
|
||||
$this->expressionLanguage->expects($this->once())
|
||||
->method('lint')
|
||||
->with($this->value, null);
|
||||
|
||||
$this->validator->validate($this->value, new ExpressionLanguageSyntax([
|
||||
'message' => 'myMessage',
|
||||
'validateNames' => false,
|
||||
]));
|
||||
|
||||
$this->assertNoViolation();
|
||||
}
|
||||
|
||||
public function testExpressionIsNotValid(): void
|
||||
{
|
||||
$this->expressionLanguage->expects($this->once())
|
||||
->method('lint')
|
||||
->with($this->value, [])
|
||||
->willThrowException(new SyntaxError('Test exception', 42));
|
||||
|
||||
$this->validator->validate($this->value, new ExpressionLanguageSyntax([
|
||||
'message' => 'myMessage',
|
||||
]));
|
||||
|
||||
$this->buildViolation('myMessage')
|
||||
->setParameter('{{ syntax_error }}', '"Test exception around position 42."')
|
||||
->setCode(ExpressionLanguageSyntax::EXPRESSION_LANGUAGE_SYNTAX_ERROR)
|
||||
->assertRaised();
|
||||
}
|
||||
|
||||
protected function createExpressionLanguage(): MockObject
|
||||
{
|
||||
return $this->getMockBuilder('\Symfony\Component\ExpressionLanguage\ExpressionLanguage')->getMock();
|
||||
}
|
||||
}
|
||||
|
|
@ -30,7 +30,7 @@
|
|||
"symfony/yaml": "^4.4|^5.0",
|
||||
"symfony/config": "^4.4|^5.0",
|
||||
"symfony/dependency-injection": "^4.4|^5.0",
|
||||
"symfony/expression-language": "^4.4|^5.0",
|
||||
"symfony/expression-language": "^5.1",
|
||||
"symfony/cache": "^4.4|^5.0",
|
||||
"symfony/mime": "^4.4|^5.0",
|
||||
"symfony/property-access": "^4.4|^5.0",
|
||||
|
|
@ -44,6 +44,7 @@
|
|||
"doctrine/lexer": "<1.0.2",
|
||||
"phpunit/phpunit": "<5.4.3",
|
||||
"symfony/dependency-injection": "<4.4",
|
||||
"symfony/expression-language": "<5.1",
|
||||
"symfony/http-kernel": "<4.4",
|
||||
"symfony/intl": "<4.4",
|
||||
"symfony/translation": "<4.4",
|
||||
|
|
@ -61,7 +62,7 @@
|
|||
"egulias/email-validator": "Strict (RFC compliant) email validation",
|
||||
"symfony/property-access": "For accessing properties within comparison constraints",
|
||||
"symfony/property-info": "To automatically add NotNull and Type constraints",
|
||||
"symfony/expression-language": "For using the Expression validator"
|
||||
"symfony/expression-language": "For using the Expression validator and the ExpressionLanguageSyntax constraints"
|
||||
},
|
||||
"autoload": {
|
||||
"psr-4": { "Symfony\\Component\\Validator\\": "" },
|
||||
|
|
|
|||
Reference in New Issue