diff --git a/backend/handler/metadata/ss_handler.py b/backend/handler/metadata/ss_handler.py index 886cc98d2..6fd6e0db6 100644 --- a/backend/handler/metadata/ss_handler.py +++ b/backend/handler/metadata/ss_handler.py @@ -2,7 +2,7 @@ import html import re from datetime import datetime from typing import Final, NotRequired, TypedDict -from urllib.parse import quote, urlparse +from urllib.parse import urlparse import pydash from unidecode import unidecode as uc @@ -624,7 +624,7 @@ class SSHandler(MetadataHandler): return None roms = await self.ss_service.search_games( - term=quote(uc(search_term), safe="/ "), + term=uc(search_term), system_id=platform_ss_id, ) @@ -873,7 +873,7 @@ class SSHandler(MetadataHandler): return [] matched_games = await self.ss_service.search_games( - term=quote(uc(search_term), safe="/ "), + term=uc(search_term), system_id=platform_ss_id, ) diff --git a/backend/tests/handler/metadata/test_ss_handler.py b/backend/tests/handler/metadata/test_ss_handler.py index 108763924..e1f559313 100644 --- a/backend/tests/handler/metadata/test_ss_handler.py +++ b/backend/tests/handler/metadata/test_ss_handler.py @@ -831,3 +831,97 @@ class TestLookupRom: ) assert result["ss_id"] is None assert is_not_game is True + + +class TestSearchTermEncoding: + """Regression tests for issue #3467: the SS name-search term must be + URL-encoded exactly once. + + The handler must pass the *raw* (un-percent-encoded) term to the service + layer, which percent-encodes it a single time when building the request URL + via ``with_query(...)``. Pre-encoding the term in the handler caused a + second round of encoding (``%2B`` -> ``%252B``), so ScreenScraper searched + for literal gibberish and returned no match for any title containing a + character that has to be URL-encoded (``+``, ``&``, an apostrophe, ...). + """ + + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("search_term", "literal", "double_encoded"), + [ + ("super mario 3d world + bowsers fury", "+", "%2B"), + ("sonic & knuckles", "&", "%26"), + ("marvel's spider-man", "'", "%27"), + ], + ) + async def test_search_rom_passes_unencoded_term_to_service( + self, search_term, literal, double_encoded + ): + """``_search_rom`` hands the service a term that is not pre-encoded.""" + handler = SSHandler() + captured: dict = {} + + async def capture(**kwargs): + captured.update(kwargs) + return [] + + with patch.object(handler.ss_service, "search_games", side_effect=capture): + await handler._search_rom(search_term, 225) + + term = captured["term"] + assert literal in term + assert double_encoded not in term + + @pytest.mark.asyncio + async def test_search_rom_still_transliterates_unicode(self): + """Unidecode is still applied so accented titles match ScreenScraper.""" + handler = SSHandler() + captured: dict = {} + + async def capture(**kwargs): + captured.update(kwargs) + return [] + + with patch.object(handler.ss_service, "search_games", side_effect=capture): + await handler._search_rom("Pokémon Snap", 14) + + assert captured["term"] == "Pokemon Snap" + + @pytest.mark.asyncio + async def test_get_matched_roms_by_name_passes_unencoded_term(self): + """``get_matched_roms_by_name`` also avoids pre-encoding the term.""" + handler = SSHandler() + captured: dict = {} + + async def capture(**kwargs): + captured.update(kwargs) + return [] + + with ( + patch("handler.metadata.ss_handler.SCREENSCRAPER_USER", "user1"), + patch("handler.metadata.ss_handler.SCREENSCRAPER_PASSWORD", "pw1"), + patch.object(handler.ss_service, "search_games", side_effect=capture), + ): + await handler.get_matched_roms_by_name(MagicMock(), "sonic & knuckles", 1) + + term = captured["term"] + assert "&" in term + assert "%26" not in term + + @pytest.mark.asyncio + async def test_search_rom_url_single_encodes_plus(self): + """End-to-end through the real service: a ``+`` is encoded exactly once + in the request URL (``%2B``), never doubly (``%252B``).""" + handler = SSHandler() + captured: dict = {} + + async def capture_request(url, *args, **kwargs): + captured["url"] = url + return {"response": {"jeux": []}} + + with patch.object(handler.ss_service, "_request", side_effect=capture_request): + await handler._search_rom("super mario 3d world + bowsers fury", 225) + + url = captured["url"] + assert "%2B" in url + assert "%252B" not in url