php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #57265 issue in php_oci_statement_fetch with more than one piecewise column
Submitted: 2006-09-29 14:26 UTC Modified: 2006-10-06 08:48 UTC
From: jeff at badtz-maru dot com Assigned:
Status: Closed Package: oci8 (PECL)
PHP Version: 5.1.6 OS: GNU/Linux
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If this is not your bug, you can add a comment by following this link.
If this is your bug, but you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: jeff at badtz-maru dot com
New email:
PHP Version: OS:

 

 [2006-09-29 14:26 UTC] jeff at badtz-maru dot com
Description:
------------
If there are multiple piecewise (LONG) columns in a query's return, php_oci_statement_fetch will misbehave.  This seems to be caused when OCIStmtSetPieceInfo is called again on a column that has already had all of its pieces retrieved.  In this scenario, OCIStmtSetPieceInfo seems to return the length that were requested even though it has actually not retrieved anything (you'd think it would return 0).  This causes column->retlen4 to be incremented by PHP_OCI_PIECE_SIZE, mangling the actual length of the fetched data

I believe that if OCIStmtGetPieceInfo is used to determine whether the piece about to be fetched is OCI_LAST_PIECE or OCI_ONE_PIECE, then a flag could be set to indicate when each piecewise column is fully retrieved.



Reproduce code:
---------------
Fetch from two tables that both contain LONG datatypes.


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2006-10-02 09:18 UTC] tony2001 at phpclub dot net
Please provide a short but complete reproduce code with actual and expected results.
 [2006-10-02 10:59 UTC] jeff at badtz-maru dot com
The issue is in oci8_statement.c, in the part of php_oci_statement_fetch shown below:

  while (statement->errcode == OCI_NEED_DATA) {
    for (i = 0; i < statement->ncolumns; i++) {
      column = php_oci_statement_get_column(statement, i + 1, NULL, 0 TSRMLS_CC);
      if (column->piecewise)  {
        if (!column->data) {
          column->data = (text *) emalloc(PHP_OCI_PIECE_SIZE);
        } else {
          column->data = erealloc(column->data, column->retlen4 + PHP_OCI_PIECE_SIZE);
        }

        column->cb_retlen = PHP_OCI_PIECE_SIZE;

        PHP_OCI_CALL(OCIStmtSetPieceInfo,
            (
             (void *) column->oci_define,
             OCI_HTYPE_DEFINE,
             statement->err,
             ((char*)column->data) + column->retlen4,
             &(column->cb_retlen),
             OCI_NEXT_PIECE,
             &column->indicator,
             &column->retcode
            )
        );
      }
    }

    PHP_OCI_CALL_RETURN(statement->errcode,  OCIStmtFetch, (statement->stmt, statement->err, nrows, OCI_FETCH_NEXT, OCI_DEFAULT));

    for (i = 0; i < statement->ncolumns; i++) {
      column = php_oci_statement_get_column(statement, i + 1, NULL, 0 TSRMLS_CC);
      if (column && column->piecewise && (column->cb_retlen<PHP_OCI_PIECE_SIZE) ) {
        column->retlen4 += column->cb_retlen;
      }
    }


As we can see, OCIStmtSetPieceInfo is called for every piecewise column, every time OCI_NEED_DATA is true.  That doesn't seem to be correct.  If OCIStmtSetPieceInfo is called for a column that does not have any pieces left to retrieve, the call will fail and return the same column->cb_retlen that was passed in (instead of the 0 that the code above seems to have expected).  That means that if there are two piecewise columns and the first has had all of its pieces retrieved, each subsequent OCI_NEED_DATA will return a bogus PHP_OCI_PIECE_SIZE buffer.

I believe that the correct way to implement the piecewise retrieval is to check OCIStmtGetPieceInfo immediately before the call to OCIStmtSetPieceInfo.  If a value of OCI_LAST_PIECE or OCI_ONE_PIECE is returned, then set a flag to prevent that column loop from trying to set the piece info for that column on any subsequent iterations.

thanks, jeff
 [2006-10-02 11:05 UTC] tony2001 at phpclub dot net
We still need a reproduce case.
 [2006-10-02 11:23 UTC] jeff at badtz-maru dot com
$db = oci_connect('user', 'pass', 'devel');

$sql1 = "
SELECT
  a.id, a.longfield, b.longfield
FROM
  TEST1 A, TEST2 B
WHERE
  a.id = b.id
";
$sth = oci_parse($db, $sql1);
oci_execute($sth);
$resarr = oci_fetch_array($sth);

In this case, a.longfield (as held in $resarr) will contain the actual contents of a.longfield plus PHP_OCI_PIECE_SIZE bytes of "garbage" data.  The query of a single long field does work properly, such as "select a.longfield from test1 a", which is also understandable after examining the php_oci_statement_fetch code.

thanks,
jeff
 [2006-10-02 11:42 UTC] tony2001 at phpclub dot net
Cannot reproduce.

SQL> insert into test1 values (123456789);
SQL> insert into test2 values (987654321);

Results in:

array(4) {
  [0]=>
  string(65544) "123456789"
  ["L1"]=>
  string(65544) "123456789"
  [1]=>
  string(65544) "987654321"
  ["L2"]=>
  string(65544) "987654321"
}
 [2006-10-02 13:30 UTC] jeff at badtz-maru dot com
The web heads I am testing this against are running kernel 2.6.9-42 and OCI client library 10.2 .  The database servers are running 10g (which I believe is 10.2) also on the same linux kernel (RH Enterprise).

create table JEFFTEST1 {LONGFIELD LONG{
create table JEFFTEST2 {LONGFIELD LONG}
insert into jefftest1 (longfield) values ('123456789')
insert into jefftest2 (longfield) values ('987654321')
SELECT a.longfield, b.longfield FROM JEFFTEST1 A, JEFFTEST2 B

I've attached the output at http://www.cmainteractive.com/jeff/dump.txt .  I can reproduce this 100% of the time and we do have a test environment where we can make changes to the OCI8 source and test the results or add debugging code or whatever, if that can be of help.  This is how we were able to observe that OCIStmtSetPieceInfo is returning 65535 in cb_retlen when it is called after it has already returned the last piece.  I wasn't able to find any documentation to supp
 [2006-10-02 13:41 UTC] tony2001 at phpclub dot net
Ah, I got it. It should have been fixed in CVS about a month ago.
Please use OCI8 from PECL or try 5.2 snapshot (http://snaps.php.net).
 [2006-10-02 15:48 UTC] jeff at badtz-maru dot com
I just tried the pecl/oci8 from the 5.2 cvs branch and it exhibits the same issue.  I checked the changelog and didn't see anything that seemed to be applicable and we still see ocistmtsetpieceinfo's length param set to the value it was before the call.  It acts like the call is failing, although column->retcode is zero.  We've done a nasty fix for now which I noticed that I accidentally pasted into the code snippet above [if (column && column->piecewise && (column->cb_retlen<PHP_OCI_PIECE_SIZE))].  But obviously that is going to lame out for longs that are longer than PHP_OCI_PIECE_SIZE.  We can probably devise the proper fix using ocistmtgetpieceinfo if this issue isn't reproduceable in your dev environment.

thanks, jeff
 [2006-10-02 15:59 UTC] tony2001 at phpclub dot net
>I just tried the pecl/oci8 from the 5.2 cvs branch and it >exhibits the same issue. 
This means you're still using old OCI8 source, since 5.2 and OCI8 from PECL use ecalloc() instead of emalloc() in this case, which sets the memory to zero.
No nasty fixes are required, as the real fix has been applied: http://cvs.php.net/viewvc.cgi/php-src/ext/oci8/oci8_statement.c?r1=1.22&r2=1.23
 [2006-10-02 16:44 UTC] jeff at badtz-maru dot com
That fix is for something later in the code, though.  The buffer that is filled with trash in our case is the one allocated on oci8_statement.c line 199 (column->data = erealloc(column->data, column->retlen4 + PHP_OCI_PIECE_SIZE)).  Then column->cb_retlen is set to PHP_OCI_PIECE_SIZE and then ocistmtsetpieceinfo is called.  Normally, the buffer is next populated by the OCI driver and the driver changes cb_retlen to indicate the actual number of bytes that the driver put into the buffer.  Then column->retlen4 is incremented by this amount.  retlen4 is presumably used later to know how many bytes are in the column buffer, total.  The issue is that when setpieceinfo blows up and returns PHP_OCI_PIECE_SIZE back, retlen4 is incremented by this amount, effectively telling all the downstream code that the OCI fetch call returned PHP_OCI_PIECE_SIZE more bytes into the piecewise columns than it actually did.  You have to remember that these oddball LONGs (and any other piecewise column) do not fetch like a normal column.  retlen4 is entirely the sum of all of the results returned in cb_retlen by the setpieceinfo calls, it is not a fixed value like the other datatypes.  When setpieceinfo is called again after all pieces are retrieved for a column, cb_retlen comes back with 65K (when nothing has actually been put into the buffer), which is added to retlen4, which tells the downstream code that oracle copied a whole bunch more data into the buffer than it actually did.  Even if you change the erealloc call to something that clears first, I suspect that you'd end up with a big blank buffer at the end because all of the properties that store the length of the data fetched into the column would still be wrong, so subsequent copies into the php var space would copy too much data.

-jeff
 [2006-10-02 17:00 UTC] jeff at badtz-maru dot com
I should also mention that if your system is fast enough that your driver has all the buffers ready and you end up only iterating the loop at line 192 [while (statement->errcode == OCI_NEED_DATA] once, you won't see the issue.  Your database server has to be slow enough so that everything isn't returned in a single OCIStmtFetch call.  Thats when setpieceinfo may end up being called on pieces that have already been fully fetched.
 [2006-10-02 17:08 UTC] tony2001 at phpclub dot net
Does this patch fix it for you ?
http://tony2001.phpclub.net/dev/tmp/oci8_more_ecalloc.diff
 [2006-10-02 19:46 UTC] jeff at badtz-maru dot com
It doesn't fix the issue but, as I suspected, the erroneous part of the buffer is now zeroed.  It's still too big, though, which is understandable.  Zeroed or not, as long as column->cb_retlen comes back with a bogus value, column->retlen4 will have the wrong size for column->data.  To prevent this, ocistmtsetpieceinfo cannot be called for a column that has no remaining pieces.  ocistmtgetpieceinfo must be used to determine what needs to be read, the oracle oci docs indicate this.
 [2006-10-03 04:34 UTC] tony2001 at phpclub dot net
I don't think I can fix something blindly.
And I'm still unable to reproduce it.
 [2006-10-03 08:46 UTC] jeff at badtz-maru dot com
That is completely understandable.  We will put together a patch here and submit it for your inspection.
 [2006-10-05 16:36 UTC] jeff at badtz-maru dot com
OK, the patch we have developed is at http://host4.cmainteractive.com/jeff/diff.txt .  This patch incorporates the patch you provided above, although that patch is not really necessary.  To outline the change in flow:

current logic used in php_oci_statement_fetch
1) loop through all piecewise columns and call setpieceinfo on each column to indicate that maxpiecesize buffer is ready for filling.
2) fetch
3) loop through all piecewise columns and add cb_retlen bytes to buffer size counter
4) reloop until no data available

The issue is that the OCI driver is being told to fill data buffers for handles that may not not have any waiting data.  The fix is to ask the OCI driver to tell us which handles have data waiting and to fill only those buffers.

modified logic:
1) if there are any piecewise columns, call getpieceinfo to find out which handle is ready for debuffering
2) loop through all piecewise columns to find matching handle (column->oci_define), then call setpieceinfo on the column that was indicated as ready for debuffering
3) fetch
4) if there are any piecewise columns, loop through them to find handle that getpieceinfo returned.  Increment retlen4 by fetched amount.
5) reloop until no data available

We will test this here in depth next week and let you know how it goes.  I assume there may be style issues that I need to resolve to make this align with the rest of the code, just let me know.

-jeff
 [2006-10-06 05:44 UTC] tony2001 at phpclub dot net
I've slightly modified your patch (no need to increase retlen4 for all piecewise columns on each fetch, we need to do it only for the column being fetched).
Here is it: http://tony2001.phpclub.net/dev/tmp/long_fetch.diff

Please check out the patch and let me know if it (still) works for you.
 [2006-10-06 08:36 UTC] jeff at badtz-maru dot com
Yes, that is a good change, I had made it here as well and tried to sneak it into the patch before you downloaded it.
 [2006-10-06 08:48 UTC] tony2001 at phpclub dot net
Ok, thanks a lot for the patch, it's now in CVS.
I'm not sure yet if it'll be available in the upcoming 5.2 release, but it will surely appear in the next PECL release of OCI8.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Apr 20 13:01:29 2024 UTC