Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#8132 closed defect (fixed)

CDS_Control/Browse can't process IDs containing '&' in GET requests

Reported by: Kris Raney <mythtv@…> Owned by: beirdo
Priority: minor Milestone: 0.25
Component: MythTV - UPnP Version: unknown
Severity: medium Keywords:
Cc: Ticket locked: no


The problem is in HTTPRequest::GetParameters? - it performs substitutions on %26 and &amp; BEFORE splitting the get request into fields. So no matter how you escape the ampersands in a program name, it always gets interpreted as a field separator.

For example, "ObjectID=RecTv?/1/key=Law%20%26%%20Order" would first get unescaped to "ObjectID=RecTv?/1/key=Law & Order", and then split up into an "ObjectID" field with a value of "RecTv?/1/key=Law " and an "Order" field.

Of course, standard UPnP would be making SOAP requests rather than GET requests, but having GET working properly is handy for testing.

Attachments (0)

Change History (8)

comment:1 Changed 8 years ago by robertm

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

comment:2 Changed 8 years ago by beirdo

  • Status changed from assigned to accepted

I will attempt to look into this once I have recorded a file with an & in it (Rizzoli & Isles -- tomorrow).

comment:3 Changed 7 years ago by beirdo

  • Milestone changed from unknown to 0.25
  • Status changed from accepted to infoneeded

What is the necessity of this? As mentioned, UPnP is meant to use SOAP. This shouldn't be necessary. Please let me know what the use case is.

comment:4 Changed 7 years ago by anonymous

One general use case is for debugging. My specific use case is an attempt to make a myth client for Yahoo! widgets, which would run natively on TVs that support them. The only real option for client requests in that API is HTTP GET requests.

Since the API supports GET requests, and it's only a matter of reshuffling two lines of code so that escaping happens after the GET is broken into separate fields, instead of before, I don't see the harm.

comment:5 Changed 7 years ago by stuartm

  • Milestone 0.25 deleted

Milestone 0.25 deleted

comment:6 Changed 7 years ago by robertm

  • Status changed from infoneeded to assigned

comment:7 Changed 7 years ago by beirdo

  • Resolution set to Fixed
  • Status changed from assigned to closed

Fixed in f76028ba358 on master.

comment:8 Changed 7 years ago by Gavin Hurlbut

  • Resolution changed from Fixed to fixed

Fix passing a URL key/value containing %26 (escaped &)

It seems that this code *only* deals with Parameters in the URL (maybe on POST too), but not in SOAP requests from UPnP clients. It should be a benign change to not mess with %26 in the params string before splitting on &. Any self-respecting client will know that & separates the keys, and not to escape them. If this breaks any specific clients, we will have to put in a workaround for the broken client perhaps.

Fixes #8132.

Changeset: f76028ba35811818f75c3d39faa4479b024bc846

Add Comment

Modify Ticket

as closed The owner will remain beirdo.
The resolution will be deleted. Next status will be 'new'.

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

Note: See TracTickets for help on using tickets.