Skip to content

Commit

Permalink
argh what's the answer - Possible regression from serialize tightening
Browse files Browse the repository at this point in the history
I can't quite figure this out but I thought I'd push up  what I've got to see if anyone has an idea.

Basically we have a contact in our DB who had a weird string (carriage returns & a .) in his name.
The dedupe script fell over on him because it could not unserialize what it had serialised for
his display name. On testing I can replicate but it seems that what it saves from the DB
is not what it retrieves so I'm not there on a fix
  • Loading branch information
eileenmcnaughton committed Mar 5, 2020
1 parent 40cbd64 commit 5f35fd0
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
6 changes: 3 additions & 3 deletions CRM/Dedupe/Finder.php
Original file line number Diff line number Diff line change
Expand Up @@ -341,14 +341,14 @@ public static function parseAndStoreDupePairs($foundDupes, $cacheKeyString) {

$mainContacts[] = $row = [
'dstID' => $dstID,
'dstName' => $displayNames[$dstID],
'dstName' => CRM_Core_DAO::escapeString($displayNames[$dstID]),
'srcID' => $srcID,
'srcName' => $displayNames[$srcID],
'srcName' => CRM_Core_DAO::escapeString($displayNames[$srcID]),
'weight' => $dupes[2],
'canMerge' => TRUE,
];

$data = CRM_Core_DAO::escapeString(serialize($row));
$data = serialize($row);
CRM_Core_BAO_PrevNextCache::setItem('civicrm_contact', $dstID, $srcID, $cacheKeyString, $data);
}
return $mainContacts;
Expand Down
16 changes: 16 additions & 0 deletions tests/phpunit/api/v3/JobTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,22 @@ public function getMergeLocationData() {

}

/**
* Test weird characters don't mess with merge & cause a fatal.
*
* @throws \CRM_Core_Exception
*/
public function testNoErrorOnOdd() {
$this->individualCreate();
$this->individualCreate(['first_name' => 'Gerrit%0a%2e%0a']);
$this->callAPISuccess('Job', 'process_batch_merge', []);
// As serialized
//a:6:{s:5:"dstID";s:1:"4";s:7:"dstName";s:23:"Mr. Anthony Anderson II";s:5:"srcID";s:1:"5";s:7:"srcName";s:31:"Mr. Gerrit%0a4e%0a Anderson II";s:6:"weight";s:2:"10";s:8:"canMerge";b:1;}
// As stored to db
//a:6:{s:5:"dstID";s:1:"4";s:7:"dstName";s:23:"Mr. Anthony Anderson II";s:5:"srcID";s:1:"5";s:7:"srcName";s:31:"Mr. Gerrit%0a4e%0a Anderson II";s:6:"weight";s:2:"10";s:8:"canMerge";b:1;}
// as retrieved
//a:6:{s:5:"dstID";s:1:"4";s:7:"dstName";s:23:"Mr. Anthony Anderson II";s:5:"srcID";s:1:"5";s:7:"srcName";s:31:"Mr. Gerrit%0a%2e%0a Anderson II";s:6:"weight";s:2:"10";s:8:"canMerge";b:1;}
}
/**
* Test the batch merge does not create duplicate emails.
*
Expand Down

0 comments on commit 5f35fd0

Please sign in to comment.