Opened 14 years ago
Closed 14 years ago
Last modified 14 years ago
#9188 closed patch (Fixed)
UPnP browsing "By Title" fails for shows with non-Latin1 characters [+PATCH]
Reported by: | Owned by: | beirdo | |
---|---|---|---|
Priority: | minor | Milestone: | 0.24.1 |
Component: | MythTV - UPnP | Version: | 0.23-fixes |
Severity: | medium | Keywords: | upnp, utf8 |
Cc: | foceni@… | Ticket locked: | yes |
Description
For example: "By Title" browsing shows a folder called "Černá zmije (18)", but when I click it open on PS3 or in Totem on the desktop, it is empty. Locating the show via "By Date" plays without fail. This is broken for everybody with non-Latin1 characters if UPnP is used.
I checked the source in my local MythTV 0.23-fixes repo and indeed, there was a bug in handling UTF8 requests. The same bug existed in SVN trunk the last time I used it (~month back).
US coders probably didn't expect the search filters to contain UTF8 characters, so they used .toLatin1() conversions all around the place. Attached is a simple fix, as switching from .toLatin1() to .toUtf8() fixes the whole issue.
Applying to upstream is as simple as "search and replace", though not all occurrences require UTF8, it doesn't hurt or break anything. E.g. for key=value pair decoding, only the "value" needs .toUtf8().
Attachments (2)
Change History (8)
Changed 14 years ago by
Attachment: | myth-0.23-fixes-UPnP-UTF8-fix.patch added |
---|
comment:1 Changed 14 years ago by
Owner: | changed from dblain to beirdo |
---|---|
Status: | new → assigned |
comment:2 Changed 14 years ago by
Keywords: | utf8 added; removed |
---|---|
Priority: | major → minor |
comment:3 Changed 14 years ago by
UPDATE: all changes from .toLatin1() to .toUtf8() in the patch are essential. Without them, the most frequent UPnP/DLNA browsing - "By Title" - doesn't work for any locale with characters outside of Latin1 (ISO-8859-1), that's all other 15 ISO/8859 code pages and even Unicode (despite QT4 being internally Unicode).
The problem is in how QStrings are converted into byte streams for QUrl::fromPercentEncoding(). This method requires %-encoded UTF8 stream. It decodes %-encoding and then decodes the resulting UTF8 byte stream into Unicode. When passed regular (non-%-encoded) UTF8, like the payload is, it just decodes it as UTF8.
All mentioned QStrings are pieces of the UPnP payload, which is correctly imported from client requests in HTTPRequest::ParseRequest?() using QString::fromUtf8(). They contain actual Unicode strings.
The original coder expected all values to be only %-encoded and thus .toLatin1() conversion for QUrl::fromPercentEncoding() would work, because %-encoded string is pure ASCII. Actually, though, UPnP payload is hardly (if ever) %-encoded and .toLatin1() applied to a Unicode string strips all non-Latin1 characters and the result, of course, isn't UTF8 byte array either. That's what's wrong.
Because QUrl::fromPercentEncoding() requires UTF8 byte array, we must use .toUtf8() and not .toLatin1() or any other local 8-bit code page. Otherwise, we feed it byte encoding not in accordance with the docs and every non-Latin1 character from the payload is stripped (in QT4 replaced by a "?"). It is these values returned by QUrl::fromPercentEncoding() that are ultimately used by Myth (for DB queries, etc).
Because this bug is apparent, without possible side effect and breaks UPnP for many locales including UTF8, I'd like to ask to commit the fix even to the *-fixes branches...
Changed 14 years ago by
Attachment: | myth-0.24-fixes-UPnP-UTF8-fix.patch added |
---|
Patch for the latest stable release-0-24-fixes, r27355
comment:4 Changed 14 years ago by
Milestone: | unknown → 0.24.1 |
---|---|
Resolution: | → Fixed |
Status: | assigned → closed |
Committed as 56dab845 in master, 1ef81e137 in fixes/0.24, 2ae967078 in fixes/0.23
comment:5 Changed 14 years ago by
I've been around this bug also i .23, it is very annoying. Any chance it will make it to .23 ?
comment:6 Changed 14 years ago by
Ticket locked: | set |
---|
It's already been applied to .23-fixes... Please read the ticket howto (not to mention the resolution of the ticket right above your query)
The patch to fix the issue.