Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 59 additions & 8 deletions backend/api/images.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from fastapi import APIRouter, Depends, HTTPException, Query, Response
from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response
from fastapi.responses import FileResponse, RedirectResponse
import hashlib
import httpx
Expand Down Expand Up @@ -50,7 +50,11 @@ def _card_back_response():
# Keep using the frontend's existing placeholder artwork. The backend only
# changes the missing-image response behavior; it must not replace the
# placeholder image data/design.
return RedirectResponse(url="/cardback.jpg", status_code=307)
return RedirectResponse(
url="/cardback.jpg",
status_code=307,
headers={"Cache-Control": "public, max-age=86400"},
)


def _set_fallback_response():
Expand All @@ -60,10 +64,17 @@ def _set_fallback_response():
return FileResponse(
_SET_FALLBACK_IMAGE,
media_type="image/svg+xml",
headers={"Cache-Control": "public, max-age=86400"},
headers={"Cache-Control": "public, max-age=2592000, immutable"},
)


def _matches_etag(if_none_match: str | None, etag: str) -> bool:
if not if_none_match:
return False
tokens = [token.strip() for token in if_none_match.split(",")]
return "*" in tokens or etag in tokens or f"W/{etag}" in tokens


def _is_globally_visible_card_image(db: Session, card: Card) -> bool:
if getattr(card, "is_custom", False):
# Manual card IDs use unguessable UUIDs. Access control is enforced on
Expand Down Expand Up @@ -179,6 +190,7 @@ def _get_or_fetch_custom_image(
def get_card_image(
card_id: str,
size: str,
request: Request = None,
db: Session = Depends(get_db),
):
if size not in ("small", "large"):
Expand All @@ -198,9 +210,26 @@ def get_card_image(
if not url:
return _card_back_response()

url_hash = hashlib.sha1(url.encode("utf-8")).hexdigest()
etag = f'"{url_hash}"'
is_custom_artwork = bool(card.is_custom or (card.custom_image_url and url == card.custom_image_url))
cache_control = (
"public, max-age=3600, must-revalidate"
if is_custom_artwork
else "public, max-age=2592000, immutable"
)

if request is not None and _matches_etag(request.headers.get("if-none-match"), etag):
return Response(
status_code=304,
headers={
"ETag": etag,
"Cache-Control": cache_control,
},
)

try:
if card.is_custom:
url_hash = hashlib.sha1(url.encode("utf-8")).hexdigest()
data, content_type = _get_or_fetch_custom_image(
db,
f"card:{card_id}:{size}:manual:{url_hash}",
Expand All @@ -210,19 +239,26 @@ def get_card_image(
elif card.custom_image_url and url == card.custom_image_url:
data, content_type = _get_or_fetch_custom_image(db, f"card:{card_id}:{size}:custom", url)
else:
url_hash = hashlib.sha1(url.encode("utf-8")).hexdigest()
data, content_type = _get_or_fetch(db, f"card:{card_id}:{size}:{url_hash}", url)
except (HTTPException, ValueError):
return _card_back_response()
return Response(
content=data,
media_type=content_type,
headers={"Cache-Control": "public, max-age=86400"},
headers={
"ETag": etag,
"Cache-Control": cache_control,
},
)


@router.get("/set/{set_id}/{image_type}")
def get_set_image(set_id: str, image_type: str, db: Session = Depends(get_db)):
def get_set_image(
set_id: str,
image_type: str,
request: Request = None,
db: Session = Depends(get_db),
):
if image_type not in ("logo", "symbol"):
raise HTTPException(status_code=400, detail="image_type must be logo or symbol")

Expand Down Expand Up @@ -250,6 +286,18 @@ def get_set_image(set_id: str, image_type: str, db: Session = Depends(get_db)):
if not url:
return _set_fallback_response()

url_hash = hashlib.sha1(url.encode("utf-8")).hexdigest()
etag = f'"{url_hash}"'

if request is not None and _matches_etag(request.headers.get("if-none-match"), etag):
return Response(
status_code=304,
headers={
"ETag": etag,
"Cache-Control": "public, max-age=2592000, immutable",
},
)

try:
data, content_type = _get_or_fetch(db, cache_key, url)
except HTTPException as exc:
Expand All @@ -259,7 +307,10 @@ def get_set_image(set_id: str, image_type: str, db: Session = Depends(get_db)):
return Response(
content=data,
media_type=content_type,
headers={"Cache-Control": "public, max-age=86400"},
headers={
"ETag": etag,
"Cache-Control": "public, max-age=2592000, immutable",
},
)


Expand Down
5 changes: 4 additions & 1 deletion backend/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
"postgresql://pokemon:changeme@localhost:5432/pokemon_tcg"
)

engine = create_engine(DATABASE_URL, pool_pre_ping=True, pool_size=10, max_overflow=20)
engine_kwargs = {"pool_pre_ping": True}
if not DATABASE_URL.startswith("sqlite"):
engine_kwargs.update({"pool_size": 10, "max_overflow": 20})
engine = create_engine(DATABASE_URL, **engine_kwargs)
SessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine)
Base = declarative_base()
logger = logging.getLogger(__name__)
Expand Down
196 changes: 196 additions & 0 deletions backend/tests/test_image_cache_headers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
import hashlib
import unittest
from unittest.mock import MagicMock, patch

try:
from fastapi import Request
from sqlalchemy import create_engine
from sqlalchemy.orm import sessionmaker

from api.images import get_card_image, get_set_image, _card_back_response, _set_fallback_response
from database import Base
from models import Card, Set, Setting

DEPS = True
except ModuleNotFoundError:
DEPS = False


def _make_request(headers=None):
scope = {
"type": "http",
"method": "GET",
"headers": [(k.lower().encode("latin-1"), v.encode("latin-1")) for k, v in (headers or {}).items()],
}
return Request(scope)


@unittest.skipUnless(DEPS, "FastAPI/SQLAlchemy not installed in this environment")
class ImageCacheHeadersTests(unittest.TestCase):
def setUp(self):
self.engine = create_engine("sqlite:///:memory:")
Base.metadata.create_all(self.engine)
self.Session = sessionmaker(bind=self.engine)
self.db = self.Session()

# Enable English sync language
self.db.add(Setting(key="tcgdex_sync_languages", value="en"))
self.db.commit()

def tearDown(self):
self.db.close()
Base.metadata.drop_all(self.engine)

@patch("api.images._get_or_fetch", return_value=(b"card-image-bytes", "image/webp"))
def test_card_image_returns_immutable_cache_control_and_etag(self, mock_fetch):
card = Card(
id="sv1-1_en",
name="Bulbasaur",
images_small="https://assets.example.test/sv1-1_small.webp",
images_large="https://assets.example.test/sv1-1_large.webp",
lang="en",
)
self.db.add(card)
self.db.commit()

expected_hash = hashlib.sha1(b"https://assets.example.test/sv1-1_small.webp").hexdigest()
expected_etag = f'"{expected_hash}"'

response = get_card_image("sv1-1_en", "small", db=self.db)

self.assertEqual(response.status_code, 200)
self.assertEqual(response.body, b"card-image-bytes")
self.assertEqual(response.headers["cache-control"], "public, max-age=2592000, immutable")
self.assertEqual(response.headers["etag"], expected_etag)

@patch("api.images._get_or_fetch")
def test_card_image_returns_304_when_if_none_match_matches(self, mock_fetch):
card = Card(
id="sv1-1_en",
name="Bulbasaur",
images_small="https://assets.example.test/sv1-1_small.webp",
images_large="https://assets.example.test/sv1-1_large.webp",
lang="en",
)
self.db.add(card)
self.db.commit()

expected_hash = hashlib.sha1(b"https://assets.example.test/sv1-1_small.webp").hexdigest()
expected_etag = f'"{expected_hash}"'
request = _make_request({"if-none-match": expected_etag})

response = get_card_image("sv1-1_en", "small", request=request, db=self.db)

self.assertEqual(response.status_code, 304)
self.assertEqual(response.body, b"")
self.assertEqual(response.headers["etag"], expected_etag)
self.assertEqual(response.headers["cache-control"], "public, max-age=2592000, immutable")
# Should not fetch blob when ETag matches
mock_fetch.assert_not_called()

@patch("api.images._get_or_fetch")
def test_card_image_accepts_weak_etag_and_comma_separated_if_none_match(self, mock_fetch):
card = Card(
id="sv1-1_en",
name="Bulbasaur",
images_small="https://assets.example.test/sv1-1_small.webp",
images_large="https://assets.example.test/sv1-1_large.webp",
lang="en",
)
self.db.add(card)
self.db.commit()

expected_hash = hashlib.sha1(b"https://assets.example.test/sv1-1_small.webp").hexdigest()
expected_etag = f'"{expected_hash}"'
weak_header = f'W/"other-etag", W/{expected_etag}'
request = _make_request({"if-none-match": weak_header})

response = get_card_image("sv1-1_en", "small", request=request, db=self.db)

self.assertEqual(response.status_code, 304)
self.assertEqual(response.headers["etag"], expected_etag)
mock_fetch.assert_not_called()

@patch("api.images._get_or_fetch_custom_image", return_value=(b"custom-card-bytes", "image/png"))
def test_custom_card_image_returns_revalidation_cache_control(self, mock_custom_fetch):
card = Card(
id="custom-1_en",
name="Custom Card",
is_custom=True,
images_small="https://assets.example.test/custom_small.png",
images_large="https://assets.example.test/custom_large.png",
lang="en",
)
self.db.add(card)
self.db.commit()

expected_hash = hashlib.sha1(b"https://assets.example.test/custom_small.png").hexdigest()
expected_etag = f'"{expected_hash}"'

response = get_card_image("custom-1_en", "small", db=self.db)

self.assertEqual(response.status_code, 200)
self.assertEqual(response.body, b"custom-card-bytes")
self.assertEqual(response.headers["cache-control"], "public, max-age=3600, must-revalidate")
self.assertEqual(response.headers["etag"], expected_etag)

@patch("api.images._get_or_fetch", return_value=(b"set-logo-bytes", "image/png"))
def test_set_image_returns_immutable_cache_control_and_etag(self, mock_fetch):
s = Set(
id="sv1_en",
tcg_set_id="sv1",
name="Scarlet & Violet",
images_logo="https://assets.example.test/sv1_logo.png",
lang="en",
)
self.db.add(s)
self.db.commit()

expected_hash = hashlib.sha1(b"https://assets.example.test/sv1_logo.png").hexdigest()
expected_etag = f'"{expected_hash}"'

response = get_set_image("sv1_en", "logo", db=self.db)

self.assertEqual(response.status_code, 200)
self.assertEqual(response.body, b"set-logo-bytes")
self.assertEqual(response.headers["cache-control"], "public, max-age=2592000, immutable")
self.assertEqual(response.headers["etag"], expected_etag)

@patch("api.images._get_or_fetch")
def test_set_image_returns_304_when_if_none_match_matches(self, mock_fetch):
s = Set(
id="sv1_en",
tcg_set_id="sv1",
name="Scarlet & Violet",
images_logo="https://assets.example.test/sv1_logo.png",
lang="en",
)
self.db.add(s)
self.db.commit()

expected_hash = hashlib.sha1(b"https://assets.example.test/sv1_logo.png").hexdigest()
expected_etag = f'"{expected_hash}"'
request = _make_request({"if-none-match": expected_etag})

response = get_set_image("sv1_en", "logo", request=request, db=self.db)

self.assertEqual(response.status_code, 304)
self.assertEqual(response.body, b"")
self.assertEqual(response.headers["etag"], expected_etag)
self.assertEqual(response.headers["cache-control"], "public, max-age=2592000, immutable")
mock_fetch.assert_not_called()

def test_card_back_fallback_redirect_has_cache_control(self):
resp = _card_back_response()
self.assertEqual(resp.status_code, 307)
self.assertEqual(resp.headers["cache-control"], "public, max-age=86400")
self.assertEqual(resp.headers["location"], "/cardback.jpg")

def test_set_fallback_response_has_immutable_cache_control(self):
resp = _set_fallback_response()
self.assertEqual(resp.status_code, 200)
self.assertEqual(resp.headers["cache-control"], "public, max-age=2592000, immutable")


if __name__ == "__main__":
unittest.main()
4 changes: 3 additions & 1 deletion frontend/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ COPY frontend/nginx.conf /etc/nginx/conf.d/default.conf
COPY frontend/docker-entrypoint.d/40-configure-public-mode.sh /docker-entrypoint.d/40-configure-public-mode.sh

RUN cp /usr/share/nginx/html/index.html /usr/share/nginx/html/index.html.template \
&& chmod +x /docker-entrypoint.d/40-configure-public-mode.sh
&& chmod +x /docker-entrypoint.d/40-configure-public-mode.sh \
&& mkdir -p /var/cache/nginx/images \
&& chown -R nginx:nginx /var/cache/nginx

EXPOSE 80

Expand Down
Loading
Loading