[vlc-devel] [PATCH 1/2] cli: remove TCP mode (--rc-host)
Alexandre Janniaux
ajanni at videolabs.io
Sat Nov 21 12:10:20 CET 2020
Hi,
That's litterally my only use case for the Cli interface
and was a point that I shared and used for the transition
from lua RC to C RC. If this is to be merged, then there's
no point in replacing the lua RC and I'll vote for restoring
the lua RC.
-1
Regards,
--
Alexandre Janniaux
Videolabs
On Fri, Nov 20, 2020 at 07:05:40PM +0200, remi at remlab.net wrote:
> 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
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list