mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-06 07:12:12 +03:00
fix(batches): validate the list endpoint's ?limit query param (#9073)
Validated in local merge-train T5 (base49+contributors+pacocartones)
This commit is contained in:
1
changelog.d/fixes/9073-batches-list-limit-validation.md
Normal file
1
changelog.d/fixes/9073-batches-list-limit-validation.md
Normal file
@@ -0,0 +1 @@
|
||||
- **fix(batches):** `GET /v1/batches` now validates the `limit` query param instead of passing `Number.parseInt(limit)` straight to the SQLite `LIMIT` bind. Previously `?limit=abc` threw an unhandled `datatype mismatch` (→ HTTP 500), `?limit=-1`/`0` returned an incoherent `has_more:true` empty page with `last_id:null`, and a large `?limit` read the entire `batches` table into memory. It now returns a `400` for any non-integer or out-of-range value (1–100, default 20), matching the `POST` handler's Zod validation and the OpenAI Batches contract ([#9073](https://github.com/diegosouzapw/OmniRoute/pull/9073))
|
||||
30
src/app/api/v1/batches/parseListLimit.ts
Normal file
30
src/app/api/v1/batches/parseListLimit.ts
Normal file
@@ -0,0 +1,30 @@
|
||||
// Validates the `limit` query param for `GET /v1/batches`. The list endpoint previously
|
||||
// passed `Number.parseInt(limit)` straight to the SQLite `LIMIT ?` bind with no validation,
|
||||
// so `?limit=abc` threw an unhandled "datatype mismatch" (→ 500), `?limit=-1|0` produced an
|
||||
// incoherent `has_more:true` empty page, and a large `?limit` read the whole table.
|
||||
//
|
||||
// This mirrors the OpenAI Batches list contract (integer, 1–100, default 20) and the repo's
|
||||
// own query-param bounds (see `max_results` in src/shared/validation/schemas/apiV1.ts:
|
||||
// `z.coerce.number().int().min(1).max(100)`). Kept dependency-free so it is unit-testable
|
||||
// in isolation; swap in the Zod schema if the maintainer prefers.
|
||||
|
||||
export const DEFAULT_BATCH_LIST_LIMIT = 20;
|
||||
export const MAX_BATCH_LIST_LIMIT = 100;
|
||||
|
||||
export type ParsedListLimit =
|
||||
| { ok: true; limit: number }
|
||||
| { ok: false; message: string };
|
||||
|
||||
export function parseBatchListLimit(raw: string | null): ParsedListLimit {
|
||||
if (raw === null || raw === "") {
|
||||
return { ok: true, limit: DEFAULT_BATCH_LIST_LIMIT };
|
||||
}
|
||||
const n = Number(raw);
|
||||
if (!Number.isInteger(n) || n < 1 || n > MAX_BATCH_LIST_LIMIT) {
|
||||
return {
|
||||
ok: false,
|
||||
message: `'limit' must be an integer between 1 and ${MAX_BATCH_LIST_LIMIT}`,
|
||||
};
|
||||
}
|
||||
return { ok: true, limit: n };
|
||||
}
|
||||
@@ -4,6 +4,7 @@ import { v1BatchCreateSchema } from "@/shared/validation/schemas";
|
||||
import { NextResponse } from "next/server";
|
||||
import { getApiKeyRequestScope } from "@/app/api/v1/_helpers/apiKeyScope";
|
||||
import { formatBatchResponse } from "./formatBatchResponse";
|
||||
import { parseBatchListLimit } from "./parseListLimit";
|
||||
|
||||
export async function OPTIONS() {
|
||||
return handleCorsOptions();
|
||||
@@ -69,7 +70,14 @@ export async function GET(request: Request) {
|
||||
const apiKeyId = scope.apiKeyId;
|
||||
|
||||
const url = new URL(request.url);
|
||||
const limit = Number.parseInt(url.searchParams.get("limit") || "20");
|
||||
const parsedLimit = parseBatchListLimit(url.searchParams.get("limit"));
|
||||
if (!parsedLimit.ok) {
|
||||
return NextResponse.json(
|
||||
{ error: { message: parsedLimit.message, type: "invalid_request_error" } },
|
||||
{ status: 400, headers: CORS_HEADERS }
|
||||
);
|
||||
}
|
||||
const limit = parsedLimit.limit;
|
||||
const after = url.searchParams.get("after") || undefined;
|
||||
|
||||
const batches = listBatches(apiKeyId || undefined, limit + 1, after);
|
||||
|
||||
46
tests/unit/batch-list-limit-validation.test.ts
Normal file
46
tests/unit/batch-list-limit-validation.test.ts
Normal file
@@ -0,0 +1,46 @@
|
||||
import { describe, test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
parseBatchListLimit,
|
||||
DEFAULT_BATCH_LIST_LIMIT,
|
||||
MAX_BATCH_LIST_LIMIT,
|
||||
} from "../../src/app/api/v1/batches/parseListLimit.ts";
|
||||
|
||||
describe("parseBatchListLimit", () => {
|
||||
test("absent or empty limit falls back to the default", () => {
|
||||
assert.deepEqual(parseBatchListLimit(null), { ok: true, limit: DEFAULT_BATCH_LIST_LIMIT });
|
||||
assert.deepEqual(parseBatchListLimit(""), { ok: true, limit: DEFAULT_BATCH_LIST_LIMIT });
|
||||
});
|
||||
|
||||
test("valid in-range integers are accepted", () => {
|
||||
assert.deepEqual(parseBatchListLimit("1"), { ok: true, limit: 1 });
|
||||
assert.deepEqual(parseBatchListLimit("20"), { ok: true, limit: 20 });
|
||||
assert.deepEqual(parseBatchListLimit("50"), { ok: true, limit: 50 });
|
||||
assert.deepEqual(parseBatchListLimit(String(MAX_BATCH_LIST_LIMIT)), {
|
||||
ok: true,
|
||||
limit: MAX_BATCH_LIST_LIMIT,
|
||||
});
|
||||
});
|
||||
|
||||
test("non-numeric limit is rejected instead of reaching the DB (no more 500 on ?limit=abc)", () => {
|
||||
const r = parseBatchListLimit("abc");
|
||||
assert.equal(r.ok, false);
|
||||
assert.match((r as { message: string }).message, /between 1 and 100/);
|
||||
});
|
||||
|
||||
test("negative and zero limits are rejected (no more incoherent has_more)", () => {
|
||||
assert.equal(parseBatchListLimit("-1").ok, false);
|
||||
assert.equal(parseBatchListLimit("0").ok, false);
|
||||
});
|
||||
|
||||
test("above-max limit is rejected (bounds the read)", () => {
|
||||
assert.equal(parseBatchListLimit("101").ok, false);
|
||||
assert.equal(parseBatchListLimit("999999999").ok, false);
|
||||
// Number("1e9") === 1e9, which parseInt would have truncated to 1 — also out of range.
|
||||
assert.equal(parseBatchListLimit("1e9").ok, false);
|
||||
});
|
||||
|
||||
test("non-integer numeric limits are rejected", () => {
|
||||
assert.equal(parseBatchListLimit("20.5").ok, false);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user