From 396a950cbc415fea1da3e7f72b80ffd8291e8939 Mon Sep 17 00:00:00 2001
From: catch <catch@35733.no-reply.drupal.org>
Date: Sat, 11 Apr 2020 19:38:24 +0100
Subject: [PATCH] Issue #3126658 by alexpott, daffie, Beakerboy: Support
enclosing reserved words with brackets
---
.../lib/Drupal/Core/Command/DbDumpCommand.php | 2 +-
core/lib/Drupal/Core/Database/Connection.php | 55 ++++++++------
.../Core/Database/Driver/mysql/Connection.php | 14 ++--
.../Core/Database/Driver/pgsql/Connection.php | 12 ++-
.../Database/Driver/sqlite/Connection.php | 12 ++-
.../Tests/Core/Database/ConditionTest.php | 10 +--
.../Tests/Core/Database/ConnectionTest.php | 76 +++++++++++++++----
.../Tests/Core/Database/OrderByTest.php | 3 -
.../Core/Database/Stub/StubConnection.php | 22 +-----
9 files changed, 112 insertions(+), 94 deletions(-)
diff --git a/core/lib/Drupal/Core/Command/DbDumpCommand.php b/core/lib/Drupal/Core/Command/DbDumpCommand.php
index 8f95455078d3..ae07e3f32fe4 100644
--- a/core/lib/Drupal/Core/Command/DbDumpCommand.php
+++ b/core/lib/Drupal/Core/Command/DbDumpCommand.php
@@ -265,7 +265,7 @@ protected function getTableIndexes(Connection $connection, $table, &$definition)
*/
protected function getTableCollation(Connection $connection, $table, &$definition) {
// Remove identifier quotes from the table name. See
- // \Drupal\Core\Database\Driver\mysql\Connection::identifierQuote().
+ // \Drupal\Core\Database\Driver\mysql\Connection::$identifierQuotes.
$table = trim($connection->prefixTables('{' . $table . '}'), '"');
$query = $connection->query("SHOW TABLE STATUS WHERE NAME = :table_name", [':table_name' => $table]);
$data = $query->fetchAssoc();
diff --git a/core/lib/Drupal/Core/Database/Connection.php b/core/lib/Drupal/Core/Database/Connection.php
index cdcbefd6b7f6..c3bfff0d512e 100644
--- a/core/lib/Drupal/Core/Database/Connection.php
+++ b/core/lib/Drupal/Core/Database/Connection.php
@@ -2,6 +2,7 @@
namespace Drupal\Core\Database;
+use Drupal\Component\Assertion\Inspector;
use Drupal\Core\Database\Query\Condition;
/**
@@ -179,6 +180,17 @@ abstract class Connection {
*/
protected $rootTransactionEndCallbacks = [];
+ /**
+ * The identifier quote characters for the database type.
+ *
+ * An array containing the start and end identifier quote characters for the
+ * database type. The ANSI SQL standard identifier quote character is a double
+ * quotation mark.
+ *
+ * @var string[]
+ */
+ protected $identifierQuotes;
+
/**
* Constructs a Connection object.
*
@@ -191,6 +203,11 @@ abstract class Connection {
* - Other driver-specific options.
*/
public function __construct(\PDO $connection, array $connection_options) {
+ if ($this->identifierQuotes === NULL) {
+ @trigger_error('In drupal:10.0.0 not setting the $identifierQuotes property in the concrete Connection class will result in an RuntimeException. See https://www.drupal.org/node/2986894', E_USER_DEPRECATED);
+ $this->identifierQuotes = ['', ''];
+ }
+ assert(count($this->identifierQuotes) === 2 && Inspector::assertAllStrings($this->identifierQuotes), '\Drupal\Core\Database\Connection::$identifierQuotes must contain 2 string values');
// The 'transactions' option is deprecated.
if (isset($connection_options['transactions'])) {
@trigger_error('Passing a \'transactions\' connection option to ' . __METHOD__ . ' is deprecated in drupal:9.1.0 and is removed in drupal:10.0.0. All database drivers must support transactions. See https://www.drupal.org/node/2278745', E_USER_DEPRECATED);
@@ -329,7 +346,7 @@ protected function setPrefix($prefix) {
$this->prefixes = ['default' => $prefix];
}
- $identifier_quote = $this->identifierQuote();
+ [$start_quote, $end_quote] = $this->identifierQuotes;
// Set up variables for use in prefixTables(). Replace table-specific
// prefixes first.
$this->prefixSearch = [];
@@ -339,8 +356,8 @@ protected function setPrefix($prefix) {
$this->prefixSearch[] = '{' . $key . '}';
// $val can point to another database like 'database.users'. In this
// instance we need to quote the identifiers correctly.
- $val = str_replace('.', $identifier_quote . '.' . $identifier_quote, $val);
- $this->prefixReplace[] = $identifier_quote . $val . $key . $identifier_quote;
+ $val = str_replace('.', $end_quote . '.' . $start_quote, $val);
+ $this->prefixReplace[] = $start_quote . $val . $key . $end_quote;
}
}
// Then replace remaining tables with the default prefix.
@@ -348,9 +365,9 @@ protected function setPrefix($prefix) {
// $this->prefixes['default'] can point to another database like
// 'other_db.'. In this instance we need to quote the identifiers correctly.
// For example, "other_db"."PREFIX_table_name".
- $this->prefixReplace[] = $identifier_quote . str_replace('.', $identifier_quote . '.' . $identifier_quote, $this->prefixes['default']);
+ $this->prefixReplace[] = $start_quote . str_replace('.', $end_quote . '.' . $start_quote, $this->prefixes['default']);
$this->prefixSearch[] = '}';
- $this->prefixReplace[] = $identifier_quote;
+ $this->prefixReplace[] = $end_quote;
// Set up a map of prefixed => un-prefixed tables.
foreach ($this->prefixes as $table_name => $prefix) {
@@ -360,20 +377,6 @@ protected function setPrefix($prefix) {
}
}
- /**
- * Returns the identifier quote character for the database type.
- *
- * The ANSI SQL standard identifier quote character is a double quotation
- * mark.
- *
- * @return string
- * The identifier quote character for the database type.
- */
- protected function identifierQuote() {
- @trigger_error('In drupal:10.0.0 this method will be abstract and contrib and custom drivers will have to implement it. See https://www.drupal.org/node/2986894', E_USER_DEPRECATED);
- return '';
- }
-
/**
* Appends a database prefix to all tables in a query.
*
@@ -413,7 +416,7 @@ public function prefixTables($sql) {
* This method should only be called by database API code.
*/
public function quoteIdentifiers($sql) {
- return str_replace(['[', ']'], $this->identifierQuote(), $sql);
+ return str_replace(['[', ']'], $this->identifierQuotes, $sql);
}
/**
@@ -579,7 +582,7 @@ public function makeSequenceName($table, $field) {
$sequence_name = $this->prefixTables('{' . $table . '}_' . $field . '_seq');
// Remove identifier quotes as we are constructing a new name from a
// prefixed and quoted table name.
- return str_replace($this->identifierQuote(), '', $sequence_name);
+ return str_replace($this->identifierQuotes, '', $sequence_name);
}
/**
@@ -1063,7 +1066,8 @@ public function condition($conjunction) {
*/
public function escapeDatabase($database) {
$database = preg_replace('/[^A-Za-z0-9_]+/', '', $database);
- return $this->identifierQuote() . $database . $this->identifierQuote();
+ [$start_quote, $end_quote] = $this->identifierQuotes;
+ return $start_quote . $database . $end_quote;
}
/**
@@ -1106,10 +1110,10 @@ public function escapeTable($table) {
public function escapeField($field) {
if (!isset($this->escapedFields[$field])) {
$escaped = preg_replace('/[^A-Za-z0-9_.]+/', '', $field);
- $identifier_quote = $this->identifierQuote();
+ [$start_quote, $end_quote] = $this->identifierQuotes;
// Sometimes fields have the format table_alias.field. In such cases
// both identifiers should be quoted, for example, "table_alias"."field".
- $this->escapedFields[$field] = $identifier_quote . str_replace('.', $identifier_quote . '.' . $identifier_quote, $escaped) . $identifier_quote;
+ $this->escapedFields[$field] = $start_quote . str_replace('.', $end_quote . '.' . $start_quote, $escaped) . $end_quote;
}
return $this->escapedFields[$field];
}
@@ -1130,7 +1134,8 @@ public function escapeField($field) {
*/
public function escapeAlias($field) {
if (!isset($this->escapedAliases[$field])) {
- $this->escapedAliases[$field] = $this->identifierQuote() . preg_replace('/[^A-Za-z0-9_]+/', '', $field) . $this->identifierQuote();
+ [$start_quote, $end_quote] = $this->identifierQuotes;
+ $this->escapedAliases[$field] = $start_quote . preg_replace('/[^A-Za-z0-9_]+/', '', $field) . $end_quote;
}
return $this->escapedAliases[$field];
}
diff --git a/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php b/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php
index 809e72cdea0c..bd9e9a7791ef 100644
--- a/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php
+++ b/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php
@@ -63,6 +63,11 @@ class Connection extends DatabaseConnection {
*/
const MIN_MAX_ALLOWED_PACKET = 1024;
+ /**
+ * {@inheritdoc}
+ */
+ protected $identifierQuotes = ['"', '"'];
+
/**
* {@inheritdoc}
*/
@@ -213,15 +218,6 @@ public function queryTemporary($query, array $args = [], array $options = []) {
return $tablename;
}
- /**
- * {@inheritdoc}
- */
- protected function identifierQuote() {
- // The database is using the ANSI option on set up so use ANSI quotes and
- // not MySQL's custom backtick quote.
- return '"';
- }
-
public function driver() {
return 'mysql';
}
diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php
index 5fa22e50c939..c2aef48a0f37 100644
--- a/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php
+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php
@@ -54,6 +54,11 @@ class Connection extends DatabaseConnection {
*/
protected $transactionalDDLSupport = TRUE;
+ /**
+ * {@inheritdoc}
+ */
+ protected $identifierQuotes = ['"', '"'];
+
/**
* Constructs a connection object.
*/
@@ -197,13 +202,6 @@ public function queryTemporary($query, array $args = [], array $options = []) {
return $tablename;
}
- /**
- * {@inheritdoc}
- */
- protected function identifierQuote() {
- return '"';
- }
-
public function driver() {
return 'pgsql';
}
diff --git a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php
index 162a615dc457..f00ffb75ffc1 100644
--- a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php
+++ b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php
@@ -60,6 +60,11 @@ class Connection extends DatabaseConnection {
*/
protected $transactionalDDLSupport = TRUE;
+ /**
+ * {@inheritdoc}
+ */
+ protected $identifierQuotes = ['"', '"'];
+
/**
* Constructs a \Drupal\Core\Database\Driver\sqlite\Connection object.
*/
@@ -375,13 +380,6 @@ public function databaseType() {
return 'sqlite';
}
- /**
- * {@inheritdoc}
- */
- protected function identifierQuote() {
- return '"';
- }
-
/**
* Overrides \Drupal\Core\Database\Connection::createDatabase().
*
diff --git a/core/tests/Drupal/Tests/Core/Database/ConditionTest.php b/core/tests/Drupal/Tests/Core/Database/ConditionTest.php
index 93eb0ef8a8bd..47a2eba20cfb 100644
--- a/core/tests/Drupal/Tests/Core/Database/ConditionTest.php
+++ b/core/tests/Drupal/Tests/Core/Database/ConditionTest.php
@@ -5,6 +5,7 @@
use Drupal\Core\Database\Connection;
use Drupal\Core\Database\Query\Condition;
use Drupal\Core\Database\Query\PlaceholderInterface;
+use Drupal\Tests\Core\Database\Stub\StubConnection;
use Drupal\Tests\Core\Database\Stub\StubPDO;
use Drupal\Tests\UnitTestCase;
use Prophecy\Argument;
@@ -195,14 +196,7 @@ class_alias('MockCondition', $mocked_namespace);
$mockPdo = $this->createMock(StubPDO::class);
- $connection = $this->getMockBuilder(Connection::class)
- ->setConstructorArgs([$mockPdo, $options])
- ->setMethods(['identifierQuote'])
- ->getMockForAbstractClass();
- // @todo In drupal:10.0.0 this function will be abstract and the mock
- // builder will automatically create it. This can be
- // can be removed at that time.
- $connection->method('identifierQuote')->willReturn(NULL);
+ $connection = new StubConnection($mockPdo, $options);
$condition = $connection->condition('AND');
$this->assertSame('MockCondition', get_class($condition));
}
diff --git a/core/tests/Drupal/Tests/Core/Database/ConnectionTest.php b/core/tests/Drupal/Tests/Core/Database/ConnectionTest.php
index 753fe87239c8..ceb5cb9761c1 100644
--- a/core/tests/Drupal/Tests/Core/Database/ConnectionTest.php
+++ b/core/tests/Drupal/Tests/Core/Database/ConnectionTest.php
@@ -78,7 +78,7 @@ public function providerTestPrefixTables() {
'SELECT * FROM test_table',
'test_',
'SELECT * FROM {table}',
- '',
+ ['', ''],
],
[
'SELECT * FROM "first_table" JOIN "second"."thingie"',
@@ -88,6 +88,16 @@ public function providerTestPrefixTables() {
],
'SELECT * FROM {table} JOIN {thingie}',
],
+ [
+ 'SELECT * FROM [first_table] JOIN [second].[thingie]',
+ [
+ 'table' => 'first_',
+ 'thingie' => 'second.',
+ ],
+ 'SELECT * FROM {table} JOIN {thingie}',
+ ['[', ']'],
+ ],
+
];
}
@@ -96,7 +106,7 @@ public function providerTestPrefixTables() {
*
* @dataProvider providerTestPrefixTables
*/
- public function testPrefixTables($expected, $prefix_info, $query, $quote_identifier = '"') {
+ public function testPrefixTables($expected, $prefix_info, $query, array $quote_identifier = ['"', '"']) {
$mock_pdo = $this->createMock('Drupal\Tests\Core\Database\Stub\StubPDO');
$connection = new StubConnection($mock_pdo, ['prefix' => $prefix_info], $quote_identifier);
$this->assertEquals($expected, $connection->prefixTables($query));
@@ -275,7 +285,8 @@ public function providerEscapeTables() {
return [
['nocase', 'nocase'],
['camelCase', 'camelCase'],
- ['backtick', '`backtick`', '`'],
+ ['backtick', '`backtick`', ['`', '`']],
+ ['brackets', '[brackets]', ['[', ']']],
['camelCase', '"camelCase"'],
['camelCase', 'camel/Case'],
// Sometimes, table names are following the pattern database.schema.table.
@@ -290,7 +301,7 @@ public function providerEscapeTables() {
* @covers ::escapeTable
* @dataProvider providerEscapeTables
*/
- public function testEscapeTable($expected, $name, $identifier_quote = '"') {
+ public function testEscapeTable($expected, $name, array $identifier_quote = ['"', '"']) {
$mock_pdo = $this->createMock(StubPDO::class);
$connection = new StubConnection($mock_pdo, [], $identifier_quote);
@@ -307,9 +318,10 @@ public function testEscapeTable($expected, $name, $identifier_quote = '"') {
*/
public function providerEscapeAlias() {
return [
- ['!nocase!', 'nocase', '!'],
- ['`backtick`', 'backtick', '`'],
- ['nocase', 'nocase', ''],
+ ['!nocase!', 'nocase', ['!', '!']],
+ ['`backtick`', 'backtick', ['`', '`']],
+ ['nocase', 'nocase', ['', '']],
+ ['[brackets]', 'brackets', ['[', ']']],
['"camelCase"', '"camelCase"'],
['"camelCase"', 'camelCase'],
['"camelCase"', 'camel.Case'],
@@ -320,7 +332,7 @@ public function providerEscapeAlias() {
* @covers ::escapeAlias
* @dataProvider providerEscapeAlias
*/
- public function testEscapeAlias($expected, $name, $identifier_quote = '"') {
+ public function testEscapeAlias($expected, $name, array $identifier_quote = ['"', '"']) {
$mock_pdo = $this->createMock(StubPDO::class);
$connection = new StubConnection($mock_pdo, [], $identifier_quote);
@@ -337,15 +349,16 @@ public function testEscapeAlias($expected, $name, $identifier_quote = '"') {
*/
public function providerEscapeFields() {
return [
- ['/title/', 'title', '/'],
- ['`backtick`', 'backtick', '`'],
- ['test.title', 'test.title', ''],
+ ['/title/', 'title', ['/', '/']],
+ ['`backtick`', 'backtick', ['`', '`']],
+ ['test.title', 'test.title', ['', '']],
['"isDefaultRevision"', 'isDefaultRevision'],
['"isDefaultRevision"', '"isDefaultRevision"'],
['"entity_test"."isDefaultRevision"', 'entity_test.isDefaultRevision'],
['"entity_test"."isDefaultRevision"', '"entity_test"."isDefaultRevision"'],
['"entityTest"."isDefaultRevision"', '"entityTest"."isDefaultRevision"'],
['"entityTest"."isDefaultRevision"', 'entityTest.isDefaultRevision'],
+ ['[entityTest].[isDefaultRevision]', 'entityTest.isDefaultRevision', ['[', ']']],
];
}
@@ -353,7 +366,7 @@ public function providerEscapeFields() {
* @covers ::escapeField
* @dataProvider providerEscapeFields
*/
- public function testEscapeField($expected, $name, $identifier_quote = '"') {
+ public function testEscapeField($expected, $name, array $identifier_quote = ['"', '"']) {
$mock_pdo = $this->createMock(StubPDO::class);
$connection = new StubConnection($mock_pdo, [], $identifier_quote);
@@ -370,10 +383,11 @@ public function testEscapeField($expected, $name, $identifier_quote = '"') {
*/
public function providerEscapeDatabase() {
return [
- ['/name/', 'name', '/'],
- ['`backtick`', 'backtick', '`'],
- ['testname', 'test.name', ''],
+ ['/name/', 'name', ['/', '/']],
+ ['`backtick`', 'backtick', ['`', '`']],
+ ['testname', 'test.name', ['', '']],
['"name"', 'name'],
+ ['[name]', 'name', ['[', ']']],
];
}
@@ -381,11 +395,41 @@ public function providerEscapeDatabase() {
* @covers ::escapeDatabase
* @dataProvider providerEscapeDatabase
*/
- public function testEscapeDatabase($expected, $name, $identifier_quote = '"') {
+ public function testEscapeDatabase($expected, $name, array $identifier_quote = ['"', '"']) {
$mock_pdo = $this->createMock(StubPDO::class);
$connection = new StubConnection($mock_pdo, [], $identifier_quote);
$this->assertEquals($expected, $connection->escapeDatabase($name));
}
+ /**
+ * @covers ::__construct
+ * @expectedDeprecation In drupal:10.0.0 not setting the $identifierQuotes property in the concrete Connection class will result in an RuntimeException. See https://www.drupal.org/node/2986894
+ * @group legacy
+ */
+ public function testIdentifierQuotesDeprecation() {
+ $mock_pdo = $this->createMock(StubPDO::class);
+ new StubConnection($mock_pdo, [], NULL);
+ }
+
+ /**
+ * @covers ::__construct
+ */
+ public function testIdentifierQuotesAssertCount() {
+ $this->expectException(\AssertionError::class);
+ $this->expectExceptionMessage('\Drupal\Core\Database\Connection::$identifierQuotes must contain 2 string values');
+ $mock_pdo = $this->createMock(StubPDO::class);
+ new StubConnection($mock_pdo, [], ['"']);
+ }
+
+ /**
+ * @covers ::__construct
+ */
+ public function testIdentifierQuotesAssertString() {
+ $this->expectException(\AssertionError::class);
+ $this->expectExceptionMessage('\Drupal\Core\Database\Connection::$identifierQuotes must contain 2 string values');
+ $mock_pdo = $this->createMock(StubPDO::class);
+ new StubConnection($mock_pdo, [], [0, '1']);
+ }
+
}
diff --git a/core/tests/Drupal/Tests/Core/Database/OrderByTest.php b/core/tests/Drupal/Tests/Core/Database/OrderByTest.php
index 9c28331ca8db..ef38627d9ff3 100644
--- a/core/tests/Drupal/Tests/Core/Database/OrderByTest.php
+++ b/core/tests/Drupal/Tests/Core/Database/OrderByTest.php
@@ -25,9 +25,6 @@ class OrderByTest extends UnitTestCase {
protected function setUp() {
$connection = $this->getMockBuilder('Drupal\Core\Database\Connection')
->disableOriginalConstructor()
- // Prevent deprecation message being triggered by
- // Connection::identifierQuote().
- ->setMethods(['identifierQuote'])
->getMockForAbstractClass();
$this->query = new Select($connection, 'test', NULL);
}
diff --git a/core/tests/Drupal/Tests/Core/Database/Stub/StubConnection.php b/core/tests/Drupal/Tests/Core/Database/Stub/StubConnection.php
index 7bdd86cc9ff5..db1a3809b296 100644
--- a/core/tests/Drupal/Tests/Core/Database/Stub/StubConnection.php
+++ b/core/tests/Drupal/Tests/Core/Database/Stub/StubConnection.php
@@ -20,13 +20,6 @@ class StubConnection extends Connection {
*/
public $driver = 'stub';
- /**
- * The identifier quote character. Can be set in the constructor for testing.
- *
- * @var string
- */
- protected $identifierQuote = '';
-
/**
* Constructs a Connection object.
*
@@ -34,21 +27,14 @@ class StubConnection extends Connection {
* An object of the PDO class representing a database connection.
* @param array $connection_options
* An array of options for the connection.
- * @param string $identifier_quote
- * The identifier quote character. Defaults to an empty string.
+ * @param string[]|null $identifier_quotes
+ * The identifier quote characters. Defaults to an empty strings.
*/
- public function __construct(\PDO $connection, array $connection_options, $identifier_quote = '') {
- $this->identifierQuote = $identifier_quote;
+ public function __construct(\PDO $connection, array $connection_options, $identifier_quotes = ['', '']) {
+ $this->identifierQuotes = $identifier_quotes;
parent::__construct($connection, $connection_options);
}
- /**
- * {@inheritdoc}
- */
- protected function identifierQuote() {
- return $this->identifierQuote;
- }
-
/**
* {@inheritdoc}
*/
--
GitLab