[vlc-devel] [PATCH 1/3] Audioscrobbler: Update the handshake protocol to the version 2.0.

Samuel Pitoiset samuel.pitoiset at gmail.com
Thu Mar 8 10:01:41 CET 2012


> +/****************************
*************************************************
> + * Sort: sort alphabetically the parameters list
> +
*****************************************************************************/
> +static int Sort(const void *elt1, const void *elt2)
> +{
> +    const audioscrobbler_param_t *p1 = elt1, *p2 = elt2;

Probably one * missing, i think Sort is called with
audioscrobbler_param_t** as argument. (pp_elem+i)

> +    return strcmp(*(char * const *) p1->key, *(char * const *) p2->key);

Probably one * too much and the type is not valid (see man strcmp)

I'm sorry, but qsort() needs the prototype int(*compar)(const void *, const
void *) (man qsort).

And the cast seems to be correct because I took this part of code in the
manpage of qsort() and I got no warnings during compilation.

> +    /* build the api signature */
> +    p_param = p_params->pp_elems[0];
> +    sprintf(psz_api_sig, "%s%s", p_param->key, p_param->val);

*psz_api_sig = '\0'; and put this one inside the loop below.

I don't understand why this is needed, could you explain, please?

> +    /* compute the length of the request */
> +    i_len += 9; /* for http:// */

That's 7 according to the comment.

Indeed.

Btw did you run this through valgrind to catch possible errors?

No, but I tried to check each malloc/free by reading the code a lot.

I'll run some tests with valgrind.

On Thu, Mar 8, 2012 at 1:16 AM, Rafaël Carré <funman at videolan.org> wrote:

> Le 2012-03-07 17:03, Samuel Pitoiset a écrit :
> > ---
> >  modules/misc/audioscrobbler.c |  501
> ++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 500 insertions(+), 1 deletion(-)
> >
> > diff --git a/modules/misc/audioscrobbler.c
> b/modules/misc/audioscrobbler.c
> > index e45c815..ddcc284 100644
> > --- a/modules/misc/audioscrobbler.c
> > +++ b/modules/misc/audioscrobbler.c
> > @@ -48,12 +48,15 @@
> >  #include <vlc_url.h>
> >  #include <vlc_network.h>
> >  #include <vlc_playlist.h>
> > +#include <vlc_xml.h>
> >
> >
>  /*****************************************************************************
> >   * Local prototypes
> >
> *****************************************************************************/
> >
> >  #define QUEUE_MAX 50
> > +#define API_KEY "c18676d0f607b5a583ac599c4b87baea"
> > +#define API_SECRET "bd0fe9ce1ec320324e2a035cc669e5ad"
> >
> >  /* Keeps track of metadata to be submitted */
> >  typedef struct audioscrobbler_song_t
> > @@ -68,6 +71,20 @@ typedef struct audioscrobbler_song_t
> >      mtime_t     i_start;            /**< playing start    */
> >  } audioscrobbler_song_t;
> >
> > +/* Keeps parameters to be submitted */
> > +typedef struct audioscrobbler_param_t
> > +{
> > +    const char *key;
> > +    const char *val;
> > +} audioscrobbler_param_t;
> > +
> > +/* Type of HTTP request */
> > +typedef enum
> > +{
> > +    GET,
> > +    POST
> > +} audioscrobbler_request_t;
> > +
> >  struct intf_sys_t
> >  {
> >      audioscrobbler_song_t   p_queue[QUEUE_MAX]; /**< songs not
> submitted yet*/
> > @@ -133,7 +150,7 @@ vlc_module_begin ()
> >                  USERNAME_TEXT, USERNAME_LONGTEXT, false)
> >      add_password("lastfm-password", "",
> >                  PASSWORD_TEXT, PASSWORD_LONGTEXT, false)
> > -    add_string("scrobbler-url", "post.audioscrobbler.com",
> > +    add_string("scrobbler-url", "ws.audioscrobbler.com/2.0",
> >                  URL_TEXT, URL_LONGTEXT, false)
> >      set_capability("interface", 0)
> >      set_callbacks(Open, Close)
> > @@ -460,6 +477,488 @@ static void Close(vlc_object_t *p_this)
> >  }
> >
> >
>  /*****************************************************************************
> > + * BuildAuthToken: generate the authentication token
> > +
> *****************************************************************************/
> > +static char *BuildAuthToken(const char *psz_username, const char
> *psz_password)
> > +{
> > +    char *psz_password_md5, *psz_auth_token_md5;
> > +    struct md5_s p_struct_md5;
> > +
> > +    /* generate a md5 hash of the password */
> > +    InitMD5(&p_struct_md5);
> > +    AddMD5(&p_struct_md5, (uint8_t*) psz_password,
> strlen(psz_password));
> > +    EndMD5(&p_struct_md5);
> > +
> > +    psz_password_md5 = psz_md5_hash(&p_struct_md5);
> > +    if (!psz_password_md5)
> > +        return NULL;
> > +
> > +    /* generate a md5 hash of :
> > +     * - username in clear text, plus
> > +     * - md5 hash of the password
> > +     */
> > +    InitMD5(&p_struct_md5);
> > +    AddMD5(&p_struct_md5, (uint8_t*) psz_username,
> strlen(psz_username));
> > +    AddMD5(&p_struct_md5, (uint8_t*) psz_password_md5, 32);
> > +    EndMD5(&p_struct_md5);
> > +
> > +    free(psz_password_md5);
> > +
> > +    psz_auth_token_md5 = psz_md5_hash(&p_struct_md5);
>
> > +    if (!psz_auth_token_md5)
> > +        return NULL;
>
> Not needed, the case is handled below.
>
> > +    return psz_auth_token_md5;
> > +}
> > +
> >
> +/*****************************************************************************
> > + * Sort: sort alphabetically the parameters list
> > +
> *****************************************************************************/
> > +static int Sort(const void *elt1, const void *elt2)
> > +{
> > +    const audioscrobbler_param_t *p1 = elt1, *p2 = elt2;
>
> Probably one * missing, i think Sort is called with
> audioscrobbler_param_t** as argument. (pp_elem+i)
>
> > +    return strcmp(*(char * const *) p1->key, *(char * const *) p2->key);
>
> Probably one * too much and the type is not valid (see man strcmp)
>
> > +}
> > +
> >
> +/*****************************************************************************
> > + * BuildApiSig: generate the api signature
> > +
> *****************************************************************************/
> > +static char *BuildApiSig(vlc_array_t *p_params, const char
> *psz_api_secret)
> > +{
> > +    char *psz_api_sig, *psz_api_sig_md5;
> > +    audioscrobbler_param_t *p_param;
> > +    struct md5_s p_struct_md5;
> > +    ssize_t i_len = 0;
> > +    int i;
>
> nitpicking: you can put the i inside the for (C99)
>
> > +
> > +    /* parameters must be sorted alphabetically */
> > +    qsort(p_params->pp_elems, p_params->i_count,
> sizeof(audioscrobbler_param_t*),
> > +            Sort);
> > +
> > +    /* compute the length of the api signature */
> > +    for (i = 0; i < p_params->i_count; i++)
> > +    {
> > +        p_param = p_params->pp_elems[i];
> > +        i_len += strlen(p_param->key) + strlen(p_param->val);
> > +    }
> > +
> > +    i_len += 32; /* md5 hash of the api secret */
> > +
> > +    psz_api_sig = malloc(i_len);
> > +    if (!psz_api_sig)
> > +        return NULL;
> > +
> > +    /* build the api signature */
> > +    p_param = p_params->pp_elems[0];
> > +    sprintf(psz_api_sig, "%s%s", p_param->key, p_param->val);
>
> *psz_api_sig = '\0'; and put this one inside the loop below.
>
> > +
> > +    for (i = 1; i < p_params->i_count; i++)
> > +    {
> > +        p_param = p_params->pp_elems[i];
> > +        strcat(psz_api_sig, p_param->key);
> > +        strcat(psz_api_sig, p_param->val);
> > +    }
> > +
> > +    /* concatenate the api secret key */
> > +    strcat(psz_api_sig, psz_api_secret);
> > +
> > +    /* generate a md5 hash of the api signature */
> > +    InitMD5(&p_struct_md5);
> > +    AddMD5(&p_struct_md5, (uint8_t*) psz_api_sig, strlen(psz_api_sig));
> > +    EndMD5(&p_struct_md5);
>
> Hm wait you allocate and build that long string just to get the md5?
>
> You can call AddMD5() repeatedly instead.
>
> > +
> > +    psz_api_sig_md5 = psz_md5_hash(&p_struct_md5);
> > +    if (!psz_api_sig_md5)
> > +        return NULL;
> > +
> > +    return psz_api_sig_md5;
> > +}
> > +
> >
> +/*****************************************************************************
> > + * AddParam: add a parameter to the list
> > +
> *****************************************************************************/
> > +static void AddParam(vlc_array_t *p_params, const char *psz_key,
> > +                    const char *psz_val)
> > +{
> > +    audioscrobbler_param_t *p_param =
> malloc(sizeof(audioscrobbler_param_t));
>
> unchecked malloc
>
> > +    p_param->key = psz_key;
> > +    p_param->val = psz_val;
> > +
> > +    vlc_array_append(p_params, p_param);
> > +}
> > +
> >
> +/*****************************************************************************
> > + * BuildGetRequest: build an HTTP GET request
> > +
> *****************************************************************************/
> > +static char *BuildGetRequest(intf_thread_t *p_this, vlc_array_t
> *p_params)
> > +{
> > +    vlc_url_t p_submit_url = p_this->p_sys->p_submit_url;
> > +    audioscrobbler_param_t *p_param;
> > +    char *psz_request;
> > +    ssize_t i_len = 0;
> > +    int i;
> > +
> > +    /* compute the length of the request */
> > +    i_len += 9; /* for http:// */
>
> That's 7 according to the comment.
>
> > +    i_len += strlen(p_submit_url.psz_host) +
> strlen(p_submit_url.psz_path);
> > +    i_len += 2; /* for /? */
> > +
> > +    for (i = 0; i < p_params->i_count; i++)
> > +    {
> > +        p_param = p_params->pp_elems[i];
> > +        i_len += strlen(p_param->key) + strlen(p_param->val);
> > +        i_len += 2; /* for & and = */
> > +    }
> > +
> > +    psz_request = malloc(i_len);
> > +    if (!psz_request)
> > +        return NULL;
> > +
> > +    /* build the url */
> > +    p_param = p_params->pp_elems[0];
> > +    sprintf(psz_request, "http://%s%s/?%s=%s", p_submit_url.psz_host,
> > +            p_submit_url.psz_path, p_param->key, p_param->val);
> > +
> > +    for (i = 1; i < p_params->i_count; i++)
> > +    {
> > +        p_param = p_params->pp_elems[i];
> > +
> > +        strcat(psz_request, "&");
> > +        strcat(psz_request, p_param->key);
> > +        strcat(psz_request, "=");
> > +        strcat(psz_request, p_param->val);
> > +    }
> > +
> > +    return psz_request;
> > +}
> > +
> >
> +/*****************************************************************************
> > + * BuildPostRequest: build an HTTP POST request
> > +
> *****************************************************************************/
> > +static char *BuildPostRequest(intf_thread_t *p_this, vlc_array_t
> *p_params)
> > +{
> > +    vlc_url_t p_submit_url = p_this->p_sys->p_submit_url;
> > +    audioscrobbler_param_t *p_param;
> > +    char *psz_data, *psz_request;
> > +    ssize_t i_len = 0;
> > +    int i;
> > +
> > +    /* compute the length of the request */
> > +    for (i = 0; i < p_params->i_count; i++)
> > +    {
> > +        p_param = p_params->pp_elems[i];
> > +        i_len += strlen(p_param->key) + strlen(p_param->val);
> > +        i_len += 2; /* for & and = */
> > +    }
> > +
> > +    psz_data = malloc(i_len);
> > +    if (!psz_data)
> > +        return NULL;
> > +
> > +    /* build the data */
> > +    p_param = p_params->pp_elems[0];
> > +    sprintf(psz_data, "%s=%s", p_param->key, p_param->val);
> > +
> > +    for (i = 1; i < p_params->i_count; i++)
> > +    {
> > +        p_param = p_params->pp_elems[i];
> > +
> > +        strcat(psz_data, "&");
> > +        strcat(psz_data, p_param->key);
> > +        strcat(psz_data, "=");
> > +        strcat(psz_data, p_param->val);
> > +    }
> > +
> > +    /* build the POST request */
> > +    if (asprintf(&psz_request,
> > +            "POST %s HTTP/1.1\n"
> > +            "Accept-Encoding: identity\n"
> > +            "Content-length: %zu\n"
> > +            "Connection: close\n"
> > +            "Content-type: application/x-www-form-urlencoded\n"
> > +            "Host: %s\n"
> > +            "User-agent: VLC media player/"VERSION"\r\n"
> > +            "\r\n"
> > +            "%s\r\n"
> > +            "\r\n",
> > +            p_submit_url.psz_path, strlen(psz_data),
> > +            p_submit_url.psz_host, psz_data
> > +    ) == -1)
> > +    {
> > +        /* out of memory */
> > +        free(psz_data);
> > +        return NULL;
> > +    }
> > +
> > +    free(psz_data);
> > +
> > +    return psz_request;
> > +}
> > +
> >
> +/*****************************************************************************
> > + * BuildRequest : build an HTTP request
> > +
> *****************************************************************************/
> > +static char *BuildRequest(intf_thread_t *p_this, vlc_array_t *p_params,
> > +                            audioscrobbler_request_t e_type)
> > +{
> > +    char *psz_api_sig, *psz_request = NULL;
> > +
> > +    /* add the api key to the parameters list */
> > +    AddParam(p_params, "api_key", API_KEY);
> > +
> > +    /* generate the api signature */
> > +    psz_api_sig = BuildApiSig(p_params, API_SECRET);
> > +    if (!psz_api_sig)
> > +        return NULL;
> > +
> > +    /* add the api signature to the parameters list */
> > +    AddParam(p_params, "api_sig", psz_api_sig);
> > +
> > +    /* build the HTTP request */
> > +    switch (e_type)
> > +    {
> > +        case GET:
> > +            psz_request = BuildGetRequest(p_this, p_params);
> > +            break;
>
> return BuildGetRequest(..) ?
>
> > +        case POST:
> > +            psz_request = BuildPostRequest(p_this, p_params);
> > +            break;
>
> idem
>
> > +        default:
> > +            msg_Err(p_this, "invalid type of request %d", e_type);
> > +            break;
> > +    }
> > +
> > +    return psz_request;
> > +}
> > +
> >
> +/*****************************************************************************
> > + * BuildSignedRequest : Build an HTTP request wich requires
> authentification
> > +
> *****************************************************************************/
> > +static char *BuildSignedRequest(intf_thread_t *p_this, vlc_array_t
> *p_params,
> > +                                 audioscrobbler_request_t e_type)
> > +{
> > +    /* add the session key to the parameters list */
> > +    AddParam(p_params, "sk", p_this->p_sys->psz_auth_token);
> > +
> > +    return BuildRequest(p_this, p_params, e_type);
> > +}
> > +
> >
> +/*****************************************************************************
> > + * Handshake : Init audioscrobbler connection
> > +
> *****************************************************************************/
> > +static int NewHandshake(intf_thread_t *p_this)
> > +{
> > +    char                *psz_username, *psz_password, *psz_auth_token;
> > +    char                *psz_scrobbler_url, *psz_handshake_url;
> > +    const char          *node;
> > +    vlc_array_t         p_params;
> > +    stream_t            *p_stream;
> > +    xml_reader_t        *p_xml_reader       = NULL;
> > +    intf_thread_t       *p_intf             = (intf_thread_t*) p_this;
> > +    intf_sys_t          *p_sys              = p_this->p_sys;
> > +
> > +    /* get the username of the user */
> > +    psz_username = var_InheritString(p_this, "lastfm-username");
> > +    if (!psz_username)
> > +        return VLC_ENOMEM;
> > +
> > +    /* get the password of the user */
> > +    psz_password = var_InheritString(p_this, "lastfm-password");
> > +    if (!psz_password)
> > +    {
> > +        free(psz_username);
> > +        return VLC_ENOMEM;
> > +    }
> > +
> > +    /* username or password have not been setup */
> > +    if (!*psz_username || !*psz_password)
> > +    {
> > +        free(psz_username);
> > +        free(psz_password);
> > +        return VLC_ENOVAR;
> > +    }
> > +
> > +    /* generate the authentication token */
> > +    psz_auth_token = BuildAuthToken(psz_username, psz_password);
> > +    free(psz_password);
> > +
> > +    if (!psz_auth_token)
> > +    {
> > +        free(psz_username);
> > +        return VLC_ENOMEM;
> > +    }
> > +
> > +    /* get the submission url */
> > +    psz_scrobbler_url = var_InheritString(p_this, "scrobbler-url");
> > +    if (!psz_scrobbler_url)
> > +    {
> > +        free(psz_username);
> > +        free(psz_auth_token);
> > +        return VLC_ENOMEM;
> > +    }
> > +
> > +    /* parse the submission url */
> > +    vlc_UrlParse(&p_sys->p_submit_url, psz_scrobbler_url, 0);
> > +    free(psz_scrobbler_url);
> > +
> > +    /* build the parameters list */
> > +    vlc_array_init(&p_params);
> > +    AddParam(&p_params, "method", "auth.getMobileSession");
> > +    AddParam(&p_params, "authToken", psz_auth_token);
> > +    AddParam(&p_params, "username", psz_username);
> > +
> > +    /* build the http request */
> > +    psz_handshake_url = BuildRequest(p_this, &p_params, GET);
> > +    free(psz_username);
> > +    free(psz_auth_token);
> > +    vlc_array_clear(&p_params);
> > +
> > +    if (!psz_handshake_url)
> > +        return VLC_ENOMEM;
> > +
> > +    /* send the http request */
> > +    p_stream = stream_UrlNew(p_intf, psz_handshake_url);
> > +    free(psz_handshake_url);
> > +
> > +    if (!p_stream)
> > +        goto proto;
> > +
> > +    /* read answer */
> > +    p_xml_reader = xml_ReaderCreate(p_this, p_stream);
> > +    if (!p_xml_reader)
> > +        goto proto;
> > +
> > +    /* check root node */
> > +    if (xml_ReaderNextNode(p_xml_reader, &node) != XML_READER_STARTELEM)
> > +    {
> > +        msg_Err(p_this, "invalid file (no root node)");
> > +        goto proto;
> > +    }
> > +
> > +    if (strcasecmp(node, "lfm"))
> > +    {
> > +        msg_Err(p_this, "invalid root node <%s>", node);
> > +        goto proto;
> > +    }
> > +
> > +    /* check response status */
> > +    const char *attr, *value;
> > +    if ((attr = xml_ReaderNextAttr(p_xml_reader, &value)))
> > +    {
> > +        if (strcasecmp(attr, "status"))
> > +        {
> > +            msg_Err(p_this, "invalid attribute \"%s\"", attr);
> > +            goto proto;
> > +        }
> > +
> > +        if (strcasecmp(value, "ok"))
> > +        {
> > +            /* an error occured */
> > +            stream_Delete(p_stream);
> > +            xml_ReaderDelete(p_xml_reader);
> > +
> > +            if (!strcasecmp(value, "4"))
> > +            {
> > +                /* authentication failed, bad username/password
> combination */
> > +                dialog_Fatal(p_this,
> > +                    _("last.fm: Authentication failed"),
> > +                    "%s", _("last.fm username or password is
> incorrect. "
> > +                      "Please verify your settings and relaunch VLC."));
> > +                return VLC_AUDIOSCROBBLER_EFATAL;
> > +            }
> > +
> > +            if (!strcasecmp(value, "10"))
> > +            {
> > +                /* invalid API key */
> > +                dialog_Fatal(p_this,
> > +                    _("last.fm: Invalid API key"),
> > +                    "%s", _("You must be granted a valid key by last.fm
> "));
> > +                return VLC_AUDIOSCROBBLER_EFATAL;
> > +            }
> > +
> > +            if (!strcasecmp(value, "26"))
> > +            {
> > +                /* suspend API key */
> > +                dialog_Fatal(p_this,
> > +                    _("last.fm: Suspend API key"),
> > +                    "%s", _("Access for your account has been
> suspended. "
> > +                      "Please contact Last.fm."));
> > +                return VLC_AUDIOSCROBBLER_EFATAL;
> > +            }
> > +
> > +            return VLC_EGENERIC;
> > +        }
> > +    }
> > +
> > +    /* read session key */
> > +    bool b_session = false, b_name = false, b_key = false,
> > +         b_subscriber = false, b_session_key = false;
> > +    int i_type;
> > +    while ((i_type = xml_ReaderNextNode(p_xml_reader, &node)) > 0)
> > +    {
> > +        switch (i_type)
> > +        {
> > +            case XML_READER_STARTELEM:
> > +                if (!strcmp(node, "session"))
> > +                    b_session = true;
> > +                if (!strcmp(node, "name"))
> > +                    b_name = true;
> > +                if (!strcmp(node, "key"))
> > +                    b_key = true;
> > +                if (!strcmp(node, "subscriber"))
> > +                    b_subscriber = true;
> > +                break;
> > +            case XML_READER_TEXT:
> > +                if (b_session && b_name && b_key && !b_subscriber)
> > +                {
> > +                    /* save the session key */
> > +                    memcpy(p_sys->psz_auth_token, node, 32);
> > +                    p_sys->psz_auth_token[32] = '\0';
> > +                    b_session_key = true; /* used for checking invalid
> XML */
> > +                }
> > +                break;
> > +            case XML_READER_ENDELEM:
> > +                if (!strcmp(node, "key"))
> > +                {
> > +                    if (!b_session_key)
> > +                    {
> > +                        msg_Err(p_this, "invalid XML (no enclosure
> markup");
> > +                        goto proto;
> > +                    }
> > +                }
> > +                break;
> > +            default:
> > +                break;
> > +        }
> > +    }
> > +
> > +    if (i_type < 0)
> > +    {
> > +        msg_Err(p_this, "unexpected end of XML data");
> > +        goto proto;
> > +    }
> > +
> > +    stream_Delete(p_stream);
> > +    xml_ReaderDelete(p_xml_reader);
> > +
> > +    return VLC_SUCCESS;
> > +
> > +proto:
> > +    if (p_stream)
> > +        stream_Delete(p_stream);
> > +    if (p_xml_reader)
> > +        xml_ReaderDelete(p_xml_reader);
> > +
> > +    msg_Err(p_intf, "Handshake: can't recognize server protocol");
> > +
> > +    return VLC_EGENERIC;
> > +}
> > +
> >
> +/*****************************************************************************
> >   * Handshake : Init audioscrobbler connection
> >
> *****************************************************************************/
> >  static int Handshake(intf_thread_t *p_this)
>
> Btw did you run this through valgrind to catch possible errors?
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
>



-- 
Best regards,
Samuel Pitoiset.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120308/146897f0/attachment.html>


More information about the vlc-devel mailing list