From aa7700e8447e041f0feb0af6da8b6f76c8993088 Mon Sep 17 00:00:00 2001 From: Richard R Date: Sat, 30 May 2026 16:41:17 -0600 Subject: [PATCH] refactor(core): improve streaming, validation, and rate limit logic - Stream request body in blob upload fallback route to enforce size limits without buffering entire payload - Enhance admin provider panel error handling for multi-status responses - Adjust positive integer validation to require value >= 1 - Use full rate limit config for job event recording and prune by largest window - Fix PDF layout job event userId usage in docx-to-pdf upload route - Add missing windows array fallback in documents register route - Minor CI workflow and env example corrections - Update audiobooks blobstore test to use beforeAll directly These changes improve efficiency, correctness, and maintainability across API, admin, rate limiting, and test modules. --- .env.example | 2 +- .github/workflows/vitest.yml | 2 ++ .../documents/blob/upload/fallback/route.ts | 34 +++++++++++++++++-- .../api/documents/docx-to-pdf/upload/route.ts | 2 +- src/app/api/documents/route.ts | 2 +- src/components/admin/AdminProvidersPanel.tsx | 16 ++++++++- src/lib/server/admin/settings.ts | 4 +-- src/lib/server/rate-limit/job-rate-limiter.ts | 13 ++++--- .../unit/audiobooks-blobstore.vitest.spec.ts | 4 +-- 9 files changed, 64 insertions(+), 15 deletions(-) diff --git a/.env.example b/.env.example index 6331ec1..69c8ed0 100644 --- a/.env.example +++ b/.env.example @@ -15,7 +15,7 @@ API_BASE=http://localhost:8880/v1 API_KEY=api_key_optional -# (Optional) TTS request/cache tuning (deaults shown below; leave unset to use defaults) +# (Optional) TTS request/cache tuning (defaults shown below; leave unset to use defaults) # TTS_CACHE_MAX_SIZE_BYTES=268435456 # 256MB # TTS_CACHE_TTL_MS=1800000 # 30 minutes # TTS_MAX_RETRIES=2 diff --git a/.github/workflows/vitest.yml b/.github/workflows/vitest.yml index f69ef42..27c2102 100644 --- a/.github/workflows/vitest.yml +++ b/.github/workflows/vitest.yml @@ -12,6 +12,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v5 + with: + persist-credentials: false - uses: actions/setup-node@v5 with: node-version: lts/* diff --git a/src/app/api/documents/blob/upload/fallback/route.ts b/src/app/api/documents/blob/upload/fallback/route.ts index 74f67df..7fc8208 100644 --- a/src/app/api/documents/blob/upload/fallback/route.ts +++ b/src/app/api/documents/blob/upload/fallback/route.ts @@ -46,14 +46,42 @@ export async function PUT(req: NextRequest) { ); } - const body = Buffer.from(await req.arrayBuffer()); - // Backstop for chunked/omitted Content-Length: enforce on the actual bytes. - if (body.byteLength > maxUploadBytes) { + // Backstop for chunked/omitted Content-Length: stream the body so we can + // bail out as soon as the running total crosses the cap instead of + // buffering the entire payload first. + const stream = req.body; + if (!stream) { + return NextResponse.json({ error: 'Missing request body' }, { status: 400 }); + } + const reader = stream.getReader(); + const chunks: Uint8Array[] = []; + let totalBytes = 0; + let overLimit = false; + try { + while (true) { + const { value, done } = await reader.read(); + if (done) break; + if (!value) continue; + totalBytes += value.byteLength; + if (totalBytes > maxUploadBytes) { + overLimit = true; + break; + } + chunks.push(value); + } + } finally { + if (overLimit) { + await reader.cancel().catch(() => {}); + } + reader.releaseLock(); + } + if (overLimit) { return NextResponse.json( { error: `Upload exceeds the maximum allowed size of ${maxUploadBytes} bytes`, maxBytes: maxUploadBytes }, { status: 413 }, ); } + const body = Buffer.concat(chunks, totalBytes); const namespace = getOpenReaderTestNamespace(req.headers); diff --git a/src/app/api/documents/docx-to-pdf/upload/route.ts b/src/app/api/documents/docx-to-pdf/upload/route.ts index 4d2b992..c849adf 100644 --- a/src/app/api/documents/docx-to-pdf/upload/route.ts +++ b/src/app/api/documents/docx-to-pdf/upload/route.ts @@ -177,7 +177,7 @@ export async function POST(req: NextRequest) { // Record upload-driven parse load (see register route for rationale). const pdfRateConfig = getPdfLayoutRateConfig(await getResolvedRuntimeConfig()); - await recordJobEvent(ctxOrRes.userId, 'pdf_layout', `docx:${randomUUID()}`, pdfRateConfig); + await recordJobEvent(storageUserId, 'pdf_layout', `docx:${randomUUID()}`, pdfRateConfig); enqueueParsePdfJob({ documentId: id, userId: storageUserId, diff --git a/src/app/api/documents/route.ts b/src/app/api/documents/route.ts index af9c38a..880b5c2 100644 --- a/src/app/api/documents/route.ts +++ b/src/app/api/documents/route.ts @@ -196,7 +196,7 @@ export async function POST(req: NextRequest) { // re-parse limiter reads. We record (not reject) here so a legitimate // bulk upload always parses; the recorded load still throttles // subsequent loopable re-parse spam via /parsed. - await recordJobEvent(ctxOrRes.userId, 'pdf_layout', `register:${randomUUID()}`, pdfRateConfig ?? { enabled: false }); + await recordJobEvent(ctxOrRes.userId, 'pdf_layout', `register:${randomUUID()}`, pdfRateConfig ?? { enabled: false, windows: [] }); enqueueParsePdfJob({ documentId: doc.id, userId: storageUserId, diff --git a/src/components/admin/AdminProvidersPanel.tsx b/src/components/admin/AdminProvidersPanel.tsx index b53b15f..0c64e97 100644 --- a/src/components/admin/AdminProvidersPanel.tsx +++ b/src/components/admin/AdminProvidersPanel.tsx @@ -72,7 +72,21 @@ async function patchDefaultProviderSlug(slug: string): Promise { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ updates: { defaultTtsProvider: slug } }), }); - if (!res.ok && res.status !== 207) throw new Error(`HTTP ${res.status}`); + if (res.status === 200 || res.status === 204) return; + if (res.status === 207) { + // Multi-status: the request succeeded for some keys but failed for others. + // Only treat it as success when the defaultTtsProvider field itself was + // accepted (no matching entry in the errors array). + const payload = (await res.json().catch(() => ({}))) as { + errors?: Array<{ key?: string; message?: string }>; + }; + const failure = payload.errors?.find((entry) => entry?.key === 'defaultTtsProvider'); + if (failure) { + throw new Error(failure.message || 'Failed to update default provider'); + } + return; + } + throw new Error(`HTTP ${res.status}`); } async function patchProviderEnabled(input: { id: string; enabled: boolean }): Promise { diff --git a/src/lib/server/admin/settings.ts b/src/lib/server/admin/settings.ts index e3b3f4b..1abbd34 100644 --- a/src/lib/server/admin/settings.ts +++ b/src/lib/server/admin/settings.ts @@ -84,11 +84,11 @@ function positiveIntValue(defaultValue: number, envVar?: string): RuntimeConfigK const trimmed = raw.trim(); if (!trimmed) return undefined; const parsed = Number(trimmed); - if (!Number.isFinite(parsed) || parsed <= 0) return undefined; + if (!Number.isFinite(parsed) || parsed < 1) return undefined; return Math.floor(parsed); }, validate(value) { - if (typeof value !== 'number' || !Number.isFinite(value) || value <= 0) return undefined; + if (typeof value !== 'number' || !Number.isFinite(value) || value < 1) return undefined; return Math.floor(value); }, }; diff --git a/src/lib/server/rate-limit/job-rate-limiter.ts b/src/lib/server/rate-limit/job-rate-limiter.ts index d65a183..f5a41e5 100644 --- a/src/lib/server/rate-limit/job-rate-limiter.ts +++ b/src/lib/server/rate-limit/job-rate-limiter.ts @@ -131,7 +131,7 @@ export async function recordJobEvent( userId: string | null | undefined, action: JobRateAction, opId: string, - config: Pick, + config: JobRateConfig, ): Promise { if (!isActive(config, userId) || !opId) return; @@ -145,15 +145,20 @@ export async function recordJobEvent( // Recording is best-effort; never block op creation on ledger writes. } - // Opportunistic prune of rows older than a generous retention window so the - // ledger stays small without a separate cron. + // Opportunistic prune of rows older than the largest configured window so + // the ledger stays small without a separate cron, but never deletes events + // that could still affect an in-window count. if (Math.random() < 0.05) { + const largestWindowMs = config.windows.reduce( + (max, w) => (Number.isFinite(w.windowMs) && w.windowMs > max ? w.windowMs : max), + 24 * 60 * 60 * 1000, + ); try { await safeDb() .delete(userJobEvents) .where(and( eq(userJobEvents.action, action), - lt(userJobEvents.createdAt, now - 24 * 60 * 60 * 1000), + lt(userJobEvents.createdAt, now - largestWindowMs), )); } catch { // ignore prune failures diff --git a/tests/unit/audiobooks-blobstore.vitest.spec.ts b/tests/unit/audiobooks-blobstore.vitest.spec.ts index 5f2c917..cb1ee96 100644 --- a/tests/unit/audiobooks-blobstore.vitest.spec.ts +++ b/tests/unit/audiobooks-blobstore.vitest.spec.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from 'vitest'; +import { beforeAll, describe, expect, test } from 'vitest'; import { audiobookKey, audiobookPrefix, @@ -15,7 +15,7 @@ function configureS3Env() { } describe('audiobooks-blobstore', () => { - test.beforeAll(() => { + beforeAll(() => { configureS3Env(); });