Re: Re: hex2bin: E_WARNING is too much for odd string?
| From: | Tjerk Meesters | Date: | Thu, 27 Jun 2013 11:55:12 +0000 |
| Subject: | Re: Re: hex2bin: E_WARNING is too much for odd string? | ||
| References: | 1 2 3 4 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-67939@lists.php.net to get a copy of this message | ||
Sent from my iPhone
On 27 Jun, 2013, at 11:05 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> Hi Sherif,
>
> I would like to have consistent behavior at least within a function.
>
> 2013/6/27 Sherif Ramadan <theanomaly.is@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
> yohgaki@ohgaki.net