Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[upnpcontrol] Fix IllegalArgumentException when downloading album art #17965

Merged
merged 2 commits into from
Dec 24, 2024
Merged

[upnpcontrol] Fix IllegalArgumentException when downloading album art #17965

merged 2 commits into from
Dec 24, 2024

Conversation

digitaldan
Copy link
Contributor

Artwork was failing to download on my new WiiM pro streamers due to whitespace in the URI.

2024-12-23 11:33:55.964 [DEBUG] [internal.handler.UpnpRendererHandler] - Invalid album art URI: http://mediaserver-cont-usc-mp1-1-v4v6.pandora.com/images/
88/0c/ed/6e/d48642d08e076af31fc3dd51/1080W_1080H.jpg
java.lang.IllegalArgumentException: Illegal character in path at index 110: http://mediaserver-cont-usc-mp1-1-v4v6.pandora.com/images/88/0c/ed/6e/d48642d0
8e076af31fc3dd51/1080W_1080H.jpg
        at java.net.URI.create(URI.java:906) ~[?:?]
        at org.eclipse.jetty.client.HttpClient.newRequest(HttpClient.java:473) ~[?:?]
        at org.openhab.core.io.net.http.HttpUtil.executeUrlAndGetReponse(HttpUtil.java:212) ~[?:?]
        at org.openhab.core.io.net.http.HttpUtil.downloadData(HttpUtil.java:443) ~[?:?]
        at org.openhab.core.io.net.http.HttpUtil.downloadImage(HttpUtil.java:406) ~[?:?]
        at org.openhab.core.io.net.http.HttpUtil.downloadImage(HttpUtil.java:377) ~[?:?]
        at org.openhab.core.io.net.http.HttpUtil.downloadImage(HttpUtil.java:363) ~[?:?]

@digitaldan digitaldan requested a review from mherwege as a code owner December 23, 2024 19:51
@jlaur jlaur changed the title Strip whitespace from album art URI [upnpcontrol] Strip whitespace from album art URI Dec 23, 2024
@lsiepel lsiepel added the bug An unexpected problem or unintended behavior of an add-on label Dec 24, 2024
mherwege
mherwege approved these changes Dec 24, 2024
View reviewed changes
Copy link
Contributor

@mherwege mherwege left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

...trol/src/main/java/org/openhab/binding/upnpcontrol/internal/handler/UpnpRendererHandler.java Show resolved Hide resolved
@jlaur jlaur changed the title [upnpcontrol] Strip whitespace from album art URI [upnpcontrol] Fix IllegalArgumentException when downloading album art Dec 24, 2024
@mherwege
Copy link
Contributor

A URI should not contain a whitespace. This solution will solve it, but I have the impression there are more issues with this streamer, see https://community.openhab.org/t/upnp-binding-already-exposed-and-exception-when-using-wz-wiim-pro/161084/1

@digitaldan
Copy link
Contributor Author

For what its worth, i don't have any of those issues with my units (on the latest firmware). I did have an issue where JUPNP would loose its registration after 20-30 mins and the device would go offline, but i have a fix for that (my Sonos connects occasionally do this as well). see #16638 (comment)

The WiiM line of products is becoming quite popular with the Hi-Fi community, so i would expect to see more of them. I would like to offer enhanced support for them, including playlists/favorites which uses a vendor specific UPNP namespace, as well as support multi-room grouping using the local HTTP API its exposes (its nice all control is local) .

@digitaldan
PR Review changes
a262d36 
Signed-off-by: Dan Cunningham <[email protected]>
@digitaldan
Copy link
Contributor Author

Ok, changed made, thanks!

lsiepel
lsiepel approved these changes Dec 24, 2024
View reviewed changes
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

@lsiepel lsiepel merged commit 0e5e48f into openhab:main Dec 24, 2024
3 checks passed
lsiepel pushed a commit that referenced this pull request Dec 24, 2024
@digitaldan @lsiepel
[upnpcontrol] Fix IllegalArgumentException when downloading album a…
49b5c51 
…rt (#17965)

* Stip whitespace from album art URI

Signed-off-by: Dan Cunningham <[email protected]>
@lsiepel lsiepel added the patch A PR that has been cherry-picked to a patch release branch label Dec 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mherwege mherwege mherwege approved these changes

@lsiepel lsiepel lsiepel approved these changes

Assignees
No one assigned
Labels
bug An unexpected problem or unintended behavior of an add-on patch A PR that has been cherry-picked to a patch release branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@digitaldan @mherwege @lsiepel