From 8dc60dbf5c2f1d1d92ead1b3b149a39c8c0e9414 Mon Sep 17 00:00:00 2001 From: Lukas Remis Date: Wed, 30 Sep 2026 21:35:26 +0200 Subject: [PATCH] Avatars: a sender without one logged a console error; phone list rows spaced 20px A missing avatar was a 404, which the browser reports as an error for every such sender on every page. It is now an empty 204: the still fails to decode it and the initials show through, but nothing is logged, and the miss carries the same one-day browser cache as a hit so it stops being re-fetched. On a phone the mail list's rows sit 20px apart instead of 7px. Co-Authored-By: Claude Opus 5.5 --- backend/app/api/avatar.py | 23 ++++++++++++-------- backend/tests/api/test_avatar.py | 9 +++++--- frontend/src/components/SenderAvatar.tsx | 2 +- frontend/src/components/list/MessageList.tsx | 5 +++-- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/backend/app/api/avatar.py b/backend/app/api/avatar.py index 0c75375..199005a 100644 --- a/backend/app/api/avatar.py +++ b/backend/app/api/avatar.py @@ -3,8 +3,9 @@ The browser only ever calls ``GET /api/avatar`` (same origin), so a sender can never see the user's IP or read activity: the server does the outbound fetch under the §10.6 SSRF guard and caches the result (both hits and misses) in Redis. A miss -returns 404, which the UI renders as its initials avatar. Disable with -``SENDER_AVATARS_ENABLED=false``. +returns 204 No Content: the fails to decode it and the UI falls back to its +initials, without the browser logging an error for every sender who has no avatar +(a 404 did, filling the console). Disable with ``SENDER_AVATARS_ENABLED=false``. """ from __future__ import annotations @@ -25,6 +26,12 @@ _CACHE_TTL_HIT = 7 * 24 * 3600 # remember a found avatar for a week _CACHE_TTL_MISS = 24 * 3600 # re-check "no avatar" daily _MAX_BYTES = 200 * 1024 _NONE = b"none" +# The browser keeps an answer as long as we would, so it stops re-asking per page. +_BROWSER_CACHE = {"Cache-Control": "private, max-age=86400"} + + +def _no_avatar() -> Response: + return Response(status_code=204, headers=_BROWSER_CACHE) def _email_md5(email: str) -> str: @@ -64,29 +71,27 @@ async def _resolve(email: str, md5: str) -> tuple[str, bytes] | None: async def sender_avatar(_user: CurrentUser, email: str = Query(...)) -> Response: email = email.strip().lower() if not get_settings().sender_avatars_enabled or "@" not in email: - return Response(status_code=404) + return _no_avatar() # Binary-safe client: the cache holds raw image bytes (and a b"none" marker). redis = get_redis_bytes() md5 = _email_md5(email) key = f"avatar:{md5}" - headers = {"Cache-Control": "private, max-age=86400"} - cached: bytes | None = await redis.get(key) if cached == _NONE: - return Response(status_code=404) + return _no_avatar() if cached: cached_ctype, _, cached_body = cached.partition(b"\n") return Response( content=cached_body, media_type=cached_ctype.decode("ascii", "ignore"), - headers=headers, + headers=_BROWSER_CACHE, ) result = await _resolve(email, md5) if result is None: await redis.set(key, _NONE, ex=_CACHE_TTL_MISS) - return Response(status_code=404) + return _no_avatar() ctype, body = result await redis.set(key, ctype.encode("ascii", "ignore") + b"\n" + body, ex=_CACHE_TTL_HIT) - return Response(content=body, media_type=ctype, headers=headers) + return Response(content=body, media_type=ctype, headers=_BROWSER_CACHE) diff --git a/backend/tests/api/test_avatar.py b/backend/tests/api/test_avatar.py index 0ad4eac..0235cf8 100644 --- a/backend/tests/api/test_avatar.py +++ b/backend/tests/api/test_avatar.py @@ -2,7 +2,7 @@ The shared Redis client decodes replies as text, so the cache of raw image bytes broke on the second request for any sender — a 500 instead of the image -(or instead of the 404 that means "draw initials"). +(or instead of the 204 that means "draw initials"). """ from __future__ import annotations @@ -68,7 +68,7 @@ async def test_cached_avatar_is_served_again( assert calls == 1 # the second came from the cache -async def test_cached_miss_is_a_404_again( +async def test_cached_miss_is_a_204_again( client: httpx.AsyncClient, monkeypatch: pytest.MonkeyPatch ) -> None: calls = 0 @@ -81,5 +81,8 @@ async def test_cached_miss_is_a_404_again( monkeypatch.setattr(avatar, "_resolve", fake_resolve) for _ in range(2): resp = await client.get("/api/avatar", params={"email": "nobody@example.com"}) - assert resp.status_code == 404 + # 204, not 404: a miss is the normal case, and a 404 put a console + # error in the browser for every sender without an avatar. + assert resp.status_code == 204 + assert "max-age" in resp.headers["cache-control"] assert calls == 1 diff --git a/frontend/src/components/SenderAvatar.tsx b/frontend/src/components/SenderAvatar.tsx index b5162ac..e074ec3 100644 --- a/frontend/src/components/SenderAvatar.tsx +++ b/frontend/src/components/SenderAvatar.tsx @@ -2,7 +2,7 @@ import { useState } from "react"; // Overlay for a server-fetched sender avatar (Gravatar / domain favicon). Drop it // inside an existing initials circle: on success the image covers the initials; on a -// miss (404/error) it renders nothing so the initials show through. The parent must +// miss (an empty 204, or an error) it renders nothing so the initials show through. The parent must // be `position: relative` and round. Fetched via /api/avatar (same origin), so the // sender never sees the user's IP or read activity. export function AvatarImage({ email }: { email?: string | null }) { diff --git a/frontend/src/components/list/MessageList.tsx b/frontend/src/components/list/MessageList.tsx index 9e1606b..2090634 100644 --- a/frontend/src/components/list/MessageList.tsx +++ b/frontend/src/components/list/MessageList.tsx @@ -146,7 +146,8 @@ export function MessageList({ const virtualizer = useVirtualizer({ count: threads.length, getScrollElement: () => parentRef.current, - estimateSize: () => 75, // 68 row + 7px gap + // 68 row + the gap above it: 7px, 20px on a phone. + estimateSize: () => 68 + (compact ? 20 : 7), overscan: 8, }); @@ -203,7 +204,7 @@ export function MessageList({ transform: `translateY(${item.start}px)`, }} // A row leaving the list lets the ones below glide up into its place. - className={`group absolute inset-x-0 px-2 pt-[7px] motion-safe:transition-transform motion-safe:duration-200 motion-safe:ease-out ${ + className={`group absolute inset-x-0 px-2 ${compact ? "pt-5" : "pt-[7px]"} motion-safe:transition-transform motion-safe:duration-200 motion-safe:ease-out ${ dimmed ? "pointer-events-none opacity-40" : "cursor-pointer" }`} >