Skip to content

Treat missing query params as absent in clampInt - #32

Merged
gbyo merged 1 commit into
mainfrom
gibby/silly-ardinghelli-c3257d
Aug 11, 2026
Merged

gbyo merged 1 commit into
mainfrom
gibby/silly-ardinghelli-c3257d

Conversation

@gbyo

@gbyo gbyo commented Aug 11, 2026

Copy link
Copy Markdown
Member

Problem

clampInt in src/admin.ts never reached its fallback for a missing query param:

const parsed = Number(value);
if (!Number.isFinite(parsed)) return fallback;
return Math.max(min, Math.min(max, Math.floor(parsed)));

Number(null) and Number("") are both 0 — finite — so the Number.isFinite guard passes and the value clamps up to min instead of falling back.

Impact

  • handleHistory / handleStudents — both call clampInt(url.searchParams.get("limit"), 1, 200, 100). GET /api/library/admin-history with no limit returned 1 row and reported "limit": 1, rather than the intended default of 100. (offset shares the bug but is harmless there, since its min is 0.)
  • handleStudentDetail — calls clampInt(url.searchParams.get("id"), 1, 1000000000, 0) and then if (!id) return json({ error: "Student ID is required." }, 400). A request with no id clamped to 1 and quietly returned student row 1, making that 400 guard unreachable.

The manage UI always sends an explicit limit, so the pagination side only affected direct API callers. The handleStudentDetail guard was reachable by anyone omitting id.

Fix

Guard for null/blank before Number():

if (value === null || value.trim() === "") return fallback;

src/admin.ts holds the only definition of clampInt — src/library.ts has no copy. The one adjacent pattern, retryPendingSheetEvents, clamps an already-numeric limit and both callers pass a literal 25, so no query-string path reaches it.

Verification

npx tsc --noEmit clean. Exercising the patched function body across the three call-site signatures:

input limit (1, 200, 100) offset (0, 100000, 0) id (1, 1e9, 0)
null 100 0 0
"" / " " 100 0 0
"abc" 100 0 0
"50" 50 50 50
"999" 200 999 999
"0" / "-5" 1 0 1

Explicitly supplied values clamp exactly as before; only the absent/blank case changed.

🤖 Generated with Claude Code

Number(null) and Number("") are both 0, which is finite, so a missing
or blank query param never reached the fallback — it clamped to `min`
instead.

Effects on the three call sites:

- handleHistory / handleStudents: GET /api/library/admin-history with
  no limit returned 1 row and reported "limit": 1 rather than the
  intended default of 100. (offset was harmless, since its min is 0.)
- handleStudentDetail: a request with no id clamped to 1 and quietly
  returned student row 1, making the `if (!id)` "Student ID is
  required." 400 guard unreachable.

Guard for null/blank before Number(), so absent params fall back and
explicitly supplied values clamp exactly as before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the area:source Changes to application source code label Aug 11, 2026
@cloudflare-workers-and-pages

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
flow 0df5908 Aug 11 2026, 11:58 AM

@gbyo
gbyo merged commit 50009bc into main Aug 11, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:source Changes to application source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant