Opened 12 years ago

Closed 11 years ago

#5435 closed patch (invalid)

Incorrect Escaping of '&' in UPNP Requests

Reported by: jeff@… Owned by: Nigel
Priority: minor Milestone: unknown
Component: upnp Version: head
Severity: medium Keywords:
Cc: Ticket locked: no


I was seeing this quite often in the debugging output from Mythbackend:

2008-06-07 14:35:48.531 HTTPRequest::Encode Input : RecTv?/0/item?ChanId?=1249&StartTime?=2008-04-23T22:00:00 2008-06-07 14:35:48.536 HTTPRequest::Encode Output : RecTv?/0/item?ChanId?=1249&StartTime?=2008-04-23T22:00:00 2008-06-07 14:35:48.622 HTTPRequest::Encode Input : RecTv?/1/key=South Park/item?ChanId??=1249amp;StartTime?=2008-04-16T22:00:00 2008-06-07 14:35:48.626 HTTPRequest::Encode Output : RecTv?/1/key=South Park/item?ChanId?=1249&StartTime?=2008-04-16T22:00:00 2008-06-07 14:35:48.650 HTTPRequest::Encode Input : RecTv?/0/item?ChanId?=1249&StartTime?=2008-04-16T22:00:00 2008-06-07 14:35:48.654 HTTPRequest::Encode Output : RecTv?/0/item?ChanId?=1249&StartTime?=2008-04-16T22:00:00

It seems like the HTTPRequest::Encode routing is getting called multiple times for the same string, and because of this, the ampersand in the escaped output was getting re-escaped.

This patch was made against MythTV 0.21p17416.

Attachments (1)

upnp.patch (1.3 KB) - added by jeff@… 12 years ago.
Fix Escaping of '&' in UPNP Requests

Download all attachments as: .zip

Change History (10)

Changed 12 years ago by jeff@…

Attachment: upnp.patch added

Fix Escaping of '&' in UPNP Requests

comment:1 Changed 11 years ago by stuartm

Owner: changed from dblain to stuartm
Status: newaccepted

comment:2 Changed 11 years ago by stuartm

Status: acceptedstarted

comment:3 Changed 11 years ago by stuartm

Resolution: fixed
Status: startedclosed

(In [18428]) Fix ampersand escaping bug in HTTPRequest. Closes #5435. I've not applied the second part of the patch since it reverses the existing logic and there is no explanation given for this in the ticket.

comment:4 Changed 11 years ago by Nigel

Resolution: fixed
Status: closednew

It seems there is a side effect from this patch. On some UPnP clients, the Recordings->All Recordings page does not appear (i.e. it probbaly has errors). Strangely, on some clients, browsing other pages first allows the All Recordings page to display. Very strange.

comment:5 Changed 11 years ago by Nigel

Milestone: 0.21.1unknown
Owner: changed from stuartm to Nigel
Status: newassigned
Version: 0.21-fixeshead

comment:6 Changed 11 years ago by Nigel

It wasn't a side-effect, the RegExp? in the patch just doesn't work, but the clients sometimes cache old data so it wasn't happening all the time. Here is the test program I used to experiment:

#include <QApplication>
#include <QRegExp>
#include <QString>

int main(int argc, char *argv[])
    QApplication a(argc, argv);

    QString data = "blah, blah & blah\n"
                   "Don't&nbsp;wrap. &#xFFFF;";

    QRegExp entities("^(quot|amp|apos|lt|gt|nbsp"


    int pos = data.indexOf('&');
    while (pos >= 0)
        QString afterAmp = data.mid(++pos);

        if (data.mid(++pos).contains(entities))
            qWarning("ENTITY contained in ");
            data.insert(pos, "amp;");

        pos = data.indexOf('&', pos);  // Find next &


comment:7 Changed 11 years ago by Nigel

(In [19122]) Fix broken & => &amp; processing (see #5435)

comment:8 Changed 11 years ago by Isaac Richards

(In [19150]) Revert [19122] and [18428] and restore the original ampersand escaping behavior. The doubled escape (&amp;amp;) is necessary.

Fixes uPnP playback on PS3, WMP, with gupnp-avp-cp, etc.

Refs #5435 - the patches attached to this bug were incorrect.

comment:9 Changed 11 years ago by Isaac Richards

Resolution: invalid
Status: assignedclosed

This patch (and associated checkins) break upnp playback on ps3, windows media player, and with the 'gupnp-tools' package on linux. Reverted in [19150].

Note: See TracTickets for help on using tickets.