From e79dafcab874f554570aa974b9cbd1610323f9e2 Mon Sep 17 00:00:00 2001 From: michalsn Date: Tue, 17 Jun 2025 07:20:28 +0200 Subject: [PATCH 1/2] fix: add BaseConnection::handleTransStatus() method to fix protected property access --- system/Database/BaseConnection.php | 16 ++++++-- system/Database/BasePreparedQuery.php | 4 +- .../Database/Live/PreparedQueryTest.php | 38 +++++++++++++++++++ user_guide_src/source/changelogs/v4.6.2.rst | 1 + 4 files changed, 53 insertions(+), 6 deletions(-) diff --git a/system/Database/BaseConnection.php b/system/Database/BaseConnection.php index 1ae432dcb79c..f09175d59b46 100644 --- a/system/Database/BaseConnection.php +++ b/system/Database/BaseConnection.php @@ -652,9 +652,7 @@ public function query(string $sql, $binds = null, bool $setEscapeFlags = true, s $query->setDuration($startTime, $startTime); // This will trigger a rollback if transactions are being used - if ($this->transDepth !== 0) { - $this->transStatus = false; - } + $this->handleTransStatus(); if ( $this->DBDebug @@ -904,6 +902,18 @@ public function resetTransStatus(): static return $this; } + /** + * Handle transaction status when a query fails + * + * @internal This method is for internal database component use only + */ + public function handleTransStatus(): void + { + if ($this->transDepth !== 0) { + $this->transStatus = false; + } + } + /** * Begin Transaction */ diff --git a/system/Database/BasePreparedQuery.php b/system/Database/BasePreparedQuery.php index 3632ccb18c0c..d5bb40bebd0a 100644 --- a/system/Database/BasePreparedQuery.php +++ b/system/Database/BasePreparedQuery.php @@ -134,9 +134,7 @@ public function execute(...$data) $query->setDuration($startTime, $startTime); // This will trigger a rollback if transactions are being used - if ($this->db->transDepth !== 0) { - $this->db->transStatus = false; - } + $this->db->handleTransStatus(); if ($this->db->DBDebug) { // We call this function in order to roll-back queries diff --git a/tests/system/Database/Live/PreparedQueryTest.php b/tests/system/Database/Live/PreparedQueryTest.php index f5fe6115b111..4f1a35eb964f 100644 --- a/tests/system/Database/Live/PreparedQueryTest.php +++ b/tests/system/Database/Live/PreparedQueryTest.php @@ -304,4 +304,42 @@ public function testInsertBinaryData(): void $this->assertSame(strlen($fileContent), strlen($file)); } + + public function testHandleTransStatusMarksTransactionFailedDuringTransaction(): void + { + $this->db->transStart(); + + // Verify we're in a transaction + $this->assertSame(1, $this->db->transDepth); + + // Prepare a query that will fail (duplicate key) + $this->query = $this->db->prepare(static fn ($db) => $db->table('without_auto_increment')->insert([ + 'key' => 'a', + 'value' => 'b', + ])); + + $this->disableDBDebug(); + + $this->assertTrue($this->query->execute('test_key', 'test_value')); + $this->assertTrue($this->db->transStatus()); + + $this->seeInDatabase($this->db->DBPrefix . 'without_auto_increment', [ + 'key' => 'test_key', + 'value' => 'test_value' + ]); + + $this->assertFalse($this->query->execute('test_key', 'different_value')); + $this->assertFalse($this->db->transStatus()); + + $this->enableDBDebug(); + + // Complete the transaction - should rollback due to failed status + $this->assertFalse($this->db->transComplete()); + + // Verify the first insert was rolled back + $this->dontSeeInDatabase($this->db->DBPrefix . 'without_auto_increment', [ + 'key' => 'test_key', + 'value' => 'test_value' + ]); + } } diff --git a/user_guide_src/source/changelogs/v4.6.2.rst b/user_guide_src/source/changelogs/v4.6.2.rst index 676d1a731ee9..11d11fd8a1f5 100644 --- a/user_guide_src/source/changelogs/v4.6.2.rst +++ b/user_guide_src/source/changelogs/v4.6.2.rst @@ -37,6 +37,7 @@ Bugs Fixed - **Cache:** Fixed a bug where a corrupted or unreadable cache file could cause an unhandled exception in ``FileHandler::getItem()``. - **Database:** Fixed a bug where ``when()`` and ``whenNot()`` in ``ConditionalTrait`` incorrectly evaluated certain falsy values (such as ``[]``, ``0``, ``0.0``, and ``'0'``) as truthy, causing callbacks to be executed unexpectedly. These methods now cast the condition to a boolean using ``(bool)`` to ensure consistent behavior with PHP's native truthiness. +- **Database:** Fixed encapsulation violation in ``BasePreparedQuery`` when accessing ``BaseConnection::transStatus`` protected property. - **Email:** Fixed a bug where ``Email::getHostname()`` failed to use ``$_SERVER['SERVER_ADDR']`` when ``$_SERVER['SERVER_NAME']`` was not set. - **Security:** Fixed a bug where the ``sanitize_filename()`` function from the Security helper would throw an error when used in CLI requests. - **Session:** Fixed a bug where using the ``DatabaseHandler`` with an unsupported database driver (such as ``SQLSRV``, ``OCI8``, or ``SQLite3``) did not throw an appropriate error. From 06630a6fa9cbaaf3f57e7eb9d4198c1477c560ea Mon Sep 17 00:00:00 2001 From: michalsn Date: Tue, 17 Jun 2025 10:10:38 +0200 Subject: [PATCH 2/2] fix test --- tests/system/Database/Live/PreparedQueryTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/system/Database/Live/PreparedQueryTest.php b/tests/system/Database/Live/PreparedQueryTest.php index 4f1a35eb964f..7549e0b75ee0 100644 --- a/tests/system/Database/Live/PreparedQueryTest.php +++ b/tests/system/Database/Live/PreparedQueryTest.php @@ -341,5 +341,7 @@ public function testHandleTransStatusMarksTransactionFailedDuringTransaction(): 'key' => 'test_key', 'value' => 'test_value' ]); + + $this->db->resetTransStatus(); } }