Filter\Validator\Require fails on nested property

[stefanides]

API Platform version(s) affected: 2.6.0+

Description
Hi, after upgrading v2.5.7 -> v2.6.4 I am facing the strange behaviour with required filter parameter on nested property. QueryParameterValidator.php (line 65) then returns FilterValidationException: Query parameter "x.y" is required even it is present in query and can work.

Once the filter is set to be required i.e. in a custom filter, the Filter\Validator\Require class fails to test requestHasQueryParameter() when the parameter name contains dot (used for nested properties). This looks to be introduced in #1723, where new Require class compares $queryParameters array in form "x.y" with result of parse_str() which converts dots to "x_y" (see e.g. https://stackoverflow.com/questions/68651/get-php-to-stop-replacing-characters-in-get-or-post-arrays) - this cannot match, then an exception is thrown.

How to reproduce

• Set Entity with nested property filter
• Create custom filter class, add getDescription() method with 'required' => true like here https://api-platform.com/docs/core/filters/#creating-custom-filters
• Call API either directy or from ApiDoc
• Filter\Validator\Require complains on missing parameter?!

• Set custom filter class with 'required' => false
• Filter\Validator\Require test passes and filter works(!)

Possible Solution
Some workaround for parse_str() like here https://stackoverflow.com/questions/22539633/parse-string-containing-dots-in-php ?

Or compare output of parse_str() with $request->query->all() which also has underscores instead of dots, as it is in v2.5.7 (https://github.com/api-platform/core/blob/37b5edf50cc05299b12cb8982be3efa035d726ce/src/Filter/QueryParameterValidateListener.php)

Or get underscores also in $queryParameters?

Additional Context
Require class https://github.com/api-platform/core/pull/1723/files#diff-900c533383b3b17876e6a5c81e525632c8a492358f36f352b0db974e4d604a09

Test simple code

<?php
parse_str("x.y", $blabla);
var_dump($blabla);

which outputs

array(1) {
  ["x_y"]=>
  string(0) ""
}

[alanpoulain]

Hello, I think the better approach would be to keep the dots if possible, like it's done in the RequestParser.
Could you make a PR to fix it?

[stefanides]

@alanpoulain yes, keep dots as much as possible is better. I am just not sure about the correct parse_str() replacement - it can easily introduce other bugs for edge cases if some translation is missing :-( If this has a reliable solution, PR is then quite easy

[alanpoulain]

Maybe just using the same method RequestParser::parseRequestParams would be alright?

[stefanides]

I do not know details of RequestParser. What says/thinks @jdeniau as original author of Require validator?

[jdeniau]

I do not remember exactly the case, and I do not know why it does not work now whereas it did worked previously. But the origin of parse_str has been required here #1692 (comment) and challenged in the same PR by @teohhanhui : #1692 (comment) but kept as-is for now.

I can try to investigate a little further tomorrow to see exactly what is happening.

Just by reading your issue, I think that the more we keep the initial key, the better, but it might be more subtle than this.

[jdeniau]

For more context, the tested value does come from RequestParser::parseRequestParams($queryString) while in older version on Require, we did use $request->query->all()[$rootName]

@alanpoulain Did you know if something did change in 2.6 on RequestParser ?

[alanpoulain]

No I don't think so.

[stefanides]

I do not remember exactly the case, and I do not know why it does not work now whereas it did
worked previously.

Originally it compared parse_str() output to $request->query->all(), now it compares the same to $queryParameters probably from RequestParser::parseRequestParams()(?) with slightly different results on dot->underscore.

But the origin of parse_str has been required here [#1692 (comment)]

This here shows as valid comment, stop using parse_str(). The key is just to find a good alternative :)

[jdeniau]

@stefanides IIRC we can not compare directly the key, as if you pass ?foo[bar]=baz as string, the corresponding request value will be $_GET['foo']['bar'] and not $_GET['foo[bar]'], but I will try to reproduce your issue tomorrow and investigate on this.

About the link on introducing parse_str, I copied the wrong link, the right one is #1692 (comment)

[jdeniau]

@alanpoulain RequestParser::parseRequestParams seems to do the trick on both dot and bracket notation, I opened #4221 to replace parse_str by parseRequestParams

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stefanides]

This should be fixed in 8d4891a , v2.6.5