-
Notifications
You must be signed in to change notification settings - Fork 157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix fatal error when $param is array #252
base: master
Are you sure you want to change the base?
Conversation
if (is_array($param)) { | ||
$param = array_reduce( | ||
$param, | ||
function ($carry, $item) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this can lead to conflicts if we just concat all items of the array. What about just using serialize or json_encode instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello,
Which conflicts? It just a line of text.
If you will use serialize() or json_encode(), you will get incorrect value in preg_match() section.
for example, when you define at BO expression for some field ^[0-9]+$ (you expect some id(s)), then:
- with json_encode you will receive "[25,98]" string, and it will be incorrect, because does not match our pattern
- with reducing array to string (via concatenation) you will receive "2598" string and it will be always correct, even if you will send multidimensional array in parameter
In your code this is simple can_cache flag and data checks only for valid format, but its context not necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With conflicts I mean something like this:
- We want to ignore requests where multiple filters for an attribute contain only the value
25
. So we add to themiss_uri_params
something like thisattribute_code=/^25$/
(I'm not 100% sure, but I guess that should work). - A request with the multiple filters for the attribute contains the values
2
and5
. The items will be concat to25
and the request gets ignored. - json_encode was just my quick idea, cause it would be possible to create a regular expression (maybe a complicated one) that only covers the example from above without ignoring other requests.
I really appreciate your PR, but the issue #191 is so old cause I haven't found a good solution that covers every case and that ignoring with regular expressions is comfortable at the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I understand what you mean.
In this case the only one solution - to walk recursive throw array and apply regexp to every entry.
And if no matches than no cache.
Maybe this not covers all situations, but this is a best that we can do, I think
It is fatal error when input parameter is array, for example like ?manufacturer[0]=Modernica