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 |