Skip to content

Fixed UPnP for full URLs with scheme#2451

Open
ypopovych wants to merge 1 commit intoMusicPlayerDaemon:v0.24.xfrom
ypopovych:v0.24.x
Open

Fixed UPnP for full URLs with scheme#2451
ypopovych wants to merge 1 commit intoMusicPlayerDaemon:v0.24.xfrom
ypopovych:v0.24.x

Conversation

@ypopovych
Copy link
Copy Markdown

Fixes #2450

To be frank I have zero confidence in URL fix, I simply patched two URL methods to return relative if it has scheme.

XML parser fix is a better one. Current version breaks on URLs with parameters. Parser sends URL by parts split on each &. With this fix all of them will be concatenated together.

Tested on Jellyfin server - works fine (slow, but works).

@MaxKellermann
Copy link
Copy Markdown
Member

Your commit consists of (at least) 3 distinct changes that deserve to be a separate commit, and your commit message documents none of these changes. It doesn't even describe the problem to be solved, let alone how and why. I don't understand this PR.

@ypopovych
Copy link
Copy Markdown
Author

ypopovych commented May 1, 2026

Sorry about that - here’s a clearer explanation of what was done and why.

These fixes were added because the UPnP database plugin does not handle URLs correctly. There are two separate issues:

Bug 1

The server may return full URLs instead of relative paths in XML responses.

The current UPnP implementation assumes that only paths are returned. It then joins the returned value with the server’s base URL. When a full URL is returned, this results in a malformed URL like: http://192.168.1.2/.../http://192.168.1.2/....

This was fixed in two places:

Bug 2

This issue lies in the XML parser logic.

The current implementation incorrectly assumes that a URL is returned as a single string. However, when a URL contains query parameters, the parser invokes the callback separately for each segment. As a result, only the last query parameter is captured instead of the full URL.

This was fixed in ‎src/db/plugins/upnp/Directory.cxx. The URL buffer is now cleared at the start of each element (once per element) and then incrementally built by appending data in the character data callback.

I hope this explanation helps, Yehor.

@ypopovych
Copy link
Copy Markdown
Author

How do you want me to split it? Do a different PRs? Or to have 2(3?) commits in the one PR?

@groutoutlook
Copy link
Copy Markdown

groutoutlook commented May 3, 2026

Not the author, but imo you can split the commits, this is still useful for me since I did struggle on those network path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants