Zend Framework

Zend_Measure_Abstract: Precision detection on type change possible bug

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.10.0
  • Fix Version/s: 1.10.3
  • Component/s: Zend_Measure
  • Labels:
    None

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

Activity

Hide
Raphael Dehousse added a comment -

This has been introduced in revision 18493:

ZF-8009 Zend_Measure:

  • added precision detection on type change
Show
Raphael Dehousse added a comment - This has been introduced in revision 18493: ZF-8009 Zend_Measure:
  • added precision detection on type change
Hide
Thomas Weidner added a comment -

Closing as non-issue.
The code works as intended and throws no warning or error.

Show
Thomas Weidner added a comment - Closing as non-issue. The code works as intended and throws no warning or error.
Hide
Raphael Dehousse added a comment -

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

Show
Raphael Dehousse added a comment - 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
Hide
Raphael Dehousse added a comment -

If the strlen($value) == 0, then, the index will be -1 => notice

I will try to give you a test case to reproduce this

Show
Raphael Dehousse added a comment - If the strlen($value) == 0, then, the index will be -1 => notice I will try to give you a test case to reproduce this
Hide
Raphael Dehousse added a comment -

$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

Show
Raphael Dehousse added a comment - $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
Hide
Raphael Dehousse added a comment -

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

Show
Raphael Dehousse added a comment - 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
Hide
Raphael Dehousse added a comment -

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

Show
Raphael Dehousse added a comment - 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
Hide
Thomas Weidner added a comment - - edited

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.

Show
Thomas Weidner added a comment - - edited 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.
Hide
Raphael Dehousse added a comment -

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

Show
Raphael Dehousse added a comment - 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
Hide
Thomas Weidner added a comment -

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
Show
Thomas Weidner added a comment - 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
Hide
Raphael Dehousse added a comment -

Test:

$ cat DummyTest.php
<?php

require_once('Zend/Measure/Time.php');

class DummyTest extends PHPUnit_Framework_TestCase
{

public function testTime()

{ $locale = new Zend_Locale(); var_dump((string) $locale); var_dump(Zend_Locale_Math::isBcmathDisabled()); $time = new Zend_Measure_Time(0, Zend_Measure_Time::SECOND); $time->setLocale($locale); $time->setType(Zend_Measure_Time::SECOND); }

}

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

Show
Raphael Dehousse added a comment - Test: $ cat DummyTest.php <?php require_once('Zend/Measure/Time.php'); class DummyTest extends PHPUnit_Framework_TestCase { public function testTime() { $locale = new Zend_Locale(); var_dump((string) $locale); var_dump(Zend_Locale_Math::isBcmathDisabled()); $time = new Zend_Measure_Time(0, Zend_Measure_Time::SECOND); $time->setLocale($locale); $time->setType(Zend_Measure_Time::SECOND); } } 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
Hide
Raphael Dehousse added a comment -

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)

Show
Raphael Dehousse added a comment - 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)
Hide
Raphael Dehousse added a comment -

Do you want I create another bug for Zend_Locale_Math_PhpMath and let this one closed?

Show
Raphael Dehousse added a comment - Do you want I create another bug for Zend_Locale_Math_PhpMath and let this one closed?
Hide
Thomas Weidner added a comment -

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.

Show
Thomas Weidner added a comment - 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.
Hide
Thomas Weidner added a comment -

At last I was able to reproduce with a different code than yours.

Fixed with r21330

Show
Thomas Weidner added a comment - At last I was able to reproduce with a different code than yours. Fixed with r21330
Hide
Raphael Dehousse added a comment -

Thanks!

Show
Raphael Dehousse added a comment - Thanks!

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: