[vlc-devel] [PATCH 1/3] Audioscrobbler: Update the handshake protocol to the version 2.0.
Rafaël Carré
funman at videolan.org
Thu Mar 8 17:35:05 CET 2012
Le 2012-03-08 04:01, Samuel Pitoiset a écrit :
>> +/****************************
> *************************************************
>> + * 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.
How about:
const audioscrobbler_param_t **p1 = elt1, **p2 = elt2;
return strcmp((*p1)->key, (*p2)->key) ?
>> + /* 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?
This could use strcat like the other strings in the loop.
strcat needs a valid string to append to, so a string of length 0 will do.
>> + /* 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.
Don't forget to use valgrind's --leak-check
>
> 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
>>
>
>
>
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list