[vlc-devel] [PATCH 1/2] cli: remove TCP mode (--rc-host)

remi at remlab.net remi at remlab.net
Fri Nov 20 18:05:40 CET 2020


From: RĂ©mi Denis-Courmont <remi at remlab.net>

There are several issues with TCP mode:
- It can only handle one client at a time, and will ignore any new
  connection until the current one properly ends, making it as
  unreliable as it comes.
- Most commands are not acknowledged.
- It has no security whatsoever.

There is not much point trying to "fix" those issues, as it would
not provide backward compatibility, which was the only reason to keep
this.

For remote control, the HTTP interface is much better. It is insecure,
but at least it handles multiple clients, acknowledges requests and
requires a password.

For reliability and security, a much better approach is to pass the
CLI commands over SSH (or similar) - which is what people actually do.
---
 modules/control/cli/cli.c | 94 ++++-----------------------------------
 1 file changed, 9 insertions(+), 85 deletions(-)

diff --git a/modules/control/cli/cli.c b/modules/control/cli/cli.c
index c741e7fcad..1cd3f846cf 100644
--- a/modules/control/cli/cli.c
+++ b/modules/control/cli/cli.c
@@ -97,11 +97,7 @@ static void msg_vprint(intf_thread_t *p_intf, const char *psz_fmt, va_list args)
     if( len < 0 )
         return;
 
-    if( p_intf->p_sys->i_socket == -1 )
-        utf8_fprintf( stdout, "%s", msg );
-    else
-        net_Write( p_intf, p_intf->p_sys->i_socket, msg, len );
-
+    utf8_fprintf( stdout, "%s", msg );
     free( msg );
 #endif
 }
@@ -247,12 +243,6 @@ static void LogOut(intf_thread_t *intf, const char *const *args, size_t count)
             vlc_close(fd);
         }
     }
-#else
-    if (sys->i_socket != -1)
-    {
-        net_Close(sys->i_socket);
-        sys->i_socket = -1;
-    }
 #endif
     (void) args; (void) count;
 }
@@ -493,9 +483,9 @@ static bool ReadWin32( intf_thread_t *p_intf, unsigned char *p_buffer, int *pi_s
 static bool ReadCommand(intf_thread_t *p_intf, char *p_buffer, int *pi_size)
 {
 #if !VLC_WINSTORE_APP
-    if( p_intf->p_sys->i_socket == -1 && !p_intf->p_sys->b_quiet )
+    if( !p_intf->p_sys->b_quiet )
         return ReadWin32( p_intf, (unsigned char*)p_buffer, pi_size );
-    else if( p_intf->p_sys->i_socket == -1 )
+    else
     {
         vlc_tick_sleep( INTF_IDLE_SLEEP );
         return false;
@@ -504,25 +494,11 @@ static bool ReadCommand(intf_thread_t *p_intf, char *p_buffer, int *pi_size)
 
     while( *pi_size < MAX_LINE_LENGTH )
     {
-        if( p_intf->p_sys->i_socket == -1 )
-        {
-            if( read( 0/*STDIN_FILENO*/, p_buffer + *pi_size, 1 ) <= 0 )
-            {   /* Standard input closed: exit */
-                libvlc_Quit( vlc_object_instance(p_intf) );
-                p_buffer[*pi_size] = 0;
-                return true;
-            }
-        }
-        else
-        {   /* Connection closed */
-            if( net_Read( p_intf, p_intf->p_sys->i_socket, p_buffer + *pi_size,
-                          1 ) <= 0 )
-            {
-                net_Close( p_intf->p_sys->i_socket );
-                p_intf->p_sys->i_socket = -1;
-                p_buffer[*pi_size] = 0;
-                return true;
-            }
+        if( read( 0/*STDIN_FILENO*/, p_buffer + *pi_size, 1 ) <= 0 )
+        {   /* Standard input closed: exit */
+            libvlc_Quit( vlc_object_instance(p_intf) );
+            p_buffer[*pi_size] = 0;
+            return true;
         }
 
         if( p_buffer[ *pi_size ] == '\r' || p_buffer[ *pi_size ] == '\n' )
@@ -574,14 +550,6 @@ static void *Run( void *data )
         bool b_complete;
 
         vlc_restorecancel( canc );
-
-        if( p_sys->pi_socket_listen != NULL && p_sys->i_socket == -1 )
-        {
-            p_sys->i_socket =
-                net_Accept( p_intf, p_sys->pi_socket_listen );
-            if( p_sys->i_socket == -1 ) continue;
-        }
-
         b_complete = ReadCommand( p_intf, p_buffer, &i_size );
         canc = vlc_savecancel( );
 
@@ -688,42 +656,6 @@ static int Activate( vlc_object_t *p_this )
 #endif /* AF_LOCAL */
 #endif /* !_WIN32 */
 
-    if( ( pi_socket == NULL ) &&
-        ( psz_host = var_InheritString( p_intf, "rc-host" ) ) != NULL )
-    {
-        vlc_url_t url;
-
-        vlc_UrlParse( &url, psz_host );
-        if( url.psz_host == NULL )
-        {
-            vlc_UrlClean( &url );
-            char *psz_backward_compat_host;
-            if( asprintf( &psz_backward_compat_host, "//%s", psz_host ) < 0 )
-            {
-                free( psz_host );
-                return VLC_EGENERIC;
-            }
-            free( psz_host );
-            psz_host = psz_backward_compat_host;
-            vlc_UrlParse( &url, psz_host );
-        }
-
-        msg_Dbg( p_intf, "base: %s, port: %d", url.psz_host, url.i_port );
-
-        pi_socket = net_ListenTCP(p_this, url.psz_host, url.i_port);
-        if( pi_socket == NULL )
-        {
-            msg_Warn( p_intf, "can't listen to %s port %i",
-                      url.psz_host, url.i_port );
-            vlc_UrlClean( &url );
-            free( psz_host );
-            return VLC_EGENERIC;
-        }
-
-        vlc_UrlClean( &url );
-        free( psz_host );
-    }
-
     intf_sys_t *p_sys = malloc( sizeof( *p_sys ) );
     if( unlikely(p_sys == NULL) )
     {
@@ -756,7 +688,6 @@ static int Activate( vlc_object_t *p_this )
     }
     p_sys->psz_unix_path = psz_unix_path;
 #else
-    p_sys->i_socket = -1;
 #if VLC_WINSTORE_APP
     p_sys->b_quiet = true;
 #else
@@ -816,9 +747,6 @@ static void Deactivate( vlc_object_t *p_this )
             unlink(p_sys->psz_unix_path);
             free(p_sys->psz_unix_path);
         }
-#else
-        if (p_sys->i_socket != -1)
-            net_Close(p_sys->i_socket);
 #endif
     }
     free( p_sys );
@@ -838,10 +766,6 @@ static void Deactivate( vlc_object_t *p_this )
 #define UNIX_LONGTEXT N_("Accept commands over a Unix socket rather than " \
                          "stdin." )
 
-#define HOST_TEXT N_("TCP command input")
-#define HOST_LONGTEXT N_("Accept commands over a socket rather than stdin. " \
-            "You can set the address and port the interface will bind to." )
-
 #ifdef _WIN32
 #define QUIET_TEXT N_("Do not open a DOS command box interface")
 #define QUIET_LONGTEXT N_( \
@@ -868,7 +792,7 @@ vlc_module_begin()
     add_string("rc-unix", NULL, UNIX_TEXT, UNIX_LONGTEXT, true)
 #endif
 #endif
-    add_string("rc-host", NULL, HOST_TEXT, HOST_LONGTEXT, true)
+    add_obsolete_string("rc-host") /* since 4.0 */
 
     set_capability("interface", 20)
 
-- 
2.29.2



More information about the vlc-devel mailing list