diff --git a/backend/packages/harness/deerflow/uploads/manager.py b/backend/packages/harness/deerflow/uploads/manager.py index 58d6b3ad2..875dc90e4 100644 --- a/backend/packages/harness/deerflow/uploads/manager.py +++ b/backend/packages/harness/deerflow/uploads/manager.py @@ -29,6 +29,8 @@ logger = logging.getLogger(__name__) UPLOAD_STAGING_PREFIX = ".upload-" UPLOAD_STAGING_SUFFIX = ".part" +_MAX_FILENAME_BYTES = 255 + def get_uploads_dir(thread_id: str, *, user_id: str | None = None) -> Path: """Return the uploads directory path for a thread (no side effects).""" @@ -66,16 +68,31 @@ def normalize_filename(filename: str) -> str: # but they indicate a Windows-style path that should be stripped or rejected. if "\\" in safe: raise ValueError(f"Filename contains backslash: {filename!r}") - if len(safe.encode("utf-8")) > 255: + if len(safe.encode("utf-8")) > _MAX_FILENAME_BYTES: raise ValueError(f"Filename too long: {len(safe)} chars") return safe +def _fit_utf8_bytes(text: str, budget: int) -> str: + """Truncate *text* to at most *budget* UTF-8 bytes without splitting a code point.""" + encoded = text.encode("utf-8") + if len(encoded) <= budget: + return text + return encoded[:budget].decode("utf-8", errors="ignore") + + def claim_unique_filename(name: str, seen: set[str]) -> str: """Generate a unique filename by appending ``_N`` suffix on collision. Automatically adds the returned name to *seen* so callers don't need to. + The deduplicated name stays within the 255-byte filename limit that + :func:`normalize_filename` enforces: when appending ``_N`` (plus the + preserved extension) would exceed it, the stem is truncated on a UTF-8 + boundary to make room. Otherwise a maximum-length upload that collides + would produce a name the filesystem (and a later ``normalize_filename`` + call on the write path) rejects. + Args: name: Candidate filename. seen: Set of filenames already claimed (mutated in place). @@ -88,10 +105,18 @@ def claim_unique_filename(name: str, seen: set[str]) -> str: return name stem, suffix = Path(name).stem, Path(name).suffix counter = 1 - candidate = f"{stem}_{counter}{suffix}" - while candidate in seen: + while True: + tag = f"_{counter}" + budget = _MAX_FILENAME_BYTES - len(tag.encode("utf-8")) - len(suffix.encode("utf-8")) + if budget < 1: + # Pathological suffix that leaves no room for a stem; keep the + # unique tag and fit the rest (stem + suffix tail) around it. + candidate = _fit_utf8_bytes(stem + suffix, _MAX_FILENAME_BYTES - len(tag.encode("utf-8"))) + tag + else: + candidate = f"{_fit_utf8_bytes(stem, budget)}{tag}{suffix}" + if candidate not in seen: + break counter += 1 - candidate = f"{stem}_{counter}{suffix}" seen.add(candidate) return candidate diff --git a/backend/tests/test_uploads_manager.py b/backend/tests/test_uploads_manager.py index 28364e951..0b5ec0eda 100644 --- a/backend/tests/test_uploads_manager.py +++ b/backend/tests/test_uploads_manager.py @@ -73,6 +73,40 @@ class TestDeduplicateFilename: claim_unique_filename("a.txt", seen) assert seen == {"a.txt", "a_1.txt"} + def test_max_length_name_stays_within_filename_limit(self): + # A 255-byte name passes normalize_filename; the deduplicated name + # must not exceed that limit, or the write path rejects it. + name = "a" * 251 + ".txt" + seen = {name} + deduped = claim_unique_filename(name, seen) + assert deduped != name + assert deduped.endswith("_1.txt") + assert len(deduped.encode("utf-8")) <= 255 + # The truncated result must round-trip through normalize_filename. + assert normalize_filename(deduped) == deduped + + def test_max_length_collisions_stay_unique_across_truncation(self): + name = "a" * 251 + ".txt" + seen = {name} + first = claim_unique_filename(name, seen) + second = claim_unique_filename(name, seen) + assert first != second + assert len(second.encode("utf-8")) <= 255 + + def test_multibyte_stem_is_truncated_on_a_codepoint_boundary(self): + # 85 CJK chars × 3 bytes = 255 bytes. + name = "深" * 85 + seen = {name} + deduped = claim_unique_filename(name, seen) + assert len(deduped.encode("utf-8")) <= 255 + assert deduped.endswith("_1") + # No replacement characters / decode artifacts. + deduped.encode("utf-8").decode("utf-8") + + def test_short_names_keep_existing_dedupe_shape(self): + seen = {"data.txt"} + assert claim_unique_filename("data.txt", seen) == "data_1.txt" + # --------------------------------------------------------------------------- # validate_path_traversal diff --git a/backend/tests/test_uploads_router.py b/backend/tests/test_uploads_router.py index 6f72bd0bd..fc8b81bcf 100644 --- a/backend/tests/test_uploads_router.py +++ b/backend/tests/test_uploads_router.py @@ -132,6 +132,50 @@ def test_upload_files_auto_renames_duplicate_form_filenames(tmp_path): assert (thread_uploads_dir / "data_1.txt").read_bytes() == b"second" +def test_upload_files_deduplicates_max_length_filenames_without_failing_the_batch(tmp_path): + # A 255-byte filename is the longest normalize_filename accepts. Before + # the byte-budget truncation in claim_unique_filename, deduplicating a + # duplicate at that length produced a 257-byte name that the write path + # rejected, failing the whole request with a 500 and rolling back files + # that had already been written. + thread_uploads_dir = tmp_path / "uploads" + thread_uploads_dir.mkdir(parents=True) + + provider = MagicMock() + provider.uses_thread_data_mounts = True + + max_length_name = "a" * 251 + ".txt" + + with ( + patch.object(uploads, "get_uploads_dir", return_value=thread_uploads_dir), + patch.object(uploads, "ensure_uploads_dir", return_value=thread_uploads_dir), + patch.object(uploads, "get_sandbox_provider", return_value=provider), + ): + result = asyncio.run( + call_unwrapped( + uploads.upload_files, + "thread-local", + request=MagicMock(), + files=[ + UploadFile(filename="innocent.txt", file=BytesIO(b"kept")), + UploadFile(filename=max_length_name, file=BytesIO(b"first")), + UploadFile(filename=max_length_name, file=BytesIO(b"second")), + ], + config=SimpleNamespace(), + ) + ) + + assert result.success is True + assert len(result.files) == 3 + deduped_name = result.files[2].filename + assert deduped_name != max_length_name + assert deduped_name.endswith("_1.txt") + assert len(deduped_name.encode("utf-8")) <= 255 + assert (thread_uploads_dir / "innocent.txt").read_bytes() == b"kept" + assert (thread_uploads_dir / max_length_name).read_bytes() == b"first" + assert (thread_uploads_dir / deduped_name).read_bytes() == b"second" + + def test_upload_files_skips_acquire_when_thread_data_is_mounted(tmp_path): thread_uploads_dir = tmp_path / "uploads" thread_uploads_dir.mkdir(parents=True)