From 2e09a60f5690e63d106a5efedfdcf888240db3e2 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Wed, 11 Mar 2020 13:09:42 +1300
Subject: [PATCH] Fix bug where a % in a serialized array can lead to the data
 being broken

It turns out that is a field in a serialized array has a %2 (for example) this gets swapped in executeQuery for the
%2 value (in this case srcID - rendering the serialized array invalid. This proposes that we
explicitly handle arrays as  a data type in compose query.

Some thoughts
1) we could make serialized arrays valid types in validate (not done here)
2) we could iterate through the  array keys  & values escaping them -
at this stage it's left in the calling function
3) there are whole bikeshed factories to keep in business on discussion of whether
'Array-1', 'Array-2' etc are the right format
---
 CRM/Dedupe/Finder.php            |  4 ++--
 CRM/Dedupe/Merger.php            |  3 +--
 tests/phpunit/api/v3/JobTest.php | 16 ++++++++++++++++
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/CRM/Dedupe/Finder.php b/CRM/Dedupe/Finder.php
index dac4bf1359..06863a2afa 100644
--- a/CRM/Dedupe/Finder.php
+++ b/CRM/Dedupe/Finder.php
@@ -348,12 +348,12 @@ class CRM_Dedupe_Finder {
         'canMerge' => TRUE,
       ];
 
-      $data = CRM_Core_DAO::escapeString(serialize($row));
       CRM_Core_DAO::executeQuery("INSERT INTO civicrm_prevnext_cache (entity_table, entity_id1, entity_id2, cacheKey, data) VALUES
-        ('civicrm_contact', %1, %2, %3, '{$data}')", [
+        ('civicrm_contact', %1, %2, %3, %4)", [
           1 => [$dstID, 'Integer'],
           2 => [$srcID, 'Integer'],
           3 => [$cacheKeyString, 'String'],
+          4 => [serialize($row), 'String'],
         ]
       );
     }
diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php
index 00f2901fa5..6c1d7d6bfd 100644
--- a/CRM/Dedupe/Merger.php
+++ b/CRM/Dedupe/Merger.php
@@ -775,10 +775,9 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
       'merged' => (int) $merged,
       'skipped' => (int) $skipped,
     ];
-    $data = serialize($data);
 
     CRM_Core_DAO::executeQuery("INSERT INTO civicrm_prevnext_cache (entity_table, entity_id1, entity_id2, cacheKey, data) VALUES
-        ('civicrm_contact', 0, 0, %1, '{$data}')", [1 => [$cacheKeyString . '_stats', 'String']]);
+        ('civicrm_contact', 0, 0, %1, %2)", [1 => [$cacheKeyString . '_stats', 'String'], 2 => [serialize($data), 'String']]);
   }
 
   /**
diff --git a/tests/phpunit/api/v3/JobTest.php b/tests/phpunit/api/v3/JobTest.php
index 57b88cfa4a..869bf470e2 100644
--- a/tests/phpunit/api/v3/JobTest.php
+++ b/tests/phpunit/api/v3/JobTest.php
@@ -828,6 +828,22 @@ class api_v3_JobTest extends CiviUnitTestCase {
 
   }
 
+  /**
+   * 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', []);
+
+    $this->individualCreate();
+    $this->individualCreate(['first_name' => '[foo\\bar\'baz']);
+    $this->callAPISuccess('Job', 'process_batch_merge', []);
+    $this->callAPISuccessGetSingle('Contact', ['first_name' => '[foo\\bar\'baz']);
+  }
+
   /**
    * Test the batch merge does not create duplicate emails.
    *
-- 
2.25.1