| Summary: | REST API: _per_page=0 crashes on Illegal division by zero | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Marcel de Rooy <m.de.rooy> |
| Component: | REST API | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Status: | Pushed to oldstable --- | QA Contact: | |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | fridolin.somers, lucas, tomascohen |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
24.05.00,23.11.06,23.05.12
|
|
| Circulation function: | |||
| Attachments: |
Bug 35129: Regression tests
Bug 35129: Return 400 if _per_page=0 passed Bug 35129: Regression tests Bug 35129: Return 400 if _per_page=0 passed Bug 35129: Regression tests Bug 35129: Return 400 if _per_page=0 passed |
||
|
Description
Marcel de Rooy
2023-10-23 12:11:42 UTC
To prohibit _per_page=0 we could use OpenAPI 3.0 with
not:
const: 0
Sorry no quick simple solution found yet. Take a try! Nice catch. Created attachment 163342 [details] [review] Bug 35129: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 163343 [details] [review] Bug 35129: Return 400 if _per_page=0 passed This patch adds a safe guard for when consumers pass _per_page=0 to endpoints. This condition is checked for on a centralized place and avoid reaching the controller in such scenarios that would provoke a division by zero exception. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/pagination.t => FAIL: We expect a 400, but get a 500 instead 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! No more explosions for this! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 163637 [details] [review] Bug 35129: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> Created attachment 163638 [details] [review] Bug 35129: Return 400 if _per_page=0 passed This patch adds a safe guard for when consumers pass _per_page=0 to endpoints. This condition is checked for on a centralized place and avoid reaching the controller in such scenarios that would provoke a division by zero exception. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/pagination.t => FAIL: We expect a 400, but get a 500 instead 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! No more explosions for this! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> Created attachment 164222 [details] [review] Bug 35129: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 164223 [details] [review] Bug 35129: Return 400 if _per_page=0 passed This patch adds a safe guard for when consumers pass _per_page=0 to endpoints. This condition is checked for on a centralized place and avoid reaching the controller in such scenarios that would provoke a division by zero exception. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/pagination.t => FAIL: We expect a 400, but get a 500 instead 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! No more explosions for this! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed for 24.05! Well done everyone, thank you! Pushed to 23.11.x for 23.11.06 Backported to 23.05.x for upcoming 23.05.12 |