From 54932c9c03e60e754b20fafae8f1b00d866c23ed Mon Sep 17 00:00:00 2001 From: Keith Smith Date: Mon, 17 Aug 2026 18:01:56 -0600 Subject: [PATCH] Expand direct image links instead of showing nothing (#43 follow-up) A URL that points straight at an image file (Content-Type: image/*) has no HTML to scrape Open Graph tags from, so the fetch found nothing and the message showed no preview at all -- reported against https://imgs.xkcd.com/comics/creepy.png. link_preview_service now recognizes an allowed image content-type (same list storage.py uses for uploads) before falling through to the HTML/og: path, and returns the URL itself as the preview (LinkPreview.is_image). No need to download the body -- the already-SSRF-validated URL is the image. The frontend renders that case as a real expandable image (message-image + lightbox, same as an actual attachment) instead of the small title+description card, which would have nothing to show anyway. Verified end-to-end against the reported URL. Co-Authored-By: Claude Sonnet 5 --- ...2_link_previews_flag_direct_image_links.py | 35 +++++++++++++++++++ backend/app/models/link_preview.py | 6 ++++ backend/app/schemas/message.py | 1 + backend/app/services/link_preview_service.py | 32 +++++++++++++++-- backend/tests/test_link_previews.py | 34 ++++++++++++++++-- frontend/src/components/ChatPane.tsx | 1 + frontend/src/components/LinkPreviewCard.tsx | 19 +++++++++- frontend/src/components/MessageList.tsx | 4 ++- frontend/src/types.ts | 4 +++ 9 files changed, 129 insertions(+), 7 deletions(-) create mode 100644 backend/alembic/versions/f3ec1c1d1992_link_previews_flag_direct_image_links.py diff --git a/backend/alembic/versions/f3ec1c1d1992_link_previews_flag_direct_image_links.py b/backend/alembic/versions/f3ec1c1d1992_link_previews_flag_direct_image_links.py new file mode 100644 index 0000000..8a5d9b3 --- /dev/null +++ b/backend/alembic/versions/f3ec1c1d1992_link_previews_flag_direct_image_links.py @@ -0,0 +1,35 @@ +"""link previews flag direct image links + +Revision ID: f3ec1c1d1992 +Revises: d00f93766fa5 +Create Date: 2026-08-17 17:56:19.078087 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = 'f3ec1c1d1992' +down_revision: Union[str, Sequence[str], None] = 'd00f93766fa5' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + # NOT NULL with a *constant* server_default (unlike a volatile one such + # as now()/clock_timestamp()) doesn't force a table rewrite in Postgres + # 11+ -- it's a metadata-only change, safe and instant regardless of + # table size. + op.add_column( + "link_previews", + sa.Column("is_image", sa.Boolean(), nullable=False, server_default=sa.false()), + ) + + +def downgrade() -> None: + """Downgrade schema.""" + op.drop_column("link_previews", "is_image") diff --git a/backend/app/models/link_preview.py b/backend/app/models/link_preview.py index 6fa7a5f..0ab1ccb 100644 --- a/backend/app/models/link_preview.py +++ b/backend/app/models/link_preview.py @@ -23,6 +23,12 @@ class LinkPreview(Base): description: Mapped[str | None] = mapped_column(String(1000)) image_url: Mapped[str | None] = mapped_column(String(2048)) site_name: Mapped[str | None] = mapped_column(String(200)) + # A direct link to an image file (Content-Type: image/*, no HTML to + # scrape og: tags from) gets image_url = the URL itself and no + # title/description/site_name -- this flags that case so the frontend + # renders it as an actual expanded image rather than the small + # title+description unfurl card. + is_image: Mapped[bool] = mapped_column(Boolean, default=False, nullable=False) # Cached separately from a "no row yet" state so a URL that genuinely # doesn't unfurl (no title, fetch error, blocked by SSRF checks) isn't # re-fetched on every message that references it within the TTL. diff --git a/backend/app/schemas/message.py b/backend/app/schemas/message.py index 97c5a49..5733e58 100644 --- a/backend/app/schemas/message.py +++ b/backend/app/schemas/message.py @@ -27,6 +27,7 @@ class LinkPreviewInfo(BaseModel): description: str | None image_url: str | None site_name: str | None + is_image: bool class MessageRead(BaseModel): diff --git a/backend/app/services/link_preview_service.py b/backend/app/services/link_preview_service.py index 4dded5c..64331ca 100644 --- a/backend/app/services/link_preview_service.py +++ b/backend/app/services/link_preview_service.py @@ -11,6 +11,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.database import async_session_factory from app.models import LinkPreview from app.services.ssrf import UnsafeUrlError, validate_target_url +from app.storage import ALLOWED_IMAGE_CONTENT_TYPES from app.ws.broadcaster import Broadcaster logger = logging.getLogger(__name__) @@ -99,8 +100,21 @@ async def _fetch_preview_data(url: str) -> dict | None: continue if response.status_code >= 400: return None - content_type = response.headers.get("content-type", "") - if "text/html" not in content_type: + content_type = response.headers.get("content-type", "").split(";")[0].strip() + # A direct link to an image file -- render the image + # itself rather than trying to scrape og: tags from + # nonexistent HTML. No need to read the body at all; + # the URL (now fully resolved through any redirects, + # each already SSRF-validated above) *is* the preview. + if content_type in ALLOWED_IMAGE_CONTENT_TYPES: + return { + "title": None, + "description": None, + "image_url": current_url, + "site_name": None, + "is_image": True, + } + if content_type != "text/html": return None body = b"" async for chunk in response.aiter_bytes(): @@ -128,6 +142,7 @@ async def _fetch_preview_data(url: str) -> dict | None: "description": (parser.og.get("og:description") or "").strip()[:1000] or None, "image_url": parser.og.get("og:image") or None, "site_name": (parser.og.get("og:site_name") or "").strip()[:200] or None, + "is_image": False, } @@ -148,6 +163,7 @@ async def _get_or_fetch(db: AsyncSession, url: str) -> LinkPreview | None: existing.description = data["description"] existing.image_url = data["image_url"] existing.site_name = data["site_name"] + existing.is_image = data["is_image"] existing.fetched_at = datetime.now(timezone.utc) await db.commit() return None if data is None else existing @@ -155,7 +171,16 @@ async def _get_or_fetch(db: AsyncSession, url: str) -> LinkPreview | None: row = LinkPreview( url=url, fetch_failed=data is None, - **(data or {"title": None, "description": None, "image_url": None, "site_name": None}), + **( + data + or { + "title": None, + "description": None, + "image_url": None, + "site_name": None, + "is_image": False, + } + ), ) db.add(row) await db.commit() @@ -192,6 +217,7 @@ async def fetch_and_broadcast_link_preview( "description": preview.description, "image_url": preview.image_url, "site_name": preview.site_name, + "is_image": preview.is_image, }, ) diff --git a/backend/tests/test_link_previews.py b/backend/tests/test_link_previews.py index 64d7bb0..f7086ef 100644 --- a/backend/tests/test_link_previews.py +++ b/backend/tests/test_link_previews.py @@ -93,7 +93,7 @@ _OG_HTML = ( ) -def _fake_client_factory(html=_OG_HTML, status_code=200, call_log=None): +def _fake_client_factory(html=_OG_HTML, status_code=200, call_log=None, content_type="text/html"): class _FakeAsyncClient: def __init__(self, *args, **kwargs): pass @@ -107,7 +107,7 @@ def _fake_client_factory(html=_OG_HTML, status_code=200, call_log=None): def stream(self, method, url, headers=None): if call_log is not None: call_log.append(url) - return _FakeStreamCtx(_FakeResponse(status_code, {"content-type": "text/html"}, html)) + return _FakeStreamCtx(_FakeResponse(status_code, {"content-type": content_type}, html)) return _FakeAsyncClient @@ -155,6 +155,36 @@ def test_ws_message_with_url_triggers_link_preview_broadcast(ws_client, monkeypa assert preview["description"] == "A description of the article." assert preview["image_url"] == "https://8.8.8.8/image.png" assert preview["site_name"] == "Example" + assert preview["is_image"] is False + + +def test_ws_message_with_direct_image_url_expands_the_image(ws_client, monkeypatch): + monkeypatch.setattr( + "app.services.link_preview_service.httpx.AsyncClient", + _fake_client_factory(html=b"", content_type="image/png"), + ) + url = _unique_url() + ".png" + + username = _unique("alice") + _register_ws(ws_client, username=username) + room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json() + + with ws_client.websocket_connect("/ws/chat") as ws: + ws.send_json({"type": "join", "room_id": room["id"]}) + assert _recv(ws)["type"] == "joined" + + ws.send_json({"type": "message", "room_id": room["id"], "content": f"lol {url}"}) + message = _recv(ws) + assert message["link_preview"] is None + + preview = _recv(ws) + assert preview["type"] == "link_preview" + assert preview["url"] == url + assert preview["image_url"] == url + assert preview["is_image"] is True + assert preview["title"] is None + assert preview["description"] is None + assert preview["site_name"] is None def test_ws_message_with_private_url_gets_no_preview(ws_client, monkeypatch): diff --git a/frontend/src/components/ChatPane.tsx b/frontend/src/components/ChatPane.tsx index ed1f731..9ea1045 100644 --- a/frontend/src/components/ChatPane.tsx +++ b/frontend/src/components/ChatPane.tsx @@ -158,6 +158,7 @@ export function ChatPane({ description: envelope.description, image_url: envelope.image_url, site_name: envelope.site_name, + is_image: envelope.is_image, } setHistory((prev) => prev.map((m) => (m.id === envelope.id ? { ...m, link_preview: linkPreview } : m))) setLive((prev) => prev.map((m) => (m.id === envelope.id ? { ...m, link_preview: linkPreview } : m))) diff --git a/frontend/src/components/LinkPreviewCard.tsx b/frontend/src/components/LinkPreviewCard.tsx index 2d60d70..72179fd 100644 --- a/frontend/src/components/LinkPreviewCard.tsx +++ b/frontend/src/components/LinkPreviewCard.tsx @@ -3,13 +3,30 @@ import './LinkPreviewCard.css' interface LinkPreviewCardProps { preview: LinkPreviewInfo + onImageClick: (src: string) => void } // Slack/Discord-style unfurl card, rendered under a message's text when the // backend found a URL in it and successfully fetched Open Graph data for it // (see link_preview_service.py -- title/description/image_url/site_name are // all independently optional, since not every page sets every og: tag). -export function LinkPreviewCard({ preview }: LinkPreviewCardProps) { +export function LinkPreviewCard({ preview, onImageClick }: LinkPreviewCardProps) { + // A direct link to an image file has no title/description/site_name to + // show (there's no HTML page to scrape them from) -- render it the same + // way a real image attachment renders (message-image + lightbox) rather + // than the small unfurl card, which would otherwise show just a tiny + // thumbnail with no text to go with it. + if (preview.is_image && preview.image_url) { + return ( + onImageClick(preview.image_url!)} + /> + ) + } + return ( (edited)} )} - {msg.link_preview && } + {msg.link_preview && ( + + )} {msg.reactions.length > 0 && (
{msg.reactions.map((r) => { diff --git a/frontend/src/types.ts b/frontend/src/types.ts index e3edb51..b603174 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -114,6 +114,9 @@ export interface LinkPreviewInfo { description: string | null image_url: string | null site_name: string | null + // A direct link to an image file -- render the image itself (like a real + // attachment) rather than the small title+description unfurl card. + is_image: boolean } export interface Message { @@ -166,6 +169,7 @@ export interface ChatLinkPreviewEnvelope { description: string | null image_url: string | null site_name: string | null + is_image: boolean } export interface ChatReactionUpdateEnvelope {