Modify
Warning Please read the Ticket HowTo before creating or commenting on a ticket. Failure to do so may cause your ticket to be rejected or result in a slower response.

Opened 3 years ago

Closed 3 years ago

Last modified 3 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@… 3 years ago.
The patch to fix the issue.
myth-0.24-fixes-UPnP-UTF8-fix.patch (3.9 KB) - added by David Kubicek <foceni@…> 3 years ago.
Patch for the latest stable release-0-24-fixes, r27355

Download all attachments as: .zip

Change History (8)

Changed 3 years ago by foceni@…

The patch to fix the issue.

comment:1 Changed 3 years ago by beirdo

  • Owner changed from dblain to beirdo
  • Status changed from new to assigned

comment:2 Changed 3 years ago by beirdo

  • Keywords utf8 added; removed
  • Priority changed from major to minor

comment:3 Changed 3 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 3 years ago by David Kubicek <foceni@…>

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

comment:4 Changed 3 years ago by beirdo

  • Milestone changed from unknown to 0.24.1
  • Resolution set to Fixed
  • Status changed from assigned to closed

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

comment:5 Changed 3 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 3 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)

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'new'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.