getActiveConnection() and getConnection() are broken when $target is not found.

From: Damien Tournoud <damien@tournoud.net>


---

 includes/database/database.inc              |   37 ++++++++++-----------------
 modules/simpletest/tests/database_test.test |   15 +++++++++++
 2 files changed, 29 insertions(+), 23 deletions(-)


diff --git includes/database/database.inc includes/database/database.inc
index 9ab7808..4784fb5 100644
--- includes/database/database.inc
+++ includes/database/database.inc
@@ -891,14 +891,17 @@ abstract class Database {
   final public static function getActiveConnection($target = 'default') {
     // This could just be a call to getConnection(), but that's an extra
     // method call for every single query.
-    if (!empty(self::$ignoreTargets[self::$activeKey][$target])) {
+
+    // If the requested target does not exist, or if it is ignored, we fall back
+    // to the default target. The target is typically either "default" or "slave",
+    // indicating to use a slave SQL server if one is available. If it's not
+    // available, then the default/master server is the correct server to use.
+    if (!empty(self::$ignoreTargets[self::$activeKey][$target]) || !isset(self::$databaseInfo[self::$activeKey][$target])) {
       $target = 'default';
     }
 
     if (!isset(self::$connections[self::$activeKey][$target])) {
-      // If we're trying to open a target that doesn't exist, we need to know
-      // what the actual target we got was.
-      $target = self::openConnection(self::$activeKey, $target);
+      self::openConnection(self::$activeKey, $target);
     }
 
     return isset(self::$connections[self::$activeKey][$target]) ? self::$connections[self::$activeKey][$target] : NULL;
@@ -911,14 +914,16 @@ abstract class Database {
    *   The corresponding connection object.
    */
   final public static function getConnection($key = 'default', $target = 'default') {
-    if (!empty(self::$ignoreTargets[$key][$target])) {
+    // If the requested target does not exist, or if it is ignored, we fall back
+    // to the default target. The target is typically either "default" or "slave",
+    // indicating to use a slave SQL server if one is available. If it's not
+    // available, then the default/master server is the correct server to use.
+    if (!empty(self::$ignoreTargets[$key][$target]) || !isset(self::$databaseInfo[$key][$target])) {
       $target = 'default';
     }
 
     if (!isset(self::$connections[$key][$target])) {
-      // If we're trying to open a target that doesn't exist, we need to know
-      // what the actual target we got was.
-      $target = self::openConnection(self::$activeKey, $target);
+      self::openConnection($key, $target);
     }
 
     return isset(self::$connections[$key][$target]) ? self::$connections[$key][$target] : NULL;
@@ -1028,10 +1033,7 @@ abstract class Database {
    *   The database connection key, as specified in settings.php.  The default
    *   is "default".
    * @param $target
-   *   The database target to open.  If the specified target does not exist,
-   *   the "default" target will be used instead.
-   * @return
-   *   The name of the target that was actually opened.
+   *   The database target to open.
    */
   final protected static function openConnection($key, $target) {
     global $db_prefix;
@@ -1041,16 +1043,9 @@ abstract class Database {
     }
     try {
       // If the requested database does not exist then it is an unrecoverable error.
-      // If the requested target does not exist, however, we fall back to the default
-      // target.  The target is typically either "default" or "slave", indicating to
-      // use a slave SQL server if one is available.  If it's not available, then the
-      // default/master server is the correct server to use.
       if (!isset(self::$databaseInfo[$key])) {
         throw new Exception('DB does not exist');
       }
-      if (!isset(self::$databaseInfo[$key][$target])) {
-        $target = 'default';
-      }
 
       if (!$driver = self::$databaseInfo[$key][$target]['driver']) {
         throw new Exception('Drupal is not set up');
@@ -1074,10 +1069,6 @@ abstract class Database {
       if (preg_match("/^simpletest\d+$/", $_SERVER['HTTP_USER_AGENT'])) {
         $db_prefix .= $_SERVER['HTTP_USER_AGENT'];
       }
-
-      // Return the target that was actually opened in case the requested one
-      // didn't exist.
-      return $target;
     }
     catch (Exception $e) {
       // It is extremely rare that an exception will be generated here other
diff --git modules/simpletest/tests/database_test.test modules/simpletest/tests/database_test.test
index f8bf819..bbde2bd 100644
--- modules/simpletest/tests/database_test.test
+++ modules/simpletest/tests/database_test.test
@@ -168,6 +168,21 @@ class DatabaseConnectionTestCase extends DatabaseTestCase {
     $this->assertNotNull($db1, t('default connection is a real connection object.'));
     $this->assertNotNull($db2, t('slave connection is a real connection object.'));
     $this->assertNotIdentical($db1, $db2, t('Each target refers to a different connection.'));
+
+    // Try to open an those target another, that should return the same objects.
+    $db1b = Database::getConnection('default', 'default');
+    $db2b = Database::getConnection('default', 'slave');
+    $this->assertIdentical($db1, $db1b, t('A second call to getConnection() returns the same object.'));
+    $this->assertIdentical($db2, $db2b, t('A second call to getConnection() returns the same object.'));
+
+    // Try to open an unknown target.
+    $db3 = Database::getConnection('default', 'anothertarget');
+    $this->assertNotNull($db3, t('another connection is a real connection object.'));
+    $this->assertIdentical($db1, $db3, t('An unknown target opens the default connection.'));
+
+    // Try to open an unknown target another time, that should return the same object.
+    $db3b = Database::getConnection('default', 'anothertarget');
+    $this->assertIdentical($db3, $db3b, t('A second call to getConnection() returns the same object.'));
   }
 
   /**