fix(photon): strip markdown when the sidecar downgrades to the text builder
chooseSendFormat() routes markdown containing a raw http(s) URL through spectrum-ts' text() builder, because the markdown builder's iMessage data detection 500s on those messages (#73615). text() ships the payload verbatim, and nothing strips the markers on the way down, so any reply that mentions a link arrives in iMessage as literal markdown source: **Release 1.2.0** is out - **EUR 5** off this month https://example.com/releases/1.2.0 Every ** is visible in the bubble. Remove the URL from that same reply and it renders correctly, which is what makes the URL the trigger rather than the content. spectrum-ts documents markdown() as degrading to readable plain text on platforms without native support "instead of surfacing raw ** markers". Selecting text() ourselves opts out of that guarantee, so the adapter has to honour it instead. Strip in _sidecar_send() when the payload is markdown and the sidecar will downgrade it. The format key is deliberately preserved: the sidecar owns the builder choice (test_rich_links.py pins that contract), and an older sidecar without chooseSendFormat must keep rendering natively. _send_plain_fallback() selects the text builder explicitly via markdown=False and had the same leak, so it strips too. Reuses the shared strip_markdown() helper rather than adding a second implementation, so the PHOTON_MARKDOWN=false path and this path produce identical output and both inherit any future fix to that helper. Diagnosis previously reported in #85733, which was closed unmerged. Stripping must not take the URL with it, though. The shared helper collapsed [label](url) to label alone, which on this path is worse than raw markdown: iMessage auto-links bare URLs and nothing else, so the reply arrives with a description and no way to reach the link. Open the itinerary on Google Flights <- URL gone entirely So strip_markdown() takes keep_link_targets, which rewrites [label](https://url) as "label\nurl" (own line, because these URLs are often long) and leaves non-http targets such as mailto: or relative paths label-only, since iMessage won't linkify those either. The default is unchanged, so the SMS, IRC, Feishu and QQ callers keep dropping the target as before. plugins/platforms/line/adapter.py already carries a private strip_markdown_preserving_urls() for exactly this reason ("LINE auto-links bare URLs only"). This moves the behaviour behind the shared helper instead, so Photon's three plain-text paths -- the downgrade, _send_plain_fallback(), and PHOTON_MARKDOWN=false -- all agree. gateway/platforms/bluebubbles.py is the same iMessage surface with the same loss; left alone here to keep this change to one platform.
This commit is contained in:
@@ -88,8 +88,9 @@ def bounded_put(store: MutableMapping[str, Any], key: str, value: Any, cap: int)
|
||||
del store[next(iter(store))]
|
||||
|
||||
|
||||
# Markdown-stripping rules, applied in order: bold, italic, bold/italic underscore,
|
||||
# code fence markers, inline code, headings, links, then newline squeeze.
|
||||
# Inline markdown-stripping rules, applied in order: bold, italic, bold/italic
|
||||
# underscore, code fence markers, inline code, headings. Links and the newline
|
||||
# squeeze run after these, in that order (see ``strip_markdown``).
|
||||
_STRIP_RULES = (
|
||||
(re.compile(r"\*\*(.+?)\*\*", re.DOTALL), r"\1"),
|
||||
(re.compile(r"\*(.+?)\*", re.DOTALL), r"\1"),
|
||||
@@ -98,16 +99,40 @@ _STRIP_RULES = (
|
||||
(re.compile(r"```[a-zA-Z0-9_+-]*\n?"), ""),
|
||||
(re.compile(r"`(.+?)`"), r"\1"),
|
||||
(re.compile(r"^#{1,6}\s+", re.MULTILINE), ""),
|
||||
(re.compile(r"\[([^\]]+)\]\([^\)]+\)"), r"\1"),
|
||||
(re.compile(r"\n{3,}"), "\n\n"),
|
||||
)
|
||||
_MD_LINK_RE = re.compile(r"\[([^\]]+)\]\(([^\)]+)\)")
|
||||
_HTTP_TARGET_RE = re.compile(r"https?://", re.IGNORECASE)
|
||||
_NEWLINE_SQUEEZE_RE = re.compile(r"\n{3,}")
|
||||
|
||||
|
||||
def strip_markdown(text: str) -> str:
|
||||
"""Strip markdown formatting for plain-text platforms (SMS, iMessage, etc.)."""
|
||||
def _drop_link_target(match: "re.Match[str]") -> str:
|
||||
return match.group(1)
|
||||
|
||||
|
||||
def _keep_link_target(match: "re.Match[str]") -> str:
|
||||
r"""``[label](https://url)`` -> ``label\nurl``.
|
||||
|
||||
The bare URL is the only thing a platform with its own data detection
|
||||
(iMessage) can turn back into a tap target, so dropping it makes the link
|
||||
unreachable rather than merely unformatted. Non-http targets (``mailto:``,
|
||||
relative paths) are not auto-linked, so they keep the label-only behaviour.
|
||||
"""
|
||||
label, target = match.group(1).strip(), match.group(2).strip()
|
||||
if not _HTTP_TARGET_RE.match(target):
|
||||
return match.group(1)
|
||||
return target if label == target else f"{label}\n{target}"
|
||||
|
||||
|
||||
def strip_markdown(text: str, *, keep_link_targets: bool = False) -> str:
|
||||
r"""Strip markdown formatting for plain-text platforms (SMS, iMessage, etc.).
|
||||
|
||||
``keep_link_targets`` rewrites ``[label](https://url)`` as ``label\nurl``
|
||||
instead of discarding the URL; pass it on platforms that auto-link bare URLs.
|
||||
"""
|
||||
for pattern, repl in _STRIP_RULES:
|
||||
text = pattern.sub(repl, text)
|
||||
return text.strip()
|
||||
text = _MD_LINK_RE.sub(_keep_link_target if keep_link_targets else _drop_link_target, text)
|
||||
return _NEWLINE_SQUEEZE_RE.sub("\n\n", text).strip()
|
||||
|
||||
|
||||
class ThreadParticipationTracker:
|
||||
|
||||
@@ -297,6 +297,32 @@ def _markdown_enabled() -> bool:
|
||||
return _get_scoped_secret("PHOTON_MARKDOWN", "true").strip().lower() not in {"false", "0", "no"}
|
||||
|
||||
|
||||
# Mirrors URL_RE in plugins/platforms/photon/sidecar/send-format.mjs. Keep the
|
||||
# two in sync: the sidecar owns the builder choice, this owns the payload that
|
||||
# choice implies.
|
||||
_SIDECAR_URL_RE = re.compile(r"https?://[^\s)'\"<>]+", re.IGNORECASE)
|
||||
|
||||
|
||||
def _strip_for_imessage(text: str) -> str:
|
||||
"""Strip markdown, but keep ``[label](url)`` targets as a bare URL on its own line.
|
||||
|
||||
Every plain-text path here ends at iMessage, which auto-links bare URLs and
|
||||
nothing else, so discarding a link target makes the link unreachable — see
|
||||
the ``chooseSendFormat`` comment for why these payloads lose markdown at all.
|
||||
"""
|
||||
return strip_markdown(text, keep_link_targets=True)
|
||||
|
||||
|
||||
def _sidecar_downgrades_to_text(text: str) -> bool:
|
||||
"""True when the sidecar will route a markdown payload to the text builder.
|
||||
|
||||
``chooseSendFormat`` sends markdown containing a raw http(s) URL through
|
||||
spectrum-ts' ``text()`` builder, because the markdown builder's iMessage
|
||||
data detection 500s on those messages.
|
||||
"""
|
||||
return bool(_SIDECAR_URL_RE.search(text or ""))
|
||||
|
||||
|
||||
def _url_only_candidate(text: str) -> Optional[str]:
|
||||
candidate = (text or "").strip()
|
||||
if not re.fullmatch(r"https?://\S+", candidate, flags=re.IGNORECASE):
|
||||
@@ -1293,7 +1319,7 @@ class PhotonAdapter(BasePlatformAdapter):
|
||||
|
||||
def format_message(self, content: str) -> str:
|
||||
# Markdown passes through verbatim (sidecar markdown() builder); PHOTON_MARKDOWN=false strips.
|
||||
return content if _markdown_enabled() else strip_markdown(content)
|
||||
return content if _markdown_enabled() else _strip_for_imessage(content)
|
||||
|
||||
@staticmethod
|
||||
def _is_retryable_error(error: Optional[str]) -> bool:
|
||||
@@ -1322,7 +1348,8 @@ class PhotonAdapter(BasePlatformAdapter):
|
||||
"""No Markdown banner (replies are markdown or already-stripped plain text); bypass
|
||||
richlink() so a rich-link outage doesn't strand a sendable URL."""
|
||||
return await self._sidecar_send(
|
||||
chat_id, self.format_message(content)[: self.MAX_MESSAGE_LENGTH], richlink=False, markdown=False)
|
||||
chat_id, _strip_for_imessage(self.format_message(content))[: self.MAX_MESSAGE_LENGTH],
|
||||
richlink=False, markdown=False)
|
||||
|
||||
async def _post_send(self, path: str, body: Dict[str, Any], *, structured: bool = False) -> SendResult:
|
||||
"""POST a send-like body and wrap the outcome as a SendResult. ``structured`` carries
|
||||
@@ -1349,11 +1376,19 @@ class PhotonAdapter(BasePlatformAdapter):
|
||||
return rich_result
|
||||
logger.warning("[photon] rich-link send failed, falling back to plain text: %s", rich_result.error)
|
||||
markdown = False
|
||||
send_markdown = markdown and _markdown_enabled()
|
||||
if send_markdown and _sidecar_downgrades_to_text(text):
|
||||
# spectrum-ts' markdown() degrades to readable plain text on its own;
|
||||
# text() does not, and ships the source verbatim. Strip before the
|
||||
# sidecar downgrades, or iMessage renders literal ** markers.
|
||||
# The format key stays: the sidecar owns the builder choice, and an
|
||||
# older sidecar without chooseSendFormat must keep rendering natively.
|
||||
text = _strip_for_imessage(text)
|
||||
if len(text) > self.MAX_MESSAGE_LENGTH:
|
||||
logger.warning("[photon] truncating outbound from %d to %d chars", len(text), self.MAX_MESSAGE_LENGTH)
|
||||
text = text[: self.MAX_MESSAGE_LENGTH]
|
||||
body: Dict[str, Any] = {"spaceId": space_id, "text": text}
|
||||
if markdown and _markdown_enabled(): # key omitted when disabled: pre-`format` sidecars still accept
|
||||
if send_markdown: # key omitted when disabled: pre-`format` sidecars still accept
|
||||
body["format"] = "markdown"
|
||||
return await self._post_send("/send", body, structured=True)
|
||||
|
||||
|
||||
39
tests/gateway/test_strip_markdown_links.py
Normal file
39
tests/gateway/test_strip_markdown_links.py
Normal file
@@ -0,0 +1,39 @@
|
||||
"""``strip_markdown`` link handling.
|
||||
|
||||
The default drops a link target (SMS, IRC, Feishu, QQ all rely on that);
|
||||
``keep_link_targets=True`` leaves the bare URL behind for platforms whose own
|
||||
data detection is the only thing that can re-linkify it.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from gateway.platforms.helpers import strip_markdown
|
||||
|
||||
_LINK = "[Open the itinerary](https://example.com/i?tfs=CBwQ&hl=en)"
|
||||
|
||||
|
||||
def test_link_target_is_dropped_by_default() -> None:
|
||||
assert strip_markdown(f"see {_LINK} now") == "see Open the itinerary now"
|
||||
|
||||
|
||||
def test_keep_link_targets_puts_the_url_on_its_own_line() -> None:
|
||||
assert strip_markdown(_LINK, keep_link_targets=True) == (
|
||||
"Open the itinerary\nhttps://example.com/i?tfs=CBwQ&hl=en"
|
||||
)
|
||||
|
||||
|
||||
def test_keep_link_targets_does_not_duplicate_a_self_labelled_link() -> None:
|
||||
url = "https://example.com/x"
|
||||
assert strip_markdown(f"[{url}]({url})", keep_link_targets=True) == url
|
||||
|
||||
|
||||
def test_keep_link_targets_ignores_non_http_targets() -> None:
|
||||
"""``mailto:``/relative targets are not auto-linked, so emitting them is noise."""
|
||||
text = "[mail](mailto:a@example.com) and [rel](/docs/page)"
|
||||
assert strip_markdown(text, keep_link_targets=True) == "mail and rel"
|
||||
|
||||
|
||||
def test_keep_link_targets_still_strips_inline_formatting() -> None:
|
||||
text = f"**Release 1.2.0**\n- `code` here\n{_LINK}"
|
||||
out = strip_markdown(text, keep_link_targets=True)
|
||||
assert "**" not in out and "`" not in out
|
||||
assert out.endswith("https://example.com/i?tfs=CBwQ&hl=en")
|
||||
@@ -103,3 +103,111 @@ async def test_standalone_send_includes_markdown_format(
|
||||
|
||||
assert result.get("success") is True
|
||||
assert posted[0][1]["format"] == "markdown"
|
||||
|
||||
|
||||
_MD_WITH_URL = (
|
||||
"**Release 1.2.0** is out\n"
|
||||
"- **\u20ac5** off this month\n"
|
||||
"https://example.com/releases/1.2.0"
|
||||
)
|
||||
_MD_WITH_LINK = "**Release 1.2.0** is out\n[Read the notes](https://example.com/releases/1.2.0)"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_url_bearing_markdown_is_stripped_before_the_text_builder(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""A markdown payload the sidecar downgrades must not ship raw markers.
|
||||
|
||||
``chooseSendFormat`` routes markdown containing a raw URL through
|
||||
spectrum-ts' ``text()`` builder, which sends the source verbatim.
|
||||
"""
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
calls = _capture_sidecar(adapter)
|
||||
|
||||
await adapter.send("space-1", _MD_WITH_URL)
|
||||
|
||||
path, body = calls[-1]
|
||||
assert path == "/send"
|
||||
# The format key is deliberately preserved: the sidecar owns the builder
|
||||
# choice (see test_rich_links.py). Only the payload changes.
|
||||
assert body["format"] == "markdown"
|
||||
assert "**" not in body["text"]
|
||||
# Stripping must not damage the parts iMessage still needs.
|
||||
assert "https://example.com/releases/1.2.0" in body["text"]
|
||||
assert "\u20ac5" in body["text"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_url_free_markdown_still_renders_natively(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
calls = _capture_sidecar(adapter)
|
||||
|
||||
await adapter.send("space-1", _MD)
|
||||
|
||||
_, body = calls[-1]
|
||||
assert body["format"] == "markdown"
|
||||
assert body["text"] == _MD, "URL-free markdown must reach markdown() untouched"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_plain_fallback_strips_markdown(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""The explicit fallback selects the text builder, so it must strip too."""
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
calls = _capture_sidecar(adapter)
|
||||
|
||||
await adapter._send_plain_fallback("space-1", _MD, reply_to=None, metadata=None)
|
||||
|
||||
_, body = calls[-1]
|
||||
assert "format" not in body
|
||||
assert "**" not in body["text"]
|
||||
assert "`" not in body["text"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_markdown_link_keeps_a_tappable_url(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""A ``[label](url)`` link must not lose its URL on the way to iMessage.
|
||||
|
||||
The URL inside ``](...)`` is enough for ``chooseSendFormat`` to pick the text
|
||||
builder, so this payload gets stripped — and iMessage auto-links bare URLs
|
||||
only. Dropping the target (the shared default) leaves the link unreachable.
|
||||
"""
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
calls = _capture_sidecar(adapter)
|
||||
|
||||
await adapter.send("space-1", _MD_WITH_LINK)
|
||||
|
||||
_, body = calls[-1]
|
||||
assert body["text"] == (
|
||||
"Release 1.2.0 is out\nRead the notes\nhttps://example.com/releases/1.2.0"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_plain_fallback_keeps_link_urls(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
calls = _capture_sidecar(adapter)
|
||||
|
||||
await adapter._send_plain_fallback("space-1", _MD_WITH_LINK, reply_to=None, metadata=None)
|
||||
|
||||
_, body = calls[-1]
|
||||
assert "**" not in body["text"]
|
||||
assert "https://example.com/releases/1.2.0" in body["text"]
|
||||
|
||||
|
||||
def test_markdown_disabled_keeps_link_urls(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""``PHOTON_MARKDOWN=false`` strips through the same iMessage-aware helper."""
|
||||
monkeypatch.setenv("PHOTON_MARKDOWN", "false")
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
|
||||
assert adapter.format_message(_MD_WITH_LINK) == (
|
||||
"Release 1.2.0 is out\nRead the notes\nhttps://example.com/releases/1.2.0"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user