Skip to content

Commit

Permalink
Change message encoding to support URLs with ? (#47)
Browse files Browse the repository at this point in the history
Before this change, if I request to play the URL

    http://192.168.0.10/test.mp3?auth%3Dsomething

through Home Assistant, it will result in this message being sent:

    heos://browse/play_stream?sequence=498&pid=-646915758&url=http://192.168.0.10/test.mp3?auth%3Dsomething

but despite saying in CLI specification that special characters in
arguments need to be encoded, HEOS seems to take the url argument
and pass it to the stream server verbatim, as demonstrated by the
nginx log resulting from this command:

    Jan 04 12:03:27 lillia nginx[351316]: lillia nginx: 192.168.0.209 - - [04/Jan/2025:12:03:27 +0100] "GET /test.mp3?auth%3Dsomething HTTP/1.1" 404 146 "-" "AvegaMediaServer/2.0 Linux/2.6"

I've been playing URLs with ? in them for some months now by inserting
them into the HEOS command without quoting them, for example:

    heos://browse/play_stream?sequence=498&pid=-646915758&url=http://192.168.0.10/test.mp3?auth=something

This seems to be consistent with the requirement that url be the last
argument, because then the controller can treat the rest of the command
as URL without processing it.

This change introduces special handling of the url argument, so that it
bypasses quoting in addition to being the last argument.
  • Loading branch information
Michcioperz authored Jan 5, 2025
1 parent 263a678 commit bc7fe5d
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 5 deletions.
4 changes: 2 additions & 2 deletions pyheos/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ def _encode_query(cls, items: dict[str, Any], *, mask: bool = False) -> str:
for key in sorted(items.keys()):
value = const.MASK if mask and key in const.MASKED_PARAMS else items[key]
item = f"{key}={HeosCommand._quote(value)}"
# Ensure 'url' goes last per CLI spec
# Ensure 'url' goes last per CLI spec and is not quoted
if key == const.ATTR_URL:
pairs.append(item)
pairs.append(f"{key}={value}")
else:
pairs.insert(0, item)
return "&".join(pairs)
Expand Down
2 changes: 1 addition & 1 deletion tests/fixtures/browse.play_stream.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"heos": {"command": "browse/play_stream", "result": "success", "message": "pid=1&url=https://my.website.com/podcast.mp3"}}
{"heos": {"command": "browse/play_stream", "result": "success", "message": "pid=1&url=https://my.website.com/podcast.mp3?patron-auth=qwerty"}}
7 changes: 5 additions & 2 deletions tests/test_player.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,14 @@ async def test_play_preset_station_invalid_index(player: HeosPlayer) -> None:

@calls_command(
"browse.play_stream",
{const.ATTR_PLAYER_ID: 1, const.ATTR_URL: "https://my.website.com/podcast.mp3"},
{
const.ATTR_PLAYER_ID: 1,
const.ATTR_URL: "https://my.website.com/podcast.mp3?patron-auth=qwerty",
},
)
async def test_play_url(player: HeosPlayer) -> None:
"""Test the play url."""
await player.play_url("https://my.website.com/podcast.mp3")
await player.play_url("https://my.website.com/podcast.mp3?patron-auth=qwerty")


@pytest.mark.parametrize("quick_select", [0, 7])
Expand Down

0 comments on commit bc7fe5d

Please sign in to comment.