[vlc-devel] [PATCH] HTTP proxy support the MMS protocol (not MMSH)

Laurent Aimar fenrir at via.ecp.fr
Sun May 17 16:40:44 CEST 2009


Hi,

On Sun, May 17, 2009, meta tech wrote:
>    Here is a patch to provide HTTP proxy support for the MMS protocol (not
>    MMSH).
>    It also features BASIC proxy authentication (username/password).
> 
>    This is my first patch for VLC (I did some patches for Wireshark in the
>    past though), please be gentle with me :-)
>    Please commit.

> From d850a8cb15d6b36d0e37d9de15a3fa3a72fc5125 Mon Sep 17 00:00:00 2001
> From: Meta Tech <metatechbe at gmail.com>
> Date: Sat, 25 Apr 2009 12:15:14 +0200
> Subject: [PATCH] This patch provides HTTP proxy support the MMS protocol (not MMSH).
>  BASIC proxy authentication with username and password is also supported.
> 
> ---
>  modules/access/mms/mmstu.c |  163 +++++++++++++++++++++++++++++++++++++++++--
>  modules/access/mms/mmstu.h |    4 +
>  2 files changed, 159 insertions(+), 8 deletions(-)
> 
> diff --git a/modules/access/mms/mmstu.c b/modules/access/mms/mmstu.c
> index f0010a7..3ea4a62 100644
> --- a/modules/access/mms/mmstu.c
> +++ b/modules/access/mms/mmstu.c
> @@ -62,6 +62,10 @@
>  #include "mms.h"
>  #include "mmstu.h"
>  
> +#ifdef HAVE_PROXY_H
> +#    include "proxy.h"
 Is it a system installed header ? if so use <>

>  /****************************************************************************
> @@ -498,20 +502,163 @@ static int MMSOpen( access_t  *p_access, vlc_url_t *p_url, int  i_proto )
>      int          i_streams;
>      int          i_first;
>      char         *mediapath;
> +    char         *psz;
>  
> +    p_sys->b_proxy = false;
> +    /* Check proxy */
> +    psz = var_CreateGetNonEmptyString( p_access, "http-proxy" );
> +    if( psz )
> +    {
> +        p_sys->b_proxy = true;
> +        vlc_UrlParse( &p_sys->proxy, psz, 0 );
> +        free( psz );
> +    }
> +#ifdef HAVE_PROXY_H
> +    else
> +    {
> +        pxProxyFactory *pf = px_proxy_factory_new();
> +        if (pf)
> +        {
> +            char *buf;
 Missing psz_ prefix.
> +            int i;
> +            i=asprintf(&buf, "%s://%s", p_access->psz_access, p_access->psz_path);
> +            if (i >= 0)
 Useless temporary variable especially when its meaning changes below.
> +            {
> +                msg_Dbg(p_access, "asking libproxy about url '%s'", buf);
> +                char **proxies = px_proxy_factory_get_proxies(pf, buf);
 Missing ppsz_ prefix.
> +                if (proxies[0])
> +                {
> +                    msg_Dbg(p_access, "libproxy suggest to use '%s'", proxies[0]);
> +                    if(strcmp(proxies[0],"direct://") != 0)
 Wouldn't it be better to iterate over proxies until a non direct:// is found ?
> +                    {
> +                        p_sys->b_proxy = true;
> +                        vlc_UrlParse( &p_sys->proxy, proxies[0], 0);
> +                    }
> +                }
> +                for(i=0;proxies[i];i++) free(proxies[i]);
> +                free(proxies);
> +                free(buf);
> +            }
> +            px_proxy_factory_free(pf);
> +        }
> +        else
> +        {
> +            msg_Err(p_access, "Allocating memory for libproxy failed");
> +        }
 overall inconsistant coding style with the current file.

> +    }
> +#elif HAVE_GETENV
> +    else
> +    {
> +        psz = getenv( "http_proxy" );
> +        if( psz )
> +        {
> +            p_sys->b_proxy = true;
> +            vlc_UrlParse( &p_sys->proxy, psz, 0 );
> +        }
> +    }
> +#endif

To avoid all the:
> +            p_sys->b_proxy = true;
> +            vlc_UrlParse( &p_sys->proxy, psz, 0 );
 It could be better to simply set a psz_proxy string (default NULL) and then
do the above code. (If not simpler, then ignore it).

> -    /* *** Open a TCP connection with server *** */
> -    msg_Dbg( p_access, "waiting for connection..." );
> -    p_sys->i_handle_tcp = net_ConnectTCP( p_access, p_url->psz_host, p_url->i_port );
> -    if( p_sys->i_handle_tcp < 0 )
> +    if( p_sys->b_proxy )
>      {
> -        msg_Err( p_access, "failed to open a connection (tcp)" );
> -        return VLC_EGENERIC;
> +        if( p_sys->proxy.psz_host == NULL || *p_sys->proxy.psz_host == '\0' )
> +        {
> +            msg_Warn( p_access, "invalid proxy host" );
> +            return -1;
 VLC_EGENERIC
 Wouldn't it be better to simply disable proxy usage ?

> +        }
> +        if( p_sys->proxy.i_port <= 0 )
> +        {
> +            p_sys->proxy.i_port = 80;
> +        }
> +    }

 The whole code above seems duplicated from http.c. It must be avoided. A simple
inline function in a .h included by http/mms would fix that.


> +    vlc_url_t      srv = p_sys->b_proxy ? p_sys->proxy : p_sys->url;
> +
> +    /* Open connection */
> +    p_sys->i_handle_tcp = net_ConnectTCP( p_access, srv.psz_host, srv.i_port );
> +    if( p_sys->i_handle_tcp == -1 )
> +    {
> +        msg_Err( p_access, "cannot connect to %s:%d", srv.psz_host, srv.i_port );
> +        return -1;
 VLC_EGENERIC

>      }
>      msg_Dbg( p_access,
>               "connection(tcp) with \"%s:%d\" successful",
> -             p_url->psz_host,
> -             p_url->i_port );
> +             srv.psz_host,
> +             srv.i_port );
> +
> +    if( p_sys->b_proxy )
> +    {
> +        /* CONNECT to establish MMS tunnel through HTTP proxy */
> +        char *psz;
> +        unsigned i_status = 0;
> +
> +        /* Proxy Authentication */
> +        if( p_sys->proxy.psz_username && *p_sys->proxy.psz_username )
> +        {
> +            char *buf;
> +            char *b64;
> +
> +            if( asprintf( &buf, "%s:%s", p_sys->proxy.psz_username,
> +                       p_sys->proxy.psz_password ? p_sys->proxy.psz_password : "" ) == -1 )
> +                return VLC_ENOMEM;
> +
> +            b64 = vlc_b64_encode( buf );
> +            free( buf );
> +
> +            net_Printf( VLC_OBJECT(p_access), p_sys->i_handle_tcp, NULL,
> +                "CONNECT %s:%d HTTP/1.1\r\nProxy-Authorization: Basic %s\r\nHost: %s:%d\r\n\r\n",
> +                p_sys->url.psz_host, p_sys->url.i_port, b64,
> +                p_sys->url.psz_host, p_sys->url.i_port);
> +
> +                free( b64 );
> +        } else {
> +            net_Printf( VLC_OBJECT(p_access), p_sys->i_handle_tcp, NULL,
> +                "CONNECT %s:%d HTTP/1.1\r\nHost: %s:%d\r\n\r\n",
> +                p_sys->url.psz_host, p_sys->url.i_port,
> +                p_sys->url.psz_host, p_sys->url.i_port);
> +        }
> +
> +        psz = net_Gets( VLC_OBJECT(p_access), p_sys->i_handle_tcp, NULL );
> +        if( psz == NULL )
> +        {
> +            msg_Err( p_access, "cannot establish HTTP/MMS tunnel" );
> +            net_Close( p_sys->i_handle_tcp );
> +            return -1;
> +        }
> +
> +        sscanf( psz, "HTTP/%*u.%*u %3u", &i_status );
> +        free( psz );
> +
> +        if( ( i_status / 100 ) != 2 )
> +        {
> +            msg_Err( p_access, "HTTP/MMS tunnel through proxy denied" );
> +            net_Close( p_sys->i_handle_tcp );
> +            return -1;
> +        }
> +
> +        do
> +        {
> +            psz = net_Gets( VLC_OBJECT(p_access), p_sys->i_handle_tcp, NULL );
> +            if( psz == NULL )
> +            {
> +                msg_Err( p_access, "HTTP proxy connection failed" );
> +                net_Close( p_sys->i_handle_tcp );
> +                return -1;
> +            }
> +
> +            if( *psz == '\0' )
> +                i_status = 0;
> +
> +            free( psz );
> +
> +            if( !vlc_object_alive (p_access) || p_access->b_error )
> +            {
> +                net_Close( p_sys->i_handle_tcp );
> +                return -1;
> +            }
> +        }
> +        while( i_status );
> +    }
 Again it seems to be duplicated with http.c and must be avoided.

Regards,

-- 
fenrir



More information about the vlc-devel mailing list