Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pdo_firebird: PDOException has wrong code and message since PHP 8.4 #17383

Open
basc opened this issue Jan 6, 2025 · 7 comments
Open

pdo_firebird: PDOException has wrong code and message since PHP 8.4 #17383

basc opened this issue Jan 6, 2025 · 7 comments

Comments

@basc
Copy link

basc commented Jan 6, 2025

Description

Since PHP 8.4 in Windows the PDOException has wrong infos.

If PHP connects to a firebird server with an invalid password the call PDOException->getCode() returns 0.
In PHP 8.3.15 getCode() returns 335544472.

I dont know if linux is also affected or if this only happens in windows.
Other PDO is not affected. I checked PDO_Mysql. Code and message is OK.

The following code shows the problem.

<?php

// OS: Windows 11 (24H2)
// Firebird Server 4.0.5
// fbclient.dll (5.0.1 64bit) - Client Library Version => WI-V6.3.1.1469 Firebird 5.0

function testPdoException($dsn, $username, $password) {
	try {
		$pdo = new PDO($dsn, $username, $password);
	} catch (PDOException $e) {
		echo 'PDOException code: ' . $e->getCode() . PHP_EOL;
		echo 'PDOException message: ' . $e->getMessage() . PHP_EOL;
	}
}

// connect to firebird server with invalid password
testPdoException("firebird:dbname=localhost:database", "user", "invalidPassword");

// connect to unknown host
testPdoException("firebird:dbname=unknownHost:database", "user", "password");

Resulted in this output in PHP 8.4.2:

// connect to firebird server with invalid password
PDOException code: 0
PDOException message: SQLSTATE[08003] [0] invalid database handle (no active connection)

// connect to unknown host
PDOException code: 0
PDOException message: SQLSTATE[08003] [0] invalid database handle (no active connection)

But I expected this output instead like in PHP 8.3.15:

// connect to firebird server with invalid password
PDOException code: 335544472
PDOException message: SQLSTATE[HY000] [335544472] Your user name and password are not defined. Ask your database administrator to set up a Firebird login.

// connect to unknown host
PDOException code: 335544721
PDOException message: SQLSTATE[HY000] [335544721] Unable to complete network request to host "unknownHost".

PHP Version

PHP 8.4.2

Operating System

Windows 11 (24H2)

@SakiTakamachi
Copy link
Member

I was not feeling well and was late to check.
I will check this issue later.

@cmb69
Copy link
Member

cmb69 commented Jan 27, 2025

The original exception is now chained ($e->getPrevious()), possibly due to

php-src/ext/pdo/pdo_dbh.c

Lines 522 to 526 in ef74ea0

/* XXX raise exception */
zend_restore_error_handling(&zeh);
if (!EG(exception)) {
zend_throw_exception(pdo_exception_ce, "Constructor failed", 0);
}

I also think that the following should not be executed if ret == 0 (since H->db == NULL):

if (dbh->auto_commit && !H->tr) {
ret = php_firebird_begin_transaction(dbh, /* auto commit mode */ true);
}

@SakiTakamachi
Copy link
Member

It is my understanding that the change in the message here is a change in the specifications and is not a bug.
As Christoph says, it's in prev.

Regarding the other thing Christophe mentioned, I'll fix it.

@cmb69
Copy link
Member

cmb69 commented Feb 2, 2025

I had a look at this again, and found the culprit. If attaching to the DB fails, we call firebird_handle_closer(), which then does:

if (isc_detach_database(H->isc_status, &H->db)) {

However, https://docwiki.embarcadero.com/InterBase/2020/en/Isc_detach_database() states:

db_handle returns an error in status_vector if it is NULL.

So if we know that an error will be returned, and that we don't need to detach, since attaching failed in the first place, we could apply (in addition to #17632):

 ext/pdo_firebird/firebird_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/pdo_firebird/firebird_driver.c b/ext/pdo_firebird/firebird_driver.c
index 9d7cb20d2c..20ca16eb78 100644
--- a/ext/pdo_firebird/firebird_driver.c
+++ b/ext/pdo_firebird/firebird_driver.c
@@ -594,7 +594,7 @@ static void firebird_handle_closer(pdo_dbh_t *dbh) /* {{{ */
 	}
 	H->in_manually_txn = 0;
 
-	if (isc_detach_database(H->isc_status, &H->db)) {
+	if (H->db != NULL && isc_detach_database(H->isc_status, &H->db)) {
 		php_firebird_error(dbh);
 	}
 

That would restore the behavior of PHP-8.3 (and also avoid another Firebird exception to be thrown).

@SakiTakamachi
Copy link
Member

SakiTakamachi commented Mar 15, 2025

@cmb69

Sorry for the late reply.
That code seems to result in a build error.

https://github.com/php/php-src/actions/runs/13867298424/job/38808811844#step:10:1

edit:
I'll look into it in more detail later

@cmb69
Copy link
Member

cmb69 commented Mar 15, 2025

That code seems to result in a build error.

https://github.com/php/php-src/actions/runs/13867298424/job/38808811844#step:10:1

Oh, in this case the handle is an integer. See also

https://github.com/FirebirdSQL/firebird/blob/1672adf4fe656633f84a9bb1be0bad019173f9c9/src/include/firebird/impl/types_pub.h#L52-L56

So we should probably only check that H->db (not that H->db != NULL).

@SakiTakamachi
Copy link
Member

@cmb69

Great, thanks!
I'm out now, so I'll change it later :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants