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 <img> 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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 <img> 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)
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user