[RFR] Use 1 as page value if the submited one is invalid #1826
Conversation
| @@ -103,7 +103,9 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator | |||
| throw new InvalidArgumentException('Item per page parameter should not be less than 0'); | |||
| } | |||
|
|
|||
| $page = $this->getPaginationParameter($request, $this->pageParameterName, 1); | |||
| if (0 >= $page = $this->getPaginationParameter($request, $this->pageParameterName, 1)) { | |||
| $page = 1; | |||
There was a problem hiding this comment.
what about throwing an error instead?
There was a problem hiding this comment.
@soyuka, it can be useful if the front call the api with empty page param for first page: /resources?page=
There was a problem hiding this comment.
it can be useful if the front call the api with empty
pageparam for first page:/resources?page=
I'm actually surprised that we're not casting to integer somewhere...
ottaviano
force-pushed
the
fix-invalid-page-value
branch
from
321ecf6
to
ac13118
Compare
| $page = $this->getPaginationParameter($request, $this->pageParameterName, 1); | ||
| if (0 >= $page = $this->getPaginationParameter($request, $this->pageParameterName, 1)) { | ||
| $page = 1; | ||
| } | ||
|
|
||
| if (0 === $itemsPerPage && 1 < $page) { |
There was a problem hiding this comment.
This test does exactly the same thing isn't it?
There was a problem hiding this comment.
@dunglas not exactly,
Need to check the negative or zero value for $page which the $firstResult can be negative.
$firstResult = ($page - 1) * $itemsPerPage;
| @@ -103,7 +103,9 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator | |||
| throw new InvalidArgumentException('Item per page parameter should not be less than 0'); | |||
| } | |||
|
|
|||
| $page = $this->getPaginationParameter($request, $this->pageParameterName, 1); | |||
There was a problem hiding this comment.
I would just cast the result of this expression to (int), or throw after calling ctype_digit?
There was a problem hiding this comment.
@dunglas, cast don't fix the negative page value ?page=-10
There was a problem hiding this comment.
We can cast, and also check for negative values. If the value is 0 we could default to 1. But if it's negative, maybe throw an exception?
ottaviano
force-pushed
the
fix-invalid-page-value
branch
from
ac13118
to
2ce47e4
Compare
ottaviano
changed the title
ottaviano
force-pushed
the
fix-invalid-page-value
branch
from
f4948d0
to
6f8330a
Compare
|
I think we can close this since #1871 is already merged. |
Hi, it's a little fix to avoid this error
LIMIT argument offset=-24 is not validif invalidpagevalue is submitted.