From 27b31bb7ff69d2a84f7889f221333d67593453cb Mon Sep 17 00:00:00 2001 From: Drexuxux Date: Thu, 16 Jul 2026 15:42:32 +0300 Subject: [PATCH] fix(relay): normalize a 0/negative max_message_length at the descriptor boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the truncate_message split-loop floor. Two review points: - CapabilityDescriptor.from_json trusted the wire max_message_length verbatim, so a connector advertising 0 ('no limit') — or a buggy one sending 0/negative — produced a descriptor whose bound flowed straight into the adapter's MAX_MESSAGE_LENGTH and truncate_message. Normalize it to the documented 4096 default (mirrors from_platform_entry's 'or 4096' and docs/relay-connector-contract.md), fixing the degenerate budget at its source rather than only surviving it downstream. - Document the truncate_message length contract for a budget too small for one codepoint (max_length=1 with a 2-unit surrogate pair under utf16_len): the chunk intentionally exceeds max_length by that one indivisible codepoint, because emitting it whole preserves content where the alternatives are data loss or an infinite loop. Tests: from_json normalizes 0 and negative bounds to 4096 and passes a real positive bound through unchanged; the sub-codepoint budget emits whole codepoints with no data loss (all emojis preserved) and a chunk that necessarily exceeds the 1-unit budget. --- gateway/platforms/base.py | 11 +++++++++ gateway/relay/descriptor.py | 13 +++++++++++ tests/gateway/relay/test_descriptor.py | 32 ++++++++++++++++++++++++++ tests/gateway/test_platform_base.py | 22 ++++++++++++++++++ 4 files changed, 78 insertions(+) diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 5d98943a80ec..9a2b9fd2c9ff 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -5694,6 +5694,17 @@ class BasePlatformAdapter(ABC): # the whole budget — leaves split_at at 0, so ``remaining`` # never shrinks and the while-loop spins forever appending # empty chunks (an unbounded hang / OOM). + # + # Length contract for a degenerate budget: a codepoint is the + # smallest indivisible unit, so when the budget is smaller than + # one codepoint (e.g. max_length=1 with a 2-unit surrogate pair + # under utf16_len) the emitted chunk WILL exceed max_length by + # that one codepoint. That is intentional — emitting the + # codepoint whole preserves the content, whereas the only + # alternatives are dropping it (data loss) or looping forever. + # Real callers never hit this: platform caps are hundreds/ + # thousands, and the relay path normalizes a 0/negative + # descriptor bound to 4096 (see gateway/relay/descriptor.py). split_at = max(1, _cp_limit) # Avoid splitting inside an inline code span (`...`). diff --git a/gateway/relay/descriptor.py b/gateway/relay/descriptor.py index 9ddbee011869..6645a08563f2 100644 --- a/gateway/relay/descriptor.py +++ b/gateway/relay/descriptor.py @@ -80,6 +80,19 @@ class CapabilityDescriptor: raw = json.loads(data) known = {f for f in cls.__dataclass_fields__} # type: ignore[attr-defined] filtered = {k: v for k, v in raw.items() if k in known} + # Normalize the chunking bound at the trust boundary. A connector may + # advertise max_message_length 0 ("no limit"), and a buggy/hostile one + # may send 0 or a negative; either is a degenerate value that would flow + # straight into the adapter's MAX_MESSAGE_LENGTH and truncate_message(). + # Map it to the documented 4096 default (docs/relay-connector-contract.md; + # mirrors from_platform_entry's `or 4096`) so from_json never yields a + # descriptor that can't chunk a real message. + if "max_message_length" in filtered: + try: + if int(filtered["max_message_length"]) <= 0: + filtered["max_message_length"] = 4096 + except (TypeError, ValueError): + filtered["max_message_length"] = 4096 return cls(**filtered) @classmethod diff --git a/tests/gateway/relay/test_descriptor.py b/tests/gateway/relay/test_descriptor.py index 10471a2c1a70..3145e557025c 100644 --- a/tests/gateway/relay/test_descriptor.py +++ b/tests/gateway/relay/test_descriptor.py @@ -46,6 +46,38 @@ def test_from_json_ignores_unknown_keys(): assert restored == d +def test_from_json_normalizes_zero_max_message_length_to_default(): + """A connector may advertise max_message_length 0 ("no limit"). from_json + must normalize it to the documented 4096 default so the receiver never + carries a degenerate chunking bound into truncate_message().""" + raw = ( + '{"contract_version": 1, "platform": "x", "label": "X", ' + '"max_message_length": 0, "supports_draft_streaming": false, ' + '"supports_edit": false, "supports_threads": false, ' + '"markdown_dialect": "plain", "len_unit": "chars"}' + ) + d = CapabilityDescriptor.from_json(raw) + assert d.max_message_length == 4096 + + +def test_from_json_normalizes_negative_max_message_length_to_default(): + """A buggy/hostile connector sending a negative bound is normalized too.""" + raw = ( + '{"contract_version": 1, "platform": "x", "label": "X", ' + '"max_message_length": -5, "supports_draft_streaming": false, ' + '"supports_edit": false, "supports_threads": false, ' + '"markdown_dialect": "plain", "len_unit": "chars"}' + ) + d = CapabilityDescriptor.from_json(raw) + assert d.max_message_length == 4096 + + +def test_from_json_keeps_a_real_positive_bound(): + """A normal positive bound is passed through unchanged.""" + d = CapabilityDescriptor.from_json(_telegram_descriptor(max_message_length=2000).to_json()) + assert d.max_message_length == 2000 + + def test_from_json_fills_optional_defaults(): """Optional fields (emoji/platform_hint/pii_safe) fall back to defaults.""" minimal = ( diff --git a/tests/gateway/test_platform_base.py b/tests/gateway/test_platform_base.py index 3db791f73fdd..064af7ae65cb 100644 --- a/tests/gateway/test_platform_base.py +++ b/tests/gateway/test_platform_base.py @@ -1651,6 +1651,28 @@ class TestTruncateMessage: assert chunks assert "😀" in "".join(chunks) + def test_sub_codepoint_budget_emits_whole_codepoints_without_data_loss(self): + """Length contract for a budget too small to fit one codepoint. + + A codepoint is indivisible, so with max_length=1 and utf16_len a 2-unit + emoji cannot fit — the loop emits it whole rather than dropping it or + spinning. The documented, intentional consequence is that such a chunk + EXCEEDS max_length by that one codepoint; in return every codepoint is + preserved (no data loss) and the call terminates. + """ + import re + + from gateway.platforms.base import utf16_len + + chunks = self._truncate_with_timeout("😀😀😀", 1, len_fn=utf16_len) + assert chunks + bodies = [re.sub(r"\s*\(\d+/\d+\)$", "", c) for c in chunks] + # No data loss: all three emojis survive across the chunks. + assert "".join(bodies).count("😀") == 3 + # Contract: a chunk carrying a 2-unit emoji necessarily exceeds the + # 1-unit budget — assert that explicitly so the behavior is pinned. + assert any(utf16_len(b) > 1 for b in bodies) + def test_code_block_first_chunk_closed(self): adapter = self._adapter() msg = "Before\n```python\n" + "x = 1\n" * 100 + "```\nAfter"