Issues

ZF-8486: Zend_Db_Table->getRow() fails due to incorrect check in seek method

Description

When using Zend_Db_Table for fetching data from certain row from DB table Zend Framework ncorrectly checks range of available/seekable positions in Zend_Db_Table_Rowset_Abstract::seek() on line 323 and throws exception "Illegal index is ...".

The solution is to change condition which triggers exception with fixing comparison from grather or equal to grather.

BEFORE:

if ($position < 0 || $position >= $this->_count) { ...

AFTER:

if ($position < 0 || $position > $this->_count) { ...

Comments

The issue still exists on 1.9.7, already checked!

This happens when you apply a foreach (for example) on a rowset and then try to get a particular row of this same rowset. After the foreach, the Iterator index is left at the end, after the last element (max(index)+1). The getRow() method fails to seek back to the former index because it simply doesn't exists in the Iterator. Happens in 1.10.0.

Still exists on 1.10.0

I'm looking into this today

The issue is actually not in seek(), seek() seems to be preforming the proper checks. I think the problem you are experiencing is using getRow().

Currently, when you iterate over a rowset, after iteration the internal pointer is set to point beyond the final row of the rowset. While this pauses no general issue with iteration, it will cause an issue when you call getRow($count-1, false), where false is whether or not to seek() BACK to the pointers original position, which of course if invalid.

I'll have to research if having a pointer point beyond the bounds after iteration is really what we want. In the mean time, try to rewind() the rowset after use to avoid any issues with getRow() trying to seek to an invalid position.

Adding patch for unit test.

The fix for the illegal index is if ($seek == true) { $this->seek($key); }

The problem is a simple out of range checking, as proposed on the issue.

If you has a Rowset with 4 rows and you check the last on a while loop will assume:

 
$this->_count = 4 // The last index is 3 (count - 1)
$position = 4 // The now working last index, not count - 1

The script:

 
if ($position < 0 || $position >= $this->_count) {

The result on Issue

 
if (4 < 0 || 4 >= 4) { 
TRUE

Proposed

 
if ($position < 0 || $position > $this->_count) {

The result

 
if (4 < 0 || 4 > 4) { 
FALSE

It NOT throws an Exception and assuming the last pos is 4, and not 3 ( count - 1 ).

And Ralph, do the unit-testing with proposed one, and check! I repeat myself: Its a simple "out of range", the count value is NEVER the last index!

I will agree that calling getRow() after an iteration throwing an exception is a bug. The problem is this: iteration happens, the pointer is left if an invalid state (basically the last element plus 1)- which is the correct behavior in PHP. The problem comes in that getRow() is (wrongly) attempting to leverage seeking and current() to find the desired row. THIS is the wrong thing to do, especially since it will try to restore the seek position after it is done. Since the seek position was invalid to begin off with, it will throw an exception when attempting to restore the seek position to the invalid state.

When you say this:


if ($position < 0 || $position > $this->_count) {

is what should be done, this is wrong. The last valid position is the count - 1, which is why the right hand side is $position >= $this->_count. In other words, if the count is 4, the last valid position is index 3, not 4. This is why we want the exception to be thrown. Put yet another way, the valid range for seek() is 0 to N-1, Not 1 to N.

I have a fix none-the-less. Will apply soon.

-ralph

Fixed at r23379 in trunk and at r23380 in release branch 1.11