🐛 bug: prevent negative paginate start overflow

[gaby]

Motivation

• Prevent PageInfo.Start() from returning negative offsets when cursor mode leaves Page unset or when extremely large page values overflow the multiplication.
• The middleware previously capped only limit while page and offset could be attacker-controlled and produce negative or wrapped results used as slice/SQL offsets.
• Cursor support being enabled by default meant a crafted cursor could trigger the bug in handlers that follow the documented pageInfo.Start() usage.

Description

• Harden PageInfo.Start() to return 0 if Page < 1 or Limit < 1 and to clamp extremely large page-based starts to MaxInt to avoid integer overflow.
• Preserve existing behavior for normal page/offset values while preventing negative/wrapped results from untrusted inputs.
• Add test cases to Test_PageInfoStart covering Page = 0 (cursor mode) and an overflow-scale Page value.
• Include updated generated interface files (ctx_interface_gen.go and req_interface_gen.go) produced by repository tooling.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: cca8cdea-19e9-41e6-a215-d9b58f9a5273

📥 Commits

Reviewing files that changed from the base of the PR and between 9041e42 and 3d60b0a.

📒 Files selected for processing (2)

• middleware/paginate/page_info.go
• middleware/paginate/paginate_test.go

✅ Files skipped from review due to trivial changes (1)

• middleware/paginate/paginate_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• middleware/paginate/page_info.go

---

Walkthrough

Rewords IsFromLocal() docs to specify loopback IPs; PageInfo.Start() treats Limit < 1 as invalid and adds an int overflow guard returning maxInt; tests updated with additional Start() edge-case assertions.

Changes

Safety and Documentation Improvements

Layer / File(s)	Summary
API Documentation Clarity ctx_interface_gen.go, req_interface_gen.go	IsFromLocal() documentation reworded to specify "loopback IP" instead of generic "local" phrasing in both Ctx and Req interface definitions.
Pagination Safety and Validation middleware/paginate/page_info.go	PageInfo.Start() now validates that Limit ≥ 1 (previously only checked Page ≥ 1) and adds integer overflow guard to return maxInt when (Page-1)*Limit would overflow.
Pagination Tests middleware/paginate/paginate_test.go	Test_PageInfoStart table expanded with edge cases: Page=1, Limit=1, Page=0, and an extreme Page value that triggers clamping to maxInt.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• gofiber/fiber#4127: The main PR's changes to middleware/paginate/page_info.go (modifying PageInfo.Start to handle Limit<=0 and add overflow saturation) directly touch the same PageInfo.Start implementation introduced in the retrieved pagination middleware PR, so they are related.
• gofiber/fiber#4270: Both PRs touch the Ctx/Req "IsFromLocal" API (the main PR narrows its doc comment to loopback-only while the retrieved PR formally enforces that behavior and adds an IsFromUnixSocket helper), so they are related.

Suggested reviewers

• sixcolors
• ReneWerner87
• efectn

Poem

I hop through code with careful paws, 🐰
I mark the loopback without a pause,
Counts clipped safe where overflows creep,
Tests snugly guard the bounds I keep,
A rabbit’s patch — the repo sleeps.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description provides clear motivation, describes the changes made, and references testing, but does not follow the required template structure with checklist items and required sections.	Restructure the description to follow the template: add 'Fixes #' reference, organize changes under the template sections, complete relevant checklist items, and confirm all requirements are met.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title focuses on preventing negative paginate start overflow, which is the core bug fix, but the emoji and 'bug:' prefix add noise; the core message is clear and directly related to the main change.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-paginate-middleware-negative-offset-vulnerability

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.1)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@middleware/paginate/page_info.go`:
- Around line 72-73: The overflow guard in the pagination calculation using
p.Page and p.Limit is incorrect: replace the current check that compares p.Page
to (maxInt/p.Limit)+1 with a non-overflowing comparison using (p.Page - 1) >
(maxInt / p.Limit) so the logic in the function/method that computes page end
(referencing p.Page, p.Limit and maxInt in page_info.go) correctly detects
overflow without causing (maxInt/p.Limit)+1 to wrap when Limit == 1; update the
conditional and keep returning maxInt when the new condition is true.

In `@middleware/paginate/paginate_test.go`:
- Around line 127-128: Add a new unit test case to paginate_test.go to cover the
Limit==1 boundary: in the table of test cases that uses PageInfo (e.g., the
slice containing {"Zero page", PageInfo{...}, ...} and the overflow case
PageInfo{Page: int(^uint(0) >> 1), Limit: 100}), add a case with PageInfo{Page:
2, Limit: 1} and an expected result of 1; this ensures the pagination
overflow-guard math in the code that consumes PageInfo is exercised for the
fragile Limit==1 boundary.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 23c2e965-16fe-45c3-bc6c-cbf5f31422ce

📥 Commits

Reviewing files that changed from the base of the PR and between c48bbc2 and 9041e42.

📒 Files selected for processing (4)

• ctx_interface_gen.go
• middleware/paginate/page_info.go
• middleware/paginate/paginate_test.go
• req_interface_gen.go

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.26%. Comparing base (c48bbc2) to head (3d60b0a).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4272      +/-   ##
==========================================
+ Coverage   91.21%   91.26%   +0.04%     
==========================================
  Files         130      130              
  Lines       12760    12763       +3     
==========================================
+ Hits        11639    11648       +9     
+ Misses        709      704       -5     
+ Partials      412      411       -1     

Flag	Coverage Δ
unittests	91.26% <100.00%> (+0.04%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR hardens the paginate middleware’s PageInfo.Start() calculation to prevent negative/wrapped offsets caused by invalid inputs or integer overflow, and updates tests and regenerated interface files accordingly.

Changes:

• Add input validation and overflow clamping logic to PageInfo.Start() for safer pagination start computation.
• Extend Test_PageInfoStart with cases for cursor-mode Page=0 and very large Page values.
• Update generated interface comments for IsFromLocal to clarify it checks for loopback IPs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
middleware/paginate/page_info.go	Adds Start() validation + overflow clamp logic for safer offsets.
middleware/paginate/paginate_test.go	Adds regression tests for cursor-mode and overflow-scale inputs.
ctx_interface_gen.go	Regenerated interface comment clarifying loopback semantics of IsFromLocal.
req_interface_gen.go	Regenerated interface comment clarifying loopback semantics of IsFromLocal.

[gaby]

@copilot apply changes based on the comments in this thread

[gemini-code-assist]

Code Review

This pull request updates the documentation for the IsFromLocal method in the context and request interfaces and introduces overflow protection in the pagination middleware's Start method. A review comment correctly identified a logic error in the new overflow check where a limit of 1 would cause an integer overflow during the comparison, and a safer code suggestion was provided to fix this issue.

[Copilot]

@copilot apply changes based on the comments in this thread

Addressed in 3d60b0a. The overflow guard now compares page-1 against the safe threshold, and I added Limit=1 regression coverage for Page=1 and Page=2.