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.

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

[9.x] Cast $perPage to integer on Paginator #41073

Merged

Conversation

gdebrauwer
Copy link
Contributor

@gdebrauwer gdebrauwer commented Feb 17, 2022

The $perPage parameter of LengthAwarePaginator/CursorPaginator must be an integer according to docblocks, but when the parameter is an integer in a string, it is never cast to an integer. Such casting is already done on $lastPage and $currentPage, but not yet on $perPage. This PR fixes that.

@driesvints
Copy link
Member

driesvints commented Feb 17, 2022

I don't think we should cast this. If you send in something else than an integer then there's a bug in your app somewhere and I think you should be made aware of that.

@gdebrauwer
Copy link
Contributor Author

gdebrauwer commented Feb 17, 2022

When you use the paginate-method with a "per page" value from a querystring parameter, then that value is an integer in a string. This is similar to the default page parameter in the querystring that is used by the paginator. The currentPage value is, because of that, also an integer as a string by default, but it is cast in the constructor of the LenghtAwarePaginator. So I think it is logical to do the same with the perPage value.

This ensures the json-encoded version of the paginator always returns per_page as an integer, because you now may get in the situation that per_page json value is sometimes a string and sometimes an integer

@gdebrauwer
Copy link
Contributor Author

When you provided an actual string as perPage, not an integer in a string, you will still get an exception because the query will fail when using a string as a limit amount

@gdebrauwer gdebrauwer changed the title [9.x] Cast $perPage to integer on LengthAwarePaginator [9.x] Cast $perPage to integer on Paginator Feb 17, 2022
@taylorotwell taylorotwell merged commit e4c93c4 into laravel:9.x Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants