Edit report at https://bugs.php.net/bug.php?id=61660&edit=1
ID: 61660
Comment by: ni...@php.net
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:
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).
Previous Comments:
------------------------------------------------------------------------
[2012-04-08 13:20:35] krtek4+php at gmail dot com
You are right, there was a bug in the patch I sent. I updated it on github.
Thanks for the comment !
------------------------------------------------------------------------
[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, 00001000 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.
------------------------------------------------------------------------
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
