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

From: Date: Sun, 08 Apr 2012 16:23:50 +0000
Subject: Bug #61660 [Com]: bin2hex(hex2bin($data)) != $data
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-169339@lists.php.net to get a copy of this message
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:             Open
 Type:               Bug
 Package:            *General Issues
 Operating System:   Debian Linux
 PHP Version:        5.4.1RC1
 Block user comment: N
 Private report:     N

 New Comment:

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.

:)


Previous Comments:
------------------------------------------------------------------------
[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).

------------------------------------------------------------------------
[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.

------------------------------------------------------------------------


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


Thread (18 messages)

« previous php.bugs (#169339) next »