Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#9188 closed patch (Fixed)

UPnP browsing "By Title" fails for shows with non-Latin1 characters [+PATCH]

Reported by: foceni@… 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)

myth-0.23-fixes-UPnP-UTF8-fix.patch (3.9 KB) - added by foceni@… 13 years ago.
The patch to fix the issue.
myth-0.24-fixes-UPnP-UTF8-fix.patch (3.9 KB) - added by David Kubicek <foceni@…> 13 years ago.
Patch for the latest stable release-0-24-fixes, r27355

Download all attachments as: .zip

Change History (8)

Changed 13 years ago by foceni@…

The patch to fix the issue.

comment:1 Changed 13 years ago by beirdo

Owner: changed from dblain to beirdo
Status: newassigned

comment:2 Changed 13 years ago by beirdo

Keywords: utf8 added; removed
Priority: majorminor

comment:3 Changed 13 years ago by David Kubicek <foceni@…>

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 13 years ago by David Kubicek <foceni@…>

Patch for the latest stable release-0-24-fixes, r27355

comment:4 Changed 13 years ago by beirdo

Milestone: unknown0.24.1
Resolution: Fixed
Status: assignedclosed

Committed as 56dab845 in master, 1ef81e137 in fixes/0.24, 2ae967078 in fixes/0.23

comment:5 Changed 13 years ago by 2@…

I've been around this bug also i .23, it is very annoying. Any chance it will make it to .23 ?

comment:6 Changed 13 years ago by robertm

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)

Note: See TracTickets for help on using tickets.