[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