Comment 32 for bug 1329202

Thanks for all the effort you're putting here. Let me reply to each open point.

> What hare your test results. Why is it worse? [...]
> As a workaround for Spreaker [...]

We reported this issue more than 1 year ago, and we immediately fixed it on the server side, otherwise Mixxx would still be not-compatible with our servers. We re-implemented HTTP request query string parsing, doing some hacks to correctly parse non-escaped query string parameters.

> In Marco's case, it seems he wants the mount string to be escaped for the metadata, but not the SOURCE call.

My original issue was: Icecast2 protocol is an HTTP-based protocol, thus it follows HTTP specs for the request/response.

Given the mountpoint "/live?stream_id=1&tracking=2" then the SOURCE's request first line is something like:
SOURCE /live?stream_id=1&tracking=2 HTTP/1.1

When Mixxx sends metadata, what it actually does is an HTTP GET request, that should follow HTTP specs. The request is something like:
GET /admin/metadata?mode=updinfo&mount=<mount>&song=title HTTP/1.1

where <mount> is the query string value that should be replaced with the original mountpoint. According to HTTP specs, the query string parameters MUST be escaped.

Right know, when Mixxx (well, libshout) does the request it's something like:
GET /admin/metadata?mode=updinfo&mount=/live?stream_id=1&tracking=2&song=title HTTP/1.1

but it's actually wrong, because the query string params MUST be escaped. It should be something like:
GET /admin/metadata?mode=updinfo&mount=2Flive%3Fstream_id%3D1%26tracking%3D2&song=title HTTP/1.1

> However, warning about unescaped strings might be okay in the interim while we look at better solutions.

If I correctly understand how the warning works, it conducts to a worse UX, IMO. When you insert a mountpoint in Mixxx it shouldn't be escaped. It should just be a standard URL, with no double-escaping applied to it. If you display a warning when the user insert the mountpoint "/live?stream_id=1&tracking=2", he will be disoriented, because he actually inserted a good URL and there's no reason to warn him that the URL is invalid (or unescaped).

> This looks like a bug in libshout [...]

I agree that should be fixed in libshout.