Skip to content

33589 fix array values in catalogsearch parameters #33682

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

Merged
merged 6 commits into from
Sep 23, 2021
Merged

33589 fix array values in catalogsearch parameters #33682

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
58adfbf
33589 fix array values in catalogsearch parameters
Aug 4, 2021
0c72905
Merge branch '2.4-develop' into 33589-catalogsearch-strlen-exception
andrewbess Aug 5, 2021
d25aba7
33589 change advanced search model and add test
Aug 19, 2021
e5df80f
Merge branch '33589-catalogsearch-strlen-exception' of github.com:Sil…
Aug 19, 2021
dd0124b
33589 change test data
Aug 23, 2021
44ed899
33589 fix integration test
Aug 24, 2021
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
19 changes: 19 additions & 0 deletions app/code/Magento/CatalogSearch/Model/Advanced.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,13 @@ protected function getPreparedSearchCriteria($attribute, $value)
if (!empty($value['from']) || !empty($value['to'])) {
$from = '';
$to = '';
if (is_array($value['from'])) {
$value['from'] = $this->getFirstArrayElement($value['from']);
}
if (is_array($value['to'])) {
$value['to'] = $this->getFirstArrayElement($value['to']);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, in case if we got an incorrect type - it's better to do assume that this parameter doesn't exist at all. I think we're expecting here the only string.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ihor-sviziev Hi! May be you are right and we should create a redirect if $value['from'] or $value['to'] is array. But I think we can check this arrays for some values and create a valid response.

cc: @andrewbess, @eduard13

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SilinMykola I'm agree with @ihor-sviziev that we should ignore this parameter if it's not valid instead of trying to understand what we received and extract some data from it.
Looks like patch for all occasions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, @Den4ik! Locally I removed this code and work on tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following comment is related to this discussion #33589 (comment)


if (isset($value['currency'])) {
/** @var $currencyModel Currency */
$currencyModel = $this->_currencyFactory->create()->load($value['currency']);
Expand Down Expand Up @@ -433,4 +440,16 @@ public function getSearchCriterias()
{
return $this->_searchCriterias;
}

/**
* Return first value in array
*
* @param $value
*
* @return mixed
*/
private function getFirstArrayElement($value)
{
return is_array($value) ? $this->getFirstArrayElement(array_shift($value)) : $value;
}
}