mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-14 16:16:48 +00:00
fix(attachments,usage): validate content behind a BOM, judge by the live parser table
Review follow-ups. A BOM told the sniff which encoding to read, but was also taken as the verdict: three prepended bytes let any binary through, including as notes.txt. A BOM now only selects the test — UTF-8 falls through to the byte rules on the remainder, UTF-16/32 decode and judge the characters (NUL, unprintable, or replacement chars from bytes the decoder could not read). Real Notepad-Unicode text still passes, mp4-behind-a-BOM does not, in either language. The gate treated the full parser table as a given, but without docling the fallback extractor has no .tif/.tiff/.bmp/.webp/.vtt/.xml handler, so those suffixes skipped the content check and reached the plain-text fallthrough — the original bug, one install away. The worker now passes the keys of the extractor it actually built, making the second gate stricter than the route's static one rather than a copy of it. Cache bins: extraction coerced a missing value to 0 and only non-zero bins were recorded, so a provider reporting cached_tokens=0 persisted as NULL — indistinguishable from "not reported", and OpenAI reports exactly that on every uncached request. Bins are now carried as Optional and recorded when not None, which is what the nullable columns and the NULL-means-unknown comment already assumed. Anthropic's cache_read/cache_creation bins had the same shape and are fixed alongside; the int-or-None coercion is shared in llm/base.py.
This commit is contained in:
1 parent
f7cd94668f
commit
9db11f18f5
10 files changed
+341
-66
No files matched your search
@@ -1583,5 +1583,26 @@ def test_record_responses_metadata_captures_cache_write_bin(monkeypatch):
|
||||
),
|
||||
))
|
||||
# A pure cache-write turn (first request of a prefix) has reads=0 and
|
||||
# writes>0; the details must still be recorded.
|
||||
assert llm._last_usage["prompt_tokens_details"] == {"cache_write_tokens": 9}
|
||||
# writes>0; both bins were reported, so both are recorded — a reported
|
||||
# zero is "no cache hits", not "unknown".
|
||||
assert llm._last_usage["prompt_tokens_details"] == {
|
||||
"cached_tokens": 0,
|
||||
"cache_write_tokens": 9,
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_record_responses_metadata_omits_unreported_cache_bins(monkeypatch):
|
||||
"""A provider that says nothing about caching must persist as NULL, not 0."""
|
||||
llm = _make_llm(monkeypatch, _responses_caps())
|
||||
llm._record_responses_metadata(_ns(
|
||||
id="resp_2",
|
||||
usage=_ns(
|
||||
input_tokens=10,
|
||||
output_tokens=7,
|
||||
total_tokens=17,
|
||||
input_tokens_details=None,
|
||||
output_tokens_details=None,
|
||||
),
|
||||
))
|
||||
assert "prompt_tokens_details" not in llm._last_usage
|
||||
@@ -121,6 +121,49 @@ def test_enforce_parseable_attachment_accepts_bom_marked_unicode_text(
|
||||
enforce_parseable_attachment(path, "notes.txt")
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"bom",
|
||||
[codecs.BOM_UTF8, codecs.BOM_UTF16_LE, codecs.BOM_UTF16_BE, codecs.BOM_UTF32_BE],
|
||||
)
|
||||
def test_enforce_parseable_attachment_rejects_binary_behind_a_bom(bom, tmp_path):
|
||||
"""A BOM says which encoding to read, not that the content is text.
|
||||
|
||||
Otherwise three prepended bytes buy any binary a pass.
|
||||
"""
|
||||
from application.upload_limits import (
|
||||
enforce_parseable_attachment,
|
||||
UnsupportedUploadTypeError,
|
||||
)
|
||||
|
||||
path = tmp_path / "notes.txt"
|
||||
path.write_bytes(bom + MP4_HEADER + bytes(range(256)) * 8)
|
||||
|
||||
with pytest.raises(UnsupportedUploadTypeError):
|
||||
enforce_parseable_attachment(path, "notes.txt")
|
||||
|
||||
|
||||
def test_enforce_parseable_attachment_uses_the_extractor_it_is_given(tmp_path):
|
||||
"""The worker holds the live parser table; a trimmed install must not admit on trust.
|
||||
|
||||
Without docling the fallback extractor has no .webp handler, so a .webp
|
||||
would otherwise skip the content check and be read as plain text.
|
||||
"""
|
||||
from application.upload_limits import (
|
||||
enforce_parseable_attachment,
|
||||
UnsupportedUploadTypeError,
|
||||
)
|
||||
|
||||
path = tmp_path / "scan.webp"
|
||||
path.write_bytes(b"RIFF\x00\x00\x00\x00WEBPVP8 " + bytes(range(256)))
|
||||
|
||||
# Default list: .webp is parser-backed, admitted on its name.
|
||||
enforce_parseable_attachment(path, "scan.webp")
|
||||
|
||||
# The extractor actually loaded has no .webp parser.
|
||||
with pytest.raises(UnsupportedUploadTypeError):
|
||||
enforce_parseable_attachment(path, "scan.webp", {".pdf", ".docx"})
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"filename",
|
||||
[
|
||||
@@ -177,6 +220,10 @@ def test_enforce_parseable_attachment_accepts_text_without_a_parser(filename, tm
|
||||
(b"\x1b[31mred log line\x1b[0m\n", True),
|
||||
(codecs.BOM_UTF16_LE + "hi\n".encode("utf-16-le"), True),
|
||||
(codecs.BOM_UTF8 + b"hi\n", True),
|
||||
(codecs.BOM_UTF32_LE + "hi\n".encode("utf-32-le"), True),
|
||||
# A BOM in front of binary is still binary.
|
||||
(codecs.BOM_UTF8 + b"\x00\x01\x02", False),
|
||||
(codecs.BOM_UTF16_LE + MP4_HEADER, False),
|
||||
(b"text\x00with nul", False),
|
||||
(bytes(range(32)) * 4, False),
|
||||
(b"\x7f\x7f\x7f\x7f" + b"a" * 16, False),
|
||||
|
||||
@@ -379,6 +379,53 @@ class TestAttachmentTypeGuard:
|
||||
failed = [payload for name, payload in events if name == "attachment.failed"]
|
||||
assert failed and failed[0]["error"] == "Unsupported file type: .mp4"
|
||||
|
||||
def test_suffix_the_loaded_extractor_cannot_parse_is_refused(
|
||||
self, pg_conn, patch_worker_db, task_self, monkeypatch, tmp_path
|
||||
):
|
||||
"""The guard judges against the parser table actually loaded.
|
||||
|
||||
Without docling the fallback extractor has no .webp parser, so a
|
||||
.webp the route admitted on its name would otherwise be opened as
|
||||
plain text here.
|
||||
"""
|
||||
from application import worker
|
||||
|
||||
local_path = tmp_path / "scan.webp"
|
||||
local_path.write_bytes(b"RIFF\x00\x00\x00\x00WEBPVP8 " + bytes(range(256)))
|
||||
|
||||
events = []
|
||||
fake_storage = MagicMock(name="storage")
|
||||
fake_storage.process_file.side_effect = lambda path, callback: callback(
|
||||
str(local_path)
|
||||
)
|
||||
monkeypatch.setattr(worker.StorageCreator, "get_storage", lambda: fake_storage)
|
||||
# The docling-less fallback table: images are handled, .webp is not.
|
||||
monkeypatch.setattr(
|
||||
worker,
|
||||
"get_default_file_extractor",
|
||||
lambda ocr_enabled=False, pdf_text_fast_path=False: {".png": object()},
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
worker,
|
||||
"publish_user_event",
|
||||
lambda user, name, payload, **kwargs: events.append((name, payload)),
|
||||
)
|
||||
|
||||
file_info = {
|
||||
"filename": "scan.webp",
|
||||
"attachment_id": "507f1f77bcf86cd799439014",
|
||||
"path": "uploads/user1/attachments/scan.webp",
|
||||
"metadata": {"source": "chat"},
|
||||
}
|
||||
|
||||
with pytest.raises(
|
||||
worker.AttachmentRejectedError, match=r"Unsupported file type: \.webp"
|
||||
):
|
||||
worker.attachment_worker(task_self, file_info, "user1")
|
||||
|
||||
failed = [payload for name, payload in events if name == "attachment.failed"]
|
||||
assert failed and failed[0]["error"] == "Unsupported file type: .webp"
|
||||
|
||||
def test_text_without_a_parser_is_parsed(
|
||||
self, pg_conn, patch_worker_db, task_self, monkeypatch, tmp_path
|
||||
):
|
||||
|
||||
Reference in new issue
Block a user