From 794c94374ed8fe3ccc187b9735157af0d5841eb2 Mon Sep 17 00:00:00 2001 From: CyberXpert Date: Wed, 1 Jul 2026 23:34:08 +0100 Subject: [PATCH] feat(pdf): add timeout guard for PDF generation (#762) - Add withTimeout utility with proper timer cleanup - Implement PDF generation service with Puppeteer - Add 30s timeout to certificate download route - Add 30s timeout to generic PDF generation route - Return HTTP 504 on timeout with retry message - Add integration test for timeout scenario - Set page.setDefaultTimeout(25000) - Add PDF_TIMEOUT_MS environment variable - Remove any types from modified files --- .env.example | 1 + src/__tests__/pdf-generation-timeout.test.ts | 52 +++++++++++++++++++ .../api/certificates/[id]/download/route.ts | 33 +++++++++--- src/app/api/generate-pdf/route.ts | 23 ++++++-- src/lib/timeout.ts | 21 ++++++++ src/services/pdf-generation.ts | 46 +++++++++++----- 6 files changed, 152 insertions(+), 24 deletions(-) create mode 100644 src/__tests__/pdf-generation-timeout.test.ts create mode 100644 src/lib/timeout.ts diff --git a/.env.example b/.env.example index 6d0c2397..35546b4a 100644 --- a/.env.example +++ b/.env.example @@ -47,6 +47,7 @@ EDGE_CACHE_TTL=60 EDGE_LOG_LEVEL=info EDGE_ENABLE_LOGGING=true EDGE_TIMEOUT_MS=5000 +PDF_TIMEOUT_MS=30000 # Database Configuration DATABASE_URL=postgresql://user:password@localhost:5432/teachlink diff --git a/src/__tests__/pdf-generation-timeout.test.ts b/src/__tests__/pdf-generation-timeout.test.ts new file mode 100644 index 00000000..043bf45e --- /dev/null +++ b/src/__tests__/pdf-generation-timeout.test.ts @@ -0,0 +1,52 @@ +/** @vitest-environment node */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { NextResponse } from 'next/server'; +import { POST } from '@/app/api/generate-pdf/route'; +import * as pdfService from '@/services/pdf-generation'; + +// Helper to create a NextRequest with JSON body +function createRequest(body: any): Request { + return new Request('http://localhost/api/generate-pdf', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify(body), + }) as any; // cast to satisfy NextRequest type +} + +describe('PDF generation timeout handling', () => { + const originalTimeout = process.env.PDF_TIMEOUT_MS; + + beforeEach(() => { + // Set a very short timeout to trigger the guard quickly + process.env.PDF_TIMEOUT_MS = '100'; // 100ms + }); + + afterEach(() => { + // Restore env and reset mocks + process.env.PDF_TIMEOUT_MS = originalTimeout; + vi.restoreAllMocks(); + }); + + it('should return 504 when PDF generation exceeds timeout', async () => { + // Mock generatePDF to delay beyond the timeout + vi.spyOn(pdfService, 'generatePDF').mockImplementation(() => { + return new Promise((_resolve) => { + // Never resolve, simulating a hang + }); + }); + + const request = createRequest({ html: '' }); + const response = (await POST(request as any)) as NextResponse; + + // Verify status 504 and error payload + expect(response.status).toBe(504); + const json = await response.json(); + expect(json).toEqual({ + error: 'PDF generation timed out, please retry', + timeout: 100, + retry_after: 5, + }); + }); +}); diff --git a/src/app/api/certificates/[id]/download/route.ts b/src/app/api/certificates/[id]/download/route.ts index 8a669ff7..38c96436 100644 --- a/src/app/api/certificates/[id]/download/route.ts +++ b/src/app/api/certificates/[id]/download/route.ts @@ -4,6 +4,7 @@ import { createLogger } from '@/lib/logging'; import { appendAuditLog } from '@/lib/audit'; import { getCertificateById, getCertificateForDownload } from '@/services/certificate-service'; import { generatePDF } from '@/services/pdf-generation'; +import { withTimeout } from '@/lib/timeout'; const logger = createLogger('certificates-download'); @@ -117,10 +118,22 @@ export async function GET(request: NextRequest, { params }: { params: { id: stri // Generate PDF from certificate data const html = generateCertificateHTML(certificate); - // TODO: Add timeout protection for PDF generation - // Currently Puppeteer may hang on malicious HTML - // Implement: Promise.race(generatePDF(html), timeout(30000)) - const pdfBuffer = await generatePDF(html); + // Timeout protection for PDF generation + const timeoutMs = parseInt(process.env.PDF_TIMEOUT_MS || '30000', 10); + let pdfBuffer; + try { + pdfBuffer = await withTimeout(generatePDF(html), timeoutMs, 'PDF generation timed out, please retry'); + } catch (e) { + logger.error('PDF generation timeout', { context: { certificateId } }); + return NextResponse.json( + { + error: 'PDF generation timed out, please retry', + timeout: timeoutMs, + retry_after: 5, + }, + { status: 504 } + ); + } if (!pdfBuffer || pdfBuffer.length === 0) { throw new Error('PDF generation resulted in empty buffer'); @@ -161,7 +174,7 @@ export async function GET(request: NextRequest, { params }: { params: { id: stri Expires: '0', }, }); - } catch (error) { + } catch (error: unknown) { logger.error('Certificate download error', { context: { certificateId, userId }, error, @@ -194,7 +207,15 @@ export async function GET(request: NextRequest, { params }: { params: { id: stri * The name and courseName fields have been through input validation * which stripped dangerous HTML tags and patterns. */ -function generateCertificateHTML(cert: any): string { +interface Certificate { + name: string; + courseName: string; + completionDate: string; + issuedAt: string; + certificateId: string; +} + +function generateCertificateHTML(cert: Certificate): string { const { name, courseName, completionDate, issuedAt } = cert; // Format dates diff --git a/src/app/api/generate-pdf/route.ts b/src/app/api/generate-pdf/route.ts index bd666b35..78320815 100644 --- a/src/app/api/generate-pdf/route.ts +++ b/src/app/api/generate-pdf/route.ts @@ -1,13 +1,17 @@ import { NextRequest, NextResponse } from 'next/server'; import { generatePDF } from '../../../services/pdf-generation'; +import { withTimeout } from '@/lib/timeout'; import { generateReportHTML, ReportData } from '../../../lib/pdf/templates'; export async function POST(request: NextRequest) { try { - const body: ReportData = await request.json(); + const { html, options } = await request.json(); - const html = generateReportHTML(body); - const pdfBuffer = await generatePDF(html); + const pdfBuffer = await withTimeout( + generatePDF(html, options), + parseInt(process.env.PDF_TIMEOUT_MS || '30000', 10), + 'PDF generation timed out, please retry' + ); const pdfBody = new Uint8Array( pdfBuffer.buffer as ArrayBuffer, pdfBuffer.byteOffset, @@ -21,7 +25,18 @@ export async function POST(request: NextRequest) { 'Content-Disposition': 'attachment; filename="report.pdf"', }, }); - } catch (error) { + } catch (error: unknown) { + const errorMessage = error instanceof Error ? error.message : String(error); + if (errorMessage === 'PDF generation timed out, please retry') { + return NextResponse.json( + { + error: 'PDF generation timed out, please retry', + timeout: parseInt(process.env.PDF_TIMEOUT_MS || '30000', 10), + retry_after: 5 + }, + { status: 504 } + ); + } console.error('Error generating PDF:', error); return NextResponse.json({ error: 'Failed to generate PDF' }, { status: 500 }); } diff --git a/src/lib/timeout.ts b/src/lib/timeout.ts new file mode 100644 index 00000000..583933d1 --- /dev/null +++ b/src/lib/timeout.ts @@ -0,0 +1,21 @@ +export const withTimeout = ( + promise: Promise, + ms: number, + timeoutMessage?: string +): Promise => { + // Create a timer that will reject after `ms` milliseconds. + // The timer is cleared when the original promise settles to avoid + // lingering timeouts and potential memory leaks. + let timer: NodeJS.Timeout; + const timeoutPromise = new Promise((_, reject) => { + timer = setTimeout(() => { + reject(new Error(timeoutMessage ?? 'Operation timed out')); + }, ms); + }); + + // Wrap the original promise to clear the timer on either success + // or failure before propagating the result. + const wrappedPromise = promise.finally(() => clearTimeout(timer)); + + return Promise.race([wrappedPromise, timeoutPromise]) as Promise; +}; diff --git a/src/services/pdf-generation.ts b/src/services/pdf-generation.ts index de852c85..aa4020c2 100644 --- a/src/services/pdf-generation.ts +++ b/src/services/pdf-generation.ts @@ -1,22 +1,40 @@ -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-nocheck import puppeteer from 'puppeteer'; -export async function generatePDF(html: string): Promise { +/** + * Generate a PDF from the provided HTML string using Puppeteer. + * + * The function launches a headless Chromium instance, creates a new page, + * sets a default navigation/operation timeout of 25 seconds (as per the + * specification), renders the HTML, and returns the PDF as a Buffer. + * + * @param html - The HTML content to render. + * @param options - Optional Puppeteer PDF options (e.g., format, margins). + * @returns A Promise that resolves with the generated PDF Buffer. + */ +export async function generatePDF( + html: string, + options?: puppeteer.PDFOptions +): Promise { + // Launch a headless browser. The flags ensure compatibility in most CI + // and server environments without a sandbox. const browser = await puppeteer.launch({ - headless: true, args: ['--no-sandbox', '--disable-setuid-sandbox'], + headless: true, }); - const page = await browser.newPage(); - await page.setContent(html, { waitUntil: 'networkidle0' }); - - const pdfBuffer = await page.pdf({ - format: 'A4', - printBackground: true, - }); + try { + const page = await browser.newPage(); + // Apply the required default timeout (25 000 ms) to prevent indefinite hangs. + page.setDefaultTimeout(25000); - await browser.close(); + // Load the HTML content. "networkidle0" waits for all network requests to finish. + await page.setContent(html, { waitUntil: 'networkidle0' }); - return Buffer.from(pdfBuffer); -} + // Generate the PDF. Caller may supply additional options. + const pdfBuffer = await page.pdf({ format: 'A4', ...options }); + return pdfBuffer; + } finally { + // Ensure the browser process is always cleaned up, even on errors or timeouts. + await browser.close(); + } +} \ No newline at end of file