Escape media_content_id in media player proxy (#77811)

* Escape media_content_id in media player proxy

* Change usage in kodi

* Change usage in roku

* Change usage in sonos

* Add test

* Add comment

* Change path regex instead of double quoting

* Use .+ instead of .*

homeassistant/components/kodi/media_player.py

@@ -7,7 +7,6 @@ from functools import wraps
 import logging
 import re
 from typing import Any, TypeVar
-import urllib.parse
 
 from jsonrpc_base.jsonrpc import ProtocolError, TransportError
 from pykodi import CannotConnectError
@@ -920,7 +919,7 @@ class KodiEntity(MediaPlayerEntity):
 
             return self.get_browse_image_url(
                 media_content_type,
-                urllib.parse.quote_plus(media_content_id),
+                media_content_id,
                 media_image_id,
             )
 

homeassistant/components/media_player/__init__.py

@@ -13,7 +13,7 @@ from http import HTTPStatus
 import logging
 import secrets
 from typing import Any, cast, final
-from urllib.parse import urlparse
+from urllib.parse import quote, urlparse
 
 from aiohttp import web
 from aiohttp.hdrs import CACHE_CONTROL, CONTENT_TYPE
@@ -1100,7 +1100,9 @@ class MediaPlayerEntity(Entity):
         """Generate an url for a media browser image."""
         url_path = (
             f"/api/media_player_proxy/{self.entity_id}/browse_media"
-            f"/{media_content_type}/{media_content_id}"
+            # quote the media_content_id as it may contain url unsafe characters
+            # aiohttp will unquote the path automatically
+            f"/{media_content_type}/{quote(media_content_id)}"
         )
 
         url_query = {"token": self.access_token}
@@ -1117,7 +1119,10 @@ class MediaPlayerImageView(HomeAssistantView):
     url = "/api/media_player_proxy/{entity_id}"
     name = "api:media_player:image"
     extra_urls = [
-        url + "/browse_media/{media_content_type}/{media_content_id}",
+        # Need to modify the default regex for media_content_id as it may
+        # include arbitrary characters including '/','{', or '}'
+        url
+        + "/browse_media/{media_content_type}/{media_content_id:.+}",
     ]
 
     def __init__(self, component: EntityComponent) -> None:

homeassistant/components/roku/browse_media.py

@@ -3,7 +3,6 @@ from __future__ import annotations
 
 from collections.abc import Callable
 from functools import partial
-from urllib.parse import quote_plus
 
 from homeassistant.components import media_source
 from homeassistant.components.media_player import BrowseMedia
@@ -64,7 +63,7 @@ def get_thumbnail_url_full(
 
     return get_browse_image_url(
         media_content_type,
-        quote_plus(media_content_id),
+        media_content_id,
         media_image_id,
     )
 

homeassistant/components/sonos/media_browser.py

@@ -6,7 +6,6 @@ from contextlib import suppress
 from functools import partial
 import logging
 from typing import cast
-from urllib.parse import quote_plus, unquote
 
 from soco.data_structures import DidlFavorite, DidlObject
 from soco.ms_data_structures import MusicServiceItem
@@ -64,7 +63,7 @@ def get_thumbnail_url_full(
 
     return get_browse_image_url(
         media_content_type,
-        quote_plus(media_content_id),
+        media_content_id,
         media_image_id,
     )
 
@@ -201,7 +200,7 @@ def build_item_response(
 
     if not title:
         try:
-            title = unquote(payload["idstring"].split("/")[1])
+            title = payload["idstring"].split("/")[1]
         except IndexError:
             title = LIBRARY_TITLES_MAPPING[payload["idstring"]]
 

tests/components/media_player/test_init.py

@@ -280,3 +280,34 @@ async def test_enqueue_alert_exclusive(hass):
             },
             blocking=True,
         )
+
+
+async def test_get_async_get_browse_image_quoting(
+    hass, hass_client_no_auth, hass_ws_client
+):
+    """Test get browse image using media_content_id with special characters.
+
+    async_get_browse_image() should get called with the same string that is
+    passed into get_browse_image_url().
+    """
+    await async_setup_component(
+        hass, "media_player", {"media_player": {"platform": "demo"}}
+    )
+    await hass.async_block_till_done()
+
+    entity_comp = hass.data.get("entity_components", {}).get("media_player")
+    assert entity_comp
+
+    player = entity_comp.get_entity("media_player.bedroom")
+    assert player
+
+    client = await hass_client_no_auth()
+
+    with patch(
+        "homeassistant.components.media_player.MediaPlayerEntity."
+        "async_get_browse_image",
+    ) as mock_browse_image:
+        media_content_id = "a/b c/d+e%2Fg{}"
+        url = player.get_browse_image_url("album", media_content_id)
+        await client.get(url)
+        mock_browse_image.assert_called_with("album", media_content_id, None)