Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#12006 closed Patch - Bug Fix (fixed)

upnp broken by Changeset 3d9155909

Reported by: steve_g@… Owned by: Karl Egly
Priority: major Milestone: 0.28
Component: MythTV - UPnP Version: Master Head
Severity: high Keywords: upnp soap
Cc: Ticket locked: yes

Description

Changeset 3d9155909 needs either to be reverted or the attached patch done.

There are 2 places in libmythupnp where the incomin packets are decoded, in one place they are converted to lowercase, in the other place they are not. This patch had made it lowercase in both places but did not change the code that accessed the data, the caused serious discombobulation and the end of upnp watching through out the world.

---attach starts here diff --git a/mythtv/libs/libmythupnp/upnpcds.cpp b/mythtv/libs/libmythupnp/upnpcds.cpp index 3cb0eb1..a36ef52 100644 --- a/mythtv/libs/libmythupnp/upnpcds.cpp +++ b/mythtv/libs/libmythupnp/upnpcds.cpp @@ -296,17 +296,17 @@ void UPnpCDS::HandleBrowse?( HTTPRequest *pRequest )

UPnpCDSRequest request;

DetermineClient?( pRequest, &request );

  • request.m_sObjectId = pRequest->m_mapParams[ "ObjectID" ];
  • request.m_sContainerID = pRequest->m_mapParams[ "ContainerID" ];

+ request.m_sObjectId = pRequest->m_mapParams[ "objectid" ]; + request.m_sContainerID = pRequest->m_mapParams[ "containerid" ];

request.m_sParentId = "0"; request.m_eBrowseFlag =

+ GetBrowseFlag?( pRequest->m_mapParams[ "browseflag" ] ); + request.m_sFilter = pRequest->m_mapParams[ "filter" ];

request.m_nStartingIndex =

+ pRequest->m_mapParams[ "startingindex" ].toLong();

request.m_nRequestedCount =

+ pRequest->m_mapParams[ "requestedcount"].toLong(); + request.m_sSortCriteria = pRequest->m_mapParams[ "sortcriteria" ];

#if 0

LOG(VB_UPNP, LOG_DEBUG, QString("UPnpCDS::ProcessRequest? \n"

@@ -493,15 +493,15 @@ void UPnpCDS::HandleSearch?( HTTPRequest *pRequest )

QString sResultXML;

DetermineClient?( pRequest, &request );

  • request.m_sObjectId = pRequest->m_mapParams[ "ObjectID" ];
  • request.m_sContainerID = pRequest->m_mapParams[ "ContainerID" ];
  • request.m_sFilter = pRequest->m_mapParams[ "Filter" ];

+ request.m_sObjectId = pRequest->m_mapParams[ "objectid" ]; + request.m_sContainerID = pRequest->m_mapParams[ "containerid" ]; + request.m_sFilter = pRequest->m_mapParams[ "filter" ];

request.m_nStartingIndex =

+ pRequest->m_mapParams[ "startingIndex" ].toLong();

request.m_nRequestedCount =

+ pRequest->m_mapParams[ "requestedcount"].toLong(); + request.m_sSortCriteria = pRequest->m_mapParams[ "sortcriteria" ]; + request.m_sSearchCriteria = pRequest->m_mapParams[ "searchcriteria"];

LOG(VB_UPNP, LOG_INFO,

QString("UPnpCDS::HandleSearch? ObjectID=%1, ContainerId?=%2")

The attached patch changes where the array is accessed to lowercase.

tested on v0.28-pre-673-g3bbc531-dirty

Attachments (1)

upnpcase.patch (2.8 KB) - added by steve_@… 6 years ago.
the patch as a file.

Download all attachments as: .zip

Change History (9)

Changed 6 years ago by steve_@…

Attachment: upnpcase.patch added

the patch as a file.

comment:1 Changed 6 years ago by Karl Egly

Type: Bug Report - GeneralPatch - Bug Fix

comment:2 Changed 6 years ago by Karl Dietz <dekarl@…>

Resolution: fixed
Status: newclosed

In e9a22c587713f2933402232b15daf82d4272dd2d/mythtv:

make parameter names consistently lowercase

Since [3d9155909f] the parameter names used inconsistent upper and lower case.

There are two places in libmythupnp where the incoming packets are decoded, in
one place they are converted to lowercase, in the other place they are not.
This commit had made it lowercase in both places but did not change the code
that accessed the data, the caused serious discombobulation and the end of UPNP
watching through out the world.

Patch by Steve Beinicke

Fixes #12006

comment:3 Changed 6 years ago by Karl Dietz <dekarl@…>

In e50af7d5d1c119b8a7cb3fb9d7444de4b7e270dc/mythtv:

make parameter names consistently lowercase

Since [3d9155909f] the parameter names used inconsistent upper and lower case.

There are two places in libmythupnp where the incoming packets are decoded, in
one place they are converted to lowercase, in the other place they are not.
This commit had made it lowercase in both places but did not change the code
that accessed the data, the caused serious discombobulation and the end of UPNP
watching through out the world.

Patch by Steve Beinicke

Fixes #12006
(cherry picked from commit e9a22c587713f2933402232b15daf82d4272dd2d)

comment:4 Changed 6 years ago by Karl Egly

Milestone: 0.280.27.1
Owner: changed from dblain to Karl Egly

comment:5 Changed 6 years ago by Karl Egly

Milestone: 0.27.10.28

reverted in fixes/0.27 in [a8a79f05c9]

comment:6 Changed 4 years ago by bjoernv@…

Why was this patch reverted in fixes/0.27? I tried both versions with and without the patch and my UPnP does not work in both versions. My UPnP problem may be unrelated to this bug, but I tried to fix the problems with the patch suggested here.

The UPnP collection is empty and I get VLC errors like this:

VLC media player 2.2.1 Terry Pratchett (Weatherwax) (revision 2.2.1-0-ga425c42)
[00000000011e7148] core libvlc: Running vlc with the default interface. Use 'cvlc' to use vlc without interface.
[00007fee24238348] upnp services discovery: Initializing libupnp on '(null)' interface
[00007fee24238348] upnp services discovery error: UPNP_E_BAD_RESPONSE when trying the send() action with URL: http://XXX.XXX.XXX.XXX:6544/CDS_Controlsc
[00007fee24238348] upnp services discovery error: No response from browse() action
QObject::~QObject: Timers cannot be stopped from another thread
Last edited 4 years ago by Stuart Auchterlonie (previous) (diff)

comment:7 Changed 4 years ago by Karl Egly

Ticket locked: set

The patch was reverted because the original issue was not brought into fixes/0.27. This led to the presumed fix actually introducing a problem.

Please open a new ticket for your issue with logs from VLC and mythbackend (with --verbose http,upnp)

comment:8 Changed 4 years ago by stuartm

VLC's uPnP implementation is completely and utterly broken, even if you can persuade it to connect it launches a DOS attack on the upnp server. One individual metadata request for every media item on the server instead of batch loading. Really appalling work and when I discussed it with them they couldn't be bothered to fix it.

Note: See TracTickets for help on using tickets.