laminas-api-tools / api-tools-hal

Laminas Module providing Hypermedia Application Language assets and rendering
https://api-tools.getlaminas.org/documentation
BSD 3-Clause "New" or "Revised" License
6 stars 12 forks source link

Invalid page size parameter causes a 500 Internal Server Error #10

Open weierophinney opened 4 years ago

weierophinney commented 4 years ago

If you configure a page size parameter, values that are not positive integers (or -1) will result in a 500 Internal Server Error and a stack trace. The exceptions that trigger this (vendor/zfcampus/zf-hal/src/Collection.php line 302 and 310) indicate the problem is one of validation rather than something going wonky on the server. IMHO, this should not be a 500, but could either be a 400 or 422 or it could "fail safe" back to using the configured default and not causing an error at all.


Originally posted by @dstockto at https://github.com/zfcampus/zf-hal/issues/141

weierophinney commented 4 years ago

Since this was opened, the class has been updated so that -1 indicates no pagination. So the above is still true for values that are not positive integers, other than -1. The exceptions thrown are now on 287 and 295 instead of the original 302 and 310.

It appears the fix for this would probably be in zf-rest in \ZF\Rest\RestController in prepareHalCollection. The call to $collection->setPageSize($this->getPageSize()); could be wrapped in a try..catch and return an ApiProblem rather than letting the ZF\Hal\Exception\InvalidArgumentException bubble all the way out and become a 500. If this sounds like an acceptable way of handling it, I'm happy to make a PR. I don't think this should be a 5xx error though.


Originally posted by @dstockto at https://github.com/zfcampus/zf-hal/issues/141#issuecomment-244665727