-
Notifications
You must be signed in to change notification settings - Fork 139
Fix Memory Exhaustion and DoS for PDF uploads (#1549) #1550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
b1bb2d8
9ca5b8a
dc3ea2d
2759ba2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| const axios = require("axios"); | ||
| const crypto = require("crypto"); | ||
| const fs = require("fs"); | ||
| const { generateWithFallback } = require("../utils/geminiHelper"); | ||
| const { | ||
| inspectPdfBuffer, | ||
|
|
@@ -103,14 +104,16 @@ function parseAiJson(raw) { | |
| * @example | ||
| */ | ||
| const summarizeNotes = async (req, res) => { | ||
| let uploadedFilePath = null; | ||
| try { | ||
| let buffer; | ||
| let fileName; | ||
| let sourceType; | ||
| let sourceUrl = null; | ||
|
|
||
| if (req.file) { | ||
| buffer = req.file.buffer; | ||
| uploadedFilePath = require('path').join(require('os').tmpdir(), require('path').basename(req.file.path)); | ||
| buffer = await fs.promises.readFile(uploadedFilePath); | ||
|
github-advanced-security[bot] marked this conversation as resolved.
Fixed
coderabbitai[bot] marked this conversation as resolved.
|
||
| fileName = req.file.originalname; | ||
| sourceType = "upload"; | ||
| } else { | ||
|
|
@@ -139,6 +142,9 @@ const summarizeNotes = async (req, res) => { | |
|
|
||
| const readingTime = computeReadingTime(pdfStats); | ||
| const contentHash = crypto.createHash("sha256").update(buffer).digest("hex"); | ||
| const fileSize = buffer.length; | ||
| const base64Data = buffer.toString("base64"); | ||
| buffer = null; // Release Buffer memory | ||
|
|
||
| const prompt = `You are an expert academic tutor helping a student decide whether a set of study notes is useful before they read it. | ||
|
|
||
|
|
@@ -168,7 +174,7 @@ DO NOT wrap the response in markdown code blocks. Return ONLY the raw JSON objec | |
| prompt, | ||
| { | ||
| inlineData: { | ||
| data: buffer.toString("base64"), | ||
| data: base64Data, | ||
| mimeType: "application/pdf", | ||
| }, | ||
| }, | ||
|
|
@@ -202,7 +208,7 @@ DO NOT wrap the response in markdown code blocks. Return ONLY the raw JSON objec | |
| res.status(200).json({ | ||
| success: true, | ||
| fileName, | ||
| fileSize: buffer.length, | ||
| fileSize, | ||
| sourceType, | ||
| sourceUrl, | ||
| pageCount: pdfStats.numPages, | ||
|
|
@@ -238,6 +244,12 @@ DO NOT wrap the response in markdown code blocks. Return ONLY the raw JSON objec | |
| } | ||
|
|
||
| res.status(500).json({ success: false, message: "Failed to summarize notes." }); | ||
| } finally { | ||
| if (uploadedFilePath) { | ||
| await require('fs').promises.unlink(uploadedFilePath).catch(err => { | ||
| if (err.code !== 'ENOENT') console.error("Failed to delete temp notes PDF:", err); | ||
| }); | ||
| } | ||
|
Comment on lines
+247
to
+252
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π©Ί Stability & Availability | π Major | ποΈ Heavy lift Make temporary-file cleanup cover the complete post-upload lifecycle. The controller
π Affects 2 files
π€ Prompt for AI Agents |
||
| } | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| const axios = require('axios'); | ||
| const FormData = require('form-data'); | ||
| const fs = require('fs'); | ||
| const { generateWithFallback } = require('../utils/geminiHelper'); | ||
|
|
||
| /** | ||
|
|
@@ -100,13 +101,20 @@ const compileResume = async (req, res) => { | |
| * } | ||
| */ | ||
| const analyzeResume = async (req, res) => { | ||
| let uploadedFilePath = null; | ||
| try { | ||
| if (!req.file) { | ||
| return res.status(400).json({ message: "No resume file uploaded" }); | ||
| } | ||
|
|
||
| uploadedFilePath = require('path').join(require('os').tmpdir(), require('path').basename(req.file.path)); | ||
| let fileBuffer = await fs.promises.readFile(uploadedFilePath); | ||
|
|
||
| const targetRole = req.body.targetRole || "General Professional"; | ||
|
|
||
| const base64Data = fileBuffer.toString("base64"); | ||
| fileBuffer = null; // Release Buffer memory | ||
|
Comment on lines
+111
to
+116
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π Performance & Scalability | π Major | ποΈ Heavy lift π§© Analysis chainπ Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 6 'readFile|inlineData|uploadFile|fileSize|uploadResume|uploadNotes' backendRepository: Canopus-Labs/PrepPilot Length of output: 15047 π Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- upload limits and resume route ---'
sed -n '1,210p' backend/middlewares/uploadMiddleware.js
sed -n '1,80p' backend/routes/resumeRoutes.js
printf '%s\n' '--- resume controller and fallback helper references ---'
sed -n '90,165p' backend/controllers/resumeController.js
rg -n -C 8 'function generateWithFallback|const generateWithFallback|generateWithFallback|aiLimiter' backendRepository: Canopus-Labs/PrepPilot Length of output: 38065 π Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- fallback implementation ---'
sed -n '1,120p' backend/utils/geminiHelper.js
printf '%s\n' '--- controller cleanup and response path ---'
sed -n '150,245p' backend/controllers/resumeController.js
printf '%s\n' '--- exact memory multipliers at the configured resume limit ---'
python3 - <<'PY'
size = 5 * 1024 * 1024
base64_size = 4 * ((size + 2) // 3)
print({
"resume_limit_bytes": size,
"base64_bytes": base64_size,
"peak_buffer_plus_base64_bytes": size + base64_size,
"peak_buffer_plus_base64_mib": (size + base64_size) / (1024 * 1024),
"peak_for_15_concurrent_uploads_mib": 15 * (size + base64_size) / (1024 * 1024),
})
PYRepository: Canopus-Labs/PrepPilot Length of output: 7990 Bound concurrent resume analyses before
π€ Prompt for AI Agents |
||
|
|
||
| // 2. Prompt Engineering | ||
| const prompt = `You are an expert ATS (Applicant Tracking System) and Senior Technical Recruiter. | ||
| Analyze the attached PDF resume against the target role: "${targetRole}". | ||
|
|
@@ -133,7 +141,7 @@ DO NOT wrap the response in markdown blocks like \`\`\`json. Return ONLY the raw | |
| prompt, | ||
| { | ||
| inlineData: { | ||
| data: req.file.buffer.toString("base64"), | ||
| data: base64Data, | ||
| mimeType: "application/pdf" | ||
| } | ||
| } | ||
|
|
@@ -161,6 +169,12 @@ DO NOT wrap the response in markdown blocks like \`\`\`json. Return ONLY the raw | |
| } catch (error) { | ||
| console.error("Resume Analysis Error:", error); | ||
| res.status(500).json({ message: "Failed to analyze resume" }); | ||
| } finally { | ||
| if (uploadedFilePath) { | ||
| await require('fs').promises.unlink(uploadedFilePath).catch(err => { | ||
| if (err.code !== 'ENOENT') console.error("Failed to delete temp resume PDF:", err); | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.