[vlc-commits] [Git][videolan/vlc][master] 5 commits: upnp: encode the object ID in the exposed URL

Steve Lhomme (@robUx4) gitlab at videolan.org
Tue May 28 19:05:15 UTC 2024



Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
5d5ccbc0 by Alaric Senat at 2024-05-28T14:15:10+00:00
upnp: encode the object ID in the exposed URL

Object ID is not supposed to be URI-encoded by default and can be pretty
much any valid string. Since we add it to a valid encoded URL, we need
to encode it to preserve the URL validity.

- - - - -
0cba1475 by Alaric Senat at 2024-05-28T14:15:10+00:00
upnp: avoid double URL query start values

This patch completes ac99cee9f34fa17e0595d8b31a2dd1ff723c0719.
It avoids corrupting URLs with an already present query list.

- - - - -
bc1d58c0 by Alaric Senat at 2024-05-28T14:15:10+00:00
upnp: implement root MRL forging with std::string

Switching to std::string simplifies the next patch changing the URL
scheme.

- - - - -
44de051a by Alaric Senat at 2024-05-28T14:15:10+00:00
upnp: enforce HTTP(S) as the only supported protocols

In preparation for the next commit, explicitly refuse any protocols
that are not supported by libpupnp. It's unclear if it's needed and
pupnp probably already perform this check, but the next commit rely
heavily on the scheme matching "http" or "https" only.

- - - - -
241fed31 by Alaric Senat at 2024-05-28T14:15:10+00:00
upnp: fix exposed directory URLs schemes

Previous implementation generated input item directories with URLs not
compliant with *RFC 3986* in an attempt to keep the original URL while
triggering the UPNP directory acces properly. Here's an exemple of a
previous upnp directory url:

> upnp://http://192.168.1.109:32469/cds?ObjectID=0

The stacking of schemes (`upnp://http://`) is problematic and leads to
most of the validators failing on those generated URLs (see the
referenced issue).

This patch fix the issue by simply replacing the original `http://`
scheme by `upnp://` instead of stacking both. To avoid any potential
regression with some obscure usage forcing https, a shortcut of the
directory access is introduced for https specificaly.

The example above would then be fixed like that:

> upnp://192.168.1.109:32469/cds?ObjectID=0

Potential use-cases with https would instead generate the following:

> upnps://192.168.1.109:32469/cds?ObjectID=0

Refs VLCKit#728

- - - - -


2 changed files:

- modules/services_discovery/upnp.cpp
- modules/services_discovery/upnp.hpp


Changes:

=====================================
modules/services_discovery/upnp.cpp
=====================================
@@ -153,6 +153,7 @@ vlc_module_begin()
         set_subcategory( SUBCAT_INPUT_ACCESS )
         set_callbacks( Access::OpenAccess, Access::CloseAccess )
         set_capability( "access", 0 )
+        add_shortcut( "upnp", "upnps" )
 
     VLC_SD_PROBE_SUBMODULE
 
@@ -406,17 +407,31 @@ bool MediaServerList::addServer( MediaServerDesc* desc )
             free( psz_playlist_option );
         }
     } else {
-        char* psz_mrl;
-        // We might already have some options specified in the location.
-        char opt_delim = desc->location.find( '?' ) == std::string::npos ? '?' : '&';
-        if( asprintf( &psz_mrl, "upnp://%s%cObjectID=0", desc->location.c_str(), opt_delim ) < 0 )
+        const auto loc_starts_with = [desc](const char *s) {
+            return desc->location.compare(0, strlen(s), s) == 0;
+        };
+
+        if ( !loc_starts_with("http://") && !loc_starts_with("https://") )
+        {
+            msg_Warn( m_sd, "Unexpected underlying protocol, the UPNP module "
+                      "fully supports HTTP and has partial support for HTTPS" );
             return false;
+        } 
+
+        std::string mrl = desc->location;
+
+        // Replace the scheme to trigger the directory access.
+        mrl.replace( 0, 4, "upnp" );
+
+        // Forge a root object ID in the MRL, this is used in the dir access.
+        if ( desc->location.find( '?' ) == std::string::npos )
+            mrl += "?ObjectID=0";
+        else 
+            mrl += "&ObjectID=0";
 
-        p_input_item = input_item_NewDirectory( psz_mrl,
+        p_input_item = input_item_NewDirectory( mrl.c_str(),
                                                 desc->friendlyName.c_str(),
                                                 ITEM_NET );
-        free( psz_mrl );
-
         if ( !p_input_item )
             return false;
 
@@ -990,8 +1005,17 @@ namespace
             if ( objectID == NULL || title == NULL )
                 return NULL;
 
+            char *encoded_id = vlc_uri_encode( objectID );
+            if ( unlikely(encoded_id == NULL) )
+                return NULL;
+
+            const char opt_delim = strrchr( psz_root, '?' ) == NULL ? '?' : '&';
+
             char* psz_url;
-            if( asprintf( &psz_url, "upnp://%s?ObjectID=%s", psz_root, objectID ) < 0 )
+            const int ret = asprintf( &psz_url, "%s%cObjectID=%s", psz_root,
+                                      opt_delim, encoded_id );
+            free( encoded_id );
+            if( ret < 0 )
                 return NULL;
 
             input_item_t* p_item = input_item_NewDirectory( psz_url, title, ITEM_NET );
@@ -1053,15 +1077,17 @@ MediaServer::MediaServer( stream_t *p_access, input_item_node_t *node )
     , m_node( node )
 
 {
-    m_psz_root = strdup( p_access->psz_location );
+    m_psz_root = strdup( p_access->psz_url );
     char* psz_objectid = strstr( m_psz_root, "ObjectID=" );
     if ( psz_objectid != NULL )
     {
         // Remove this parameter from the URL, since it might cause some servers to fail
         // Keep in mind that we added a '&' or a '?' to the URL, so remove it as well
         *( psz_objectid - 1) = 0;
-        m_psz_objectId = &psz_objectid[strlen( "ObjectID=" )];
+        m_psz_objectId = vlc_uri_decode( &psz_objectid[strlen("ObjectID=")] );
     }
+
+    m_original_url = std::string( m_psz_root ).replace(0, 4, "http");
 }
 
 MediaServer::~MediaServer()
@@ -1253,7 +1279,7 @@ IXML_Document* MediaServer::_browseAction( const char* psz_object_id_,
      * interrupted by vlc_interrupt_kill */
     i11eCb = new Upnp_i11e_cb( sendActionCb, &p_response );
     i_res = UpnpSendActionAsync( sys->p_upnp->handle(),
-              m_psz_root,
+              m_original_url.c_str(),
               CONTENT_DIRECTORY_SERVICE_TYPE,
               NULL, /* ignored in SDK, must be NULL */
               p_action,


=====================================
modules/services_discovery/upnp.hpp
=====================================
@@ -119,6 +119,7 @@ private:
 private:
     char* m_psz_root;
     char* m_psz_objectId;
+    std::string m_original_url;
     stream_t* m_access;
     input_item_node_t* m_node;
 };



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/a44480e820f66227ff2d2d059750ab4769b198db...241fed315be92f4a12ced2afdffd79f5fd638ff4

-- 
This project does not include diff previews in email notifications.
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/a44480e820f66227ff2d2d059750ab4769b198db...241fed315be92f4a12ced2afdffd79f5fd638ff4
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list