Bug #61660 [Com]: bin2hex(hex2bin($data)) != $data

Edit report at https://bugs.php.net/bug.php?id=61660&edit=1 ID: 61660 Comment by: theanomaly dot is at gmail dot com Reported by: krtek4+php at gmail dot com Summary: bin2hex(hex2bin($data)) != $data Status: Closed Type: Bug Package: *General Issues Operating System: Debian Linux PHP Version: 5.4.1RC1 Assigned To: nikic Block user comment: N Private report: N New Comment: The patch currently in master still leaves a remanent problem. First, var_dump(hex2bin('')) still returns string(0) "". Whether or not this is acceptable is arguable. Since 0x00 is null, but an empty string isn't really null. For example: var_dump(unpack('H*',hex2bin(''))); // will give me string(0) "" Whereas: var_dump(unpack('H*',hex2bin('00'))); // will give me string(2) "" and: var_dump(hex2bin('00')); // will give me string(1) "" # which is what I expect So the warning needs to be amended for an empty string as well as odd-sized hex. Additionally, the function now returns a bool(false), breaking BC. I suggest a second optional argument to return partial binary (potentially broken binary) even on error in the event the user wants to maintain backwards compatibility. I'll be happy to submit another patch for this if there are no objections. Previous Comments: ------------------------------------------------------------------------ [2012-04-08 20:51:36] nikic@php.net After some further discussion on IRC we agreed that it is best to throw a warning. The reasoning is as outlined in my previous comment. The warning is implemented with https://github.com/php/php-src/commit/7ae93a2c4c8a51cc2aec9977ce3c83c100e382a0. ------------------------------------------------------------------------ [2012-04-08 20:45:41] nikic@php.net Automatic comment on behalf of nikic Revision: http://git.php.net/?p=php-src.git;a=commit;h=7ae93a2c4c8a51cc2aec9977ce3c83c100e382a0 Log: Fix bug #61660: bin2hex(hex2bin($data)) != $data ------------------------------------------------------------------------ [2012-04-08 16:23:50] theanomaly dot is at gmail dot com We have no problem type juggling a string to an int as the first non-whitespace, non-zero number character... var_dump(1 + "\t\r\n 0001"); // int(2) Then, equally, we should have no problem interpreting a hexadecimal representation of 1 as 01. :) ------------------------------------------------------------------------ [2012-04-08 16:05:02] rasmus@php.net > The reason is simple: A padding can be either added on the left or on the right. E.g. "af52b" could be interpreted both as "0af52b" and as "af52b0". Why would af52b ever mean af52b0 ? That's like saying 15 could be interpreted as 150 sometimes. ------------------------------------------------------------------------ [2012-04-08 15:25:52] nikic@php.net In my eyes hex2bin should not try to fix the corrupt data by padding it with zero. Instead it should throw a warning. The reason is simple: A padding can be either added on the left or on the right. E.g. "af52b" could be interpreted both as "0af52b" and as "af52b0". Both are valid interpretations, depending on the use case. Because of this I think a warning is more appropriate. It signals the user that the data is wrong. He can always easily fix it by adding a zero to either of the sides, whatever is more appropriate in his particular situation. Blindly adding the padding on the other hand will probably be quite unexpected. Especially when adding the 0 on the left side all the data will be shifted by 4 bits. This means that the whole data will be corrupted. All bytes will change. (Adding a padding on the right is less intrusive as it only changes one byte). ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=61660 -- Edit this bug report at https://bugs.php.net/bug.php?id=61660&edit=1