ZF-9078: Zend_Measure_Abstract: Precision detection on type change possible bug
Description
Dears,
In this code (line 287)
$slength = strlen($value); $length = 0; for($i = 1; $i <= 25; ++$i) { if ($value[$slength - $i] != '0') { $length = 26 - $i; break; } }
$this->_value = Zend_Locale_Math::round($value, $length); $this->_type = $type;
A possible error can occur if $slength is < 25... We will have $value[$indexNegative]. A simple fix would be
for($i = 1; $i <= $slength && $i <= 25; ++$i) {
Regards,
Raphaël Dehousse
Comments
Posted by Raphael Dehousse (thymus) on 2010-02-04T00:19:01.000+0000
This has been introduced in revision 18493:
[ZF-8009] Zend_Measure:
Posted by Thomas Weidner (thomas) on 2010-02-05T13:07:54.000+0000
Closing as non-issue. The code works as intended and throws no warning or error.
Posted by Raphael Dehousse (thymus) on 2010-02-07T05:34:48.000+0000
Thomas,
With all the respect I owe you, I do not understand why a notice is not considered as a possible bug. With this code, I get notices than causes unit tests to fail. A simple fix I provided you resolve the problem. Why not integrate it?
Regards,
Raphaël Dehousse
Posted by Raphael Dehousse (thymus) on 2010-02-07T06:53:49.000+0000
If the strlen($value) == 0, then, the index will be -1 => notice
I will try to give you a test case to reproduce this
Posted by Raphael Dehousse (thymus) on 2010-02-07T07:00:17.000+0000
$time = new Zend_Measure_Time(0, Zend_Measure_Time::SECOND);
$time->setType(Zend_Measure_Time::SECOND);
Uninitialized string offset: -1
Zend/Measure/Abstract.php:293
Regards,
Raphaël Dehousse
Posted by Raphael Dehousse (thymus) on 2010-02-07T07:07:55.000+0000
This is because with 0, strlen($value) = 1,
for($i = 1; $i <= 25; ++$i) { if ($value[$slength - $i] != '0') { $length = 26 - $i; break; } }
$value[1 - 1] == 0 => won't break $value[1 - 2] => negative offset...
Won't occur with $i <= $slength
Thank for your help :)
Regards
Raphaël Dehousse
Posted by Raphael Dehousse (thymus) on 2010-02-15T07:52:05.000+0000
Dear Thomas,
I really like the Zend Framework and I think that I have shown good will by providing patches to the issues I reported.
Would it be possible to see any action from you? A response?
Thank you very much for your cooperation.
Raphaël Dehousse
Posted by Thomas Weidner (thomas) on 2010-02-18T14:56:56.000+0000
The input value is fixed to a length of 25 by using Zend_Locale_Math. Using your example code, a vardump of $value on line 290 returns: string '0.0000000000000000000000000' (length=27)
Therefor still not reproducable. (speaking of revision 20096 or higher)
PS: Please note that I am always replying, even if I am one week away due to my real live job or because I am working on other issues... no reason to get impatient.
Posted by Raphael Dehousse (thymus) on 2010-03-01T02:36:29.000+0000
Dear Thomas,
First, please accept my excuses about my many comments.
Let's back to the issue :)
I tested on Linux and on Windows and indeed, the result is different. On Windows, if I dump $value line around 289-290, I receive string(27) "0.0000000000000000000000000". On Linux, I receive : string(1) "0".
So, the "problem" could be here. Then the issue is maybe in Zend_Locale_Math?
PS: I tested against ZF 1.10.1 and 1.10.2
Best regards,
Raphaël Dehousse
Posted by Thomas Weidner (thomas) on 2010-03-01T12:44:42.000+0000
Note: When you say that unittests fail it is always recommended, and in my eyes useful, when you provide the returned informations from unittests to see which test fails on which line with which returned value.
And we need also some other informations for reproducation in all issues: * PHP release? * BCMath available? * Used locale
Posted by Raphael Dehousse (thymus) on 2010-03-02T00:58:09.000+0000
Test:
$ cat DummyTest.php <?php
require_once('Zend/Measure/Time.php');
class DummyTest extends PHPUnit_Framework_TestCase {
}
Test output:
$ phpunit DummyTest.php PHPUnit 3.4.6 by Sebastian Bergmann.
string(5) "en_US" bool(true) E
Time: 0 seconds, Memory: 5.75Mb
There was 1 error:
1) DummyTest::testTime Uninitialized string offset: -1
/usr/share/php5/Zend/Measure/Abstract.php:292 /home/rdehouss/DummyTest.php:12
FAILURES! Tests: 1, Assertions: 0, Errors: 1.
PHP release: 5.2.12 BC math disabled Locale: en_US
Posted by Raphael Dehousse (thymus) on 2010-03-02T01:27:26.000+0000
With bcmath compiled with php, the problem does not occur:
$ phpunit DummyTest.php PHPUnit 3.4.6 by Sebastian Bergmann.
string(5) "en_US" bool(false) .
Time: 1 second, Memory: 5.75Mb
OK (1 test, 0 assertions)
Posted by Raphael Dehousse (thymus) on 2010-03-02T02:39:14.000+0000
Do you want I create another bug for Zend_Locale_Math_PhpMath and let this one closed?
Posted by Thomas Weidner (thomas) on 2010-03-03T12:18:42.000+0000
You said that unitests fail..
So I don't need a new test from you, but the information which unittests from ZF fail, with the exact failure, the exceptionstack and so on.
Give me not only the unitests from Zend_Measure but also the testresults from Zend_Locale_AllTests. These tests are deeper.
Thnx
PS: Please use styling and don't write code plaintext.
Posted by Thomas Weidner (thomas) on 2010-03-04T14:08:07.000+0000
At last I was able to reproduce with a different code than yours.
Fixed with r21330
Posted by Raphael Dehousse (thymus) on 2010-03-05T01:59:37.000+0000
Thanks!