Change message encoding to support URLs with ? #47
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
Michcioperz
force-pushed
the
play-unencoded-url
branch
from
40329a5 to
e71f923
Compare
andrewsayre
approved these changes
Jan 5, 2025
|
Mmm, not to my knowledge. Honestly I saw the issue and dug down to the source until I could find it. I see there is home-assistant/core#107442 (comment) where this problem is mentioned, but it's only a half of that issue's cause, the other being the character limit. |
|
@Michcioperz thank you! I'll get an update in HA this week! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this change, if I request to play the URL
through Home Assistant, it will result in this message being sent:
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:
I've been playing URLs with ? in them for some months now by inserting them into the HEOS command without quoting them, for example:
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.
Checklist:
.coveragercpermitted.README.MDupdated (if necessary)