fix(telegram): truncate media captions by UTF-16 units, not code points - #62620
pierrenode wants to merge 1 commit into
Conversation
Code Review Summary\n\nFound potential security issues. Please review.\n\n---\nReviewed by Hermes Agent |
Related to closed #49324 (same outbound-caption UTF-16 fix, never merged) and to the inbound entity-offset fix #49325 / issue #55439 (different direction and site). This is the outbound-caption half and is a valid re-implementation of the closed #49324 — not a duplicate. |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address the same independently identified Telegram outbound-caption defect; no filed issue is present in this complex. Both replace code-point slicing at all nine media-caption sites with UTF-16-unit-aware truncation, while #62620 applies the fix to the current plugin path and adds focused regression coverage.
Related pull requests
- #49324 [closed]
duplicate— (+80/-9) — historical reference, superseded by #62620: the diff correctly replaced all nine naive caption slices with_prefix_within_utf16_limit(..., 1024)and tested document/video captions, but it was closed for staleness without being merged and targets the formergateway/platforms/telegram.pylocation. - #62620
related— (+105/-9) — merge candidate: the diff reimplements the same nine-site UTF-16-aware fix in the currentplugins/platforms/telegram/adapter.pylocation and tests over-limit astral captions plus preservation of short captions. The non-contributor security warning provides no specific finding tied to the shown low-risk truncation diff.
Duplicates
#49324 and #62620 implement essentially the same outbound Telegram caption fix; as the contributor discussion notes, #62620 is the valid current-tree reimplementation rather than an operational duplicate to close.
Suggested consolidation
Merge #62620 — it applies the complete nine-site fix to the current Telegram adapter and includes focused regression tests; keep already-closed #49324 as the superseded historical reference, with no additional open duplicate requiring closure.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup49324 ["PRs duplicating each other"]
P49324["PR #49324 (closed)"]
P62620["PR #62620 (open)"]
end
class P49324 closed
class P62620 open
class P62620 target
click P49324 "https://github.com/NousResearch/hermes-agent/pull/49324"
click P62620 "https://github.com/NousResearch/hermes-agent/pull/62620"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 18 kB of PR diffs, 7 kB of issue/PR text, 3 kB of discussion (5 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
TelegramAdapter.send_voice/send_multiple_images/send_image_file/ send_document/send_video/send_image/send_animation cap captions with caption[:1024] — a code-point slice. Telegram's 1024-character caption limit is measured in UTF-16 code units, the same as the message-length limit that utf16_len()/_prefix_within_utf16_limit() already protect elsewhere in this file (7 call sites). Characters outside the Basic Multilingual Plane (emoji, rare CJK) are surrogate pairs and cost two UTF-16 units each, so a caption whose code-point length is <=1024 can still have a UTF-16 length up to 2048 and go out unchanged — Telegram's Bot API rejects it outright and the media never sends. Fix: route all 9 caption[:1024]/alt_text[:1024] sites through the existing _prefix_within_utf16_limit() helper (already imported and used for message-text truncation in this same file). Symmetric with how message-text truncation already works; no behavior change for captions that were already within the UTF-16 limit.
be61c41 to
180d9ae
Compare
|
Rebased onto current `upstream/main` and squashed to a single commit. 8 of the 9 `caption[:1024]`/`alt_text[:1024]` sites auto-merged cleanly. The `send_voice` site had a real conflict: upstream added a MarkdownV2-formatted-caption-with-plain-fallback retry loop (`_caption_variants`) around it since this PR was opened. Kept upstream's retry-loop structure entirely and applied this fix to the one line that still matters — the plain-caption fallback variant (`_caption_variants.append((_prefix_within_utf16_limit(caption, 1024), None))`), which was still doing a raw code-point slice. All 9 sites now route through `_prefix_within_utf16_limit()` (the same helper the message-length path already uses), verified by grep — zero `caption[:1024]`/`alt_text[:1024]` sites remain. Targeted suite (3 tests) passes; mutation-verified (reverting the `send_document` site back to `caption[:1024]` makes `test_send_document_caption_truncated_by_utf16_units` fail with `assert 2048 <= 1024`). Broader `telegram`-tagged sweep (619 tests across `tests/gateway/`, `tests/tools/`, `tests/hermes_cli/`, plus the PTB-specific file) has 6 pre-existing failures in `tests/test_telegram_polling_progress_ptb.py` — confirmed unrelated to this change (same 6 failures reproduce with this PR's test file excluded entirely; the file's own "real PTB" tests are polluted by `sys.modules["telegram"]` mocking from other files in the combined run, not from this PR). Ruff clean. Fresh competitor search found no other PR touching these call sites. |
What does this PR do?
TelegramAdapter.send_voice/send_multiple_images/send_image_file/send_document/send_video/send_image/send_animation(plugins/platforms/telegram/adapter.py) cap the outgoingcaptionwithcaption[:1024]/alt_text[:1024]— a Python code-point slice.Telegram's 1024-character caption limit is measured in UTF-16 code units, the same unit the 4096-character message-length limit uses — which this same file already handles correctly via
utf16_len()/_prefix_within_utf16_limit()at 7 message-text call sites. Characters outside the Basic Multilingual Plane (emoji, rare CJK extensions) are surrogate pairs and cost two UTF-16 units each but only one Python code point, so a caption whose code-point length is<=1024can still have a true UTF-16 length of up to 2048. The Bot API then rejects the send outright and the media never reaches the chat.Fix: route all 9 caption-truncation sites through the existing
_prefix_within_utf16_limit()helper (already imported/used in this file for message-text truncation), instead of the raw[:1024]slice.Related Issue
No existing issue — found via a systematic UTF-16-truncation sweep after fixing the same bug class for Discord auto-thread names (#60252).
Type of Change
Changes Made
plugins/platforms/telegram/adapter.py: import_prefix_within_utf16_limitfromgateway.platforms.base(already used for message-text truncation in the same file); replace all 9caption[:1024]/alt_text[:1024]sites (send_voicex2,send_multiple_images,send_image_file,send_document,send_video,send_imagex2,send_animation) with_prefix_within_utf16_limit(caption, 1024).tests/gateway/test_telegram_caption_utf16_truncation.py(new): regression tests — a caption of 1024 astral-plane emoji (code-point length 1024, UTF-16 length 2048) must come out<=1024UTF-16 units without splitting a surrogate pair; a short caption is passed through unchanged.How to Test
Mutation-verified: the new tests fail against the pre-fix code (
AssertionError: assert 2048 <= 1024) and pass after the fix. Also ran the full neighboring Telegram caption/send suites (test_telegram_caption_merge.py,test_telegram_send_path_health.py,test_telegram_send_draft_format.py,test_telegram_documents.py,test_telegram_photo_interrupts.py,test_telegram_voice_v0_regressions.py,test_telegram_rich_messages.py,test_telegram_overflow_partial.py,tests/tools/test_telegram_send_message_caption.py) — 151 tests, all pass.Checklist