Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

f0ska
Copy link

@f0ska f0ska commented Aug 26, 2016

It is fatal error when input parameter is array, for example like ?manufacturer[0]=Modernica

if (is_array($param)) {
$param = array_reduce(
$param,
function ($carry, $item) {
Copy link
Owner

@GordonLesti GordonLesti Aug 29, 2016

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?

Copy link
Author

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:

  1. with json_encode you will receive "[25,98]" string, and it will be incorrect, because does not match our pattern
  2. 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.

Copy link
Owner

@GordonLesti GordonLesti Aug 29, 2016

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 the miss_uri_params something like this attribute_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 and 5. The items will be concat to 25 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.

Copy link
Author

@f0ska f0ska Aug 29, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants