Improve error handling process_play_media_url (#68322)

homeassistant/components/media_player/browse_media.py

@@ -10,7 +10,9 @@ import yarl
 
 from homeassistant.components.http.auth import async_sign_path
 from homeassistant.core import HomeAssistant, callback
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers.network import (
+    NoURLAvailableError,
     get_supervisor_network_url,
     get_url,
     is_hass_url,
@@ -28,11 +30,15 @@ def async_process_play_media_url(
     for_supervisor_network: bool = False,
 ) -> str:
     """Update a media URL with authentication if it points at Home Assistant."""
-    if media_content_id[0] != "/" and not is_hass_url(hass, media_content_id):
-        return media_content_id
-
     parsed = yarl.URL(media_content_id)
 
+    if parsed.is_absolute():
+        if not is_hass_url(hass, media_content_id):
+            return media_content_id
+    else:
+        if media_content_id[0] != "/":
+            raise ValueError("URL is relative, but does not start with a /")
+
     if parsed.query:
         logging.getLogger(__name__).debug(
             "Not signing path for content with query param"
@@ -46,13 +52,23 @@ def async_process_play_media_url(
         media_content_id = str(parsed.join(yarl.URL(signed_path)))
 
     # convert relative URL to absolute URL
-    if media_content_id[0] == "/" and not allow_relative_url:
+    if not parsed.is_absolute() and not allow_relative_url:
         base_url = None
         if for_supervisor_network:
             base_url = get_supervisor_network_url(hass)
 
         if not base_url:
-            base_url = get_url(hass)
+            try:
+                base_url = get_url(hass)
+            except NoURLAvailableError as err:
+                msg = "Unable to determine Home Assistant URL to send to device"
+                if (
+                    hass.config.api
+                    and hass.config.api.use_ssl
+                    and (not hass.config.external_url or not hass.config.internal_url)
+                ):
+                    msg += ". Configure internal and external URL in general settings."
+                raise HomeAssistantError(msg) from err
 
         media_content_id = f"{base_url}{media_content_id}"
 

homeassistant/components/roku/media_player.py

@@ -402,9 +402,6 @@ class RokuMediaPlayer(RokuEntity, MediaPlayerEntity):
             stream_name = original_media_id
             stream_format = guess_stream_format(media_id, mime_type)
 
-        # If media ID is a relative URL, we serve it from HA.
-        media_id = async_process_play_media_url(self.hass, media_id)
-
         if media_type == FORMAT_CONTENT_TYPE[HLS_PROVIDER]:
             media_type = MEDIA_TYPE_VIDEO
             mime_type = FORMAT_CONTENT_TYPE[HLS_PROVIDER]
@@ -412,6 +409,9 @@ class RokuMediaPlayer(RokuEntity, MediaPlayerEntity):
             stream_format = "hls"
 
         if media_type in (MEDIA_TYPE_MUSIC, MEDIA_TYPE_URL, MEDIA_TYPE_VIDEO):
+            # If media ID is a relative URL, we serve it from HA.
+            media_id = async_process_play_media_url(self.hass, media_id)
+
             parsed = yarl.URL(media_id)
 
             if mime_type is None:

homeassistant/helpers/network.py

@@ -62,7 +62,13 @@ def get_supervisor_network_url(
 
 def is_hass_url(hass: HomeAssistant, url: str) -> bool:
     """Return if the URL points at this Home Assistant instance."""
-    parsed = yarl.URL(normalize_url(url))
+    parsed = yarl.URL(url)
+
+    if not parsed.is_absolute():
+        return False
+
+    if parsed.is_default_port():
+        parsed = parsed.with_port(None)
 
     def host_ip() -> str | None:
         if hass.config.api is None or is_loopback(ip_address(hass.config.api.local_ip)):

homeassistant/util/network.py

@@ -82,6 +82,6 @@ def is_ipv6_address(address: str) -> bool:
 def normalize_url(address: str) -> str:
     """Normalize a given URL."""
     url = yarl.URL(address.rstrip("/"))
-    if url.is_default_port():
+    if url.is_absolute() and url.is_default_port():
         return str(url.with_port(None))
     return str(url)

tests/components/cast/test_media_player.py

@@ -974,7 +974,7 @@ async def test_entity_play_media(hass: HomeAssistant, quick_play_mock):
         {
             ATTR_ENTITY_ID: entity_id,
             media_player.ATTR_MEDIA_CONTENT_TYPE: "audio",
-            media_player.ATTR_MEDIA_CONTENT_ID: "best.mp3",
+            media_player.ATTR_MEDIA_CONTENT_ID: "http://example.com/best.mp3",
             media_player.ATTR_MEDIA_EXTRA: {"metadata": {"metadatatype": 3}},
         },
         blocking=True,
@@ -985,7 +985,7 @@ async def test_entity_play_media(hass: HomeAssistant, quick_play_mock):
         chromecast,
         "default_media_receiver",
         {
-            "media_id": "best.mp3",
+            "media_id": "http://example.com/best.mp3",
             "media_type": "audio",
             "metadata": {"metadatatype": 3},
         },
@@ -1523,13 +1523,15 @@ async def test_group_media_control(hass, mz_mock, quick_play_mock):
     assert not chromecast.media_controller.stop.called
 
     # Verify play_media is not forwarded
-    await common.async_play_media(hass, "music", "best.mp3", entity_id)
+    await common.async_play_media(
+        hass, "music", "http://example.com/best.mp3", entity_id
+    )
     assert not grp_media.play_media.called
     assert not chromecast.media_controller.play_media.called
     quick_play_mock.assert_called_once_with(
         chromecast,
         "default_media_receiver",
-        {"media_id": "best.mp3", "media_type": "music"},
+        {"media_id": "http://example.com/best.mp3", "media_type": "music"},
     )
 
 
@@ -1803,7 +1805,7 @@ async def test_cast_platform_play_media(hass: HomeAssistant, quick_play_mock, ca
         {
             ATTR_ENTITY_ID: entity_id,
             media_player.ATTR_MEDIA_CONTENT_TYPE: "audio",
-            media_player.ATTR_MEDIA_CONTENT_ID: "best.mp3",
+            media_player.ATTR_MEDIA_CONTENT_ID: "http://example.com/best.mp3",
             media_player.ATTR_MEDIA_EXTRA: {"metadata": {"metadatatype": 3}},
         },
         blocking=True,
@@ -1811,7 +1813,7 @@ async def test_cast_platform_play_media(hass: HomeAssistant, quick_play_mock, ca
 
     # Assert the media player attempt to play media through the cast platform
     cast_platform_mock.async_play_media.assert_called_once_with(
-        hass, entity_id, chromecast, "audio", "best.mp3"
+        hass, entity_id, chromecast, "audio", "http://example.com/best.mp3"
     )
 
     # Assert pychromecast is used to play media

tests/components/forked_daapd/test_media_player.py

@@ -557,7 +557,7 @@ async def test_async_play_media_from_paused(hass, mock_api_object):
         SERVICE_PLAY_MEDIA,
         {
             ATTR_MEDIA_CONTENT_TYPE: MEDIA_TYPE_MUSIC,
-            ATTR_MEDIA_CONTENT_ID: "somefile.mp3",
+            ATTR_MEDIA_CONTENT_ID: "http://example.com/somefile.mp3",
         },
     )
     state = hass.states.get(TEST_MASTER_ENTITY_NAME)
@@ -581,7 +581,7 @@ async def test_async_play_media_from_stopped(
         SERVICE_PLAY_MEDIA,
         {
             ATTR_MEDIA_CONTENT_TYPE: MEDIA_TYPE_MUSIC,
-            ATTR_MEDIA_CONTENT_ID: "somefile.mp3",
+            ATTR_MEDIA_CONTENT_ID: "http://example.com/somefile.mp3",
         },
     )
     state = hass.states.get(TEST_MASTER_ENTITY_NAME)
@@ -616,7 +616,7 @@ async def test_async_play_media_tts_timeout(hass, mock_api_object):
             SERVICE_PLAY_MEDIA,
             {
                 ATTR_MEDIA_CONTENT_TYPE: MEDIA_TYPE_MUSIC,
-                ATTR_MEDIA_CONTENT_ID: "somefile.mp3",
+                ATTR_MEDIA_CONTENT_ID: "http://example.com/somefile.mp3",
             },
         )
         state = hass.states.get(TEST_MASTER_ENTITY_NAME)
@@ -725,7 +725,7 @@ async def test_librespot_java_play_media(hass, pipe_control_api_object):
         SERVICE_PLAY_MEDIA,
         {
             ATTR_MEDIA_CONTENT_TYPE: MEDIA_TYPE_MUSIC,
-            ATTR_MEDIA_CONTENT_ID: "somefile.mp3",
+            ATTR_MEDIA_CONTENT_ID: "http://example.com/somefile.mp3",
         },
     )
     state = hass.states.get(TEST_MASTER_ENTITY_NAME)
@@ -747,7 +747,7 @@ async def test_librespot_java_play_media_pause_timeout(hass, pipe_control_api_ob
             SERVICE_PLAY_MEDIA,
             {
                 ATTR_MEDIA_CONTENT_TYPE: MEDIA_TYPE_MUSIC,
-                ATTR_MEDIA_CONTENT_ID: "somefile.mp3",
+                ATTR_MEDIA_CONTENT_ID: "http://example.com/somefile.mp3",
             },
         )
         state = hass.states.get(TEST_MASTER_ENTITY_NAME)

tests/components/media_player/test_browse_media.py

@@ -7,6 +7,8 @@ from homeassistant.components.media_player.browse_media import (
     async_process_play_media_url,
 )
 from homeassistant.config import async_process_ha_core_config
+from homeassistant.exceptions import HomeAssistantError
+from homeassistant.helpers.network import NoURLAvailableError
 
 from tests.common import mock_component
 
@@ -48,6 +50,11 @@ async def test_process_play_media_url(hass, mock_sign_path):
         async_process_play_media_url(hass, "http://192.168.123.123:8123/path")
         == "http://192.168.123.123:8123/path?authSig=bla"
     )
+    with pytest.raises(HomeAssistantError), patch(
+        "homeassistant.components.media_player.browse_media.get_url",
+        side_effect=NoURLAvailableError,
+    ):
+        async_process_play_media_url(hass, "/path")
 
     # Test skip signing URLs that have a query param
     assert (
@@ -61,6 +68,9 @@ async def test_process_play_media_url(hass, mock_sign_path):
         == "http://192.168.123.123:8123/path?hello=world"
     )
 
+    with pytest.raises(ValueError):
+        async_process_play_media_url(hass, "hello")
+
 
 async def test_process_play_media_url_for_addon(hass, mock_sign_path):
     """Test it uses the hostname for an addon if available."""

tests/helpers/test_network.py

@@ -681,6 +681,9 @@ async def test_is_hass_url(hass):
     assert hass.config.external_url is None
 
     assert is_hass_url(hass, "http://example.com") is False
+    assert is_hass_url(hass, "bad_url") is False
+    assert is_hass_url(hass, "bad_url.com") is False
+    assert is_hass_url(hass, "http:/bad_url.com") is False
 
     hass.config.api = Mock(use_ssl=False, port=8123, local_ip="192.168.123.123")
     assert is_hass_url(hass, "http://192.168.123.123:8123") is True

tests/util/test_network.py

@@ -91,3 +91,4 @@ def test_normalize_url():
         network_util.normalize_url("https://example.com:443/test/")
         == "https://example.com/test"
     )
+    assert network_util.normalize_url("/test/") == "/test"