[vlc-devel] [BUG] upnp:f020c4f5 from 2012-03-08 breaks resolve_rel_url() - VLC:UPnP to MythTV

[Philipp Matthias Hahn]

Hello,

today I tried to debug a problem of VLC no longer being able to browse
the MythTV UPnP server, which was reported in
<http://www.mythtv.org/pipermail/mythtv-users/2013-October/354498.html>.

It's a bug in src/genlib/net/uri/uri.c:581 resolve_rel_url(), which can
be demonstrated by the following short programm:

 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>

 const char psz_base_url[] = "http://192.168.178.33:6544/getDeviceDesc";
 const char psz_event_sub_url[] = "CDS_Event";

 int main(void) {
     int ret;
     char *psz_url = (char *)malloc(strlen(psz_base_url) + strlen(psz_event_sub_url) + 1);

     ret = UpnpResolveURL(psz_base_url, psz_event_sub_url, psz_url);
     printf("%d %s\n", ret, psz_url);
     return 0;
 }

It returns
 "http://192.168.178.33:6544/CDS_EventDesc"
instead of
 "http://192.168.178.33:6544/CDS_Event"; notice the trailing "Desc".


This is causes by commit 0edaf336:
 Remove most of strcpy, sprintf and strcat

 Replace strcpy, sprintf and strcat by strncpy, snprintf and strncat to
 avoid buffer overflows.

 (forward port of commit 97a17ff5add73c97844e2fa74456bab4df0800f1)

which blindly replaces several
 strcpy(dest, src)
by
 strncpy(dest, src, strlen(src))
, which are NOT equivalent, as strlen() returns the length without the
ending '\0'. Thus the string is not terminated!


I reviewed that commit and changed several cases to simply use strdup()
instead of using strlen() every time.
Fixing resolve_rel_url() took a lot longer than expected, because there
were other problems hiding:
1. The conversion from if() to switch() broke the getaddrinfo() logic,
2. remove_dots() did not handle all cases from RFC 3986 correctly,
3. resolve_rel_url() did not handle queries and fragments correctly.

Please find attached my patch set including a new unit test; whith those
applied I'm no able to at least get VLC to connect to MythTV using UPnP.
But VLC/MythTV now has different problem...

Sincerely
Philipp
-- 
  / /  (_)__  __ ____  __ Philipp Hahn
 / /__/ / _ \/ // /\ \/ /
/____/_/_//_/\_,_/ /_/\_\ pmhahn at pmhahn.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-getaddrinfo-loop.patch
Type: text/x-diff
Size: 1520 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20140501/589bdc3f/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Directly-use-strdup.patch
Type: text/x-diff
Size: 4595 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20140501/589bdc3f/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Fix-broken-strncat-.-strlen.patch
Type: text/x-diff
Size: 3444 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20140501/589bdc3f/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Fix-resolve_rel_url.patch
Type: text/x-diff
Size: 8387 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20140501/589bdc3f/attachment-0003.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-Fix-remove_dots.patch
Type: text/x-diff
Size: 6205 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20140501/589bdc3f/attachment-0004.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-Add-unit-test-for-parse_uri.patch
Type: text/x-diff
Size: 6653 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20140501/589bdc3f/attachment-0005.patch>