-
Notifications
You must be signed in to change notification settings - Fork 0
File size limit #76
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
base: main
Are you sure you want to change the base?
File size limit #76
Changes from all commits
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -54,23 +54,14 @@ async def upload_job_description( | |||||
| if not file.filename: | ||||||
| raise HTTPException(status_code=400, detail="File must have a filename") | ||||||
|
|
||||||
| is_valid, _ = await validate_document_file(file) | ||||||
|
|
||||||
| if not is_valid: | ||||||
| raise HTTPException( | ||||||
| status_code=400, | ||||||
| detail=f"Invalid file type. Accepted formats: PDF, DOC, DOCX" | ||||||
| ) | ||||||
| is_valid, tmp_file_path, _, _ = await validate_document_file(file) | ||||||
|
|
||||||
| # Validate company name is picked by user | ||||||
| if not company_name: | ||||||
| raise HTTPException(status_code=400, detail="Company name is required") | ||||||
|
|
||||||
| # try block to process the document with Azure Content Understanding | ||||||
| tmp_file_path = None | ||||||
| try: | ||||||
| tmp_file_path = await save_upload_file_tmp(file) | ||||||
|
|
||||||
| subscription_key = os.getenv("AZURE_CONTENT_UNDERSTANDING_SUBSCRIPTION_KEY") | ||||||
| if not subscription_key: | ||||||
| raise HTTPException(status_code=500, detail="Azure API key not configured") | ||||||
|
|
@@ -457,16 +448,11 @@ async def upload_culture_document( | |||||
|
|
||||||
| if not file.filename: | ||||||
| raise HTTPException(status_code=400, detail="File must have a filename") | ||||||
| is_valid, _ = await validate_document_file(file) | ||||||
| if not is_valid: | ||||||
| raise HTTPException(status_code=400, detail="Invalid file type. Accepted formats: PDF, DOC, DOCX") | ||||||
| is_valid, tmp_file_path, _, _ = await validate_document_file(file) | ||||||
|
||||||
| is_valid, tmp_file_path, _, _ = await validate_document_file(file) | |
| _, tmp_file_path, _, _ = await validate_document_file(file) |
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,13 +2,17 @@ | |||||||||||||||
| import os | ||||||||||||||||
| import tempfile | ||||||||||||||||
| import logging | ||||||||||||||||
| from typing import Tuple | ||||||||||||||||
| from fastapi import HTTPException, UploadFile | ||||||||||||||||
| from bson import ObjectId | ||||||||||||||||
| from bson.errors import InvalidId | ||||||||||||||||
|
|
||||||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||||||
|
|
||||||||||||||||
| ALLOWED_MIME_TYPES= { | ||||||||||||||||
| 'application/pdf', | ||||||||||||||||
| 'application/msword', | ||||||||||||||||
| 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| def validate_object_id(id_str: str) -> None: | ||||||||||||||||
| """Validate that a string is a valid MongoDB ObjectId format.""" | ||||||||||||||||
|
|
@@ -55,36 +59,86 @@ def cleanup_temp_file(file_path: str) -> None: | |||||||||||||||
| except Exception as e: | ||||||||||||||||
| logger.warning(f"Failed to cleanup temp file {file_path}: {e}") | ||||||||||||||||
|
|
||||||||||||||||
| # function for validating document file types | ||||||||||||||||
| async def validate_document_file(file: UploadFile) -> Tuple[bool, str]: | ||||||||||||||||
| # function for validating document file size and type | ||||||||||||||||
| async def validate_document_file(file: UploadFile) -> tuple[bool, str, int, str]: | ||||||||||||||||
| """ | ||||||||||||||||
| Orchestrates the full document ingestion pipeline: validates the uploaded file, | ||||||||||||||||
| saves it to a temporary location, and converts it to PDF if necessary. | ||||||||||||||||
|
Comment on lines
+65
to
+66
|
||||||||||||||||
| Orchestrates the full document ingestion pipeline: validates the uploaded file, | |
| saves it to a temporary location, and converts it to PDF if necessary. | |
| Validates the uploaded file, saves it to a temporary location, | |
| and returns the validation result and file metadata. |
Copilot
AI
Apr 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validate_document_file reads and saves the upload to a temp file via save_upload_file_tmp(file), which consumes the UploadFile stream. Callers that subsequently do await file.read() will get empty bytes unless the stream is rewound. Either rewind the upload (await file.seek(0)) after saving, or change this helper to avoid consuming the stream / return the bytes, so downstream endpoints can still read the file content reliably.
Copilot
AI
Apr 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validate_file_size is annotated to return int, but the docstring says it returns a tuple "(is_valid, file_size_bytes)". Please correct the Returns section so it matches the actual return type/value.
| Tuple of (is_valid, file_size_bytes) | |
| The file size in bytes. |
Copilot
AI
Apr 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validate_file_type calls python-docx's Document(...) directly inside an async request handler. This is synchronous and can block the event loop for large/complex files. Consider running DOCX parsing in a threadpool (or using a lightweight ZIP/content-type check) so uploads don't stall other requests.
Copilot
AI
Apr 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The HTTP 400 error detail for DOCX validation includes the raw exception ({e}), which may leak internal parsing details to clients. Prefer logging the exception server-side and returning a generic client-facing message (e.g., "Invalid DOCX file") with a stable error code/context.
| raise HTTPException( | |
| status_code=400, | |
| detail=f"Error occurred while validating DOCX file {file.filename}: {e}" | |
| logger.warning(f"Invalid DOCX file during validation: {file.filename}: {e}") | |
| raise HTTPException( | |
| status_code=400, | |
| detail="Invalid DOCX file." |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -145,16 +145,7 @@ async def validate_and_save_temp_file(file: UploadFile) -> str: | |||||
| Returns: temporary file path | ||||||
| """ | ||||||
| # Validate file type | ||||||
| is_valid, _ = await validate_document_file(file) | ||||||
|
|
||||||
| if not is_valid: | ||||||
| raise HTTPException( | ||||||
| status_code=400, | ||||||
| detail=f"Invalid file type. Accepted formats: PDF, DOC, DOCX" | ||||||
| ) | ||||||
|
|
||||||
| # Save to temporary location | ||||||
| tmp_file_path = await save_upload_file_tmp(file) | ||||||
| is_valid, tmp_file_path, _, _ = await validate_document_file(file) | ||||||
|
||||||
| is_valid, tmp_file_path, _, _ = await validate_document_file(file) | |
| _, tmp_file_path, _, _ = await validate_document_file(file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
is_validvalue returned byvalidate_document_fileis assigned but never used. Since the helper now raisesHTTPExceptionon invalid input, consider unpacking to_(or removing the boolean from the helper's return type) to avoid unused variables and make the control flow clearer.