diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php index cacae24..0811842 100644 --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php @@ -186,6 +186,64 @@ public function queryTemporary($query, array $args = array(), array $options = a return $tablename; } + /** + * {@inheritdoc} + */ + public function escapeField($field) { + $escaped = parent::escapeField($field); + + // Escape any invalid start character. + $escaped = preg_replace('/^[^A-Za-z0-9_]/', '', $escaped); + + // The pgsql database driver does not support field names that contain + // periods (supported by PostgreSQL server) because this method may be + // called by a field with a table alias as part of SQL conditions or + // order by statements. This will consider a period as a table alias + // identifier, and split the string at the first period. + if (preg_match('/^([A-Za-z0-9_]+)"?[.]"?([A-Za-z0-9_.]+)/', $escaped, $parts)) { + $table = $parts[1]; + $column = $parts[2]; + + // Use escape alias because escapeField may contain multiple periods that + // need to be escaped. + $escaped = $this->escapeTable($table) . '.' . $this->escapeAlias($column); + } + elseif (preg_match('/[A-Z]/', $escaped)) { + // Quote the field name for case-sensitivity. + $escaped = '"' . $escaped . '"'; + } + + return $escaped; + } + + /** + * {@inheritdoc} + */ + public function escapeAlias($field) { + $escaped = preg_replace('/[^A-Za-z0-9_]+/', '', $field); + + // Escape the alias in quotes for case-sensitivity. + if (preg_match('/[A-Z]/', $escaped)) { + $escaped = '"' . $escaped . '"'; + } + + return $escaped; + } + + /** + * {@inheritdoc} + */ + public function escapeTable($table) { + $escaped = parent::escapeTable($table); + + // Quote identifier to make it case-sensitive. + if (preg_match('/[A-Z]/', $escaped)) { + $escaped = '"' . $escaped . '"'; + } + + return $escaped; + } + public function driver() { return 'pgsql'; } diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php index 2190ee5..aac30e6 100644 --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php @@ -248,7 +248,8 @@ protected function createTableSql($name, $table) { * The field specification, as per the schema data structure format. */ protected function createFieldSql($name, $spec) { - $sql = $name . ' ' . $spec['pgsql_type']; + // The PostgreSQL server converts names into lowercase, unless quoted. + $sql = '"' . $name . '" ' . $spec['pgsql_type']; if (isset($spec['type']) && $spec['type'] == 'serial') { unset($spec['not null']); diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Select.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Select.php index de9409f..8c63044 100644 --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Select.php +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Select.php @@ -82,7 +82,7 @@ public function orderBy($field, $direction = 'ASC') { // Also check expression aliases. foreach ($this->expressions as $expression) { - if ($expression['alias'] == $field) { + if ($expression['alias'] == $this->connection->escapeAlias($field)) { return $return; } } @@ -112,6 +112,31 @@ public function orderBy($field, $direction = 'ASC') { /** * {@inheritdoc} */ + public function addExpression($expression, $alias = NULL, $arguments = array()) { + if (empty($alias)) { + $alias = 'expression'; + } + + // This implements counting in the same manner as the parent method. + $alias_candidate = $alias; + $count = 2; + while (!empty($this->expressions[$alias_candidate])) { + $alias_candidate = $alias . '_' . $count++; + } + $alias = $alias_candidate; + + $this->expressions[$alias] = array( + 'expression' => $expression, + 'alias' => $this->connection->escapeAlias($alias_candidate), + 'arguments' => $arguments, + ); + + return $alias; + } + + /** + * {@inheritdoc} + */ public function execute() { $this->connection->addSavepoint(); try { diff --git a/core/modules/system/src/Tests/Database/SelectTest.php b/core/modules/system/src/Tests/Database/SelectTest.php index 26dd109..bba0d73 100644 --- a/core/modules/system/src/Tests/Database/SelectTest.php +++ b/core/modules/system/src/Tests/Database/SelectTest.php @@ -39,10 +39,10 @@ function testSimpleComment() { $records = $result->fetchAll(); $query = (string) $query; - $expected = "/* Testing query comments */ SELECT test.name AS name, test.age AS age\nFROM \n{test} test"; + $expected = "/* Testing query comments */"; $this->assertEqual(count($records), 4, 'Returned the correct number of rows.'); - $this->assertEqual($query, $expected, 'The flattened query contains the comment string.'); + $this->assertNotEqual(FALSE, strpos($query, $expected), 'The flattened query contains the comment string.'); } /** @@ -57,10 +57,10 @@ function testVulnerableComment() { $records = $result->fetchAll(); $query = (string) $query; - $expected = "/* Testing query comments SELECT nid FROM {node}; -- */ SELECT test.name AS name, test.age AS age\nFROM \n{test} test"; + $expected = "/* Testing query comments SELECT nid FROM {node}; -- */"; $this->assertEqual(count($records), 4, 'Returned the correct number of rows.'); - $this->assertEqual($query, $expected, 'The flattened query contains the sanitised comment string.'); + $this->assertNotEqual(FALSE, strpos($query, $expected), 'The flattened query contains the sanitised comment string.'); } /** diff --git a/core/modules/system/src/Tests/Database/UpdateComplexTest.php b/core/modules/system/src/Tests/Database/UpdateComplexTest.php index 19eff49..abe2730 100644 --- a/core/modules/system/src/Tests/Database/UpdateComplexTest.php +++ b/core/modules/system/src/Tests/Database/UpdateComplexTest.php @@ -139,15 +139,12 @@ function testSubSelectUpdate() { $query = db_update('test') ->expression('age', $subselect) ->condition('name', 'Ringo'); - // Save the query for a __toString test later. - $string_test = $query; - $query->execute(); + // Save the number of rows that updated for assertion later. + $num_updated = $query->execute(); $after_age = db_query('SELECT age FROM {test} WHERE name = :name', array(':name' => 'Ringo'))->fetchField(); $expected_age = $select->execute()->fetchField(); $this->assertEqual($after_age, $expected_age); - // Replace whitespace with a single space. - $query_string = preg_replace('/\s+/', ' ', $string_test); - $this->assertIdentical('UPDATE {test} SET age= (SELECT MAX(priority) + :increment AS max_priority FROM {test_task} t) WHERE (name = :db_condition_placeholder_0)', trim($query_string)); + $this->assertEqual(1, $num_updated, t('Expected 1 row to be updated in subselect update query.')); } } diff --git a/core/tests/Drupal/Tests/Core/Database/Driver/pgsql/PostgresqlConnectionTest.php b/core/tests/Drupal/Tests/Core/Database/Driver/pgsql/PostgresqlConnectionTest.php new file mode 100644 index 0000000..a915f95 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Database/Driver/pgsql/PostgresqlConnectionTest.php @@ -0,0 +1,123 @@ +mock_pdo = $this->getMock('Drupal\Tests\Core\Database\Stub\StubPDO'); + $connection = $this->getMockBuilder('Drupal\Core\Database\Connection') + ->disableOriginalConstructor() + ->getMock(); + } + + /** + * Data provider for testEscapeTable. + * + * @return [] + * An indexed array of where each value is an array of arguments to pass to + * testEscapeField. The first value is the expected value, and the second + * value is the value to test. + */ + public function providerEscapeTables() { + return array( + array('nocase', 'nocase'), + array('"camelCase"', 'camelCase'), + array('"camelCase"', '"camelCase"'), + array('"camelCase"', 'camel/Case'), + ); + } + + /** + * Data provider for testEscapeAlias. + * + * @return [] + * Array of arrays with the following elements: + * - Expected escaped string. + * - String to escape. + */ + public function providerEscapeAliases() { + return array( + array('nocase', 'nocase'), + array('"camelCase"', '"camelCase"'), + array('"camelCase"', 'camelCase'), + array('"camelCase"', 'camel.Case'), + ); + } + + /** + * Data provider for testEscapeField. + * + * @return [] + * Array of arrays with the following elements: + * - Expected escaped string. + * - String to escape. + */ + public function providerEscapeFields() { + return array( + array('title', 'title'), + array('"isDefaultRevision"', 'isDefaultRevision'), + array('"isDefaultRevision"', '"isDefaultRevision"'), + array('entity_test."isDefaultRevision"', 'entity_test.isDefaultRevision'), + array('entity_test."isDefaultRevision"', '"entity_test"."isDefaultRevision"'), + array('"entityTest"."isDefaultRevision"', '"entityTest"."isDefaultRevision"'), + array('"entityTest"."isDefaultRevision"', 'entityTest.isDefaultRevision'), + array('entity_test."isDefaultRevision"', 'entity_test.is.Default.Revision'), + ); + } + + /** + * Assert that the escapeTable method returns a valid table name that is + * quoted. + * + * @dataProvider providerEscapeTables + */ + public function testEscapeTable($expected, $name) { + $pgsql_connection = new Connection($this->mock_pdo, array()); + + $this->assertEquals($expected, $pgsql_connection->escapeTable($name)); + } + + /** + * Assert that the escapeAlias method returns a valid alias name that is + * quoted. + * + * @dataProvider providerEscapeAliases + */ + public function testEscapeAliases($expected, $name) { + $pgsql_connection = new Connection($this->mock_pdo, array()); + + $this->assertEquals($expected, $pgsql_connection->escapeAlias($name)); + } + + /** + * Assert that the escapeField method returns a valid field value that is + * quoted. + * + * @dataProvider providerEscapeFields + */ + public function testEscapeField($expected, $name) { + $pgsql_connection = new Connection($this->mock_pdo, array()); + + $this->assertEquals($expected, $pgsql_connection->escapeField($name)); + } + +}