Merge pull request #21072 from colemanw/apiTableName

APIv4 - Throw exception instead of munging illegal join aliases
civicrm · Aug 10, 2021 · 7eb8aaf · 7eb8aaf
2 parents a933031 + 1930b3e
commit 7eb8aaf
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 1 deletion.
diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php
@@ -680,7 +680,10 @@ private function addExplicitJoins() {
         continue;
       }
       // Ensure alias is a safe string, and supply default if not given
-      $alias = $alias ? \CRM_Utils_String::munge($alias, '_', 256) : strtolower($entity);
+      $alias = $alias ?: strtolower($entity);
+      if ($alias === self::MAIN_TABLE_ALIAS || !preg_match('/^[-\w]{1,256}$/', $alias)) {
+        throw new \API_Exception('Illegal join alias: "' . $alias . '"');
+      }
       // First item in the array is a boolean indicating if the join is required (aka INNER or LEFT).
       // The rest are join conditions.
       $side = array_shift($join);

diff --git a/tests/phpunit/api/v4/Action/FkJoinTest.php b/tests/phpunit/api/v4/Action/FkJoinTest.php
@@ -121,6 +121,40 @@ public function testExcludeJoin() {
     $this->assertNotContains($this->getReference('test_contact_1')['id'], $contacts);
   }
 
+  public function testInvalidJoinAlias() {
+    // Not allowed to use same alias as the base table
+    try {
+      Contact::get(FALSE)->addJoin('Address AS a')->execute();
+    }
+    catch (\API_Exception $e) {
+      $message = $e->getMessage();
+    }
+    $this->assertEquals('Illegal join alias: "a"', $message);
+
+    // Not allowed to use dots in the alias
+    try {
+      Contact::get(FALSE)->addJoin('Address AS add.ress')->execute();
+    }
+    catch (\API_Exception $e) {
+      $message = $e->getMessage();
+    }
+    $this->assertEquals('Illegal join alias: "add.ress"', $message);
+
+    // Not allowed to use an alias > 256 characters
+    try {
+      $longAlias = str_repeat('z', 257);
+      Contact::get(FALSE)->addJoin("Address AS $longAlias")->execute();
+    }
+    catch (\API_Exception $e) {
+      $message = $e->getMessage();
+    }
+    $this->assertEquals("Illegal join alias: \"$longAlias\"", $message);
+
+    // Alpha-numeric with dashes 256 characters long - weird but allowed
+    $okAlias = str_repeat('-0_a-9Z_', 32);
+    Contact::get(FALSE)->addJoin("Address AS $okAlias")->execute();
+  }
+
   public function testJoinToTheSameTableTwice() {
     $cid1 = Contact::create(FALSE)
       ->addValue('first_name', 'Aaa')