[vlc-devel] [PATCH 1/5] access/http: Introduce a struct for cookie data

Rémi Denis-Courmont remi at remlab.net
Tue Jul 22 12:32:04 CEST 2014


    Heippa,

Le 2014-07-22 13:09, Antti Ajanki a écrit :
> ---
>  modules/access/http.c |  112
> +++++++++++++++++++++++++++----------------------
>  1 file changed, 61 insertions(+), 51 deletions(-)
>
> diff --git a/modules/access/http.c b/modules/access/http.c
> index 78a186c..75ed1cf 100644
> --- a/modules/access/http.c
> +++ b/modules/access/http.c
> @@ -188,6 +188,13 @@ struct access_sys_t
>      vlc_array_t * cookies;
>  };
>
> +typedef struct http_cookie_t
> +{
> +    char *psz_name;
> +    char *psz_value;
> +    char *psz_domain;
> +} http_cookie_t;

Fine but did you consider creating a cookie_s_ structure and getting 
rid of the generic array?

Also, I'd move the cookie code to a separate file, http.c is getting a 
bit large :-/ and is very messy pile of spaghettis :-(

> +
>  /* */
>  static int OpenWithCookies( vlc_object_t *p_this, const char 
> *psz_access,
>                              unsigned i_redirect, vlc_array_t 
> *cookies );
> @@ -204,10 +211,11 @@ static int Request( access_t *p_access,
> uint64_t i_tell );
>  static void Disconnect( access_t * );
>
>  /* Small Cookie utilities. Cookies support is partial. */
> +static http_cookie_t * cookie_parse( const char * cookie_header );
> +static void cookie_destroy( http_cookie_t * cookie );
>  static char * cookie_get_content( const char * cookie );
>  static char * cookie_get_domain( const char * cookie );
> -static char * cookie_get_name( const char * cookie );
> -static void cookie_append( vlc_array_t * cookies, char * cookie );
> +static void cookie_append( vlc_array_t * cookies, const char * 
> cookie );
>
>
>  static void AuthReply( access_t *p_acces, const char *psz_prefix,
> @@ -606,7 +614,7 @@ error:
>      {
>          int i;
>          for( i = 0; i < vlc_array_count( p_sys->cookies ); i++ )
> -            free(vlc_array_item_at_index( p_sys->cookies, i ));
> +            cookie_destroy(vlc_array_item_at_index( p_sys->cookies, 
> i ));
>          vlc_array_destroy( p_sys->cookies );
>      }
>
> @@ -648,7 +656,7 @@ static void Close( vlc_object_t *p_this )
>      {
>          int i;
>          for( i = 0; i < vlc_array_count( p_sys->cookies ); i++ )
> -            free(vlc_array_item_at_index( p_sys->cookies, i ));
> +            cookie_destroy(vlc_array_item_at_index( p_sys->cookies, 
> i ));
>          vlc_array_destroy( p_sys->cookies );
>      }
>
> @@ -1191,23 +1199,17 @@ static int Request( access_t *p_access,
> uint64_t i_tell )
>          int i;
>          for( i = 0; i < vlc_array_count( p_sys->cookies ); i++ )
>          {
> -            const char * cookie = vlc_array_item_at_index(
> p_sys->cookies, i );
> -            char * psz_cookie_content = cookie_get_content( cookie 
> );
> -            char * psz_cookie_domain = cookie_get_domain( cookie );
> -
> -            assert( psz_cookie_content );
> +            const http_cookie_t * cookie = vlc_array_item_at_index(
> p_sys->cookies, i );
>
>              /* FIXME: This is clearly not conforming to the rfc */
> -            bool is_in_right_domain = (!psz_cookie_domain || strstr(
> p_sys->url.psz_host, psz_cookie_domain ));
> +            bool is_in_right_domain = (!cookie->psz_domain ||
> strstr( p_sys->url.psz_host, cookie->psz_domain ));
>
>              if( is_in_right_domain )
>              {
> -                msg_Dbg( p_access, "Sending Cookie %s",
> psz_cookie_content );
> -                if( net_Printf( p_access, p_sys->fd, pvs, "Cookie:
> %s\r\n", psz_cookie_content ) < 0 )
> +                msg_Dbg( p_access, "Sending Cookie %s=%s",
> cookie->psz_name, cookie->psz_value );
> +                if( net_Printf( p_access, p_sys->fd, pvs, "Cookie:
> %s=%s\r\n", cookie->psz_name, cookie->psz_value ) < 0 )
>                      msg_Err( p_access, "failed to send Cookie" );
>              }
> -            free( psz_cookie_content );
> -            free( psz_cookie_domain );
>          }
>      }
>
> @@ -1505,7 +1507,7 @@ static int Request( access_t *p_access,
> uint64_t i_tell )
>              if( p_sys->cookies )
>              {
>                  msg_Dbg( p_access, "Accepting Cookie: %s", p );
> -                cookie_append( p_sys->cookies, strdup(p) );
> +                cookie_append( p_sys->cookies, p );
>              }
>              else
>                  msg_Dbg( p_access, "We have a Cookie we won't
> remember: %s", p );
> @@ -1581,6 +1583,41 @@ static void Disconnect( access_t *p_access )
>   * them) (FIXME: only support the "domain=" param)
>
> 
> *****************************************************************************/
>
> +static http_cookie_t * cookie_parse( const char * cookie_header )
> +{
> +    char *content = cookie_get_content( cookie_header );
> +    if ( !content )
> +        return NULL;
> +
> +    http_cookie_t* cookie = malloc(sizeof(http_cookie_t));
> +    memset(cookie, 0, sizeof(http_cookie_t));

Use calloc() instead of malloc+memset.

If you cannot be bothered to implement all error handling paths, at 
least use x-prefixed functions so that the program will abort cleanly.
Ditto for strdup->xstrdup. Feel free to add xstrndup() if we do not 
already have it.

> +
> +    const char *eq = strchr(content, '=');
> +    if ( eq )
> +    {
> +        cookie->psz_name = strndup( content, eq-content );
> +        cookie->psz_value = strdup( eq + 1 );
> +    }
> +    else
> +    {
> +        cookie->psz_name = strdup( content );
> +        cookie->psz_value = strdup( "" );
> +    }
> +    cookie->psz_domain = cookie_get_domain( cookie_header );
> +    return cookie;
> +}
> +
> +static void cookie_destroy( http_cookie_t * cookie )
> +{
> +    if ( !cookie )
> +        return;
> +
> +    free(cookie->psz_name);
> +    free(cookie->psz_value);
> +    free(cookie->psz_domain);
> +    free(cookie);
> +}
> +
>  /* Get the NAME=VALUE part of the Cookie */
>  static char * cookie_get_content( const char * cookie )
>  {
> @@ -1624,61 +1661,34 @@ static char * cookie_get_domain( const char *
> cookie )
>      return NULL;
>  }
>
> -/* Get NAME in the NAME=VALUE field */
> -static char * cookie_get_name( const char * cookie )
> -{
> -    char * ret = cookie_get_content( cookie ); /* NAME=VALUE */
> -    if( !ret ) return NULL;
> -    char * str = ret;
> -    while( *str && *str != '=' ) str++;
> -    *str = 0;
> -    return ret;
> -}
> -
>  /* Add a cookie in cookies, checking to see how it should be added 
> */
> -static void cookie_append( vlc_array_t * cookies, char * cookie )
> +static void cookie_append( vlc_array_t * cookies, const char *
> cookie_header )
>  {
>      int i;
>
> +    http_cookie_t *cookie = cookie_parse(cookie_header);
>      if( !cookie )
>          return;
>
> -    char * cookie_name = cookie_get_name( cookie );
> -
> -    /* Don't send invalid cookies */
> -    if( !cookie_name )
> -        return;
> -
> -    char * cookie_domain = cookie_get_domain( cookie );
>      for( i = 0; i < vlc_array_count( cookies ); i++ )
>      {
> -        char * current_cookie = vlc_array_item_at_index( cookies, i 
> );
> -        char * current_cookie_name = cookie_get_name( current_cookie 
> );
> -        char * current_cookie_domain = cookie_get_domain( 
> current_cookie );
> +        http_cookie_t *iter = vlc_array_item_at_index( cookies, i );
>
> -        assert( current_cookie_name );
> +        assert( iter->psz_name );
>
>          bool is_domain_matching = (
> -                      ( !cookie_domain && !current_cookie_domain ) 
> ||
> -                      ( cookie_domain && current_cookie_domain &&
> -                        !strcmp( cookie_domain, 
> current_cookie_domain ) ) );
> +                      ( !cookie->psz_domain && !iter->psz_domain ) 
> ||
> +                      ( cookie->psz_domain && iter->psz_domain &&
> +                        !strcmp( cookie->psz_domain, 
> iter->psz_domain ) ) );
>
> -        if( is_domain_matching && !strcmp( cookie_name,
> current_cookie_name )  )
> +        if( is_domain_matching && !strcmp( cookie->psz_name,
> iter->psz_name )  )
>          {
>              /* Remove previous value for this cookie */
> -            free( current_cookie );
>              vlc_array_remove( cookies, i );
> -
> -            /* Clean */
> -            free( current_cookie_name );
> -            free( current_cookie_domain );
> +            cookie_destroy(iter);
>              break;
>          }
> -        free( current_cookie_name );
> -        free( current_cookie_domain );
>      }
> -    free( cookie_name );
> -    free( cookie_domain );
>      vlc_array_append( cookies, cookie );
>  }

-- 
Rémi Denis-Courmont



More information about the vlc-devel mailing list