cakephp: Paginator::generateUrl returns invalid URL for all query parameters
This is a (multiple allowed):
- bug
- enhancement
- feature-discussion (RFC)
- CakePHP Version: 3.2.12
- Platform and Target: Apache 2.4, MySQL, PHP 7
What you did
echo $this->Paginator->generateUrl(['test'=>'thing']);
Generated a custom URL for use with pagination.
Expected Behavior
Output a URL that contains the extra query parameters.
Actual Behavior
Output URL contains escaped HTML entities so that & is formatted as &
I traced this down to the UrlHelper::build() method which is a simple function:
public function build($url = null, $full = false)
{
return h(Router::url($url, $full));
}
Why is the resulting URL being passed to h(....) which will encode all the entities including the query parameter joiner &
About this issue
- Original URL
- State: closed
- Created 8 years ago
- Comments: 27 (27 by maintainers)
@dereuromark I have a PR coming that does exactly this. Sorry, I’ve been so busy that I hadn’t had time to finish it. This PR will also close another issue on this topic.
Why did you say a raw URL is a security issue? What are the risks?
Let’s wait for the PR and then look into it. I don’t think a rawLink() makes sense.
Because people always forget to escape it then, leaving XSS possibilities as well as invalid DOM.
I just took a look into the source code (which should always be the first step before opening a ticket), as this tells a lot about what the class internally and externally provides and does. And I see a
generateUrl()method ( https://github.com/cakephp/cakephp/blob/master/src/View/Helper/PaginatorHelper.php#L477 ).The doc block of it states:
Maybe we should provide a link() method then which internally uses this or some url method to build a link as you expect it?
Because it doesn’t exists in 3.x, does it?
I don’t probably understand the security implications so I’ll just get used to how it works.
URL encoding aside, being able to generate links with the current pagination context is still something that isn’t ‘easy’ to do with the current methods. Is that gap something someone wants to address?
@markstory @dereuromark okay guys. My apologizes for wasting your time. I really thought this was wrong, but after googling it turns out you’re completely correct.
https://perishablepress.com/how-to-write-valid-url-query-string-parameters/
Not encoding the
&would produce invalid HTML.Still much to learn. Thanks.
The above URL is absolutely correct and valid. The characters must be properly escaped when outputted in HTML context. Crawlers know how to treat them. And so do all browsers, of course.
I can see how that would be expected to work. We could add a more general purpose
Paginator->link()method that lets you make customized link elements that include pagination context. We used to have a similar method in 2.x, and I’ve forgotten why it was removed.