mirror/php-e107
1
0
Fork 0
mirror of https://github.com/e107inc/e107.git synced 2025-08-04 05:37:32 +02:00
Code Issues Releases Wiki Activity

e_db_pdo::copyRow() - Greatly decrease collision chance

Browse Source 
Increased possible random strings for unique fields in e_db_pdo::copyRow() from 1000 to 59^11 (UserHandler::generateRandomString() "alphanumeric" should have 59 characters to choose from)

If a collision still happens, e_db_pdo::copyRow() retries up to 3 times for a successful copy.

Fixes: #3678
This commit is contained in:
Nick Liu
2019-12-27 21:54:11 +01:00
parent 5e9b3bee14
commit fabb0c2757
2 changed files with 82 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
e107_handlers/e_db_pdo_class.php
View File

@@ -2295,41 +2295,46 @@ class e_db_pdo implements e_db
return false; return false;
} }
if($fields === '*') for ($retries = 0; $retries < 3; $retries ++) {
{ if ($fields === '*') {
$fields = $this->db_FieldList($table); $fieldList = $this->db_FieldList($table);
$unique = $this->_getUnique($table); $unique = $this->_getUnique($table);
$flds = array(); $flds = array();
// randomize fields that must be unique. // randomize fields that must be unique.
foreach($fields as $fld) foreach ($fieldList as $fld) {
{ if (isset($unique[$fld])) {
if(isset($unique[$fld])) $flds[] = $unique[$fld] === 'PRIMARY' ? 0 :
{ "'rand-" . e107::getUserSession()->generateRandomString('***********') . "'";
$flds[] = $unique[$fld] === 'PRIMARY' ? 0 : "'rand-".rand(0,999)."'"; // keep it short.
continue; continue;
} }
$flds[] = $fld; $flds[] = $fld;
} }
$fieldList = implode(",", $fields); $fieldList = implode(",", $fieldList);
$fieldList2 = implode(",", $flds); $fieldList2 = implode(",", $flds);
} } else {
else
{
$fieldList = $fields; $fieldList = $fields;
$fieldList2 = $fieldList; $fieldList2 = $fieldList;
} }
if(empty($fieldList)) if (empty($fieldList)) {
{
$this->mysqlLastErrText = "copyRow \$fields list was empty"; $this->mysqlLastErrText = "copyRow \$fields list was empty";
return false; return false;
} }
$id = $this->gen("INSERT INTO ".$this->mySQLPrefix.$table."(".$fieldList.") SELECT ".$fieldList2." FROM ".$this->mySQLPrefix.$table." WHERE ".$args); $beforeLastInsertId = $this->lastInsertId();
$id = $this->gen(
"INSERT INTO " . $this->mySQLPrefix . $table .
"(" . $fieldList . ") SELECT " .
$fieldList2 .
" FROM " . $this->mySQLPrefix . $table .
" WHERE " . $args
);
$lastInsertId = $this->lastInsertId(); $lastInsertId = $this->lastInsertId();
if ($beforeLastInsertId !== $lastInsertId) break;
}
return ($id && $lastInsertId) ? $lastInsertId : false; return ($id && $lastInsertId) ? $lastInsertId : false;

49
e107_tests/tests/unit/e_db_pdoTest.php
View File

@@ -118,4 +118,53 @@ class e_db_pdoTest extends e_db_abstractTest
$qry = $this->db->getLastErrorText(); $qry = $this->db->getLastErrorText();
$this->assertGreaterThan(1,$result, $qry); $this->assertGreaterThan(1,$result, $qry);
} }
public function test_Db_CopyRowRNGRetry()
{
$original_user_handler = e107::getRegistry('core/e107/singleton/UserHandler');
$evil_user_handler = $this->make('UserHandler', [
'generateRandomString' => function($pattern = '', $seed = '')
{
static $index = 0;
$mock_values = ['same0000000', 'same0000000', 'different00'];
return $mock_values[$index ++];
}
]);
e107::setRegistry('core/e107/singleton/UserHandler', $evil_user_handler);
// test with table that has unique keys.
$result = $this->db->db_CopyRow('core_media_cat', '*', "media_cat_id = 1");
$qry = $this->db->getLastErrorText();
$this->assertGreaterThan(1,$result, $qry);
// test with table that has unique keys. (same row again) - make sure copyRow duplicates it regardless.
$result = $this->db->db_CopyRow('core_media_cat', '*', "media_cat_id = 1");
$qry = $this->db->getLastErrorText();
$this->assertGreaterThan(1,$result, $qry);
e107::setRegistry('core/e107/singleton/UserHandler', $original_user_handler);
}
public function test_Db_CopyRowRNGGiveUp()
{
$original_user_handler = e107::getRegistry('core/e107/singleton/UserHandler');
$evil_user_handler = $this->make('UserHandler', [
'generateRandomString' => function($pattern = '', $seed = '')
{
return 'neverchange';
}
]);
e107::setRegistry('core/e107/singleton/UserHandler', $evil_user_handler);
// test with table that has unique keys.
$result = $this->db->db_CopyRow('core_media_cat', '*', "media_cat_id = 1");
$result = $this->db->db_CopyRow('core_media_cat', '*', "media_cat_id = 1");
$qry = $this->db->getLastErrorText();
$this->assertFalse($result,
"Intentionally broken random number generator should have prevented row copy with unique keys"
);
e107::setRegistry('core/e107/singleton/UserHandler', $original_user_handler);
}
} }