mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(uploads): avoid case-variant filename collisions (#6466)
* fix(uploads): avoid case-variant filename collisions * test(uploads): cover case collisions across requests --------- Co-authored-by: haotianqi666 <250384991+haotianqi666@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
69f4093ce9
commit
4946b29d03
@ -96,7 +96,7 @@ def _fit_utf8_bytes(text: str, budget: int) -> str:
|
||||
|
||||
|
||||
def claim_unique_filename(name: str, seen: set[str]) -> str:
|
||||
"""Generate a unique filename by appending ``_N`` suffix on collision.
|
||||
"""Generate a case-insensitively unique filename by appending ``_N`` on collision.
|
||||
|
||||
Automatically adds the returned name to *seen* so callers don't need to.
|
||||
|
||||
@ -112,9 +112,10 @@ def claim_unique_filename(name: str, seen: set[str]) -> str:
|
||||
seen: Set of filenames already claimed (mutated in place).
|
||||
|
||||
Returns:
|
||||
A filename not present in *seen* (already added to *seen*).
|
||||
A filename not present in *seen* even ignoring case (already added to *seen*).
|
||||
"""
|
||||
if name not in seen:
|
||||
claimed = {existing.casefold() for existing in seen}
|
||||
if name.casefold() not in claimed:
|
||||
seen.add(name)
|
||||
return name
|
||||
stem, suffix = Path(name).stem, Path(name).suffix
|
||||
@ -128,7 +129,7 @@ def claim_unique_filename(name: str, seen: set[str]) -> str:
|
||||
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:
|
||||
if candidate.casefold() not in claimed:
|
||||
break
|
||||
counter += 1
|
||||
seen.add(candidate)
|
||||
|
||||
@ -2,6 +2,10 @@
|
||||
|
||||
Backend tests must preserve the runtime invariants they exercise without changing production execution topology.
|
||||
|
||||
Upload case-collision coverage uses separate HTTP requests and preserves both
|
||||
reported payloads. Observe real filename-claim inputs to pin the disk seed;
|
||||
case-insensitive hosts can otherwise mask a missing seed through link retries.
|
||||
|
||||
Channel reload cancellation regressions must assert the worker returned the
|
||||
stale snapshot before checking that newer runtime config survived. Completion
|
||||
alone cannot prove the race: loader exceptions are caught and return `None`.
|
||||
|
||||
@ -282,6 +282,29 @@ class TestResolveAttachments:
|
||||
|
||||
|
||||
class TestInboundFileIngestion:
|
||||
def test_case_variant_names_keep_both_inbound_attachments(self, tmp_path):
|
||||
from app.channels import manager
|
||||
|
||||
uploads_dir = tmp_path / "uploads"
|
||||
uploads_dir.mkdir()
|
||||
msg = InboundMessage(
|
||||
channel_name="telegram",
|
||||
chat_id="chat-1",
|
||||
user_id="user-1",
|
||||
text="see attachments",
|
||||
files=[
|
||||
{"type": "file", "filename": "Report.txt", "_content": b"first"},
|
||||
{"type": "file", "filename": "report.txt", "_content": b"second"},
|
||||
],
|
||||
)
|
||||
|
||||
with patch("deerflow.uploads.manager.ensure_uploads_dir", return_value=uploads_dir):
|
||||
result = _run(manager._ingest_inbound_files("thread-1", msg))
|
||||
|
||||
assert [item["filename"] for item in result] == ["Report.txt", "report_1.txt"]
|
||||
assert (uploads_dir / "Report.txt").read_bytes() == b"first"
|
||||
assert (uploads_dir / "report_1.txt").read_bytes() == b"second"
|
||||
|
||||
def test_consumes_inline_channel_bytes_without_exposing_them_downstream(self, tmp_path):
|
||||
from app.channels import manager
|
||||
|
||||
|
||||
@ -136,6 +136,11 @@ class TestDeduplicateFilename:
|
||||
assert claim_unique_filename("data.txt", seen) == "data_1.txt"
|
||||
assert "data_1.txt" in seen
|
||||
|
||||
def test_case_insensitive_collision_including_suffix(self):
|
||||
seen = {"Report.txt", "report_1.TXT"}
|
||||
assert claim_unique_filename("report.txt", seen) == "report_2.txt"
|
||||
assert "report_2.txt" in seen
|
||||
|
||||
def test_triple_collision(self):
|
||||
seen = {"data.txt", "data_1.txt", "data_2.txt"}
|
||||
assert claim_unique_filename("data.txt", seen) == "data_3.txt"
|
||||
|
||||
@ -16,6 +16,7 @@ from fastapi import HTTPException, UploadFile
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app.gateway import upload_ingestion
|
||||
from app.gateway.auth.models import User
|
||||
from app.gateway.deps import get_config
|
||||
from app.gateway.routers import uploads
|
||||
from deerflow.sandbox.lease import get_sandbox_lease_manager
|
||||
@ -141,6 +142,46 @@ 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_case_variants_across_requests(tmp_path):
|
||||
thread_uploads_dir = tmp_path / "uploads"
|
||||
thread_uploads_dir.mkdir()
|
||||
user = User(email="upload-test@example.com")
|
||||
app = make_authed_test_app(user_factory=lambda: user, bind_current_user=True)
|
||||
app.include_router(uploads.router)
|
||||
app.dependency_overrides[get_config] = lambda: SimpleNamespace()
|
||||
|
||||
claims = []
|
||||
real_claim = uploads.claim_unique_filename
|
||||
|
||||
def capture_claim(name, seen):
|
||||
claims.append((name, set(seen)))
|
||||
return real_claim(name, seen)
|
||||
|
||||
with (
|
||||
patch.object(uploads, "ensure_uploads_dir", return_value=thread_uploads_dir),
|
||||
patch.object(uploads, "get_sandbox_provider", return_value=_mounted_provider()),
|
||||
patch.object(uploads, "claim_unique_filename", side_effect=capture_claim),
|
||||
TestClient(app) as client,
|
||||
):
|
||||
first = client.post("/api/threads/thread-local/uploads", files={"files": ("Report.txt", b"first")})
|
||||
assert first.status_code == 200
|
||||
assert first.json()["success"] is True
|
||||
assert [entry["filename"] for entry in first.json()["files"]] == ["Report.txt"]
|
||||
|
||||
second = client.post("/api/threads/thread-local/uploads", files={"files": ("report.txt", b"second")})
|
||||
|
||||
assert second.status_code == 200
|
||||
assert second.json()["success"] is True
|
||||
assert [entry["filename"] for entry in second.json()["files"]] == ["report_1.txt"]
|
||||
assert second.json()["files"][0]["original_filename"] == "report.txt"
|
||||
assert (thread_uploads_dir / first.json()["files"][0]["filename"]).read_bytes() == b"first"
|
||||
assert (thread_uploads_dir / second.json()["files"][0]["filename"]).read_bytes() == b"second"
|
||||
assert {path.name for path in thread_uploads_dir.iterdir()} == {"Report.txt", "report_1.txt"}
|
||||
# On case-insensitive hosts the link retry can hide a missing disk seed.
|
||||
# Observe the real claim inputs so this invariant is checked on every host.
|
||||
assert claims == [("Report.txt", set()), ("report.txt", {"Report.txt"})]
|
||||
|
||||
|
||||
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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user