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.

Sign up for GitHub

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
Open

fix fatal error when $param is array #252

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a1a311
fix fatal error when $param is array
Aug 26, 2016
6548a9f
fix code style
Aug 29, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion app/code/community/Lesti/Fpc/Helper/Data.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,8 @@ public function canCacheRequest()
foreach ($missParams as $missParam) {
$pair = array_map('trim', explode('=', $missParam));
$key = $pair[0];
$param = $request->getParam($key);
$param = $this->_paramToString($request->getParam($key));

if ($param && isset($pair[1]) && preg_match($pair[1], $param)) {
return false;
}
Expand Down Expand Up @@ -258,4 +259,30 @@ public function getContentType(\Mage_Core_Controller_Response_Http $response)

return 'text/html; charset=UTF-8';
}

/**
* Transform array parameter to string to avoid error and make correct validation in regexp pattern
*
* @param mixed $param Input parameter
*
* @return mixed
*/
protected function _paramToString($param)
{
if (is_array($param)) {
$param = array_reduce(
$param,
function ($carry, $item) {
Copy link
Owner

@GordonLesti GordonLesti Aug 29, 2016 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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

if (!is_array($item)) {
return $carry . $item;
}

return '';
},
''
);
}

return $param;
}
}