Repository navigation
Fix/filename #167
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
Fix/filename #167
Changes from 5 commits
55dff02
7fb5f08
158388b
62d2cfc
5c8a9fb
512d3fa
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,11 +1,35 @@ | ||||||
| import asyncio | ||||||
| import re | ||||||
| import secrets | ||||||
| import time | ||||||
| from pathlib import Path | ||||||
| from typing import Dict, Optional | ||||||
|
|
||||||
| import aiofiles | ||||||
| from fastapi import UploadFile | ||||||
| import consts | ||||||
| import time | ||||||
| import secrets | ||||||
| import asyncio | ||||||
| from typing import Dict, Optional | ||||||
| from fastapi import UploadFile | ||||||
|
|
||||||
|
|
||||||
| def sanitize_filename(filename: str) -> str: | ||||||
| # Split filename into name and extension | ||||||
| path = Path(filename) | ||||||
| name = path.stem | ||||||
| ext = path.suffix | ||||||
|
|
||||||
| # Remove special characters (keep only word characters and hyphens temporarily) | ||||||
| name = re.sub(r"[^\w\-]", "_", name) | ||||||
|
|
||||||
| # Replace hyphens with underscores | ||||||
| name = name.replace("-", "_") | ||||||
|
|
||||||
| # Collapse multiple underscores | ||||||
| name = re.sub(r"_+", "_", name) | ||||||
|
Collaborator
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. Is is a problem to have multiple underscore? I know I have files with them, this case seems legit to me
Collaborator
Author
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. openrag/openrag/components/files.py Lines 21 to 22 in 8bd8f1c
With this code, if there are consecutive special characters, each one is replaced with “_”, which can result in filenames containing many underscores. I added that line for further cleaning, but having multiple underscores doesn’t cause any issues during indexing. |
||||||
|
|
||||||
| # Remove leading/trailing underscores | ||||||
| name = name.strip("_") | ||||||
|
|
||||||
| # Reconstruct filename | ||||||
| return name + ext | ||||||
|
|
||||||
|
|
||||||
| def make_unique_filename(filename: str) -> Path: | ||||||
|
|
@@ -64,6 +88,5 @@ async def serialize_file(task_id: str, path: str, metadata: Optional[Dict] = {}) | |||||
| except Exception: | ||||||
| raise | ||||||
| else: | ||||||
|
|
||||||
| ray.cancel(future, recursive=True) | ||||||
| raise TimeoutError(f"Serialization task {task_id} timed out after seconds") | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,9 @@ | ||
| import io | ||
| import uuid | ||
| from pathlib import Path | ||
|
|
||
| from fastapi import UploadFile | ||
| import pytest | ||
|
|
||
| from components.files import save_file_to_disk | ||
| from components.files import sanitize_filename, save_file_to_disk | ||
| from fastapi import UploadFile | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
|
|
@@ -18,11 +16,7 @@ async def test_save_file_to_disk_writes_content(tmp_path: Path): | |
|
|
||
| dest_dir = tmp_path / "uploads" | ||
|
|
||
| saved_path = await save_file_to_disk( | ||
| file=upload, | ||
| dest_dir=dest_dir, | ||
| chunk_size=4 | ||
| ) | ||
| saved_path = await save_file_to_disk(file=upload, dest_dir=dest_dir, chunk_size=4) | ||
|
|
||
| assert saved_path.exists() | ||
| assert saved_path.parent == dest_dir | ||
|
|
@@ -34,10 +28,8 @@ async def test_save_file_to_disk_writes_content(tmp_path: Path): | |
| assert saved_content == content | ||
|
|
||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_save_file_to_disk_with_random_prefix(tmp_path, monkeypatch): | ||
|
|
||
| def fake_make_unique_filename(filename: str) -> str: | ||
| assert filename == "test.txt" | ||
| return "PREFIX_1234_test.txt" | ||
|
|
@@ -63,3 +55,29 @@ def fake_make_unique_filename(filename: str) -> str: | |
| assert saved_path.name == "PREFIX_1234_test.txt" | ||
| assert saved_path.exists() | ||
| assert saved_path.read_bytes() == file_content | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "input_name,expected", | ||
| [ | ||
| # Basic cases | ||
| ("simple_file.txt", "simple_file.txt"), | ||
| ("file-name.pdf", "file_name.pdf"), | ||
| # Spaces and commas | ||
| ("my file.txt", "my_file.txt"), | ||
| ("file,name.txt", "file_name.txt"), | ||
| ("multiple spaces.txt", "multiple_spaces.txt"), | ||
| # Special characters | ||
| ("file@name#2024.txt", "file_name_2024.txt"), | ||
| ("doc$with%special&chars.pdf", "doc_with_special_chars.pdf"), | ||
| # Multiple underscores | ||
| ("file___name.txt", "file_name.txt"), | ||
| ("file__name__here.txt", "file_name_here.txt"), | ||
| # Edge cases | ||
| ("", ""), | ||
| ("file(1).txt", "file_1.txt"), | ||
|
Collaborator
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. Could you add a test with
Collaborator
Author
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. Added |
||
| ("file.with.dot.txt", "file_with_dot.txt"), | ||
| ], | ||
| ) | ||
| def test_sanitize_filename(input_name, expected): | ||
| assert sanitize_filename(input_name) == expected | ||
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.
Is hyphens really problematic ? Lot of files have it
Uh oh!
There was an error while loading. Please reload this page.
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.
This isn’t an issue during indexation, but it becomes one with Chainlit.
For example, the filename
allocution_de_Mr_trump,__au_COP21_-_Hello_20251119065407.txtindexes correctly, but when displayed in the Chainlit UI, some underscores are interpreted as Markdown italics markers, breaking the file links.No issue with filenames like:
allocution_de_Mr_trump,__au_COP21__Hello_20251119065407.txt(only underscores)allocution_de_Mr_trump,-au-COP21-Hello-20251119065407.txt(only hyphens)The issue happens specifically with:
allocution_de_Mr_trump,__au_COP21_-_Hello_20251119065407.txtas shown in the screenshot.
This suggests that Chainlit doesn’t fail on
_or-individually, but on certain combinations of both.To keep things simple, we can sanitize the
filenamefor safe display and during indexation. In another file metadata we can store the original name, such as"original_filename".Uh oh!
There was an error while loading. Please reload this page.
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.
Ok, let's keep it like this then and add the
original_filename