Sent from my iPhone

On 27 Jun, 2013, at 11:05 AM, Yasuo Ohgaki <yohg...@ohgaki.net> wrote:

> Hi Sherif,
> 
> I would like to have consistent behavior at least within a function.
> 
> 2013/6/27 Sherif Ramadan <theanomaly...@gmail.com>
> 
>> I thought you wanted to add an extra error for malformed hex, which I
>> would have been fine with, but removing the error entirely? The error is
>> useful. It informs the user that they may have buggy code since the
>> function is clearly documented to expect even length hex encoded strings.
> 
> 
> It is good to have additional errors for invalid inputs. It would be
> beneficial for many users.
> I've checked some conversion(decoder) functions quickly.
> 
> Functions raise E_WARNING / E_NOTICE for invalid inputs
>  unserialize()
>  convert_uudecode()
>  xmlreader module
> 
> Functions simply return FALSE for invalid inputs
>  base64_decode()
>  pg_unescape_bytea()
>  mb_decode_mimiheader()
>  mb_decode_numericentity()
>  mb_convert_kana()
>  mb_convert_encoding()
>  mb_convert_variables()
>    Note: mbstring functions raise errors for invalid encoding, otherwise
> simply return FALSE.
> 
> Functions do not check validity
>  quoted_printable_decode()
>  xml_utf8_decode() - replaces bad chars to '?'
> 
> Functions have separate error message function
>  json_decode() - returns FALSE, but can get errors via json_last_error()
> 
> Decoding errors are usually a bug or some kind of attack, so I agree to add
> E_NOTICEs. Exception is decoders that supposed to accept external inputs.
> e.g. base64_decode() and mbstring functions.
> 
> I think pg_unescape_bytea() should raise E_WARNING, so I'll add it later.
> Adding E_WARNING to it will never be BC issue. It's obvious bug.
> 
> hex2bin() will not be used for handling external inputs almost always, so
> raising E_WARNING make sense.
> 
> I've updated pull request. (Added E_WARNING for bad hex)
> 
> https://github.com/php/php-src/pull/369
> 
> Everyone is OK with this?

The thread started with the assertion that it raises a warning and the commits 
first remove the warning and then adds it again later, so isn't the whole PR a 
noop? :)

> 
> Regards,
> 
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to