From 260b40b84acd914314a698b8fc9d3b986dc165ce Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sat, 30 Apr 2011 23:06:27 -0700
Subject: [PATCH] Plug the establishConnection() Lisk isolation hole

Summary:
Currently you can still punch through Lisk isolation by calling
establishConnection(), and we do that all over the place. Rename getConnection()
to establishConnection() so that all existing callers are safe, and rename
establishConnection() to establishLiveConnection() so that it's not surprising
when this fails to stub in unit tests.

Not wedded to the name if anyone thinks "establishExternalConnection" or
something is clearer.

Test Plan:
Loaded site, browsed around, ran unit tests.

Reviewed By: aran
Reviewers: aran, tuomaspelkonen, jungejason
CC: aran
Differential Revision: 201
---
 .../base/storage/lisk/PhabricatorLiskDAO.php  |  2 +-
 src/storage/lisk/dao/LiskDAO.php              | 20 +++++++++----------
 .../dao/__tests__/LiskIsolationTestCase.php   |  2 +-
 .../dao/__tests__/LiskIsolationTestDAO.php    |  2 +-
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/applications/base/storage/lisk/PhabricatorLiskDAO.php b/src/applications/base/storage/lisk/PhabricatorLiskDAO.php
index fc282ed8a0..3db43dfc78 100644
--- a/src/applications/base/storage/lisk/PhabricatorLiskDAO.php
+++ b/src/applications/base/storage/lisk/PhabricatorLiskDAO.php
@@ -19,7 +19,7 @@
 
 abstract class PhabricatorLiskDAO extends LiskDAO {
 
-  public function establishConnection($mode) {
+  public function establishLiveConnection($mode) {
     $conf_provider = PhabricatorEnv::getEnvConfig(
       'mysql.configuration_provider', 'DatabaseConfigurationProvider');
     PhutilSymbolLoader::loadClass($conf_provider);
diff --git a/src/storage/lisk/dao/LiskDAO.php b/src/storage/lisk/dao/LiskDAO.php
index 27e62ca943..e45613ed25 100644
--- a/src/storage/lisk/dao/LiskDAO.php
+++ b/src/storage/lisk/dao/LiskDAO.php
@@ -42,15 +42,15 @@
  * =Building New Objects=
  *
  * To create new Lisk objects, extend @{class:LiskDAO} and implement
- * @{method:establishConnection}. It should return an AphrontDatabaseConnection;
- * this will tell Lisk where to save your objects.
+ * @{method:establishLiveConnection}. It should return an
+ * AphrontDatabaseConnection; this will tell Lisk where to save your objects.
  *
  *   class Dog extends LiskDAO {
  *
  *     protected $name;
  *     protected $breed;
  *
- *     public function establishConnection() {
+ *     public function establishLiveConnection() {
  *       return $some_connection_object;
  *     }
  *   }
@@ -175,7 +175,7 @@ abstract class LiskDAO {
     }
   }
 
-  abstract protected function establishConnection($mode);
+  abstract protected function establishLiveConnection($mode);
 
 
 /* -(  Configuring Lisk  )--------------------------------------------------- */
@@ -370,7 +370,7 @@ abstract class LiskDAO {
 
 
   protected function loadRawDataWhere($pattern/*, $arg, $arg, $arg ... */) {
-    $connection = $this->getConnection('r');
+    $connection = $this->establishConnection('r');
 
     $lock_clause = '';
     if ($connection->isReadLocking()) {
@@ -600,7 +600,7 @@ abstract class LiskDAO {
    *
    * @task   info
    */
-  protected function getConnection($mode) {
+  public function establishConnection($mode) {
     if ($mode != 'r' && $mode != 'w') {
       throw new Exception("Unknown mode '{$mode}', should be 'r' or 'w'.");
     }
@@ -617,7 +617,7 @@ abstract class LiskDAO {
     // or on 'w' queries against reading
 
     if (!isset($this->__connections[$mode])) {
-      $this->__connections[$mode] = $this->establishConnection($mode);
+      $this->__connections[$mode] = $this->establishLiveConnection($mode);
     }
 
     return $this->__connections[$mode];
@@ -732,7 +732,7 @@ abstract class LiskDAO {
       $map[$k] = $v;
     }
 
-    $conn = $this->getConnection('w');
+    $conn = $this->establishConnection('w');
 
     foreach ($map as $key => $value) {
       $map[$key] = qsprintf($conn, '%C = %ns', $key, $value);
@@ -781,7 +781,7 @@ abstract class LiskDAO {
   public function delete() {
     $this->willDelete();
 
-    $conn = $this->getConnection('w');
+    $conn = $this->establishConnection('w');
     $conn->query(
       'DELETE FROM %T WHERE %C = %d',
       $this->getTableName(),
@@ -835,7 +835,7 @@ abstract class LiskDAO {
 
     $this->willWriteData($data);
 
-    $conn = $this->getConnection('w');
+    $conn = $this->establishConnection('w');
 
     $columns = array_keys($data);
 
diff --git a/src/storage/lisk/dao/__tests__/LiskIsolationTestCase.php b/src/storage/lisk/dao/__tests__/LiskIsolationTestCase.php
index 1b11b7a675..6bf6e36ad4 100644
--- a/src/storage/lisk/dao/__tests__/LiskIsolationTestCase.php
+++ b/src/storage/lisk/dao/__tests__/LiskIsolationTestCase.php
@@ -42,7 +42,7 @@ class LiskIsolationTestCase extends PhabricatorTestCase {
     $dao = new LiskIsolationTestDAO();
 
     try {
-      $dao->establishConnection('r');
+      $dao->establishLiveConnection('r');
 
       $this->assertFailure(
         "LiskIsolationTestDAO did not throw an exception when instructed to ".
diff --git a/src/storage/lisk/dao/__tests__/LiskIsolationTestDAO.php b/src/storage/lisk/dao/__tests__/LiskIsolationTestDAO.php
index 72d806bbe2..bd0a3aeea6 100644
--- a/src/storage/lisk/dao/__tests__/LiskIsolationTestDAO.php
+++ b/src/storage/lisk/dao/__tests__/LiskIsolationTestDAO.php
@@ -31,7 +31,7 @@ class LiskIsolationTestDAO extends LiskDAO {
     return PhabricatorPHID::generateNewPHID('TISO');
   }
 
-  public function establishConnection($mode) {
+  public function establishLiveConnection($mode) {
     throw new LiskIsolationTestDAOException(
       "Isolation failure! DAO is attempting to connect to an external ".
       "resource!");