Re: [PATCH]ppg push - crashing bug trusted-pi/no user

Stipe,

On Thursday, March 20, 2003, at 01:26 PM, Stipe Tolj wrote:

> why is the
>
>   if (username == NULL || (octstr_str_compare(username,"") == 0)) {
>
>   } else {
>
>   }
>
> necessary?!

If there's no user associated with the request (with "trusted-pi" = true 
and no incoming user information (no url parameters, no http basic 
auth), username gets set to "" (the NULL check might not be necessary, 
just to be safe I added it), and if there's no username to feed to 
wap_push_ppg_pushuser_smsc_id_get to work with, it doesn't make sense to 
even call it hence the check for availability of a username.


>
> AFAIS, wap_push_ppg_pushuser_smsc_id_get() will return NULL in case no
> user is found. Ok at least it may be a performance thing not to look
> into the dict hash if anyway no username is given.

It actually crashes if username equals "", the code was like this:

     u = user_find_by_username(username);

     if ((smsc_id = forced_smsc(u)) != NULL)
         return octstr_duplicate(smsc_id);

When user_find_by_username returns NULL, which is possible, forced_smsc 
does this:

     return u->smsc_id;

on a NULL u hence it crashes.


In theory, only the patch to wap_push_ppg_pushuser.c is needed to 
prevent the crash. However, why bother to have pap_request_thread call 
wap_push_ppg_pushuser_smsc_id_get when you already know there is no 
username to make wap_push_ppg_pushuser_smsc_id_get do something usefull? 
Also, it allows to differentiate between the 2 situations if needed 
(e.g. logging).

But it's your call ;)

Regards,

Bas.