Skip to content

Commit

Permalink
Prevent race condition when saving plugin settings (#15327)
Browse files Browse the repository at this point in the history 
* Prevent race condition when saving plugin settings

* use delete method
  • Loading branch information
@tsteur
tsteur committed Dec 30, 2019
1 parent 520ba48 commit 6131735
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 18 deletions.
76 changes: 58 additions & 18 deletions core/Tracker/Db/Mysqli.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,17 +165,20 @@ public function fetchAll($query, $parameters = array())
$timer = $this->initProfiler();
}

$rows = array();
$query = $this->prepare($query, $parameters);
$rs = mysqli_query($this->connection, $query);
if (is_bool($rs)) {
$stmt = $this->executeQuery($query, $parameters);
$result = $stmt->get_result();

if ($result === false) {
throw new DbException('fetchAll() failed: ' . mysqli_error($this->connection) . ' : ' . $query);
}

while ($row = mysqli_fetch_array($rs, MYSQLI_ASSOC)) {
$rows = array();
while ($row = $result->fetch_array(MYSQLI_ASSOC)) {
$rows[] = $row;
}
mysqli_free_result($rs);
$stmt->free_result();
$result->free();
$stmt->close();

if (self::$profiling && isset($timer)) {
$this->recordQueryProfile($query, $timer);
Expand Down Expand Up @@ -205,14 +208,17 @@ public function fetch($query, $parameters = array())
$timer = $this->initProfiler();
}

$query = $this->prepare($query, $parameters);
$rs = mysqli_query($this->connection, $query);
if (is_bool($rs)) {
$stmt = $this->executeQuery($query, $parameters);
$result = $stmt->get_result();

if ($result === false) {
throw new DbException('fetch() failed: ' . mysqli_error($this->connection) . ' : ' . $query);
}

$row = mysqli_fetch_array($rs, MYSQLI_ASSOC);
mysqli_free_result($rs);
$row = $result->fetch_array(MYSQLI_ASSOC);
$stmt->free_result();
$result->free();
$stmt->close();

if (self::$profiling && isset($timer)) {
$this->recordQueryProfile($query, $timer);
Expand Down Expand Up @@ -243,16 +249,24 @@ public function query($query, $parameters = array())
$timer = $this->initProfiler();
}

$query = $this->prepare($query, $parameters);
$result = mysqli_query($this->connection, $query);
$stmt = $this->executeQuery($query, $parameters);
$result = $stmt->get_result();

if (!empty($stmt->error)) {
throw new DbException('query() failed: ' . mysqli_error($this->connection) . ' : ' . $query);
}

$returnval = $stmt;

if (!is_bool($result)) {
mysqli_free_result($result);
$result->free();
$returnval = $result;
}

if (self::$profiling && isset($timer)) {
$this->recordQueryProfile($query, $timer);
}
return $result;
return $returnval;
} catch (Exception $e) {
throw new DbException("Error query: " . $e->getMessage() . "
In query: $query
Expand All @@ -270,6 +284,32 @@ public function lastInsertId()
return mysqli_insert_id($this->connection);
}

private function executeQuery($sql, $bind) {

$stmt = mysqli_prepare($this->connection, $sql);

if (!$stmt) {
throw new DbException('preparing query failed: ' . mysqli_error($this->connection) . ' : ' . $sql);
}

if (!is_array($bind)) {
$bind = array($bind);
}

if (!empty($bind)) {
array_unshift($bind, str_repeat('s', count($bind)));
$refs = array();
foreach ($bind as $key => $value) {
$refs[$key] = &$bind[$key];
}

call_user_func_array(array($stmt, 'bind_param'), $refs);
}

$stmt->execute();

return $stmt;
}
/**
* Input is a prepared SQL statement and parameters
* Returns the SQL statement
Expand All @@ -285,19 +325,16 @@ private function prepare($query, $parameters)
} elseif (!is_array($parameters)) {
$parameters = array($parameters);
}

$this->paramNb = 0;
$this->params = & $parameters;
$query = preg_replace_callback('/\?/', array($this, 'replaceParam'), $query);

return $query;
}

public function replaceParam($match)
{
$param = & $this->params[$this->paramNb];
$this->paramNb++;

if ($param === null) {
return 'NULL';
} else {
Expand All @@ -318,6 +355,9 @@ public function isErrNo($e, $errno)
*/
public function rowCount($queryResult)
{
if (!empty($queryResult) && is_object($queryResult) && $queryResult instanceof \mysqli_stmt) {
return $queryResult->affected_rows;
}
return mysqli_affected_rows($this->connection);
}

Expand Down
112 changes: 112 additions & 0 deletions tests/PHPUnit/Integration/Tracker/DbTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@
*/
class DbTest extends IntegrationTestCase
{
private $tableName;
public function setUp()
{
parent::setUp();
$this->tableName = Common::prefixTable('option');
}

public function test_rowCount_whenUpdating_returnsAllMatchedRowsNotOnlyUpdatedRows()
{
$db = Tracker::getDatabase();
Expand All @@ -43,4 +50,109 @@ public function test_rowCount_whenUpdating_returnsAllMatchedRowsNotOnlyUpdatedRo
$result = $db->query($sqlUpdate . " WHERE option_name = 'rowid'");
$this->assertSame(1, $db->rowCount($result));
}

public function test_rowCount_whenInserting()
{
$db = Tracker::getDatabase();
// insert one record
$result = $this->insertRowId();

$this->assertSame(1, $db->rowCount($result));
}

/**
* @expectedExceptionMessage doesn't exist
* @expectedException \Piwik\Tracker\Db\DbException
*/
public function test_fetchOne_notExistingTable()
{
$db = Tracker::getDatabase();
$this->insertRowId(3);
$val = $db->fetchOne('SELECT option_value FROM foobarbaz where option_value = "rowid"');
$this->assertEquals('3', $val);
}

/**
* @expectedExceptionMessage Duplicate entry
* @expectedException \Piwik\Tracker\Db\DbException
*/
public function test_query_error_whenInsertingDuplicateRow()
{
$this->insertRowId();
$this->insertRowId();
}

public function test_fetchOne()
{
$db = Tracker::getDatabase();
$this->insertRowId(3);
$val = $db->fetchOne('SELECT option_value FROM `' . $this->tableName . '` where option_name = "rowid"');
$this->assertEquals('3', $val);
}

public function test_fetchOne_noMatch()
{
$db = Tracker::getDatabase();
$val = $db->fetchOne('SELECT option_value from `' . $this->tableName . '` where option_name = "foobar"');
$this->assertFalse($val);
}

public function test_fetchRow()
{
$db = Tracker::getDatabase();
$this->insertRowId(3);
$val = $db->fetchRow('SELECT option_value from `' . $this->tableName . '` where option_name = "rowid"');
$this->assertEquals(array(
'option_value' => '3'
), $val);
}

public function test_fetchRow_noMatch()
{
$db = Tracker::getDatabase();
$val = $db->fetchRow('SELECT option_value from `' . $this->tableName . '` where option_name = "foobar"');
$this->assertNull($val);
}

public function test_fetch()
{
$db = Tracker::getDatabase();
$this->insertRowId(3);
$val = $db->fetch('SELECT option_value from `' . $this->tableName . '` where option_name = "rowid"');
$this->assertEquals(array(
'option_value' => '3'
), $val);
}

public function test_fetch_noMatch()
{
$db = Tracker::getDatabase();
$val = $db->fetch('SELECT option_value from `' . $this->tableName . '` where option_name = "foobar"');
$this->assertNull($val);
}

public function test_fetchAll()
{
$db = Tracker::getDatabase();
$this->insertRowId(3);
$val = $db->fetchAll('SELECT option_value from `' . $this->tableName . '` where option_name = "rowid"');
$this->assertEquals(array(
array(
'option_value' => '3'
)
), $val);
}

public function test_fetchAll_noMatch()
{
$db = Tracker::getDatabase();
$val = $db->fetchAll('SELECT option_value from `' . $this->tableName . '` where option_name = "foobar"');
$this->assertEquals(array(), $val);
}

private function insertRowId($value = '1')
{
$db = Tracker::getDatabase();
return $db->query("INSERT INTO `" . $this->tableName . "` VALUES ('rowid', '$value', false)");
}
}

0 comments on commit 6131735

Please sign in to comment.