1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-19 12:00:55 +01:00

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
This commit is contained in:
epriestley 2011-04-30 23:06:27 -07:00
parent 72e33c9e5a
commit 260b40b84a
4 changed files with 13 additions and 13 deletions

View file

@ -19,7 +19,7 @@
abstract class PhabricatorLiskDAO extends LiskDAO { abstract class PhabricatorLiskDAO extends LiskDAO {
public function establishConnection($mode) { public function establishLiveConnection($mode) {
$conf_provider = PhabricatorEnv::getEnvConfig( $conf_provider = PhabricatorEnv::getEnvConfig(
'mysql.configuration_provider', 'DatabaseConfigurationProvider'); 'mysql.configuration_provider', 'DatabaseConfigurationProvider');
PhutilSymbolLoader::loadClass($conf_provider); PhutilSymbolLoader::loadClass($conf_provider);

View file

@ -42,15 +42,15 @@
* =Building New Objects= * =Building New Objects=
* *
* To create new Lisk objects, extend @{class:LiskDAO} and implement * To create new Lisk objects, extend @{class:LiskDAO} and implement
* @{method:establishConnection}. It should return an AphrontDatabaseConnection; * @{method:establishLiveConnection}. It should return an
* this will tell Lisk where to save your objects. * AphrontDatabaseConnection; this will tell Lisk where to save your objects.
* *
* class Dog extends LiskDAO { * class Dog extends LiskDAO {
* *
* protected $name; * protected $name;
* protected $breed; * protected $breed;
* *
* public function establishConnection() { * public function establishLiveConnection() {
* return $some_connection_object; * return $some_connection_object;
* } * }
* } * }
@ -175,7 +175,7 @@ abstract class LiskDAO {
} }
} }
abstract protected function establishConnection($mode); abstract protected function establishLiveConnection($mode);
/* -( Configuring Lisk )--------------------------------------------------- */ /* -( Configuring Lisk )--------------------------------------------------- */
@ -370,7 +370,7 @@ abstract class LiskDAO {
protected function loadRawDataWhere($pattern/*, $arg, $arg, $arg ... */) { protected function loadRawDataWhere($pattern/*, $arg, $arg, $arg ... */) {
$connection = $this->getConnection('r'); $connection = $this->establishConnection('r');
$lock_clause = ''; $lock_clause = '';
if ($connection->isReadLocking()) { if ($connection->isReadLocking()) {
@ -600,7 +600,7 @@ abstract class LiskDAO {
* *
* @task info * @task info
*/ */
protected function getConnection($mode) { public function establishConnection($mode) {
if ($mode != 'r' && $mode != 'w') { if ($mode != 'r' && $mode != 'w') {
throw new Exception("Unknown mode '{$mode}', should be 'r' or '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 // or on 'w' queries against reading
if (!isset($this->__connections[$mode])) { if (!isset($this->__connections[$mode])) {
$this->__connections[$mode] = $this->establishConnection($mode); $this->__connections[$mode] = $this->establishLiveConnection($mode);
} }
return $this->__connections[$mode]; return $this->__connections[$mode];
@ -732,7 +732,7 @@ abstract class LiskDAO {
$map[$k] = $v; $map[$k] = $v;
} }
$conn = $this->getConnection('w'); $conn = $this->establishConnection('w');
foreach ($map as $key => $value) { foreach ($map as $key => $value) {
$map[$key] = qsprintf($conn, '%C = %ns', $key, $value); $map[$key] = qsprintf($conn, '%C = %ns', $key, $value);
@ -781,7 +781,7 @@ abstract class LiskDAO {
public function delete() { public function delete() {
$this->willDelete(); $this->willDelete();
$conn = $this->getConnection('w'); $conn = $this->establishConnection('w');
$conn->query( $conn->query(
'DELETE FROM %T WHERE %C = %d', 'DELETE FROM %T WHERE %C = %d',
$this->getTableName(), $this->getTableName(),
@ -835,7 +835,7 @@ abstract class LiskDAO {
$this->willWriteData($data); $this->willWriteData($data);
$conn = $this->getConnection('w'); $conn = $this->establishConnection('w');
$columns = array_keys($data); $columns = array_keys($data);

View file

@ -42,7 +42,7 @@ class LiskIsolationTestCase extends PhabricatorTestCase {
$dao = new LiskIsolationTestDAO(); $dao = new LiskIsolationTestDAO();
try { try {
$dao->establishConnection('r'); $dao->establishLiveConnection('r');
$this->assertFailure( $this->assertFailure(
"LiskIsolationTestDAO did not throw an exception when instructed to ". "LiskIsolationTestDAO did not throw an exception when instructed to ".

View file

@ -31,7 +31,7 @@ class LiskIsolationTestDAO extends LiskDAO {
return PhabricatorPHID::generateNewPHID('TISO'); return PhabricatorPHID::generateNewPHID('TISO');
} }
public function establishConnection($mode) { public function establishLiveConnection($mode) {
throw new LiskIsolationTestDAOException( throw new LiskIsolationTestDAOException(
"Isolation failure! DAO is attempting to connect to an external ". "Isolation failure! DAO is attempting to connect to an external ".
"resource!"); "resource!");