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

Description

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"
                   "Nigel&apos;s\n"
                   "Error&ampError\n"
                   "Don't&nbsp;wrap. &#xFFFF;";

    QRegExp entities("^(quot|amp|apos|lt|gt|nbsp"
                     "|#\\d\\d\\d\\d|#x[0-F][0-F][0-F][0-F]|);");

    qWarning("\tString...");
    qWarning(qPrintable(data));

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

        if (data.mid(++pos).contains(entities))
        {
            qWarning("ENTITY contained in ");
            qWarning(qPrintable(afterAmp))
        }
        else
            data.insert(pos, "amp;");

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

    qWarning("\tbecomes..");
    qWarning(qPrintable(data));
}

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.