From 00b894733d9b68bf80d7ef32f8238a44c6d89ba2 Mon Sep 17 00:00:00 2001 From: Spinnich Date: Wed, 3 Jun 2026 14:14:25 +0000 Subject: [PATCH 1/3] fix(roms): return 404 when content file_ids match no files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renaming a ROM gives its file a new internal id, but the EmulatorJS player keeps a remembered file id ("disc") in localStorage and reuses it on the next launch. After a rename that id is stale, so the content download endpoint matched zero files and fell through to its multi-file ZIP path, producing a download whose only entry was an empty .m3u playlist. nginx's mod_zip decode step rejects the blank value (HTTP 400) and aborts the response, sending 0 bytes — which EmulatorJS surfaces as a generic "network error" (issue #3470). The frontend half (validating the remembered disc against the ROM's current files) already landed on master in d1696cd04. This is the backend half: when no files match the request, raise a clean 404 instead of building a broken empty-.m3u ZIP. This also covers a ROM with zero files. Add endpoint tests (auth, single-file, valid file id, stale file id -> 404, missing rom -> 404) plus a `rom_file` fixture. Written primarily by Claude Code. Co-Authored-By: Claude Opus 4.8 --- backend/endpoints/roms/__init__.py | 9 ++++ backend/tests/conftest.py | 14 +++++- backend/tests/endpoints/roms/test_rom.py | 56 ++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) diff --git a/backend/endpoints/roms/__init__.py b/backend/endpoints/roms/__init__.py index 6374a4436..1627b4985 100644 --- a/backend/endpoints/roms/__init__.py +++ b/backend/endpoints/roms/__init__.py @@ -945,6 +945,15 @@ async def get_rom_content( files = [f for f in files if f.id in file_id_values] files.sort(key=lambda x: x.file_name) + # When no files match (e.g. a stale `file_ids` the player remembered from + # before a rename gave the file a new id), return a clean 404 instead of + # building an empty `.m3u` ZIP that nginx aborts as a 0-byte response. See #3470. + if not files: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"No files found for rom {id}", + ) + log.info( f"User {hl(current_username, color=BLUE)} is downloading {hl(rom.fs_name)}" ) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index ab111c4a6..2847011d5 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -23,7 +23,7 @@ from models.device import Device from models.device_save_sync import DeviceSaveSync from models.platform import Platform from models.play_session import PlaySession -from models.rom import Rom +from models.rom import Rom, RomFile from models.sync_session import SyncSession from models.user import Role, User @@ -88,6 +88,18 @@ def rom(admin_user: User, platform: Platform): return rom +@pytest.fixture +def rom_file(rom: Rom): + """A single content file attached to the `rom` fixture.""" + rom_file = RomFile( + rom_id=rom.id, + file_name="test_rom.zip", + file_path=rom.fs_path, + file_size_bytes=1000, + ) + return db_rom_handler.add_rom_file(rom_file) + + @pytest.fixture def save(rom: Rom, platform: Platform, admin_user: User): """Slot-bound save (the canonical device-uploaded shape). diff --git a/backend/tests/endpoints/roms/test_rom.py b/backend/tests/endpoints/roms/test_rom.py index 498c5593b..4e81973d7 100644 --- a/backend/tests/endpoints/roms/test_rom.py +++ b/backend/tests/endpoints/roms/test_rom.py @@ -59,6 +59,62 @@ def test_get_all_roms( assert items[0]["id"] == rom.id +def test_get_rom_content_requires_auth(client: TestClient, rom: Rom, rom_file): + response = client.get(f"/api/roms/{rom.id}/content/test_rom.zip") + assert response.status_code == status.HTTP_403_FORBIDDEN + + +def test_get_rom_content_single_file( + client: TestClient, access_token: str, rom: Rom, rom_file +): + response = client.get( + f"/api/roms/{rom.id}/content/test_rom.zip", + headers={"Authorization": f"Bearer {access_token}"}, + follow_redirects=False, + ) + assert response.status_code == status.HTTP_200_OK + # Single-file roms are proxied through nginx via X-Accel-Redirect. + assert "X-Accel-Redirect" in response.headers + + +def test_get_rom_content_valid_file_id( + client: TestClient, access_token: str, rom: Rom, rom_file +): + response = client.get( + f"/api/roms/{rom.id}/content/test_rom.zip", + headers={"Authorization": f"Bearer {access_token}"}, + params={"file_ids": str(rom_file.id)}, + follow_redirects=False, + ) + assert response.status_code == status.HTTP_200_OK + assert "X-Accel-Redirect" in response.headers + + +def test_get_rom_content_stale_file_id_returns_404( + client: TestClient, access_token: str, rom: Rom, rom_file +): + # Regression test for #3470: a remembered file id that no longer exists + # (e.g. after a rename gave the file a new id) must return a clean 404 + # instead of an empty-.m3u ZIP that nginx aborts as a 0-byte response. + stale_file_id = rom_file.id + 999 + response = client.get( + f"/api/roms/{rom.id}/content/test_rom.zip", + headers={"Authorization": f"Bearer {access_token}"}, + params={"file_ids": str(stale_file_id)}, + follow_redirects=False, + ) + assert response.status_code == status.HTTP_404_NOT_FOUND + + +def test_get_rom_content_missing_rom_returns_404(client: TestClient, access_token: str): + response = client.get( + "/api/roms/999999/content/missing.zip", + headers={"Authorization": f"Bearer {access_token}"}, + follow_redirects=False, + ) + assert response.status_code == status.HTTP_404_NOT_FOUND + + @patch.object(FSRomsHandler, "rename_fs_rom") @patch.object(IGDBHandler, "get_rom_by_id", return_value=IGDBRom(igdb_id=None)) def test_update_rom( From ceaab1875ba55f1a46dbd2814f3633f31ef9a6ac Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi <3247106+gantoine@users.noreply.github.com> Date: Wed, 3 Jun 2026 15:08:51 -0400 Subject: [PATCH 2/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- backend/endpoints/roms/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/endpoints/roms/__init__.py b/backend/endpoints/roms/__init__.py index 1627b4985..a013cd209 100644 --- a/backend/endpoints/roms/__init__.py +++ b/backend/endpoints/roms/__init__.py @@ -951,7 +951,7 @@ async def get_rom_content( if not files: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, - detail=f"No files found for rom {id}", + detail=f"No files found for ROM {id}", ) log.info( From 20bae48ea97af0b4381c64ea8dd7dc39033e72b4 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Wed, 3 Jun 2026 15:41:55 -0400 Subject: [PATCH 3/3] add ot HEAD --- backend/endpoints/roms/__init__.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/backend/endpoints/roms/__init__.py b/backend/endpoints/roms/__init__.py index a013cd209..06f2407b0 100644 --- a/backend/endpoints/roms/__init__.py +++ b/backend/endpoints/roms/__init__.py @@ -868,6 +868,12 @@ async def head_rom_content( files = [f for f in files if f.id in file_id_values] files.sort(key=lambda x: x.file_name) + if not files: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"No files found for ROM {id}", + ) + # Serve the file directly in development mode for emulatorjs if DEV_MODE: if len(files) == 1: @@ -945,9 +951,6 @@ async def get_rom_content( files = [f for f in files if f.id in file_id_values] files.sort(key=lambda x: x.file_name) - # When no files match (e.g. a stale `file_ids` the player remembered from - # before a rename gave the file a new id), return a clean 404 instead of - # building an empty `.m3u` ZIP that nginx aborts as a 0-byte response. See #3470. if not files: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND,