[PHP-BUG] Bug #61660 [NEW]: bin2hex(hex2bin($data)) != $data
From: Operating system: Debian Linux PHP version: 5.4.1RC1 Package: Unknown/Other Function Bug Type: Bug Bug description:bin2hex(hex2bin($data)) != $data Description: If you try to apply bin2hex on the result of hex2bin, when the length of the initial data is an odd number, the resulting data are not the same as the original. $ php -v PHP 5.4.1RC1 (cli) (built: Apr 6 2012 13:31:16) Copyright (c) 1997-2012 The PHP Group Zend Engine v2.4.0, Copyright (c) 1998-2012 Zend Technologies This is the original Debian package present in sid on the 7th April 2012 Test script: --- https://bugs.php.net/bug.php?id=61660&edit=1 -- Try a snapshot (PHP 5.4): https://bugs.php.net/fix.php?id=61660&r=trysnapshot54 Try a snapshot (PHP 5.3): https://bugs.php.net/fix.php?id=61660&r=trysnapshot53 Try a snapshot (trunk): https://bugs.php.net/fix.php?id=61660&r=trysnapshottrunk Fixed in SVN: https://bugs.php.net/fix.php?id=61660&r=fixed Fixed in SVN and need be documented: https://bugs.php.net/fix.php?id=61660&r=needdocs Fixed in release: https://bugs.php.net/fix.php?id=61660&r=alreadyfixed Need backtrace: https://bugs.php.net/fix.php?id=61660&r=needtrace Need Reproduce Script: https://bugs.php.net/fix.php?id=61660&r=needscript Try newer version: https://bugs.php.net/fix.php?id=61660&r=oldversion Not developer issue: https://bugs.php.net/fix.php?id=61660&r=support Expected behavior: https://bugs.php.net/fix.php?id=61660&r=notwrong Not enough info: https://bugs.php.net/fix.php?id=61660&r=notenoughinfo Submitted twice: https://bugs.php.net/fix.php?id=61660&r=submittedtwice register_globals: https://bugs.php.net/fix.php?id=61660&r=globals PHP 4 support discontinued: https://bugs.php.net/fix.php?id=61660&r=php4 Daylight Savings:https://bugs.php.net/fix.php?id=61660&r=dst IIS Stability: https://bugs.php.net/fix.php?id=61660&r=isapi Install GNU Sed: https://bugs.php.net/fix.php?id=61660&r=gnused Floating point limitations: https://bugs.php.net/fix.php?id=61660&r=float No Zend Extensions: https://bugs.php.net/fix.php?id=61660&r=nozend MySQL Configuration Error: https://bugs.php.net/fix.php?id=61660&r=mysqlcfg
Bug #61660 [Opn]: bin2hex(hex2bin($data)) != $data
Edit report at https://bugs.php.net/bug.php?id=61660&edit=1 ID: 61660 User updated by:krtek4+php at gmail dot com Reported by:krtek4+php at gmail dot com Summary:bin2hex(hex2bin($data)) != $data Status: Open Type: Bug Package:Unknown/Other Function Operating System: Debian Linux PHP Version:5.4.1RC1 Block user comment: N Private report: N New Comment: I'm aware that this is a problem with the internal reprensation of the binary value which has to be aligned on 8 bits. But with the actual implementation, we are losing informations. A possible solution would be to pad the binary data with 0 on the left and when converting back again to hex, remove the leading 0s. At least, something should be said in the documentation about this shortcoming. Previous Comments: [2012-04-07 15:46:35] krtek4+php at gmail dot com Description: If you try to apply bin2hex on the result of hex2bin, when the length of the initial data is an odd number, the resulting data are not the same as the original. $ php -v PHP 5.4.1RC1 (cli) (built: Apr 6 2012 13:31:16) Copyright (c) 1997-2012 The PHP Group Zend Engine v2.4.0, Copyright (c) 1998-2012 Zend Technologies This is the original Debian package present in sid on the 7th April 2012 Test script: --- https://bugs.php.net/bug.php?id=61660&edit=1
Bug #61660 [Com]: bin2hex(hex2bin($data)) != $data
Edit report at https://bugs.php.net/bug.php?id=61660&edit=1
ID: 61660
Comment by: krtek4+php at gmail dot com
Reported by:krtek4+php at gmail dot com
Summary:bin2hex(hex2bin($data)) != $data
Status: Open
Type: Bug
Package:*General Issues
Operating System: Debian Linux
PHP Version:5.4.1RC1
Block user comment: N
Private report: N
New Comment:
If I could intervene, my patch does exactly the same thing without adding a new
test for each iteration of the loop.
Mine has only one modulo (%) outside of the loop and the new test is executed
only
once at the first pass assuming the string is a correct hexadecimal value.
Like said in your last comment, the % operation can even be optimized with a '&
1'
if needed.
Previous Comments:
[2012-04-08 10:01:22] larue...@php.net
@theanomaly as we talked, since Rasmus said it's okey, then I have no objection.
2 suggestions :
1. use oddlen & 1 instead of oddlen % 2
2. since you cal oddlen % 2 twice, so it's better if you can use a var to hold
it
and it's better for you to make a pull request by yourself, let me know if you
need help on that :)
------------
[2012-04-08 10:00:43] krtek4+php at gmail dot com
The internal representation must always be aligned on 8 bits, thus we have no
choice to pad with 0 bits at the beginning, 1000 and 1000 is the exact same
value in binary and I think the actual patch is correct.
The new problem is that the reverse operation, i.e. bin2hex, should remove the
added 0 bit at the beginning.
@theanomaly ; decbin works just fine since it returns a string composed of 0s
and 1s and not a "binary value". hex2bin / bin2hex are the only function I'm
aware of working this way.
BTW, why did you sent another patch ? mine is doing exactly the same as yours
and is working fine.
[2012-04-08 09:42:25] theanomaly dot is at gmail dot com
@laruence
I've replaced the last patch with a better patch because I realized I created a
memory leak and that was a poor strategy.
I can't understand why there should be any confusion about whether it's an
octal
value or a hexadecimal value though. Since when should using bin2hex() ever
leave us with the expectation that would _ever_ get back an octal value?
I might be missing something here, but hex2bin() should always be expecting a
hexadecimal value and bin2hex() should always leave us with the expectation of
a
hexadecimal value. I see nothing wrong with padding the value to an even number
otherwise the result is hex2bin() isn't doing what it's supposed to be doing.
It
makes sense to me that even if the client sends a value of '1' that it's
completely expected behavior that '01' and '1' should both be a valid
hexadecimal value.
To me it just makes no sense to punish the client for forgetting to pad the
value by returning false data. At the very least we should be issuing a warning
to let the client know they have sent unexpected data and then this can be
documented behavior. But why waste time fixing it to issue E_WARNINGs when this
patch fixes the issue completely? Besides hex2bin is returning a string. It's
not like the user can inadvertently use it as an octal value.
var_dump('0123' + '0123'); // int(246)
This would be silly not to fix in my opinion. Especially since it's such an
easy
fix. At least run the patch and let me know which test case you can come up
with
that would break any of PHP's already existing documented behavior by making
this modification?
[2012-04-08 08:07:32] larue...@php.net
@theanomaly I have tried the similar way as you did. but the key problem is
the
result will be considered as a oct number.
reads:
"123" != "0123"
[2012-04-08 04:38:44] theanomaly dot is at gmail dot com
I've also submitted a patch which seems to work fine as far as I've tested it.
Similar to the previous patch, but mine simply prepends a 0 to the beginning of
every odd length string sent to hex2bin. It shouldn't break anything that I can
see. If anything it just ensures we always have a valid hexadecimal
representation since the implementation relies on shift 1 to translate between
hex and binary (we'll always be one off). I suggest this also becomes a part of
the dec2bin implementation since it would make sense to return a full octet in
that representation as well. Sorry if my patch looks ugly though. This is my
Bug #61660 [Opn]: bin2hex(hex2bin($data)) != $data
Edit report at https://bugs.php.net/bug.php?id=61660&edit=1
ID: 61660
User updated by:krtek4+php at gmail dot com
Reported by:krtek4+php at gmail dot com
Summary:bin2hex(hex2bin($data)) != $data
Status: Open
Type: Bug
Package:*General Issues
Operating System: Debian Linux
PHP Version:5.4.1RC1
Block user comment: N
Private report: N
New Comment:
You are right, there was a bug in the patch I sent. I updated it on github.
Thanks for the comment !
Previous Comments:
[2012-04-08 12:23:32] theanomaly dot is at gmail dot com
@krtek4+php
I didn't mean to step on any toes, honestly. I think your patch probably looks
way cleaner than mine, but when I tried compiling your patch it did not work
for
me. The test didn't pass.
var_dump(bin2hex(hex2bin(1))); // returned string(0) ""
Maybe I didn't do it right, but that's the only reason I submitted another
patch
after I tested again on the PHP-5.4 branch.
--------
[2012-04-08 10:08:14] krtek4+php at gmail dot com
If I could intervene, my patch does exactly the same thing without adding a new
test for each iteration of the loop.
Mine has only one modulo (%) outside of the loop and the new test is executed
only
once at the first pass assuming the string is a correct hexadecimal value.
Like said in your last comment, the % operation can even be optimized with a '&
1'
if needed.
[2012-04-08 10:01:22] larue...@php.net
@theanomaly as we talked, since Rasmus said it's okey, then I have no objection.
2 suggestions :
1. use oddlen & 1 instead of oddlen % 2
2. since you cal oddlen % 2 twice, so it's better if you can use a var to hold
it
and it's better for you to make a pull request by yourself, let me know if you
need help on that :)
--------------------
[2012-04-08 10:00:43] krtek4+php at gmail dot com
The internal representation must always be aligned on 8 bits, thus we have no
choice to pad with 0 bits at the beginning, 1000 and 1000 is the exact same
value in binary and I think the actual patch is correct.
The new problem is that the reverse operation, i.e. bin2hex, should remove the
added 0 bit at the beginning.
@theanomaly ; decbin works just fine since it returns a string composed of 0s
and 1s and not a "binary value". hex2bin / bin2hex are the only function I'm
aware of working this way.
BTW, why did you sent another patch ? mine is doing exactly the same as yours
and is working fine.
[2012-04-08 09:42:25] theanomaly dot is at gmail dot com
@laruence
I've replaced the last patch with a better patch because I realized I created a
memory leak and that was a poor strategy.
I can't understand why there should be any confusion about whether it's an
octal
value or a hexadecimal value though. Since when should using bin2hex() ever
leave us with the expectation that would _ever_ get back an octal value?
I might be missing something here, but hex2bin() should always be expecting a
hexadecimal value and bin2hex() should always leave us with the expectation of
a
hexadecimal value. I see nothing wrong with padding the value to an even number
otherwise the result is hex2bin() isn't doing what it's supposed to be doing.
It
makes sense to me that even if the client sends a value of '1' that it's
completely expected behavior that '01' and '1' should both be a valid
hexadecimal value.
To me it just makes no sense to punish the client for forgetting to pad the
value by returning false data. At the very least we should be issuing a warning
to let the client know they have sent unexpected data and then this can be
documented behavior. But why waste time fixing it to issue E_WARNINGs when this
patch fixes the issue completely? Besides hex2bin is returning a string. It's
not like the user can inadvertently use it as an octal value.
var_dump('0123' + '0123'); // int(246)
This would be silly not to fix in my opinion. Especially since it's such an
easy
fix. At least run the patch and let me know which test case you can come up
with
that would break any of PHP's already existing documented behavior by making
this modification?
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
