Skip to content

Commit ff2aa0d

Browse files
authored
Merge pull request #3700 from rommapp/posthog-code/display-media-files-in-details-tab
feat(roms): serve and display media files inline in details tab
2 parents d4bdfef + 80a1ea1 commit ff2aa0d

25 files changed

Lines changed: 504 additions & 254 deletions

File tree

backend/endpoints/responses/rom.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,6 @@ class RomSchema(BaseModel):
312312
url_cover: str | None
313313

314314
has_manual: bool
315-
has_manual_files: bool
316315
has_soundtrack: bool
317316
path_manual: str | None
318317
url_manual: str | None

backend/endpoints/roms/files.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from logger.logger import log
1919
from models.rom import RomFileCategory
2020
from utils.audio_tags import guess_audio_media_type
21+
from utils.media_types import guess_media_file_type, is_allowed_media_file
2122
from utils.nginx import FileRedirectResponse
2223
from utils.router import APIRouter
2324

@@ -98,13 +99,17 @@ async def get_romfile_content(
9899
# record, never from the client-supplied file_name path param — otherwise a
99100
# caller could request the same bytes with an arbitrary extension to force a
100101
# mismatched Content-Type while served inline (content-sniffing/XSS).
101-
is_audio = file.category == RomFileCategory.SOUNDTRACK
102-
media_type = (
103-
guess_audio_media_type(file.file_name)
104-
if is_audio
105-
else "application/octet-stream"
106-
)
107-
disposition = "inline" if is_audio else "attachment"
102+
# Audio, images and videos are served inline so <audio>/<video>/<img> in the
103+
# details view can render and seek them; everything else downloads.
104+
if file.category == RomFileCategory.SOUNDTRACK:
105+
media_type = guess_audio_media_type(file.file_name)
106+
disposition = "inline"
107+
elif is_allowed_media_file(file.file_name):
108+
media_type = guess_media_file_type(file.file_name)
109+
disposition = "inline"
110+
else:
111+
media_type = "application/octet-stream"
112+
disposition = "attachment"
108113

109114
# Serve the file directly in development mode for emulatorjs
110115
if DEV_MODE:

backend/endpoints/roms/screenshot.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@
2020
from logger.formatter import highlight as hl
2121
from logger.logger import log
2222
from models.rom import RomFile, RomFileCategory
23-
from utils.router import APIRouter
24-
from utils.screenshots import (
25-
ALLOWED_SCREENSHOT_EXTENSIONS,
26-
is_allowed_screenshot_file,
23+
from utils.media_types import (
24+
ALLOWED_IMAGE_EXTENSIONS,
25+
is_allowed_image_file,
2726
)
27+
from utils.router import APIRouter
2828

2929
router = APIRouter()
3030

@@ -79,12 +79,12 @@ async def add_rom_screenshots(
7979
detail="Upload filename must be a plain file name, not a path",
8080
)
8181

82-
if not is_allowed_screenshot_file(safe_filename):
82+
if not is_allowed_image_file(safe_filename):
8383
raise HTTPException(
8484
status_code=status.HTTP_400_BAD_REQUEST,
8585
detail=(
8686
f"Unsupported image file type. Allowed: "
87-
f"{', '.join(sorted(ALLOWED_SCREENSHOT_EXTENSIONS))}"
87+
f"{', '.join(sorted(ALLOWED_IMAGE_EXTENSIONS))}"
8888
),
8989
)
9090

backend/endpoints/screenshots.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@
1717
from logger.formatter import highlight as hl
1818
from logger.logger import log
1919
from utils.filesystem import sanitize_filename
20-
from utils.router import APIRouter
21-
from utils.screenshots import (
22-
ALLOWED_SCREENSHOT_EXTENSIONS,
23-
is_allowed_screenshot_file,
20+
from utils.media_types import (
21+
ALLOWED_IMAGE_EXTENSIONS,
22+
is_allowed_image_file,
2423
)
24+
from utils.router import APIRouter
2525

2626
router = APIRouter(
2727
prefix="/screenshots",
@@ -69,12 +69,12 @@ async def add_screenshot(
6969
detail=f"Invalid screenshot filename: {str(exc)}",
7070
) from exc
7171

72-
if not is_allowed_screenshot_file(sanitized_screenshot_filename):
72+
if not is_allowed_image_file(sanitized_screenshot_filename):
7373
raise HTTPException(
7474
status_code=status.HTTP_400_BAD_REQUEST,
7575
detail=(
7676
f"Unsupported image file type. Allowed: "
77-
f"{', '.join(sorted(ALLOWED_SCREENSHOT_EXTENSIONS))}"
77+
f"{', '.join(sorted(ALLOWED_IMAGE_EXTENSIONS))}"
7878
),
7979
)
8080

backend/handler/database/roms_handler.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,6 @@ def wrapper(*args, **kwargs):
236236
selectinload(Rom.notes),
237237
undefer(Rom.multi_file),
238238
undefer(Rom.top_level_file_count),
239-
undefer(Rom.has_manual_files),
240239
undefer(Rom.has_soundtrack),
241240
)
242241
return func(*args, **kwargs)
@@ -876,7 +875,6 @@ def filter_roms(
876875
query = query.options(
877876
undefer(Rom.multi_file),
878877
undefer(Rom.top_level_file_count),
879-
undefer(Rom.has_manual_files),
880878
undefer(Rom.has_soundtrack),
881879
)
882880

backend/handler/filesystem/assets_handler.py

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import zipfile
55
from mimetypes import guess_type
66
from pathlib import Path
7-
from typing import Final
87

98
import magic
109
from fastapi import HTTPException, UploadFile, status
@@ -13,18 +12,10 @@
1312
from config import ASSETS_BASE_PATH
1413
from logger.logger import log
1514
from models.user import User
15+
from utils.media_types import IMAGE_EXT_BY_MIME_TYPE
1616

1717
from .base_handler import FSHandler
1818

19-
# Image MIME types we trust to (a) accept as avatar uploads and (b) serve
20-
# inline from the raw asset endpoint
21-
SAFE_IMAGE_MIME_TYPES: Final[dict[str, str]] = {
22-
"image/png": "png",
23-
"image/jpeg": "jpg",
24-
"image/webp": "webp",
25-
"image/gif": "gif",
26-
}
27-
2819
# libmagic loads its database on construction (~few MB read from disk), so we
2920
# share a single Magic instance across requests. The underlying magic_t handle
3021
# is not thread-safe, so guard from_buffer with a lock. Endpoints that call
@@ -38,7 +29,7 @@ def validate_image_upload(upload: UploadFile, *, label: str = "Image") -> str:
3829
3930
Sniffs the leading bytes with libmagic and returns the trusted extension
4031
matching the detected MIME type. Raises HTTPException(400) if the file
41-
is not a recognized PNG/JPEG/WebP/GIF, or if MIME sniffing fails.
32+
is not a recognized image, or if MIME sniffing fails.
4233
Leaves the file cursor at 0.
4334
"""
4435
upload.file.seek(0)
@@ -55,7 +46,7 @@ def validate_image_upload(upload: UploadFile, *, label: str = "Image") -> str:
5546
detail=f"Could not determine {label.lower()} file type",
5647
) from exc
5748

58-
safe_extension = SAFE_IMAGE_MIME_TYPES.get(detected_mime)
49+
safe_extension = IMAGE_EXT_BY_MIME_TYPE.get(detected_mime)
5950
if not safe_extension:
6051
raise HTTPException(
6152
status_code=status.HTTP_400_BAD_REQUEST,
@@ -76,7 +67,7 @@ def build_asset_file_response(
7667
avatar) can't execute as stored XSS."""
7768
download_name = filename or resolved_path.name
7869
guessed_type, _ = guess_type(download_name)
79-
if guessed_type in SAFE_IMAGE_MIME_TYPES:
70+
if guessed_type in IMAGE_EXT_BY_MIME_TYPE:
8071
return FileResponse(
8172
path=str(resolved_path),
8273
filename=download_name,

backend/handler/metadata/launchbox_handler/media.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,17 @@
3030
sanitize_filename,
3131
)
3232

33-
VIDEO_EXTS: tuple[str, ...] = (".mp4", ".webm", ".avi", ".mkv", ".mov", ".wmv")
33+
# Video container extensions recognized when probing for local LaunchBox media
34+
# and when validating a metadata video URL's suffix. A tuple because the probe
35+
# order is a preference order.
36+
RECOGNIZED_VIDEO_EXTENSIONS: tuple[str, ...] = (
37+
".mp4",
38+
".webm",
39+
".avi",
40+
".mkv",
41+
".mov",
42+
".wmv",
43+
)
3444

3545

3646
def local_media_req(
@@ -356,7 +366,7 @@ def _get_video(req: MediaRequest) -> str | None:
356366
return None
357367

358368
for stem in ctx["stems"]:
359-
for ext in VIDEO_EXTS:
369+
for ext in RECOGNIZED_VIDEO_EXTENSIONS:
360370
candidate = ctx["base"] / f"{stem}{ext}"
361371
if candidate.is_file():
362372
return file_uri_for_local_path(candidate)
@@ -553,7 +563,7 @@ def populate_rom_specific_paths(
553563
rom.platform_id, rom.id, MetadataMediaType.VIDEO
554564
)
555565
ext = Path(metadata["video_url"]).suffix.lower()
556-
if ext not in VIDEO_EXTS:
566+
if ext not in RECOGNIZED_VIDEO_EXTENSIONS:
557567
ext = ".mp4"
558568
metadata["video_path"] = f"{base}/video{ext}"
559569
return metadata

backend/models/rom.py

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -443,30 +443,6 @@ def full_path(self) -> str:
443443
def has_manual(self) -> bool:
444444
return bool(self.path_manual)
445445

446-
# `has_manual_files` and `has_soundtrack` come from correlated
447-
# `EXISTS` subqueries against rom_files filtered by category.
448-
# `@declared_attr` lets us define the column_property inside the
449-
# class while still referencing `cls.id` (resolved after the
450-
# mapping is built). Deferred + opt-in via `undefer` from the
451-
# gallery query so we don't force a `Rom.files` load (the
452-
# relationship is `lazy="raise"` for that endpoint).
453-
@declared_attr
454-
def has_manual_files(cls) -> Mapped[bool]:
455-
return column_property(
456-
select(RomFile.id)
457-
.where(
458-
and_(
459-
RomFile.rom_id == cls.id,
460-
RomFile.category == RomFileCategory.MANUAL,
461-
)
462-
)
463-
.correlate_except(RomFile)
464-
.exists()
465-
.select()
466-
.scalar_subquery(),
467-
deferred=True,
468-
)
469-
470446
@declared_attr
471447
def has_soundtrack(cls) -> Mapped[bool]:
472448
return column_property(

backend/tasks/scheduled/convert_images_to_webp.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
)
1515
from logger.logger import log
1616
from tasks.tasks import PeriodicTask, TaskType, update_job_meta
17+
from utils.media_types import ALLOWED_IMAGE_EXTENSIONS
1718

1819

1920
@dataclass
@@ -30,9 +31,6 @@ class ConversionResult:
3031
class ImageConverter:
3132
"""Handles image format conversion to WebP."""
3233

33-
# Supported image formats
34-
SUPPORTED_EXTENSIONS = {".png", ".jpg", ".jpeg", ".bmp", ".tiff", ".tif", ".gif"}
35-
3634
# Image mode conversion mapping
3735
MODE_CONVERSIONS = {
3836
"P": "RGBA", # Palette-based to RGBA (preserves transparency)
@@ -147,7 +145,8 @@ def _find_convertible_images(self) -> List[Path]:
147145
for p in self.resources_path.rglob("**/cover/*")
148146
if p.is_file()
149147
and not p.is_symlink()
150-
and p.suffix.lower() in ImageConverter.SUPPORTED_EXTENSIONS
148+
and p.suffix.lower() in ALLOWED_IMAGE_EXTENSIONS
149+
and p.suffix.lower() != ".webp"
151150
and not p.with_suffix(".webp").exists()
152151
]
153152

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
from fastapi import status
2+
from fastapi.testclient import TestClient
3+
4+
from handler.database import db_rom_handler
5+
from models.platform import Platform
6+
from models.rom import Rom, RomFile, RomFileCategory
7+
from models.user import User
8+
9+
10+
def _auth(token: str) -> dict[str, str]:
11+
return {"Authorization": f"Bearer {token}"}
12+
13+
14+
def _add_file(rom: Rom, name: str, category: RomFileCategory | None) -> RomFile:
15+
return db_rom_handler.add_rom_file(
16+
RomFile(
17+
rom_id=rom.id,
18+
file_name=name,
19+
file_path=f"{rom.fs_path}/{rom.fs_name}",
20+
file_size_bytes=10,
21+
category=category,
22+
)
23+
)
24+
25+
26+
def _make_rom(admin_user: User, platform: Platform) -> Rom:
27+
rom = db_rom_handler.add_rom(
28+
Rom(
29+
platform_id=platform.id,
30+
name="media_rom",
31+
slug="media_rom_slug",
32+
fs_name="media_rom",
33+
fs_name_no_tags="media_rom",
34+
fs_name_no_ext="media_rom",
35+
fs_extension="",
36+
fs_path=f"{platform.slug}/roms",
37+
)
38+
)
39+
db_rom_handler.add_rom_user(rom_id=rom.id, user_id=admin_user.id)
40+
return rom
41+
42+
43+
def test_image_file_served_inline(
44+
client: TestClient, access_token: str, admin_user: User, platform: Platform
45+
):
46+
rom = _make_rom(admin_user, platform)
47+
file = _add_file(rom, "trailer_thumb.png", RomFileCategory.GAME)
48+
49+
r = client.get(
50+
f"/api/roms/{file.id}/files/content/trailer_thumb.png",
51+
headers=_auth(access_token),
52+
)
53+
54+
assert r.status_code == status.HTTP_200_OK
55+
assert r.headers["content-type"].startswith("image/png")
56+
assert r.headers["content-disposition"].startswith("inline")
57+
58+
59+
def test_video_file_served_inline(
60+
client: TestClient, access_token: str, admin_user: User, platform: Platform
61+
):
62+
rom = _make_rom(admin_user, platform)
63+
file = _add_file(rom, "trailer.mp4", RomFileCategory.GAME)
64+
65+
r = client.get(
66+
f"/api/roms/{file.id}/files/content/trailer.mp4",
67+
headers=_auth(access_token),
68+
)
69+
70+
assert r.status_code == status.HTTP_200_OK
71+
assert r.headers["content-type"].startswith("video/mp4")
72+
assert r.headers["content-disposition"].startswith("inline")
73+
74+
75+
def test_rom_file_served_as_attachment(
76+
client: TestClient, access_token: str, admin_user: User, platform: Platform
77+
):
78+
rom = _make_rom(admin_user, platform)
79+
file = _add_file(rom, "game.bin", RomFileCategory.GAME)
80+
81+
r = client.get(
82+
f"/api/roms/{file.id}/files/content/game.bin",
83+
headers=_auth(access_token),
84+
)
85+
86+
assert r.status_code == status.HTTP_200_OK
87+
assert r.headers["content-type"].startswith("application/octet-stream")
88+
assert r.headers["content-disposition"].startswith("attachment")
89+
90+
91+
def test_content_type_derived_from_db_not_path_param(
92+
client: TestClient, access_token: str, admin_user: User, platform: Platform
93+
):
94+
# A caller must not be able to force an inline image content-type on a
95+
# non-media file by tacking a fake extension onto the URL path param.
96+
rom = _make_rom(admin_user, platform)
97+
file = _add_file(rom, "game.bin", RomFileCategory.GAME)
98+
99+
r = client.get(
100+
f"/api/roms/{file.id}/files/content/game.bin.png",
101+
headers=_auth(access_token),
102+
)
103+
104+
assert r.status_code == status.HTTP_200_OK
105+
assert r.headers["content-type"].startswith("application/octet-stream")
106+
assert r.headers["content-disposition"].startswith("attachment")

0 commit comments

Comments
 (0)