Interbase bugfixes

php.internals

Lars Westermann

18 years ago
Hi! I've had a long mail-dialogue with Wez regarding bugfixes for the Interbase modules (the old interbase and the new PDO version). In the PHP_5_3 branch I have worked on the files mentioned below. As these files aren't changed from the PHP_5_2 branch, the bugfixes will easily go into the PHP_5_2 branch as well. I have attached the file for someone to review/comment before I commit anything. PHP_Interbase: ===================== I've made bugfixes in ext/interbase/ibase_query.c to deal with the following bug-reports: Fixed bug #30907: (ibase_query() crashes (in fact the same bug as #32143) Fixed bug #32143: (ibase_query() causing IBserver 7 crash with NULL param as link-id) Fixed bug #39056: (Interbase NUMERIC data type error) Fixed bug #39397: (invalid statement handle in Unknown on line 0) Fixed bug #39700: (NUMERIC error when result precision are 7,8 or 12-14) Fixed bug #42284: (duplicate of #39700) Best regards Lars W.

Antony Dovgal

18 years ago
On 24.10.2007 02:11, Lars Westermann wrote:
> Hi! > > I've had a long mail-dialogue with Wez regarding bugfixes for the Interbase > modules (the old interbase and the new PDO version). In the PHP_5_3 branch I > have worked on the files mentioned below. As these files aren't changed from > the PHP_5_2 branch, the bugfixes will easily go into the PHP_5_2 branch as > well. > > I have attached the file for someone to review/comment before I commit > anything.
Please attach unified diffs (diff -u), not the patched files. Also separate diffs for each bug (with comments) are very welcome.
-- Wbr, Antony Dovgal

Lars Westermann

18 years ago
Hi For ibase_query.c the unified diff is: diff -u -r1.23.2.1.2.10 ibase_query.c --- ibase_query.c 7 Jun 2007 08:59:00 -0000 1.23.2.1.2.10 +++ ibase_query.c 23 Oct 2007 18:18:28 -0000 @@ -144,8 +144,13 @@ } if (ib_query->stmt) { IBDEBUG("Dropping statement handle (free_query)..."); - if (isc_dsql_free_statement(IB_STATUS, &ib_query->stmt, DSQL_drop)) { - _php_ibase_error(TSRMLS_C); + /* Bugfix #39397: Only free statement if db-connection is still open */ + char db_items[] = {isc_info_page_size}, res_buf[40]; + if (SUCCESS == isc_database_info(IB_STATUS, &ib_query->link->handle, + sizeof(db_items), db_items, sizeof(res_buf), res_buf)) { + if (isc_dsql_free_statement(IB_STATUS, &ib_query->stmt, DSQL_drop)) { + _php_ibase_error(TSRMLS_C); + } } } if (ib_query->in_array) { @@ -302,6 +307,12 @@ static char info_type[] = {isc_info_sql_stmt_type}; char result[8]; + /* Bugfix #32143: Crashing IBserver 7 with empty querystring */ + if (*query == '\0') { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Querystring empty."); + return FAILURE; + } + ib_query->link = link; ib_query->trans = trans; ib_query->result_res_id = 0; @@ -1288,9 +1299,23 @@ static int _php_ibase_var_zval(zval *val, void *data, int type, int len, /* {{{ */ int scale, int flag TSRMLS_DC) { - static ISC_INT64 const scales[] = { 1, 10, 100, 1000, 10000, 100000, 1000000, 100000000, 1000000000, - 1000000000, LL_LIT(10000000000),LL_LIT(100000000000),LL_LIT(10000000000000),LL_LIT(100000000000000), - LL_LIT(1000000000000000),LL_LIT(1000000000000000),LL_LIT(1000000000000000000) }; + static ISC_INT64 const scales[] = { 1, 10, 100, 1000, + 10000, + 100000, + 1000000, + 10000000, + 100000000, + 1000000000, + LL_LIT(10000000000), + LL_LIT(100000000000), + LL_LIT(1000000000000), + LL_LIT(10000000000000), + LL_LIT(100000000000000), + LL_LIT(1000000000000000), + LL_LIT(10000000000000000), + LL_LIT(100000000000000000), + LL_LIT(1000000000000000000) + }; switch (type & ~1) { unsigned short l; "Antony Dovgal" <tony@daylessday.org> skrev i en meddelelse news:471E7442.4070409@daylessday.org...

Antony Dovgal

18 years ago
On 24.10.2007 02:30, Lars Westermann wrote:
> Hi > > For ibase_query.c the unified diff is:
Please don't include diffs into the message body (you can try applying it now yourself to see why). It's also become completely unreadable, especially the first part of it. And some comments on WHY the diff looks like this would be appreciated ("this fixes bug #.. which happens because.. ").
> diff -u -r1.23.2.1.2.10 ibase_query.c > --- ibase_query.c 7 Jun 2007 08:59:00 -0000 1.23.2.1.2.10 > +++ ibase_query.c 23 Oct 2007 18:18:28 -0000 > @@ -144,8 +144,13 @@ > } > if (ib_query->stmt) { > IBDEBUG("Dropping statement handle (free_query)..."); > - if (isc_dsql_free_statement(IB_STATUS, &ib_query->stmt, DSQL_drop)) { > - _php_ibase_error(TSRMLS_C); > + /* Bugfix #39397: Only free statement if db-connection is still open */ > + char db_items[] = {isc_info_page_size}, res_buf[40]; > + if (SUCCESS == isc_database_info(IB_STATUS, &ib_query->link->handle, > + sizeof(db_items), db_items, sizeof(res_buf), res_buf)) { > + if (isc_dsql_free_statement(IB_STATUS, &ib_query->stmt, DSQL_drop)) { > + _php_ibase_error(TSRMLS_C); > + } > } > } > if (ib_query->in_array) { > @@ -302,6 +307,12 @@ > static char info_type[] = {isc_info_sql_stmt_type}; > char result[8]; > + /* Bugfix #32143: Crashing IBserver 7 with empty querystring */ > + if (*query == '\0') { > + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Querystring empty."); > + return FAILURE; > + } > + > ib_query->link = link; > ib_query->trans = trans; > ib_query->result_res_id = 0; > @@ -1288,9 +1299,23 @@ > static int _php_ibase_var_zval(zval *val, void *data, int type, int len, /* > {{{ */ > int scale, int flag TSRMLS_DC) > { > - static ISC_INT64 const scales[] = { 1, 10, 100, 1000, 10000, 100000, > 1000000, 100000000, 1000000000, > - 1000000000, > LL_LIT(10000000000),LL_LIT(100000000000),LL_LIT(10000000000000),LL_LIT(100000000000000), > - > LL_LIT(1000000000000000),LL_LIT(1000000000000000),LL_LIT(1000000000000000000) > }; > + static ISC_INT64 const scales[] = { 1, 10, 100, 1000, > + 10000, > + 100000, > + 1000000, > + 10000000, > + 100000000, > + 1000000000, > + LL_LIT(10000000000), > + LL_LIT(100000000000), > + LL_LIT(1000000000000), > + LL_LIT(10000000000000), > + LL_LIT(100000000000000), > + LL_LIT(1000000000000000), > + LL_LIT(10000000000000000), > + LL_LIT(100000000000000000), > + LL_LIT(1000000000000000000) > + }; > switch (type & ~1) { > unsigned short l; > > "Antony Dovgal" <tony@daylessday.org> skrev i en meddelelse > news:471E7442.4070409@daylessday.org... >> On 24.10.2007 02:11, Lars Westermann wrote: >>> Hi! >>> >>> I've had a long mail-dialogue with Wez regarding bugfixes for the >>> Interbase >>> modules (the old interbase and the new PDO version). In the PHP_5_3 >>> branch I >>> have worked on the files mentioned below. As these files aren't changed >>> from >>> the PHP_5_2 branch, the bugfixes will easily go into the PHP_5_2 branch >>> as >>> well. >>> >>> I have attached the file for someone to review/comment before I commit >>> anything. >> >> Please attach unified diffs (diff -u), not the patched files. >> Also separate diffs for each bug (with comments) are very welcome. >> >> -- >> Wbr, >> Antony Dovgal >
-- Wbr, Antony Dovgal

Lars Westermann

18 years ago
"Live and learn" ... Starting all over! Please find attached a unified diff (diff -u) for interbase/ibase_query.c (PHP_5_3 branch), which addresses the following bug reports: Bug #30907: (ibase_query() crashes (in fact the same bug as #32143) Bug #32143: (ibase_query() causing IBserver 7 crash with NULL param as link-id) Due to the new way of parsing parameters, the first NULL parameter in the examples is treated as the querystring (which it should be according to the documentation, as it isn't a resource). With Firebird 2 this resulted in an error stating that there is a problem with the query on line 1 column 1. In IBserver 7 I believe that this empty string causes a servercrash (I do not have access to an IBserver 7, so I can't test that). In either case I believe it is reasonable to assume, that an empty query doesn't make any sense, therefore the bugfix is to issue an error message _before_ trying to pass the query to the Interbase/Firebird API (@@ -302,6 +307,12 @@) Bug #39397: (invalid statement handle in Unknown on line 0) The error occurs when a query resource is destroyed, and the database-handle is already closed (and the statement resources in the Interbase/Firebird API therefore are already released). By checking that the statement-handle is still valid (by calling isc_database_info()), isc_dsql_free_statement() is only called when there is a valid handle. This way problems with the latter functioncall which aren't related to the handle still get reported (@@ -144,8 +144,13 @@) Bug #39056: (Interbase NUMERIC data type error) Bug #39700: (NUMERIC error when result precision are 7,8 or 12-14) Bug #42284: (duplicate of #39700) These are actually addressing the same problem, and I just took the solution given in the comment sections of the bugreports and put it into place (when lined up above each other, the errors are obvious in the original code) (@@ -1288,9 +1299,23 @@). Hope this is better! Best regards Lars

Lester Caine

18 years ago
Lars Westermann wrote:
> I've had a long mail-dialogue with Wez regarding bugfixes for the Interbase > modules (the old interbase and the new PDO version). In the PHP_5_3 branch I > have worked on the files mentioned below. As these files aren't changed from > the PHP_5_2 branch, the bugfixes will easily go into the PHP_5_2 branch as > well.
Lars there is also a bug relating to blob id's which was fixed in the overnight builds of 5.2.4, but the distributed copy seems to still have the fault which is now being supplied with the Mandriva 2008.0 distribution, and I'm getting reports from other sources as well. So busy on customer sites, I've not been able to look any closer, other than dropping back to the overnight build I have here that works OK :(
-- Lester Caine - G8HFL ----------------------------- Contact - http://home.lsces.co.uk/lsces/wiki/?page=contact L.S.Caine Electronic Services - http://home.lsces.co.uk MEDW - http://home.lsces.co.uk/ModelEngineersDigitalWorkshop/ Firebird - http://www.firebirdsql.org/index.php