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
Posted by Stephan "Bladed" de Souza (bladed) on 2010-01-12T03:04:06.000+0000
The issue still exists on 1.9.7, already checked!
Posted by Nicolas Grevet (nyko18) on 2010-01-29T05:27:27.000+0000
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.
Posted by Stephan "Bladed" de Souza (bladed) on 2010-02-02T10:29:17.000+0000
Still exists on 1.10.0
Posted by Ralph Schindler (ralph) on 2010-02-04T07:20:03.000+0000
I'm looking into this today
Posted by Ralph Schindler (ralph) on 2010-02-08T08:13:55.000+0000
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.
Posted by Ralph Schindler (ralph) on 2010-02-19T12:46:09.000+0000
Adding patch for unit test.
Posted by kalyanraj (kalyanraj) on 2010-05-19T11:36:16.000+0000
The fix for the illegal index is if ($seek == true) { $this->seek($key); }
Posted by Stephan "Bladed" de Souza (bladed) on 2010-05-19T12:39:35.000+0000
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:
The script:
if ($position < 0 || $position >= $this->_count) {The result on Issue
if (4 < 0 || 4 >= 4) { TRUEProposed
if ($position < 0 || $position > $this->_count) {The result
if (4 < 0 || 4 > 4) { FALSEIt 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!
Posted by Ralph Schindler (ralph) on 2010-11-18T14:14:24.000+0000
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:
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
Posted by Ralph Schindler (ralph) on 2010-11-18T14:23:02.000+0000
Fixed at r23379 in trunk and at r23380 in release branch 1.11