From 5a193a820f526efb0b9f2ef8a49a4e249894aa5d Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Mon, 16 Jan 2023 15:29:41 +0000
Subject: [PATCH] Issue #3191623 by mondrake, Medha Kumari, daffie, alexpott:
Select queries do not escape the GROUP BY fields
---
.../lib/Drupal/Core/Database/Query/Select.php | 5 +++-
.../tests/src/Kernel/QueryGroupByTest.php | 2 +-
.../Core/Database/ReservedWordTest.php | 26 +++++++++++++++++++
.../Core/Database/SelectComplexTest.php | 4 +++
4 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/core/lib/Drupal/Core/Database/Query/Select.php b/core/lib/Drupal/Core/Database/Query/Select.php
index 93ffa081a6fb..c4fd8535f450 100644
--- a/core/lib/Drupal/Core/Database/Query/Select.php
+++ b/core/lib/Drupal/Core/Database/Query/Select.php
@@ -878,7 +878,10 @@ public function __toString() {
// GROUP BY
if ($this->group) {
- $query .= "\nGROUP BY " . implode(', ', $this->group);
+ $group_by_fields = array_map(function (string $field): string {
+ return $this->connection->escapeField($field);
+ }, $this->group);
+ $query .= "\nGROUP BY " . implode(', ', $group_by_fields);
}
// HAVING
diff --git a/core/modules/views/tests/src/Kernel/QueryGroupByTest.php b/core/modules/views/tests/src/Kernel/QueryGroupByTest.php
index 171688321e4e..20a55769fbd5 100644
--- a/core/modules/views/tests/src/Kernel/QueryGroupByTest.php
+++ b/core/modules/views/tests/src/Kernel/QueryGroupByTest.php
@@ -230,7 +230,7 @@ public function testGroupByBaseField() {
$view->displayHandlers->get('default')->options['fields']['name']['group_type'] = 'min';
unset($view->displayHandlers->get('default')->options['fields']['id']['group_type']);
$this->executeView($view);
- $this->assertStringContainsString('GROUP BY entity_test.id', (string) $view->build_info['query'], 'GROUP BY field includes the base table name when grouping on the base field.');
+ $this->assertMatchesRegularExpression('/GROUP BY .*[^\w\s]entity_test[^\w\s]\.[^\w\s]id[^\w\s]/', (string) $view->build_info['query'], 'GROUP BY field includes the base table name when grouping on the base field.');
}
/**
diff --git a/core/tests/Drupal/KernelTests/Core/Database/ReservedWordTest.php b/core/tests/Drupal/KernelTests/Core/Database/ReservedWordTest.php
index 9bc296c8e67c..c541f466d46d 100644
--- a/core/tests/Drupal/KernelTests/Core/Database/ReservedWordTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Database/ReservedWordTest.php
@@ -80,4 +80,30 @@ public function testSelectReservedWordAliasAllFields() {
$this->assertSame('27', $record->age);
}
+ /**
+ * Tests SELECT query with GROUP BY clauses on fields with reserved names.
+ */
+ public function testGroupBy() {
+ $this->connection->insert('select')
+ ->fields([
+ 'id' => 2,
+ 'update' => 'Update value 1',
+ ])
+ ->execute();
+
+ // Using aliases.
+ $query = $this->connection->select('select', 's');
+ $query->addExpression('COUNT([id])', 'num');
+ $query->addField('s', 'update');
+ $query->groupBy('s.update');
+ $this->assertSame('2', $query->execute()->fetchAssoc()['num']);
+
+ // Not using aliases.
+ $query = $this->connection->select('select');
+ $query->addExpression('COUNT([id])', 'num');
+ $query->addField('select', 'update');
+ $query->groupBy('update');
+ $this->assertSame('2', $query->execute()->fetchAssoc()['num']);
+ }
+
}
diff --git a/core/tests/Drupal/KernelTests/Core/Database/SelectComplexTest.php b/core/tests/Drupal/KernelTests/Core/Database/SelectComplexTest.php
index 281f3d4d57c9..e7df5c4386b1 100644
--- a/core/tests/Drupal/KernelTests/Core/Database/SelectComplexTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Database/SelectComplexTest.php
@@ -82,6 +82,10 @@ public function testGroupBy() {
$task_field = $query->addField('t', 'task');
$query->orderBy($count_field);
$query->groupBy($task_field);
+
+ $this->assertMatchesRegularExpression("/ORDER BY .*[^\w\s]num[^\w\s]/", (string) $query);
+ $this->assertMatchesRegularExpression("/GROUP BY .*[^\w\s]task[^\w\s]/", (string) $query);
+
$result = $query->execute();
$num_records = 0;
--
GitLab