fix(attachments): address fix-review nits (ftyp scan, text-preview, cache doc)

A review of the fix commits surfaced three refinements:

- ftyp audio sniff: scan the whole ftyp box (its declared length) for an audio brand instead of a fixed 6-slot window, so a real .m4a with the brand listed late still passes — while a pure-video file (no audio brand) still rejects.

- text-preview: accumulate body chunks until >=240 chars before cancelling the stream, instead of assuming the first chunk is large (flush boundaries can split a large body into small early chunks).

- _resolve_attachments: correct the cache comment — the memo is refreshed per send and the wire resolver only runs during a send, so a stale value is never observed between sends.
This commit is contained in:
Patrick Buckley
2026-06-15 21:06:28 -07:00
parent dd80ca3655
commit ed5c104a88
4 changed files with 44 additions and 21 deletions
+4
View File
@@ -30,6 +30,9 @@ FLAC = b"fLaC\x00\x00\x00\x22" + b"\x00" * 8
M4A = b"\x00\x00\x00\x20ftypM4A \x00\x00\x00\x00"
# Major brand mp42 but M4A in the compatible-brands list (common for real .m4a).
M4A_COMPAT = b"\x00\x00\x00\x20ftypmp42\x00\x00\x00\x00M4A mp42isom"
# M4A as a LATE compatible brand (offset 40), past the old fixed 16:40 scan window
# — must still be accepted now that the whole ftyp box is scanned.
M4A_LATE_BRAND = b"\x00\x00\x00\x2cftypmp42\x00\x00\x00\x00isomiso2mp41avc1dashmp4aM4A "
# ISO-BMFF *video* / MOV share the ftyp box — must NOT sniff as audio.
MP4_VIDEO = b"\x00\x00\x00\x20ftypisom\x00\x00\x02\x00isomiso2avc1mp41"
MOV_VIDEO = b"\x00\x00\x00\x14ftypqt \x00\x00\x00\x00qt \x00\x00\x00\x00"
@@ -60,6 +63,7 @@ class TestSniffAudio:
assert sniff_audio_mime(FLAC) == "audio/flac"
assert sniff_audio_mime(M4A) == "audio/mp4"
assert sniff_audio_mime(M4A_COMPAT) == "audio/mp4"
assert sniff_audio_mime(M4A_LATE_BRAND) == "audio/mp4"
assert sniff_audio_mime(AAC_ADTS) == "audio/aac"
assert sniff_audio_mime(WEBM) == "audio/webm"
+14 -8
View File
@@ -181,14 +181,20 @@ def sniff_audio_mime(data: bytes) -> str | None:
if data[:4] == b"fLaC":
return "audio/flac"
# ISO-BMFF: the ``ftyp`` box is shared by MP4/MOV *video* and M4A/M4B
# *audio*. Accept only audio brands (major, or an audio brand in the
# compatible-brands list) so a video file can't masquerade as audio.
if data[4:8] == b"ftyp" and (
data[8:12] in (b"M4A ", b"M4B ", b"F4A ", b"F4B ")
or b"M4A " in data[16:40]
or b"M4B " in data[16:40]
):
return "audio/mp4"
# *audio*. Accept only when an audio brand is the major brand or appears
# anywhere in the compatible-brands list — scan the whole ftyp box (its
# length is the big-endian uint32 at data[0:4]) so a real .m4a with the
# audio brand listed late still passes; a pure-video file carries none. A
# generic-MP4 (isom/mp42) audio stream with no audio brand is
# indistinguishable from video by magic bytes alone, so it falls through
# (rejected) rather than risk passing a video off as audio.
if data[4:8] == b"ftyp":
box_end = int.from_bytes(data[0:4], "big")
if not 16 <= box_end <= len(data):
box_end = len(data)
brands = data[8:box_end] # major brand (8:12) + compatible brands (16:end)
if any(b in brands for b in (b"M4A ", b"M4B ", b"F4A ", b"F4B ")):
return "audio/mp4"
if data[:4] == b"\x1aE\xdf\xa3":
return "audio/webm"
return None
+3 -1
View File
@@ -2969,7 +2969,9 @@ class ChatSession:
# re-rasterized / a blob re-base64'd once per round-trip. Key on
# (id, caps-signature): the same stored blob materializes differently per
# capability set, and a fallback to a different-caps model can resolve
# within one send. ``cache`` is None outside a send → original behavior.
# within one send. The cache is refreshed per send (set in send()); the
# wire resolver runs only during a send, so a stale value between sends is
# never read. A None cache disables memoization (the original behavior).
cache = self._wire_part_cache
caps_sig = (caps.supports_pdf, caps.supports_vision, caps.supports_audio_input)
out: dict[str, Any] = {}
+23 -12
View File
@@ -125,20 +125,31 @@ export function buildAttachmentPreview(opts) {
})
.then(function (r) {
if (!r || !r.ok) return "";
// We only render ~240 chars: read just the first body chunk and cancel
// the rest of the transfer instead of downloading the whole blob (text
// attachments are capped at 512 KiB, ~2000x the rendered size). The
// first network chunk is always many KB, so 240 chars are available.
// We only render ~240 chars: read body chunks until we have enough, then
// cancel the rest of the transfer instead of downloading the whole blob
// (text attachments are capped at 512 KiB, ~2000x the rendered size).
// Accumulate rather than assume one chunk ≥ 240 chars (flush boundaries
// can split a large body into small early chunks).
if (!r.body || typeof r.body.getReader !== "function") return r.text();
var reader = r.body.getReader();
return reader.read().then(function (res) {
try {
reader.cancel();
} catch (e) {
/* already closed */
}
return res && res.value ? new TextDecoder().decode(res.value) : "";
});
var dec = new TextDecoder();
var acc = "";
function pump() {
return reader.read().then(function (res) {
if (res && res.value)
acc += dec.decode(res.value, { stream: true });
if (res.done || acc.length >= 240) {
try {
reader.cancel();
} catch (e) {
/* already closed */
}
return acc;
}
return pump();
});
}
return pump();
})
.then(function (t) {
if (!t) {