diff --git a/backend/api/images.py b/backend/api/images.py index 68e85e90..ddd6ba88 100644 --- a/backend/api/images.py +++ b/backend/api/images.py @@ -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 @@ -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(): @@ -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 @@ -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"): @@ -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}", @@ -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") @@ -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: @@ -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", + }, ) diff --git a/backend/database.py b/backend/database.py index 8a5873cb..edf02358 100644 --- a/backend/database.py +++ b/backend/database.py @@ -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__) diff --git a/backend/tests/test_image_cache_headers.py b/backend/tests/test_image_cache_headers.py new file mode 100644 index 00000000..662ef18d --- /dev/null +++ b/backend/tests/test_image_cache_headers.py @@ -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() diff --git a/frontend/Dockerfile b/frontend/Dockerfile index b78f604f..11b9189f 100644 --- a/frontend/Dockerfile +++ b/frontend/Dockerfile @@ -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 diff --git a/frontend/nginx.conf b/frontend/nginx.conf index 43ac1fd0..9f74b8c9 100644 --- a/frontend/nginx.conf +++ b/frontend/nginx.conf @@ -1,3 +1,11 @@ +# Cache zone for card and set images proxied from backend to eliminate internal Docker network I/O +proxy_cache_path /var/cache/nginx/images + levels=1:2 + keys_zone=image_cache:20m + max_size=1g + inactive=30d + use_temp_path=off; + server { listen 80; server_name _; @@ -64,6 +72,34 @@ server { client_max_body_size 100M; } + # Cache card and set images from backend to avoid redundant multi-hop I/O + location ^~ /api/images/ { + proxy_pass http://backend:8000; + proxy_http_version 1.1; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_read_timeout 60s; + proxy_connect_timeout 30s; + proxy_send_timeout 60s; + + proxy_cache image_cache; + proxy_cache_key "$scheme$request_method$host$request_uri"; + proxy_cache_valid 200 30d; + proxy_cache_valid 301 302 307 7d; + proxy_cache_valid 404 1m; + proxy_cache_use_stale error timeout updating http_500 http_502 http_503 http_504; + proxy_cache_lock on; + proxy_cache_lock_timeout 5s; + + # Keep security headers on responses (locations with add_header do not inherit server-level add_header) + add_header X-Cache-Status $upstream_cache_status always; + add_header X-Frame-Options "SAMEORIGIN" always; + add_header X-Content-Type-Options "nosniff" always; + add_header Referrer-Policy "strict-origin-when-cross-origin" always; + add_header Permissions-Policy "camera=(self), microphone=()" always; + } + # Proxy API requests to backend location ^~ /api/ { proxy_pass http://backend:8000; diff --git a/frontend/src/components/CardImage.jsx b/frontend/src/components/CardImage.jsx index b2577bbb..95274c51 100644 --- a/frontend/src/components/CardImage.jsx +++ b/frontend/src/components/CardImage.jsx @@ -205,7 +205,7 @@ export default function CardImage({ alt={alt} className={className || 'w-full h-full object-cover'} style={{ ...(src ? {} : { opacity: 0.8 }), ...style }} - loading={loading === 'lazy' ? 'eager' : loading} + loading={loading} onError={handleError} onLoad={() => setStatus(previous => ({ source: src,