KnpLabs / KnpLabs/knp-components
Error when passing an array as sort direction parameter in ORM pagination
- Dominant language
- PHP
- Stars
- 773
- Forks
- 139
- Avg merge
- 22h 3m
- Merged PRs (30d)
- 1
Description
Affected version: 3.6.0 (possibly 4.1.0 as well)
When using ORM pagination and calling something like `http://localhost/galery/page/2?direction[test]=1&sort=columnname`, the `direction` parameter becomes an array. This will result in an array to string conversion error in the [3.6.0 QuerySubscriber](https://github.com/KnpLabs/knp-components/blob/v3.6.0/src/Knp/Component/Pager/Event/Subscriber/Sortable/Doctrine/ORM/QuerySubscriber.php#L38). The [4.1.0 QuerySubscriber](https://github.com/KnpLabs/knp-components/blob/v4.1.0/src/Knp/Component/Pager/Event/Subscriber/Sortable/Doctrine/ORM/QuerySubscriber.php#L36) looks like it could be affected, too.
First, I thought about a quick `is_string()` check. But then I noticed that other places like the [SlidingPagination in the KnpPaginatorBundle](https://github.com/KnpLabs/KnpPaginatorBundle/blob/v6.2.0/src/Pagination/SlidingPagination.php#L141-L144) rely on the `direction` parameter to be a string, too. It seems that more than one class gets the value directly from the request. So my current idea is fixing the request with a kernel event subscriber like this draft:
```
final class SanitizeKnpPaginationParameters implements EventSubscriberInterface
{
public static function getSubscribedEvents(): array
{
return [
KernelEvents::REQUEST => ['onKernelRequest', 10],
];
}
public function onKernelRequest(RequestEvent $event): void
{
$direction = $event->getRequest()->query->get('direction');
if (null === $direction || \is_string($direction)) {
return;
}
$event->getRequest()->query->set('direction', 'desc');
}
}
```
What do you think? I'm not convinced this steamroller approach is an elegant solution.
I'd be happy to open a PR when we have a good notion on how to do it. (If so, could I target a 3.x branch or are PRs only accepted on the current major?)
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by reading the linked 3.6.0 and 4.1.0 QuerySubscriber implementations, then inspect SlidingPagination.php where direction is also read from the request. Reproduce pagination with an array-valued direction parameter and trace each affected path. Done means the request no longer causes array-to-string errors across these components while normal string directions continue to work.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend, database
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100