danvics/openreader
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: address CodeRabbit review comments for PR #76

Browse source 
Security fixes:
- Validate returnTo parameter in auth route to prevent open redirect attacks

Performance fixes:
- Add concurrency limit (3) for EPUB spine extraction to prevent memory exhaustion
- Iteratively split oversized paragraphs in chunk splitting to prevent content loss
- Add iteration limit (10) and shrink-check to summary combining loop

Data integrity:
- Remap summary rows when document IDs change during sync

UX improvements:
- Clear summary base URL when switching to non-custom providers
- Add type="button" to SummarizeButton to prevent form submission
- Add dev logging to PDFViewer error handler
- Move document state detection from render to useEffect

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Sunny 2026-02-02 04:48:16 +00:00
parent c46947b238
commit 567b29abf2
8 changed files with 186 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
src/app/api/auth/route.ts
View file

@ -1,6 +1,23 @@
import { NextRequest, NextResponse } from 'next/server';
import { getAuthToken } from '@/lib/auth';
function isValidReturnTo(returnTo: string): boolean {
// Only allow relative paths starting with /
// Reject absolute URLs, protocol-relative URLs, and other schemes
if (!returnTo.startsWith('/')) return false;
if (returnTo.startsWith('//')) return false;
// Reject URLs with encoded characters that could bypass checks
if (returnTo.includes('%')) {
try {
const decoded = decodeURIComponent(returnTo);
if (decoded.startsWith('//') || decoded.includes('://')) return false;
} catch {
return false;
}
}
return true;
}
export async function GET(request: NextRequest) {
const token = request.nextUrl.searchParams.get('token');
const returnTo = request.nextUrl.searchParams.get('returnTo') || '/';
@ -9,7 +26,9 @@ export async function GET(request: NextRequest) {
return NextResponse.json({ error: 'Invalid token' }, { status: 401 });
}
const response = NextResponse.redirect(new URL(returnTo, request.url));
// Validate returnTo to prevent open redirect attacks
const safeReturnTo = isValidReturnTo(returnTo) ? returnTo : '/';
const response = NextResponse.redirect(new URL(safeReturnTo, request.url));
response.cookies.set('auth_session', token, {
httpOnly: true,
secure: process.env.NODE_ENV === 'production',

42
src/app/epub/[id]/page.tsx
View file

@ -20,6 +20,7 @@ import { SummarizeButton } from '@/components/SummarizeButton';
import { SummarizeModal } from '@/components/SummarizeModal';
import type { SummarizeMode } from '@/types/summary';
import { resolveDocumentId } from '@/lib/dexie';
import { processWithConcurrencyLimit } from '@/lib/concurrency';
const isDev = process.env.NEXT_PUBLIC_NODE_ENV !== 'production' || process.env.NODE_ENV == null;
@ -126,27 +127,38 @@ export default function EPUBPage() {
}
if (mode === 'whole_book') {
// Extract text from all spine sections
const promises: Promise<string>[] = [];
// Extract text from all spine sections with concurrency limit to prevent memory exhaustion
const spineItems: { href: string }[] = [];
const spine = book.spine;
spine.each((item: { href?: string }) => {
const url = item.href || '';
if (!url) return;
const promise = book.load(url)
.then((section) => (section as Document))
.then((section) => section.body?.textContent?.trim() || '')
.catch((err) => {
console.warn('Failed to extract text from section:', url, err);
return '';
});
promises.push(promise);
if (url) {
spineItems.push({ href: url });
}
});
const textParts = await Promise.all(promises);
return textParts.filter(text => text).join('\n\n');
// Use concurrency limit of 3 to prevent memory spikes on large books
const results = await processWithConcurrencyLimit(
spineItems,
async (item) => {
try {
const section = await book.load(item.href) as Document;
return section.body?.textContent?.trim() || '';
} catch (err) {
console.warn('Failed to extract text from section:', item.href, err);
return '';
}
},
3 // Max concurrent extractions
);
const textParts = results
.filter((r): r is { status: 'fulfilled'; value: string } => r.status === 'fulfilled')
.map(r => r.value)
.filter(text => text);
return textParts.join('\n\n');
} else {
// Extract current page text
return extractPageText(book, rendition, false);

21
src/components/PDFViewer.tsx
View file

@ -62,15 +62,15 @@ export function PDFViewer({ zoomLevel }: PDFViewerProps) {
const lastDataRef = useRef<ArrayBuffer | undefined>(undefined);
// Reset ready state when document data changes
if (currDocData !== lastDataRef.current) {
lastDataRef.current = currDocData;
if (currDocData) {
documentKeyRef.current += 1;
}
if (isDocumentReady) {
useEffect(() => {
if (currDocData !== lastDataRef.current) {
lastDataRef.current = currDocData;
if (currDocData) {
documentKeyRef.current += 1;
}
setIsDocumentReady(false);
}
}
}, [currDocData]);
// Create a Uint8Array copy to prevent "detached ArrayBuffer" errors
const pdfFileData = useMemo(() => {
@ -300,8 +300,11 @@ export function PDFViewer({ zoomLevel }: PDFViewerProps) {
onDocumentLoadSuccess(pdf);
setIsDocumentReady(true);
}}
onLoadError={() => {
// Ignore errors from destroyed documents during navigation/reload
onLoadError={(error) => {
// Log errors in development for debugging
if (process.env.NODE_ENV !== 'production') {
console.warn('PDFViewer load error (may be from destroyed document during navigation):', error);
}
}}
onItemClick={(args: PDFOnLinkClickArgs) => {
if (args?.pageNumber) {

4
src/components/SettingsModal.tsx
View file

@ -730,15 +730,19 @@ export function SettingsModal() {
switch (provider.id) {
case 'openai':
setLocalSummaryModel('gpt-4o-mini');
setLocalSummaryBaseUrl(''); // Clear custom base URL
break;
case 'anthropic':
setLocalSummaryModel('claude-3-5-haiku-latest');
setLocalSummaryBaseUrl(''); // Clear custom base URL
break;
case 'groq':
setLocalSummaryModel('llama-3.3-70b-versatile');
setLocalSummaryBaseUrl(''); // Clear custom base URL
break;
case 'openrouter':
setLocalSummaryModel('google/gemini-2.0-flash-001');
setLocalSummaryBaseUrl(''); // Clear custom base URL
break;
case 'custom-openai':
setLocalSummaryModel('');

1
src/components/SummarizeButton.tsx
View file

@ -10,6 +10,7 @@ interface SummarizeButtonProps {
export function SummarizeButton({ onClick, disabled }: SummarizeButtonProps) {
return (
<button
type="button"
onClick={onClick}
disabled={disabled}
className="inline-flex items-center py-1 px-2 rounded-md border border-offbase bg-base text-foreground text-xs hover:bg-offbase transition-all duration-200 ease-in-out hover:scale-[1.09] hover:text-accent disabled:opacity-50 disabled:cursor-not-allowed"

50
src/lib/concurrency.ts Normal file
View file

@ -0,0 +1,50 @@
/**
* Utility for processing items with a concurrency limit
*
* This enables parallel processing of items (like TTS generation) while
* respecting rate limits and preventing resource exhaustion.
*/
export type ConcurrencyResult<R> =
| { status: 'fulfilled'; value: R }
| { status: 'rejected'; reason: Error };
/**
* Process items in parallel with a maximum concurrency limit.
*
* @param items - Array of items to process
* @param processor - Async function to process each item
* @param maxConcurrent - Maximum number of concurrent processors
* @param signal - Optional AbortSignal for cancellation
* @returns Array of results in the same order as input items
*/
export async function processWithConcurrencyLimit<T, R>(
items: T[],
processor: (item: T, index: number) => Promise<R>,
maxConcurrent: number,
signal?: AbortSignal
): Promise<ConcurrencyResult<R>[]> {
const results: ConcurrencyResult<R>[] = new Array(items.length);
let currentIndex = 0;
async function worker(): Promise<void> {
while (currentIndex < items.length) {
if (signal?.aborted) break;
const index = currentIndex++;
if (index >= items.length) break;
try {
const result = await processor(items[index], index);
results[index] = { status: 'fulfilled', value: result };
} catch (error) {
results[index] = { status: 'rejected', reason: error as Error };
}
}
}
const workerCount = Math.min(maxConcurrent, items.length);
await Promise.all(Array(workerCount).fill(null).map(() => worker()));
return results;
}

19
src/lib/dexie.ts
View file

@ -329,6 +329,7 @@ async function applyDocumentIdMapping(oldId: string, newId: string): Promise<voi
db[LAST_LOCATION_TABLE],
db[APP_CONFIG_TABLE],
db[DOCUMENT_ID_MAP_TABLE],
db[SUMMARIES_TABLE],
],
async () => {
await recordDocumentIdMapping(oldId, nextId);
@ -406,6 +407,24 @@ async function applyDocumentIdMapping(oldId: string, newId: string): Promise<voi
await db[APP_CONFIG_TABLE].update('singleton', { documentListState: mapped });
}
}
// Remap summaries to use the new document ID
const oldSummaries = await db[SUMMARIES_TABLE].where('docId').equals(oldId).toArray();
for (const summary of oldSummaries) {
const newSummaryId = `${nextId}-${summary.scope}-${summary.pageNumber ?? 'all'}`;
// Check if a summary with the new ID already exists
const existing = await db[SUMMARIES_TABLE].get(newSummaryId);
if (!existing) {
// Create new summary with updated docId and id
await db[SUMMARIES_TABLE].put({
...summary,
id: newSummaryId,
docId: nextId,
});
}
// Delete the old summary
await db[SUMMARIES_TABLE].delete(summary.id);
}
},
);
});

57
src/lib/summarize.ts
View file

@ -24,15 +24,48 @@ function splitTextIntoChunks(text: string, maxTokens: number): string[] {
const chunks: string[] = [];
let current = '';
// Helper to split oversized text into multiple chunks
function splitOversizedText(oversized: string): string[] {
const parts: string[] = [];
let remaining = oversized;
while (remaining.length > 0) {
// Try to split at sentence boundary within maxChars
let splitPoint = maxChars;
if (remaining.length > maxChars) {
const searchRange = remaining.slice(0, maxChars);
const lastSentence = Math.max(
searchRange.lastIndexOf('. '),
searchRange.lastIndexOf('! '),
searchRange.lastIndexOf('? ')
);
if (lastSentence > maxChars * 0.5) {
splitPoint = lastSentence + 2; // Include the punctuation and space
}
}
parts.push(remaining.slice(0, splitPoint).trim());
remaining = remaining.slice(splitPoint).trim();
}
return parts;
}
for (const para of text.split(/\n\n+/)) {
const trimmed = para.trim();
if (!trimmed) continue;
if ((current + '\n\n' + trimmed).length > maxChars) {
if (current) chunks.push(current.trim());
current = trimmed.length > maxChars
? trimmed.slice(0, maxChars) // Force split if paragraph too long
: trimmed;
// Handle oversized paragraphs by splitting them iteratively
if (trimmed.length > maxChars) {
const splitParts = splitOversizedText(trimmed);
// Add all but the last part as separate chunks
for (let i = 0; i < splitParts.length - 1; i++) {
chunks.push(splitParts[i]);
}
// Use the last part as the new current
current = splitParts[splitParts.length - 1] || '';
} else {
current = trimmed;
}
} else {
current = current ? current + '\n\n' + trimmed : trimmed;
}
@ -101,9 +134,19 @@ export async function generateSummary(
// Combine summaries (recursively if still too large)
onProgress?.({ currentChunk: chunks.length, totalChunks: chunks.length, phase: 'combining', message: 'Combining summaries...' });
let combined = summaries.join('\n\n---\n\n');
const MAX_COMBINING_ITERATIONS = 10;
let combiningIterations = 0;
while (needsChunking(combined, contextLimit)) {
combiningIterations++;
if (combiningIterations > MAX_COMBINING_ITERATIONS) {
console.warn('Summary combining reached maximum iterations, proceeding with current result');
break;
}
const previousLength = combined.length;
const subChunks = splitTextIntoChunks(combined, maxInputTokens);
const subSummaries: string[] = [];
for (const chunk of subChunks) {
@ -112,6 +155,12 @@ export async function generateSummary(
totalTokens += result.tokensUsed || 0;
}
combined = subSummaries.join('\n\n---\n\n');
// Guard against non-convergence: if combined didn't shrink, break to avoid infinite loop
if (combined.length >= previousLength) {
console.warn('Summary combining not converging, proceeding with current result');
break;
}
}
// Final pass