Re: Bug in digest_md5_parse_challenge

On Sat, Aug 16, 2008 at 8:38 PM, Paul Lesniewski <[email protected]> wrote:
> On Sat, Aug 16, 2008 at 8:00 PM, Pablo Álvarez de Sotomayor Posadillo
> <[email protected]> wrote:
>>
>>    Paul> If base64_decode() returns FALSE, then !== should catch it.
>>    Paul> Can you explain why !== does not work?
>>
>> Well, you are right, that solve the bug, but it still remain another
>> bug, and I discovered it now, when I've check why with the !=
>> condition turn off the warnings and with !== still remain. The new bug
>> is produced because the while loops once more than the neccesary, that
>> is, in each while loop the function substract one part of the
>> $challenge string, so when the string is empty it shows a message
>> warning about that (the exact message is " Uninitialized string
>> offset: 0"). The program pass the !== condition if the variables are
>> not equals or not of the same type, so, in the case described, a
>> boolean type is diferent from a string type, passing the condition. In
>> the other hand, with the != condition your are doing an underlying
>> cast and the while stops at the correct loop. Remember that an empty
>> string ("") is considered as FALSE in a cast conversion. So, finally,
>> or you put one != condition or you put two !== condition (one with the
>> boolean comparision and another one with the string comparision), but
>> in the first case it's recommended a comment explaining that
>> comparision.
>
> The reason I chose !== was because with stable code, we need to treat
> the rest of the code as a black box and touch as little as we can with
> any changes, and the !== means it will only test for a single
> condition (the one initially identified, where base64_decode() returns
> specifically FALSE), whereas != can catch any other value that can be
> evaluated as FALSE such as 0, "", array(), and "0".  It is not good to
> make assumptions about the meaning of those other possible values.
>
> So, if we continue on that track, we'd have to add another
> type-specific check, but I'm now going to step away from the black box
> approach and look inside the loop, where we can see that it is clear
> that any of the other types that can be cast as boolean FALSE are not
> expected therein.  Therefore, the better solution is (and please, if
> you don't mind, test this and let us know if it works OK):
>
>    while (!empty($challenge)) {

Additionally, if base64_decode() returns FALSE, we need to add one
more line ABOVE the while statement so that the return value is
initialized (otherwise, it should generate a PHP notice):

$parsed = array();

Cheers,

 Paul

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
-----
squirrelmail-devel mailing list
Posting guidelines: http://squirrelmail.org/postingguidelines
List address: [email protected]
List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel
List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel