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.





Reply via email to