ZF-7342: Zend_Amf_Util_BinaryStream - Endian Detection Issue and Fix proposal
Description
On the constructor the class variable "$this->_bigEndian" is hardcoded with a "1" value, on 64 Bits processors this convert the float values incorrectly because the method "writeDouble" execute a strrev($stream), with that all the float values are displayed as 0.40998789798-E123 for example
I detected that on the constructor is the following sentence:
$testEndian = unpack("C", pack("S", 256));
and following is: $this->_bigEndian = 1;
that I believe that the intention is to detect the endian on the server but it was never used
I changed the line with $this->_bigEndian = 1;
to
$this->_bigEndian = $testEndian[2];
Please verify if the solution is correct, I tested on my both environments:
My Linux box running: Apache/2.0.59 (Unix) DAV/2 PHP/5.2.6
My Solaris Box 64bits: Apache/2.0.59 (Unix) PHP/5.2.9
Let me know if you need more details and If I can help documenting this
Comments
Posted by Marc Bennewitz (private) (mabe) on 2010-01-06T12:24:06.000+0000
I don't know how your change will work correctly because on readDouble the byte order wasn't tested before reverting the stream. Additionally this will set bigEndian to 1 on little endian systems.
Posted by Alejandro Santos (alexws) on 2010-01-06T12:37:46.000+0000
I tested on both environments little endian and big endian, we have been working that way for months and we didn't had a problem
Posted by Marc Bennewitz (private) (mabe) on 2010-01-06T12:54:59.000+0000
I added a patch.
Can you run Zend_Amf_AllTests with the applied patch and test if this working for you please.
thx
Posted by Alejandro Santos (alexws) on 2010-01-06T16:21:08.000+0000
Hi Marc, I have patched and executed the all tests for AMF and got no errors.
Posted by Marc Bennewitz (private) (mabe) on 2010-01-07T10:02:04.000+0000
fixed in r20125
Posted by Alejandro Santos (alexws) on 2010-01-07T10:30:59.000+0000
Great, Thanks Marc, there is anything else that I have to do?
Posted by Wade Arnold (wadearnold) on 2010-01-17T11:22:09.000+0000
I see that the patch was committed to trunk but there was no test case commit to trunk. Do you plan on committing a test?
Posted by Wade Arnold (wadearnold) on 2010-01-17T11:25:43.000+0000
Also thanks for the patch!!!
Posted by Wade Arnold (wadearnold) on 2010-01-17T11:30:43.000+0000
I have run AllTests with this patch on 64 and 32 bit system and everything passes. We should really add a patch for when it would fail so that people know to upgrade. I am going to promote the change to the next release.
Posted by Marc Bennewitz (private) (mabe) on 2010-01-17T14:07:41.000+0000
Hi Wade,
This hadn't do anything with 32/64bit but with little/big endian systems.
I haven't got an big-endian (e.g Solaris, SPARC) system to test it by my self. But after viewing i think the tests didn't fail on big endian systems.
Posted by Wade Arnold (wadearnold) on 2010-01-17T14:18:31.000+0000
I personally don't have access to anything without intel inside so thanks for making the patch!