From 35884ecd3fbb09a847ee76eecd979468dd1a5c45 Mon Sep 17 00:00:00 2001 From: "John Paul E. Balandan, CPA" Date: Tue, 6 Oct 2026 21:08:36 +0800 Subject: [PATCH 1/2] fix: resolve schema-qualified table names in `getFieldData()` and `protectIdentifiers()` --- system/Database/BaseConnection.php | 2 +- system/Database/Postgre/Connection.php | 13 ++++++++-- system/Database/SQLSRV/Connection.php | 8 +++++++ system/Database/SQLite3/Connection.php | 7 +++++- tests/system/Database/BaseConnectionTest.php | 5 ++++ .../Live/AbstractGetFieldDataTestCase.php | 24 +++++++++++++++++++ user_guide_src/source/changelogs/v4.7.5.rst | 1 + 7 files changed, 56 insertions(+), 4 deletions(-) diff --git a/system/Database/BaseConnection.php b/system/Database/BaseConnection.php index a7af22565cca..45a7b0c6be94 100644 --- a/system/Database/BaseConnection.php +++ b/system/Database/BaseConnection.php @@ -1338,7 +1338,7 @@ public function protectIdentifiers($item, bool $prefixSingle = false, ?bool $pro private function protectDotItem(string $item, string $alias, bool $protectIdentifiers, bool $fieldExists): string { - $parts = explode('.', $item); + $parts = array_map(fn (string $part): string => trim($part, $this->escapeChar), explode('.', $item)); // Does the first segment of the exploded item match // one of the aliases previously identified? If so, diff --git a/system/Database/Postgre/Connection.php b/system/Database/Postgre/Connection.php index 6f6233d14b74..900f8f716c38 100644 --- a/system/Database/Postgre/Connection.php +++ b/system/Database/Postgre/Connection.php @@ -325,11 +325,20 @@ protected function _listColumns($table = ''): string */ protected function _fieldData(string $table): array { + $parts = explode('.', $table); + $table = array_pop($parts); + $schema = array_pop($parts); + $sql = 'SELECT "column_name", "data_type", "character_maximum_length", "numeric_precision", "column_default", "is_nullable" FROM "information_schema"."columns" WHERE LOWER("table_name") = ' - . $this->escape(strtolower($table)) - . ' ORDER BY "ordinal_position"'; + . $this->escape(strtolower($table)); + + if ($schema !== null) { + $sql .= ' AND LOWER("table_schema") = ' . $this->escape(strtolower($schema)); + } + + $sql .= ' ORDER BY "ordinal_position"'; if (($query = $this->query($sql)) === false) { throw new DatabaseException(lang('Database.failGetFieldData')); diff --git a/system/Database/SQLSRV/Connection.php b/system/Database/SQLSRV/Connection.php index e18408c5c34d..2bfbdabc57e3 100644 --- a/system/Database/SQLSRV/Connection.php +++ b/system/Database/SQLSRV/Connection.php @@ -314,12 +314,20 @@ protected function _enableForeignKeyChecks() */ protected function _fieldData(string $table): array { + $parts = explode('.', $table); + $table = array_pop($parts); + $schema = array_pop($parts); + $sql = 'SELECT COLUMN_NAME, DATA_TYPE, CHARACTER_MAXIMUM_LENGTH, NUMERIC_PRECISION, COLUMN_DEFAULT, IS_NULLABLE FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_NAME= ' . $this->escape(($table)); + if ($schema !== null) { + $sql .= ' AND TABLE_SCHEMA = ' . $this->escape($schema); + } + if (($query = $this->query($sql)) === false) { throw new DatabaseException(lang('Database.failGetFieldData')); } diff --git a/system/Database/SQLite3/Connection.php b/system/Database/SQLite3/Connection.php index 23ca0eaa5c35..582ab2b4b2b1 100644 --- a/system/Database/SQLite3/Connection.php +++ b/system/Database/SQLite3/Connection.php @@ -270,7 +270,12 @@ public function getFieldNames($tableName) */ protected function _fieldData(string $table): array { - if (false === $query = $this->query('PRAGMA TABLE_INFO(' . $this->protectIdentifiers($table, true, null, false) . ')')) { + $parts = explode('.', $table); + $table = array_pop($parts); + $schema = array_pop($parts); + $pragma = ($schema === null ? '' : $this->escapeIdentifiers($schema) . '.') . 'TABLE_INFO(' . $this->protectIdentifiers($table, true, null, false) . ')'; + + if (false === $query = $this->query('PRAGMA ' . $pragma)) { throw new DatabaseException(lang('Database.failGetFieldData')); } diff --git a/tests/system/Database/BaseConnectionTest.php b/tests/system/Database/BaseConnectionTest.php index 1b3a9841f58e..b21b02f14c7a 100644 --- a/tests/system/Database/BaseConnectionTest.php +++ b/tests/system/Database/BaseConnectionTest.php @@ -372,6 +372,11 @@ public static function provideProtectIdentifiers(): iterable 'quoted table alias' => [false, true, false, '"jobs" "j"', '"jobs" "j"'], 'quoted table alias prefix' => [true, true, false, '"jobs" "j"', '"test_jobs" "j"'], + 'schema.table' => [false, true, false, 'tenant.jobs', '"tenant"."test_jobs"'], + 'schema.table prefix' => [true, true, false, 'tenant.jobs', '"tenant"."test_jobs"'], + 'quoted schema.table prefix' => [true, true, false, '"tenant"."test_jobs"', '"tenant"."test_jobs"'], + 'quoted schema.table no-protect' => [true, false, false, '"tenant"."test_jobs"', 'tenant.test_jobs'], + 'table.*' => [false, true, true, 'jobs.*', '"test_jobs".*'], // Prefixed because it has segments 'table.* prefix' => [true, true, true, 'jobs.*', '"test_jobs".*'], 'table.column' => [false, true, true, 'users.id', '"test_users"."id"'], // Prefixed because it has segments diff --git a/tests/system/Database/Live/AbstractGetFieldDataTestCase.php b/tests/system/Database/Live/AbstractGetFieldDataTestCase.php index 4749fabd9de7..533e1089c3d6 100644 --- a/tests/system/Database/Live/AbstractGetFieldDataTestCase.php +++ b/tests/system/Database/Live/AbstractGetFieldDataTestCase.php @@ -57,6 +57,30 @@ protected function tearDown(): void $this->forge->dropTable($this->table, true); } + protected function qualifiedTableName(): string + { + return match ($this->db->DBDriver) { + 'MySQLi' => $this->db->getDatabase() . '.' . $this->table, + 'OCI8' => $this->db->username . '.' . $this->table, + 'Postgre' => 'public.' . $this->table, + 'SQLite3' => 'main.' . $this->table, + 'SQLSRV' => 'dbo.' . $this->table, + default => $this->markTestSkipped('No qualified table name is defined for ' . $this->db->DBDriver . '.'), + }; + } + + public function testGetFieldDataWithQualifiedTableName(): void + { + $this->createTableForDefault(); + + $toArray = static fn (stdClass $field): array => (array) $field; + + $this->assertSame( + array_map($toArray, $this->db->getFieldData($this->table)), + array_map($toArray, $this->db->getFieldData($this->qualifiedTableName())), + ); + } + protected function createTableForDefault(): void { $this->forge->dropTable($this->table, true); diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index deabaf4b7ab4..aaf5a34897ac 100644 --- a/user_guide_src/source/changelogs/v4.7.5.rst +++ b/user_guide_src/source/changelogs/v4.7.5.rst @@ -54,6 +54,7 @@ Bugs Fixed - **Cookie:** Fixed a bug where ``Cookie`` instances allowed invalid characters in path, domain, and prefix attributes rejected by ``setcookie()`` and ``setrawcookie()``. - **Database:** Fixed a bug where rebuilding a SQLite3 table (e.g., ``Forge::dropColumn()``, ``Forge::modifyColumn()``, ``Forge::dropForeignKey()`` and ``Forge::dropPrimaryKey()``) corrupted the table names referenced by its foreign keys when ``DBPrefix`` was set. - **Database:** Fixed a bug where Postgre query failures were silently ignored when ``DBDebug`` was enabled and PHP warnings were disabled. A ``DatabaseException`` is now thrown. +- **Database:** Fixed a bug where ``getFieldData()`` failed for a schema-qualified table name on Postgre, SQLSRV and SQLite3, and where ``protectIdentifiers()`` prefixed an already protected segment of a dotted identifier a second time. - **Debug:** Fixed a bug where ``Timer::start()`` treated ``0.0`` as an empty value and substituted the current time. - **Files:** Fixed a bug where ``File::move()`` and ``UploadedFile::move()`` set executable and overly permissive file permissions (``0777 & ~umask()`` instead of ``0666 & ~umask()``), and ``UploadedFile::move()`` targeted the parent directory instead of the destination file for ``chmod()``. - **Helpers:** Fixed a bug where ``get_dir_file_info()`` returned incomplete entries for subdirectories and missing files instead of omitting them. From fc5c8b056dbad0be9a7f25041a614a4890b5564c Mon Sep 17 00:00:00 2001 From: "John Paul E. Balandan, CPA" Date: Fri, 9 Oct 2026 02:44:19 +0800 Subject: [PATCH 2/2] address reviews --- system/Database/BaseConnection.php | 10 ++++++++-- tests/system/Database/BaseConnectionTest.php | 21 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/system/Database/BaseConnection.php b/system/Database/BaseConnection.php index 45a7b0c6be94..1db7618e2971 100644 --- a/system/Database/BaseConnection.php +++ b/system/Database/BaseConnection.php @@ -1315,7 +1315,7 @@ public function protectIdentifiers($item, bool $prefixSingle = false, ?bool $pro // In some cases, especially 'from', we end up running through // protect_identifiers twice. This algorithm won't work when // it contains the escapeChar so strip it out. - $item = trim($item, $this->escapeChar); + $item = $this->trimEscapeChar($item); // Is there a table prefix? If not, no need to insert it if ($this->DBPrefix !== '') { @@ -1336,9 +1336,15 @@ public function protectIdentifiers($item, bool $prefixSingle = false, ?bool $pro return $item . $alias; } + private function trimEscapeChar(string $item): string + { + return trim($item, is_array($this->escapeChar) ? implode('', $this->escapeChar) : $this->escapeChar); + } + private function protectDotItem(string $item, string $alias, bool $protectIdentifiers, bool $fieldExists): string { - $parts = array_map(fn (string $part): string => trim($part, $this->escapeChar), explode('.', $item)); + $parts = array_map($this->trimEscapeChar(...), explode('.', $item)); + $item = implode('.', $parts); // Does the first segment of the exploded item match // one of the aliases previously identified? If so, diff --git a/tests/system/Database/BaseConnectionTest.php b/tests/system/Database/BaseConnectionTest.php index b21b02f14c7a..7052250eab8c 100644 --- a/tests/system/Database/BaseConnectionTest.php +++ b/tests/system/Database/BaseConnectionTest.php @@ -439,6 +439,27 @@ public static function provideProtectIdentifiers(): iterable ]; } + public function testProtectIdentifiersWithBracketEscapeChar(): void + { + $db = new MockConnection($this->options); + $db->escapeChar = ['[', ']']; + + $this->assertSame('[test_jobs]', $db->protectIdentifiers('jobs', true, true, false)); + $this->assertSame('[test_jobs]', $db->protectIdentifiers('[test_jobs]', true, true, false)); + $this->assertSame('test_jobs', $db->protectIdentifiers('[test_jobs]', true, false, false)); + $this->assertSame('[test_jobs].[id]', $db->protectIdentifiers('jobs.id')); + $this->assertSame('[test_jobs].[id]', $db->protectIdentifiers('[test_jobs].[id]')); + $this->assertSame('test_jobs.id', $db->protectIdentifiers('[test_jobs].[id]', true, false)); + } + + public function testProtectIdentifiersStripsQuotesFromDottedItemWithoutPrefix(): void + { + $db = new MockConnection([...$this->options, 'DBPrefix' => '']); + + $this->assertSame('public.jobs', $db->protectIdentifiers('"public"."jobs"', true, false, false)); + $this->assertSame('"public"."jobs"', $db->protectIdentifiers('"public"."jobs"', true, true, false)); + } + /** * These tests are intended to confirm the current behavior. */