KnpLabs / KnpLabs/knp-components

Error when passing an array as sort direction parameter in ORM pagination

Open
#322 6 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.