Skip to content

fix(notifications): clamp invalid pagination ranges - #306

Merged
ralyodio merged 2 commits into
profullstack:masterfrom
Jorel97:codex/fix-notifications-offset-297
May 29, 2026
Merged

fix(notifications): clamp invalid pagination ranges#306
ralyodio merged 2 commits into
profullstack:masterfrom
Jorel97:codex/fix-notifications-offset-297

Conversation

@Jorel97

@Jorel97 Jorel97 commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • clamps notification offset to a minimum of 0 before building the Supabase range
  • also bounds limit to 1..100 so invalid values cannot create inverted ranges

Fixes #297.

Verification

  • Inspected src/app/api/notifications/route.ts and confirmed range inputs are bounded before .range(...).
  • Full local test suite not run in this environment because the repo was updated through the GitHub API without a full dependency checkout.

@greptile-apps

greptile-apps Bot commented May 29, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a bug where negative or zero offset/limit query parameters could produce an inverted .range() call in the Supabase notifications query. It replaces the loose Number() parsing with parseInt(..., 10) and adds explicit bounds: limit is clamped to [1, 100] and offset is clamped to ≥ 0.

  • parseInt truncates float inputs to integers, eliminating the fractional-argument issue that existed in the previous Number()-based parsing.
  • Number.isFinite guards against NaN (e.g. non-numeric strings like \"abc\" or \"Infinity\"), falling back to safe defaults of 50 and 0 respectively.

Confidence Score: 5/5

The change is narrowly scoped to input sanitisation of two query params and cannot produce worse range arguments than the previous code.

Both bounds are correctly applied before the .range() call. Integer truncation via parseInt and the Number.isFinite guard together close the originally reported invalid-range bug and the float-argument edge case raised in the prior review thread. No auth, data-write, or other side-effect paths are touched.

No files require special attention.

Important Files Changed

Filename Overview
src/app/api/notifications/route.ts Replaces Number() with parseInt() for limit/offset parsing and adds explicit clamping: limit bounded to [1,100], offset bounded to ≥ 0, preventing inverted Supabase range arguments.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GET /api/notifications] --> B[parseInt limit param]
    B --> C{isFinite?}
    C -- Yes --> D[clamp to 1..100]
    C -- No/NaN --> E[default 50]
    D --> F[final limit]
    E --> F

    A --> G[parseInt offset param]
    G --> H{isFinite?}
    H -- Yes --> I[clamp to min 0]
    H -- No/NaN --> J[default 0]
    I --> K[final offset]
    J --> K

    F & K --> L[range offset to offset plus limit minus 1]
    L --> M[Supabase query]
    M --> N[Return paginated notifications]
Loading

Reviews (2): Last reviewed commit: "fix(notifications): parse pagination par..." | Re-trigger Greptile

Comment thread src/app/api/notifications/route.ts Outdated
@ralyodio
ralyodio merged commit 5ca56c5 into profullstack:master May 29, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Notifications endpoint accepts negative offset

2 participants