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

Felix Paul Kühne (@fkuehne) gitlab at videolan.org
Mon Jun 3 08:41:12 UTC 2024



Felix Paul Kühne pushed to branch 3.0.x at VideoLAN / VLC


Commits:
9b4bd076 by Alaric Senat at 2024-06-03T08:25:58+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.

(cherry picked from commit 5d5ccbc0fa309d2aa07ca840ab66e9403fd15b03)

- - - - -
dc82b76d by Alaric Senat at 2024-06-03T08:25:58+00:00
upnp: avoid double URL query start values

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

(cherry picked from commit 0cba1475ba9c2d89b19c4e81a928e4305e6e97ad)

- - - - -
a5e97d74 by Alaric Senat at 2024-06-03T08:25:58+00:00
upnp: implement root MRL forging with std::string

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

(cherry picked from commit bc1d58c065d1a359863f3bc4216351b54c60e51d)

- - - - -
237ab100 by Alaric Senat at 2024-06-03T08:25:58+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.

(cherry picked from commit 44de051a61475a899f1bd71a3db0e737a481b41f)

- - - - -
0229706f by Alaric Senat at 2024-06-03T08:25:58+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

(cherry picked from commit 241fed315be92f4a12ced2afdffd79f5fd638ff4)

- - - - -


2 changed files:

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


Changes:

=====================================
modules/services_discovery/upnp.cpp
=====================================
@@ -139,6 +139,7 @@ vlc_module_begin()
         set_subcategory( SUBCAT_INPUT_ACCESS )
         set_callbacks( Access::Open, Access::Close )
         set_capability( "access", 0 )
+        add_shortcut( "upnp", "upnps" )
 
     VLC_SD_PROBE_SUBMODULE
 vlc_module_end()
@@ -350,17 +351,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;
 
@@ -953,8 +968,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 );
@@ -1024,15 +1048,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()
@@ -1224,7 +1250,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
=====================================
@@ -164,6 +164,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/b351ccccc41b5807b93579614b0eb1d9ad86ce6e...0229706f3f6f7686ea6701a769abc50f58e6516b

-- 
This project does not include diff previews in email notifications.
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/b351ccccc41b5807b93579614b0eb1d9ad86ce6e...0229706f3f6f7686ea6701a769abc50f58e6516b
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