api-platform / core

The server component of API Platform: hypermedia and GraphQL APIs in minutes
https://api-platform.com
MIT License
2.45k stars 878 forks source link

Inconsistent validation of filter with a regular expression in the pattern #6685

Open LukasLangr opened 1 month ago

LukasLangr commented 1 month ago

API Platform version affected: 4.0.2 (Symfony 7.1)

Description
After updating the API platform from version 3 to 4, I encountered an issue. I have a regex pattern defined in the schema for a filter. The API Platform now reports an error when making requests with the filter set:'No ending delimiter '^' found.

I tried to find where the problem lies. I installed a clean instance of API Platform (API Platform 4.0.2 + Symfony 7.1). From the documentation, I took the sample Book entity and the RegexpFilter filter. I found out that when I set the filter through the #[ApiFilter()] attribute, the error does not appear. However, when I set the filter directly to a specific operation using new GetCollection(filters: [RegexpFilter::class]), the parameter gets validated via RegexValidator during the request. And since the pattern is defined in the schema in ECMA-262 format, i.e., without delimiters, it is incompatible with preg_match, where a delimiter is required. Although I can add delimiters to the pattern, which makes the processing work, it breaks the validation in SwaggerUI

How to reproduce

  1. create a custom filter
  2. In the getDescription method of the filter, set the regex pattern in the schema. Use ECMA-262 without delimiters.
  3. assign the filter to the operation through new GetCollection(filters: [RegexpFilter::class])
  4. make a request with the filter parameter

Additional Context

public function getDescription(string $resourceClass): array
{
    return [
        'title' => [
            'property' => 'title',
            'type' => Type::BUILTIN_TYPE_STRING,
            'required' => false,
            'schema' => [
                'type' => Type::BUILTIN_TYPE_STRING,
                // regex pattern in ECMA-262, used by swagger UI and by RegexValidator
                'pattern' => '^[a-z0-9]+$'
            ],
        ]
    ];
}
#[ApiResource(
    operations: [
        new GetCollection(
            filters: [
                // A filter set this way is validated through the RegexValidator, and an error is thrown.
                RegexpFilter::class
            ],
        )
    ]
)]
// This works, or rather, the validation most likely doesn't occur at all through the RegexValidator.
#[ApiFilter(RegexpFilter::class)]
class Book
{
soyuka commented 1 month ago

Can you check my patch and lmk what you think?

LukasLangr commented 1 month ago

It looks like the change in the file ParameterValidationResourceMetadataCollectionFactory.php indeed resolves the issue with the Delimiters error. This way, the validation is preserved in SwaggerUI, and it is also correctly validated during the request processing in Symfony.

I also compared the differences between ECMA-262 and PCRE and didn't find any significant difference that would prevent preg_match('#'.$schema['ECMA-262_PATTERN'].'#', $value) from working.

However, it still holds that for the filter defined through the PHP attribute #[ApiFilter(RegexpFilter::class)], parameter validation does not occur during the request, and thus the pattern in the schema is completely ignored during server-side validation.

soyuka commented 1 month ago

But RegexpFilter is not part of our core filters and the previous validation system looks broken anyways. We probably need to update the documentation to reflect the usage of QueryParameter instead of ApiFilter?

LukasLangr commented 1 month ago

RegexpFilter is just an example of a custom filter, so I can set a pattern on the schema in getDescription as a demonstration.

I am under the impression that setting the filter via #[ApiFilter(RegexpFilter::class)] is essentially equivalent to writing new GetCollection(filters: [RegexpFilter::class]) when configuring a specific operation in the ApiResource attribute, isn’t it? The only difference is that the ApiFilter attribute sets the filter for all operations.

I may not be describing the issue accurately. I will try to describe it again using the internal BackedEnumFilter:

book.status_filter:
    parent: 'api_platform.doctrine.orm.backed_enum_filter'
    arguments: [{ status: ~ }]
    tags: ['api_platform.filter']
#[ApiResource(
    operations: [
        new GetCollection(
            filters: [
                // URL parameter 'status' is VALIDATED during request processing
                'book.status_filter'
            ]
        )
    ]
)]
// URL parameter 'status' is NOT VALIDATED during request processing
#[ApiFilter(BackedEnumFilter::class, properties: ['status'])]
class Book
{
    #[ORM\Column]
    public BookStatus $status = BookStatus::unread;
}

enum BookStatus: string
{
    case read = 'read';
    case unread = 'unread';
}

Now, when I use an invalid value for my status filter in the URL, the results differ depending on how I defined the filter. I call the API like this, for example: GET https://www.example.com/api/books/?page=1&status=lorem-ipsum

Filter set by SwaggerUI Server Side API Response
 #[ApiFilter(BackedEnumFilter::class, properties: ['status'])]
✅ validated ❌ not validated 200 (Unfiltered list, filter ignored)
 new GetCollection( filters: [ 'book.status_filter' ] )
✅ validated ✅ validated 422 (The value you selected is not a valid choice.)

I'm not sure if this is purely a documentation issue. I would expect the API to behave consistently in this case, regardless of how I set the filter.

soyuka commented 1 month ago

Mhh it should be the same but is it possible that the filter isn't available at:

https://github.com/api-platform/core/blob/fc8fa00a19320b65547a60537261959c11f8e6a8/src/Metadata/Resource/Factory/ParameterResourceMetadataCollectionFactory.php#L73-L75

This means the priority of the ParameterResourceMetadataCollectionFactory should be re-adjusted :

https://github.com/api-platform/core/blob/a4d6ac41bf098bd84a0b6aaea35cf48275f77a05/src/Symfony/Bundle/Resources/config/metadata/resource.xml#L82