From f8eec38c941954b870940e99ffb4860e4a16eb8d Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 3 Sep 2019 12:09:20 -0700
Subject: [PATCH] When "mysqli->real_connect()" fails without setting an error
 code, recover more gracefully

Summary: Depends on D20779. Ref T13403. Bad parameters may cause this call to fail without setting an error code; if it does, catch the issue and go down the normal connection error pathway.

Test Plan:
  - With "mysql.port" set to "quack", ran `bin/storage probe`.
  - Before: wild mess of warnings as the code continued below and failed when trying to interact with the connection.
  - After: clean connection failure with a useful error message.

Maniphest Tasks: T13403

Differential Revision: https://secure.phabricator.com/D20780
---
 .../AphrontBaseMySQLDatabaseConnection.php    |  3 ++
 .../mysql/AphrontMySQLiDatabaseConnection.php | 32 +++++++++++++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/infrastructure/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php b/src/infrastructure/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php
index 0f9201b02d..313ea5a3b0 100644
--- a/src/infrastructure/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php
+++ b/src/infrastructure/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php
@@ -10,6 +10,9 @@ abstract class AphrontBaseMySQLDatabaseConnection
 
   private $nextError;
 
+  const CALLERROR_QUERY = 777777;
+  const CALLERROR_CONNECT = 777778;
+
   abstract protected function connect();
   abstract protected function rawQuery($raw_query);
   abstract protected function rawQueries(array $raw_queries);
diff --git a/src/infrastructure/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php b/src/infrastructure/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php
index 7a4b5193d5..6a0bc759a7 100644
--- a/src/infrastructure/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php
+++ b/src/infrastructure/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php
@@ -68,19 +68,47 @@ final class AphrontMySQLiDatabaseConnection
       $host = 'p:'.$host;
     }
 
-    @$conn->real_connect(
+    $trap = new PhutilErrorTrap();
+
+    $ok = @$conn->real_connect(
       $host,
       $user,
       $pass,
       $database,
       $port);
 
+    $call_error = $trap->getErrorsAsString();
+    $trap->destroy();
+
     $errno = $conn->connect_errno;
     if ($errno) {
       $error = $conn->connect_error;
       $this->throwConnectionException($errno, $error, $user, $host);
     }
 
+    // See T13403. If the parameters to "real_connect()" are wrong, it may
+    // fail without setting an error code. In this case, raise a generic
+    // exception. (One way to reproduce this is to pass a string to the
+    // "port" parameter.)
+
+    if (!$ok) {
+      if (strlen($call_error)) {
+        $message = pht(
+          'mysqli->real_connect() failed: %s',
+          $call_error);
+      } else {
+        $message = pht(
+          'mysqli->real_connect() failed, but did not set an error code '.
+          'or emit a message.');
+      }
+
+      $this->throwConnectionException(
+        self::CALLERROR_CONNECT,
+        $message,
+        $user,
+        $host);
+    }
+
     // See T13238. Attempt to prevent "LOAD DATA LOCAL INFILE", which allows a
     // malicious server to ask the client for any file. At time of writing,
     // this option MUST be set after "real_connect()" on all PHP versions.
@@ -152,7 +180,7 @@ final class AphrontMySQLiDatabaseConnection
             'Call to "mysqli->query()" failed, but did not set an error '.
             'code or emit an error message.');
         }
-        $this->throwQueryCodeException(777777, $message);
+        $this->throwQueryCodeException(self::CALLERROR_QUERY, $message);
       }
     }