[#36504] - [j3] Unexpected behavior of PDO driver in php 8+
- Closed
- 31 Aug 2023
- Medium
- Build: 4.0-dev
- # 36504
2) JModelListTest::testGetTotalReturnsFalseOnEmptyQuery
ValueError: PDO::prepare(): Argument # 1 ($query) cannot be empty
/drone/src/libraries/joomla/database/driver/pdo.php:725
/drone/src/libraries/src/MVC/Model/BaseDatabaseModel.php:366
/drone/src/libraries/src/MVC/Model/ListModel.php:289
/drone/src/tests/unit8/suites/libraries/legacy/model/JModelListTest.php:212
/usr/local/bin/phpunit:2287
Steps to reproduce the issue
-
Run unit tests on php 8 (with test testGetTotalReturnsFalseOnEmptyQuery activated)
-
Alternative it should also happen with an empty query
Expected result
no error
Actual result
error
System information (as much as possible)
PHP 8, PDO Driver
Additional comments
Found on refactoring the tests for phpunit 9 #36465
joomla-cms-bot
- | Labels |
Added:
No Code Attached Yet
|
||
The $this->connection->prepare, does that have something to do with PDO::prepare ?
If so, their help says following about the return value:
If the database server cannot successfully prepare the statement, PDO::prepare() returns false or emits PDOException (depending on error handling).
And the error handling part links to this: https://www.php.net/manual/en/pdo.error-handling.php
And this mentions changes from pre-8.0.0 to 8.0.0.
Could this be the thing?
Prior to PHP 8.0.0, PDO::ERRMODE_SILENT was the default mode for error handling.
As of PHP 8.0.0, PDO::ERRMODE_EXCEPTION is the default mode.
Sounds like this thing. The question would be whether we change the default error reporting or better handle the error message. I would say we should better do error handling right? Maybe within the PDO Driver and not on the reciving end? What do you think?
Maybe we can set the parameter for the error handling to PDO::ERRMODE_SILENT in the PDO drivers (MySQL PDO and PostgreSQL PDO) when connecting so that we force the old default behaviour.
I think we need a quick fix for 3.10 and 4.x and do the bigger changes later. Refactoring error handling and reporting in the driver is a bigger thing.
changing the default to the old value is at least the best way for 3.10
Ok fine for me can you do a PR?
Ok fine for me can you do a PR?
Who?
And should it be done in the CMS or in the driver?
Hmm, the error mode is already set to exception here https://github.com/joomla/joomla-cms/blob/3.10-dev/libraries/joomla/database/driver/pdomysql.php#L151 and here https://github.com/joomla/joomla-cms/blob/3.10-dev/libraries/fof/database/driver/pdomysql.php#L140 .
So that should not be the problem, should it?
What I mean is that this should not cause a change in behaviour between PHP pre-8 and 8 since we explicitly use that mode and not relied on the default.
Hackwar
- | Labels |
Added:
bug
|
||
has this been resolved?
Sorry don't know but since it didn't came up in real world it might be fixed in the framework anyway.
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-08-31 08:35:08 |
| Closed_By | ⇒ | HLeithner |
Hmm looks like the issue here is that we try to use the connection here and this fails: https://github.com/joomla/joomla-cms/blob/3.10-dev/libraries/joomla/database/driver/pdo.php#L725
The question would be whether skipping that preperation with an empty sql field would be the correct way? Just returing false there does not feel right.
Interesting is also that it only fails on php8 could it be that there are internal changes within the pdo driver which are only php8+?
What do you think?