On Fri, Jun 28, 2013 at 5:21 AM, Leszek Krupiński <leafn...@gmail.com>wrote:
> On 2013-06-27 03:33, Sherif Ramadan wrote: > >> On Wed, Jun 26, 2013 at 9:05 PM, Yasuo Ohgaki <yohg...@ohgaki.net> wrote: >> >> Hi all, >>> >>> I've sent pull request for PHP-5.5 branch. >>> https://github.com/php/php-**src/pull/369<https://github.com/php/php-src/pull/369> >>> >>> It's simple 1 liner removes E_WARNING for invalid length. >>> >>> Are there any objections? >>> >>> Yes, I object to removing the error in the same breath we're arguing for >> consistency. 5.4.0 through 5.4.3 (no error), 5.4.4 through 5.4.16 (error), >> 5.5.0 (error), 5.5.1 (no error)??? What kind of consistency is this? >> >> 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. >> > > In my opinion generating E_WARNING is too severe. Passing improper string > to hex2bin does not necessary mean "buggy code". If the string is eg. from > external input, why should I check if it's a well formed hex string if the > function does this anyway? I think E_NOTICE + returning false is enough. > > And about consistency... I don't know the rationale behind adding E_ERROR > in recent versions, but if the situation is bad, it should be fixed, not > proliferated to next versions. Yes, it'd better if the change was better > thought, but the milk has spilled. > > To be fair, the only reason we ended up adding an E_WARNING in the first place is because someone raised a concern that they had the wrong expectation about what the function does. As far as I can tell, there is no reason that the user would believe hex2bin (a pretty descriptive function name) should accept anything other than proper hex notation. Meaning that returning false on failure is fair when the string is not valid hexadecimal encoding. The issue of having odd or even length hexits, however, is a different matter entirely. People (myself included) misunderstood what the function does. The misconception being that this function took a hexadecimal **number** and converted it to a binary **number**, which is not what it does. In such a case it is perfectly acceptable to have something like "f" translate to a valid ASCII character, for example. As is with the case of pack("h1". "f"), or ord(base_convert('f', 16, 10))... This is a fruitless endeavor to complain about consistency in PHP where we are the ones actually creating unnecessary inconsistencies that can't be justified from an objective point-of-view, in my own personal opinion.
