fix(pagination): guard against division by zero when Size is 0#59
Merged
Erlend Ellefsen (erlendellefsen) merged 1 commit intoJan 24, 2026
Merged
Conversation
Collaborator
Erlend Ellefsen (erlendellefsen)
commented
Jan 24, 2026
Erlend Ellefsen (erlendellefsen)
commented
- Clamp pagination.Size to minimum of 1 before division operations
- Add regression tests for Size=0 edge case
- Clamp pagination.Size to minimum of 1 before division operations - Add regression tests for Size=0 edge case
Copilot started reviewing on behalf of
Erlend Ellefsen (erlendellefsen)
January 24, 2026 21:04
View session
Erlend Ellefsen (erlendellefsen)
deleted the
fix/1.4-pagination-division-by-zero
branch
There was a problem hiding this comment.
Pull request overview
Guards JSON:API pagination logic against DivideByZeroException when clients pass page[size]=0 by clamping the size to a minimum of 1.
Changes:
- Clamp
pagination.Sizeto at least1in pagination calculations (ApplyPagination,CreatePaginationMetaAsync). - Add regression tests covering the
Size=0edge case for both pagination application and meta generation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| JsonApiToolkit/Extensions/Querying/Handlers/PaginationHandler.cs | Clamps page size before division/Skip/Take to avoid division by zero. |
| JsonApiToolkit.Tests/Extensions/QueryableExtensionTests.cs | Adds tests ensuring Size=0 is treated as Size=1 for results and pagination metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+21
to
24
| int size = Math.Max(1, pagination.Size); | ||
| int totalCount = query.Count(); | ||
| int totalPages = (int)Math.Ceiling(totalCount / (double)pagination.Size); | ||
| int totalPages = (int)Math.Ceiling(totalCount / (double)size); | ||
| int effectivePage = Math.Max(1, Math.Min(pagination.Number, Math.Max(totalPages, 1))); |
There was a problem hiding this comment.
This change clamps pagination.Size inside PaginationHandler, but the request Size can still be 0 elsewhere. In particular, JsonApiToolkit/Controllers/JsonApiController.cs:280 computes Math.Ceiling(totalCount / (double)parameters.Pagination.Size) and will still throw DivideByZeroException when Size == 0. Consider applying the same clamp there (and using the clamped value for PageSize), or centralizing the size-normalization logic so all pagination/meta code paths are protected.
Comment on lines
+48
to
50
| int size = Math.Max(1, pagination.Size); | ||
| int totalPages = (int)Math.Ceiling(totalCount / (double)size); | ||
| int effectivePage = Math.Max(1, Math.Min(pagination.Number, Math.Max(totalPages, 1))); |
There was a problem hiding this comment.
The size-clamping logic (Math.Max(1, pagination.Size)) is now duplicated in both ApplyPagination and CreatePaginationMetaAsync. To avoid future drift (e.g., if you later decide to also clamp page number or apply a max page size), consider extracting this into a shared helper (private method on PaginationHandler or an extension on PaginationParameters).
Erlend Ellefsen (erlendellefsen)
pushed a commit
that referenced
this pull request
Jan 24, 2026
🤖 I have created a release *beep* *boop* --- ## [1.3.0](Intility.JsonApiToolkit-v1.2.5...Intility.JsonApiToolkit-v1.3.0) (2026-01-24) ### Features * ✨ `AllowedIncludesAttribute` to whitelist allowed include paths ([6a26c29](6a26c29)) * ✨ `JsonApiOkAsync` ([d22466d](d22466d)) * ✨ `JsonApiOkAsync` ([bc26940](bc26940)) * ✨ add filtering support for included resources ([4e81c99](4e81c99)) * ✨ add support for filtering in primary resource with included r… ([0b0bb87](0b0bb87)) * ✨ add support for filtering in primary resource with included relationships ([5c90d41](5c90d41)) * ✨ add too many reqyests exeption ([94a810d](94a810d)) * ✨ Allow collections and json columns to be mapped ([6a096bc](6a096bc)) * ✨ Code cleanup and standardization of error handling ([fec75f5](fec75f5)) * ✨ Enhance QueryHelpers with enum support and additional types ([4949c26](4949c26)) * ✨ general-purpose exception class ([0cdf9a5](0cdf9a5)) * ✨ general-purpose exception class ([e222bb4](e222bb4)) * ✨ Overall project cleanup ([c8c10f4](c8c10f4)) * ✨ Remove IncludeAsAttribute and related logic ([a1593be](a1593be)) * ✨ Support complex JsonCols ([b744b8e](b744b8e)) * 📚 add comprehensive debugging guide and enhance logging for better troubleshooting ([0ecc0cf](0ecc0cf)) * 🚀 add ApplyFiltersOnly method for pre-aggregation filtering and add documentation on statistics and aggregations ([b9c6546](b9c6546)) * 🚀 enhance query processing with AsSingleQuery for pagination and add detailed logging for inclusion processing ([7824da9](7824da9)) * **errors:** add JsonApiErrorCodes and JsonApiErrors factory methods ([#60](#60)) ([8531ad3](8531ad3)) * **errors:** complete refactor Phase 1 with exception filter tests a… ([#61](#61)) ([e5b50be](e5b50be)) ### Bug Fixes * 🐛 single included resources are no longer ignored ([1e2e4b6](1e2e4b6)) * 🚑️ `[JsonIgnore]` not being respected ([af12b0b](af12b0b)) * 🚑️ adds support for filtering on included collection fields ([ee2eb19](ee2eb19)) * 🚑️ adds support for filtering on included collection fields ([1194fd6](1194fd6)) * 🚑️ adjust query processing order for filtered and regular includes to enhance EF Core compatibility ([8d21509](8d21509)) * 🚑️ bracket nested filtering without the nessesary includes breaking main filtering ([e1e5785](e1e5785)) * 🚑️ correct version number in project file to match release version ([a0a51dd](a0a51dd)) * 🚑️ error responses for forbidden includes did not include meta information ([9610878](9610878)) * 🚑️ filtering on includes not working on 2-level ([c658327](c658327)) * 🚑️ Fixed the filtering issue for included resources. ([86cab81](86cab81)) * 🚑️ improve error messages for forbidden includes to clarify not found status ([07ea15e](07ea15e)) * 🚑️ Initial working fix. Needs further testing and validation. ([0fa5628](0fa5628)) * 🚑️ JsonApiOk and JsonApiCreated methods not adding includes ([903eda3](903eda3)) * 🚑️ refactor querying files and fix single resource relationship issues ([962d4d4](962d4d4)) * 🚑️ reorder query processing to apply sorting before includes for better EF Core compatibility ([20bf0d9](20bf0d9)) * 🚑️ three level nested values and collection include filters ([7f9a336](7f9a336)) * 🚑️ three level nested values and collection include filters ([044aaf0](044aaf0)) * 🚑️ use single query mode to prevent EF Core split query correlation issues with filtered includes ([ff48615](ff48615)) * add defensive reflection checks with ReflectionMethodCache ([#57](#57)) ([75eb978](75eb978)) * **mapping:** remove dead AddIncludedResourcesRecursive method ([#55](#55)) ([bbc8c17](bbc8c17)) * **pagination:** guard against division by zero when Size is 0 ([#59](#59)) ([0863dee](0863dee)) * **parsing:** guard unsafe string parsing in filter parsers ([#58](#58)) ([9fb463d](9fb463d)) * **security:** prevent log forging and add workflow permissions ([#51](#51)) ([5fbbaba](5fbbaba)) * **security:** prevent log forging and update tooling ([#52](#52)) ([52d73ce](52d73ce)) * support JsonPropertyName attribute and fix many-to-many collecti… ([634abff](634abff)) * support JsonPropertyName attribute and fix many-to-many collection filtering ([6f1d961](6f1d961)) ### Refactoring * 🔨 follow ts-package renaming ([4cd1e7e](4cd1e7e)) * 🔨 optimize logging and add XML documentation ([8c14bc0](8c14bc0)) * 🔨 remove Microsoft.Identity.Abstractions package reference ([55933b7](55933b7)) * 🔨 remove the OR max count ([65107d5](65107d5)) * 🔨 remove the OR max count ([5a3aa87](5a3aa87)) * 🔨 Update JsonApiOk function and docs to align with what it actually does ([bfe7635](bfe7635)) ### Documentation * 📝 update stats docs ([549743c](549743c)) * 📜 add too many request exeption to docs ([872ae2a](872ae2a)) * 📜 Clarify that filtering is only on main entity ([5ee3568](5ee3568)) * 📜 Update Claude.md ([88502bb](88502bb)) * 📜 update error message for forbidden includes to clarify not found status ([95ab6ce](95ab6ce)) ### Dependencies * **actions:** bump actions/checkout from 4 to 6 ([#47](#47)) ([a16ab53](a16ab53)) * **actions:** bump actions/setup-dotnet from 4 to 5 ([#45](#45)) ([db8c0d1](db8c0d1)) * **actions:** bump actions/upload-pages-artifact from 3 to 4 ([#44](#44)) ([c5e35fb](c5e35fb)) * **actions:** bump github/codeql-action from 3 to 4 ([#46](#46)) ([4bad70c](4bad70c)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: intility-release-bot[bot] <175299729+intility-release-bot[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.