[vlc-devel] [GCI] last.fm access module

[Ilkka Ollakka]

On Sun, Mar 13, 2011 at 11:24:11AM +0100, Jakub Wieczorek wrote:
> Hello,

> I'm attaching the latest version of the last.fm access module branch.
> It's been updated per the recent XML reader changes.
> The branch had not been merged in before due to uncertainty over
> conformity of this module to the ToS of last.fm's API. Jean told me
> that it should be safe to get this now.

> I'd love someone to take a look once again. Thanks.

Hi,

> Subject: [PATCH 1/2] Add an access module that works with last.fm radio stations.

> --- /dev/null
> +++ b/modules/access/lastfm.c

> +/*****************************************************************************
> + * Authenticates: performs the authentication of the user
> + *****************************************************************************/
> +// FIXME: We should be doing this only once by storing the session key
> +// persistently and then reusing next time it's needed.
> +static int Authenticate( access_t *p_access )
> +{
> +    char *psz_lastfm_username = var_InheritString( p_access, "lastfm-username" );
> +    char *psz_lastfm_password = var_InheritString( p_access, "lastfm-password" );
> +    char *psz_auth_token = NULL;
...
> +    if( !psz_lastfm_username || !psz_lastfm_password ||
> +        *psz_lastfm_username == '\0' || *psz_lastfm_password == '\0' )
> +    {
> +        msg_Err( p_access, "last.fm credentials missing" );
> +        i_retcode = VLC_ENOVAR;
> +        goto error;
> +    }
> +
> +    psz_auth_token = AuthToken( psz_lastfm_username, psz_lastfm_password );
> +    if( !psz_auth_token )
> +    {
> +        msg_Err( p_access, "out of memory" );
> +        i_retcode = VLC_ENOMEM;
> +        goto error;
> +    }
> +
> +    msg_Dbg( p_access, "username: %s; password: %s; auth token: %s",
> +             psz_lastfm_username, psz_lastfm_password, psz_auth_token );

I think it's not that good idea to log password in plaintext, and you
could free psz_lastfm_password in this point too? Also why don't add
some logic and just store generated md5 hash to lastfm-password instead
keep it plaintext (Yes I know we don't do that in last.fm submit plugin
either, just an idea).

> +/*****************************************************************************
> +* CurlWriteData: a callback that receives the data
> +*****************************************************************************/
> +size_t CurlWriteData( void *p_data, size_t i_size, size_t i_nmemb, void *p_userdata )
> +{
> +    i_buffersize = NextPowerOfTwo( p_buffer->i_used + i_realsize );

Why not just something like ? Assuming that i_buffersize starts from
some power of 2
while i_buffersize <= (p_buffer->i_used+i_realsize )
    i_buffersize <<= 1;

> +static stream_t *Request( access_t *p_access, struct url_argument_t *p_arguments, int i_method )
> +{
> +    msg_Dbg( p_access, "Issuing a %s request...", i_method == REST_GET ? "GET" : "POST" );
> +
> +
> +    p_curl_handle = curl_easy_init();
> +    curl_easy_setopt( p_curl_handle, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1 );
> +    curl_easy_setopt( p_curl_handle, CURLOPT_URL, psz_url );
> +    curl_easy_setopt( p_curl_handle, CURLOPT_USERAGENT, "VLC media player" );
> +    curl_easy_setopt( p_curl_handle, CURLOPT_WRITEFUNCTION, CurlWriteData );
> +    curl_easy_setopt( p_curl_handle, CURLOPT_WRITEDATA, p_access );
> +    if( psz_data )
> +        curl_easy_setopt( p_curl_handle, CURLOPT_POSTFIELDS, psz_data );

Is it really harder/requires more code to do http post by hand over
TCP-connection?

> +        p_argument->psz_name = curl_escape( p_argument->psz_name, strlen( p_argument->psz_name ) );
> +        p_argument->psz_value = curl_escape( p_argument->psz_value, strlen( p_argument->psz_value ) );

encode_URI_component?
and maybe vlc_url_t is the struct youre looking for?

-- 
Ilkka Ollakka
A child of five could understand this!  Fetch me a child of five.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20110313/553ad43d/attachment.sig>