Avatars: a cached avatar or miss failed on every later request

The shared Redis client decodes replies as UTF-8 text, but the avatar cache
holds raw image bytes and a b"none" miss marker: an image could not be read
back at all, and a miss came back as str and crashed on .partition(b"\n").
Only a sender's first request worked; after that, a 500. The cache now goes
through a binary-safe client, get_redis_bytes().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

backend/app/api/avatar.py

@@ -17,7 +17,7 @@ from fastapi import APIRouter, Query, Response
 from app.accounts.ssrf import SsrfError, guarded_request
 from app.config import get_settings
 from app.identity.deps import CurrentUser
-from app.redis_client import get_redis
+from app.redis_client import get_redis_bytes
 
 router = APIRouter(prefix="/avatar", tags=["mail"])
 
@@ -66,7 +66,8 @@ async def sender_avatar(_user: CurrentUser, email: str = Query(...)) -> Response
     if not get_settings().sender_avatars_enabled or "@" not in email:
         return Response(status_code=404)
 
-    redis = get_redis()
+    # 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"}

backend/app/redis_client.py

@@ -20,6 +20,7 @@ def aw(result: Any) -> Awaitable[Any]:
 
 
 _client: aioredis.Redis | None = None
+_bytes_client: aioredis.Redis | None = None
 
 
 def get_redis() -> aioredis.Redis:
@@ -34,9 +35,25 @@ def get_redis() -> aioredis.Redis:
     return _client
 
 
+def get_redis_bytes() -> aioredis.Redis:
+    """Return the process-wide Redis client for binary values.
+
+    ``get_redis()`` decodes every reply as UTF-8 text, which fails on (or
+    silently changes the type of) raw bytes such as a cached image. Anything that
+    stores bytes goes through this one instead.
+    """
+    global _bytes_client
+    if _bytes_client is None:
+        _bytes_client = aioredis.Redis.from_url(get_settings().redis_url, decode_responses=False)
+    return _bytes_client
+
+
 async def close_redis() -> None:
-    """Close the Redis client (used on app shutdown)."""
+    """Close the Redis clients (used on app shutdown)."""
-    global _client
+    global _client, _bytes_client
     if _client is not None:
         await _client.aclose()
         _client = None
+    if _bytes_client is not None:
+        await _bytes_client.aclose()
+        _bytes_client = None

backend/tests/api/test_avatar.py

@@ -0,0 +1,85 @@
+"""Sender avatar cache (SPEC §9.1): a cached hit or miss must be served again.
+
+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").
+"""
+
+from __future__ import annotations
+
+import uuid
+from collections.abc import AsyncIterator
+
+import httpx
+import pytest
+import pytest_asyncio
+from httpx import ASGITransport
+from sqlalchemy import delete
+from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine
+from sqlalchemy.pool import NullPool
+
+from app.api import avatar
+from app.config import get_settings
+from app.identity.passwords import hash_password
+from app.identity.sessions import create_session
+from app.main import create_app
+from app.models import User
+
+# A PNG signature: not valid UTF-8, exactly what a text-decoding client chokes on.
+PNG = b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\xff\xfe"
+
+
+@pytest_asyncio.fixture
+async def client() -> AsyncIterator[httpx.AsyncClient]:
+    engine = create_async_engine(get_settings().database_url, poolclass=NullPool)
+    session = async_sessionmaker(engine, expire_on_commit=False)()
+    user = User(handle=f"av{uuid.uuid4().hex[:8]}@test.local", password_hash=hash_password("pw"))
+    session.add(user)
+    await session.commit()
+    token = await create_session(user.id, "pytest")
+    async with httpx.AsyncClient(
+        transport=ASGITransport(app=create_app()),
+        base_url="http://testserver",
+        cookies={"session": token},
+    ) as c:
+        yield c
+    await session.execute(delete(User).where(User.id == user.id))
+    await session.commit()
+    await session.close()
+    await engine.dispose()
+
+
+async def test_cached_avatar_is_served_again(
+    client: httpx.AsyncClient, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    calls = 0
+
+    async def fake_resolve(email: str, md5: str) -> tuple[str, bytes] | None:
+        nonlocal calls
+        calls += 1
+        return "image/png", PNG
+
+    monkeypatch.setattr(avatar, "_resolve", fake_resolve)
+    for _ in range(2):
+        resp = await client.get("/api/avatar", params={"email": "alice@example.com"})
+        assert resp.status_code == 200
+        assert resp.content == PNG
+        assert resp.headers["content-type"] == "image/png"
+    assert calls == 1  # the second came from the cache
+
+
+async def test_cached_miss_is_a_404_again(
+    client: httpx.AsyncClient, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    calls = 0
+
+    async def fake_resolve(email: str, md5: str) -> tuple[str, bytes] | None:
+        nonlocal calls
+        calls += 1
+        return None
+
+    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
+    assert calls == 1