Skip to content

Commit

Permalink
fix(MapQueryParameter) convert pcre regex to ecma
Browse files Browse the repository at this point in the history
  • Loading branch information
xavierleune committed Jan 21, 2025
1 parent 8b4803f commit 178d540
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 11 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# CHANGELOG

## 4.36.2
- Support of attribute MapQueryParameter with a regexp has been improved, it now converts the regexp from PCRE to ECMA-262 for better compliance with OpenApi.

## 4.36.1
- Passing an array key `value` with a list of strings to the `Areas` annotation/attribute is deprecated. Pass the list of strings directly.
```diff
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ private function describeValidateFilter(?int $filter, int $flags, array $options
}

if (FILTER_VALIDATE_REGEXP === $filter) {
return ['type' => 'string', 'pattern' => $options['regexp']];
return ['type' => 'string', 'pattern' => $this->getEcmaRegexpFromPCRE($options['regexp'])];
}

if (FILTER_VALIDATE_URL === $filter) {
Expand All @@ -141,4 +141,35 @@ private function describeValidateFilter(?int $filter, int $flags, array $options

return [];
}

private function getEcmaRegexpFromPCRE(string $pcreRegex): string
{
// Check if PCRE regex has delimiters
if (!preg_match('/^(.)(.*)\1([a-zA-Z]*)$/s', $pcreRegex, $matches)) {
throw new \InvalidArgumentException('Invalid PCRE regex format. Missing delimiters.');
}

[$fullMatch, $delimiter, $pattern, $flags] = $matches;

// Check for unsupported PCRE specific constructs
$unsupportedFeatures = [
'\A', // Start of string (use ^ in JavaScript)
'\z', // End of string (use $ in JavaScript)
'\Z', // End of string before newline (not supported in JavaScript)
'\R', // Any Unicode newline sequence (not supported in JavaScript)
'\K', // Resets the start of the current match (not supported in JavaScript)
];

foreach ($unsupportedFeatures as $feature) {
if (str_contains($pattern, $feature)) {
throw new \InvalidArgumentException("Unsupported PCRE feature found: {$feature}");

Check warning on line 165 in src/RouteDescriber/RouteArgumentDescriber/SymfonyMapQueryParameterDescriber.php

View check run for this annotation

Codecov / codecov/patch

src/RouteDescriber/RouteArgumentDescriber/SymfonyMapQueryParameterDescriber.php#L165

Added line #L165 was not covered by tests
}
}

// Remove escaped delimiters in the pattern
$pattern = str_replace('\\'.$delimiter, $delimiter, $pattern);

// Return only the pattern (without flags or delimiters)
return $pattern;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Nelmio\ApiDocBundle\Tests\Functional\Controller;

use OpenApi\Attributes as OA;
use Symfony\Component\HttpKernel\Attribute\MapQueryParameter;
use Symfony\Component\Routing\Annotation\Route;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

/*
* This file is part of the NelmioApiDocBundle package.
*
* (c) Nelmio
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Nelmio\ApiDocBundle\Tests\Functional\Controller;

use OpenApi\Attributes as OA;
use Symfony\Component\HttpKernel\Attribute\MapQueryParameter;
use Symfony\Component\Routing\Annotation\Route;

class MapQueryParameterWithInvalidPCREController
{
#[Route('/article_map_query_parameter_invalid_regexp', methods: ['GET'])]
#[OA\Response(response: '200', description: '')]
public function fetchArticleWithInvalidRegexp(
#[MapQueryParameter(filter: FILTER_VALIDATE_REGEXP, options: ['regexp' => 'This is not a valid regexp'])]
string $regexp,
) {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

/*
* This file is part of the NelmioApiDocBundle package.
*
* (c) Nelmio
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Nelmio\ApiDocBundle\Tests\Functional\Controller;

use OpenApi\Attributes as OA;
use Symfony\Component\HttpKernel\Attribute\MapQueryParameter;
use Symfony\Component\Routing\Annotation\Route;

class MapQueryParameterWithUnsupportedFlagInPCREController
{
#[Route('/article_map_query_parameter_invalid_regexp', methods: ['GET'])]
#[OA\Response(response: '200', description: '')]
public function fetchArticleWithUnsupportedRegexpFlag(
#[MapQueryParameter(filter: FILTER_VALIDATE_REGEXP, options: ['regexp' => 'This is not a valid regexp'])]
string $regexp,
) {
}
}
47 changes: 47 additions & 0 deletions tests/Functional/ControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,35 @@ public function testControllers(?array $controller, ?string $fixtureName = null,
);
}

/**
* @dataProvider provideExceptionsTestCases
*
* @param array{name: string, type: string}|null $controller
* @param Bundle[] $extraBundles
* @param string[] $extraConfigs
*/
public function testControllersThrowingExceptions(?array $controller, array $extraBundles = [], array $extraConfigs = []): void
{
if (version_compare(Kernel::VERSION, '6.3.0', '<')) {
$this->markTestSkipped();

Check failure on line 90 in tests/Functional/ControllerTest.php

View workflow job for this annotation

GitHub Actions / PHPStan

Dynamic call to static method PHPUnit\Framework\Assert::markTestSkipped().
}
$controllerName = $controller['name'] ?? null;
$controllerType = $controller['type'] ?? null;

$routingConfiguration = function (RoutingConfigurator $routes) use ($controllerName, $controllerType) {
if (null === $controllerName) {
return;
}

$routes->withPath('/')->import(__DIR__."/Controller/$controllerName.php", $controllerType);
};

$this->configurableContainerFactory->create($extraBundles, $routingConfiguration, $extraConfigs);

$this->expectException(\InvalidArgumentException::class);
$this->getOpenApiDefinition();
}

public static function provideAttributeTestCases(): \Generator
{
if (PHP_VERSION_ID < 80100) {
Expand Down Expand Up @@ -212,6 +241,24 @@ public static function provideUniversalTestCases(): \Generator
];
}

public static function provideExceptionsTestCases(): \Generator
{
$type = 'attribute';

yield 'Symfony 6.3 MapQueryParameter attribute with invalid PCRE' => [
[
'name' => 'MapQueryParameterWithInvalidPCREController',
'type' => $type,
],
];
yield 'Symfony 6.3 MapQueryParameter attribute with unsupported flag in PCRE' => [
[
'name' => 'MapQueryParameterWithUnsupportedFlagInPCREController',
'type' => $type,
],
];
}

private static function getFixture(string $fixture): string
{
if (!file_exists($fixture)) {
Expand Down
24 changes: 14 additions & 10 deletions tests/Functional/Fixtures/MapQueryParameterController.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
}
],
"responses": {
"default": {
"200": {
"description": ""
}
}
Expand Down Expand Up @@ -133,7 +133,7 @@
"required": true,
"schema": {
"type": "string",
"pattern": "/^test/"
"pattern": "^test"
}
},
{
Expand All @@ -147,7 +147,7 @@
}
],
"responses": {
"default": {
"200": {
"description": ""
}
}
Expand All @@ -168,7 +168,7 @@
}
],
"responses": {
"default": {
"200": {
"description": ""
}
}
Expand All @@ -189,7 +189,7 @@
}
],
"responses": {
"default": {
"200": {
"description": ""
}
}
Expand All @@ -202,24 +202,28 @@
{
"name": "id",
"in": "query",
"description": "Query parameter id description",
"required": false,
"schema": {
"type": "integer",
"nullable": true
}
},
"example": 123
},
{
"name": "changedType",
"in": "query",
"description": "Incorrectly described query parameter",
"required": false,
"schema": {
"type": "string",
"nullable": true
}
"type": "int",
"nullable": false
},
"example": 123
}
],
"responses": {
"default": {
"200": {
"description": ""
}
}
Expand Down

0 comments on commit 178d540

Please sign in to comment.