From ea40c9306c8ad7f9e2f34f4f4d819dac694864a2 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 15 Feb 2018 17:51:42 +1300 Subject: [PATCH] [NFC] pass arrays rather than strings to construct urls in dedupe code When we pass a query it is urlencoded and any quotes in the string are not subsequently htmlentity encoded plus I think the url construction is generally cleaner https://github.com/civicrm/civicrm-core/pull/11658 Change-Id: I2300bac6aff180318f01784ce5b12ae1627a2e37 --- CRM/Contact/Form/Merge.php | 47 ++++++++++++++++++++------------- CRM/Contact/Page/AJAX.php | 11 ++++++-- CRM/Contact/Page/DedupeFind.php | 2 +- 3 files changed, 38 insertions(+), 22 deletions(-) diff --git a/CRM/Contact/Form/Merge.php b/CRM/Contact/Form/Merge.php index 9a567873eb07..8894c58df6a0 100644 --- a/CRM/Contact/Form/Merge.php +++ b/CRM/Contact/Form/Merge.php @@ -74,7 +74,8 @@ public function preProcess() { $this->_gid = $gid = CRM_Utils_Request::retrieve('gid', 'Positive', $this, FALSE); $this->_mergeId = CRM_Utils_Request::retrieve('mergeId', 'Positive', $this, FALSE); $this->limit = CRM_Utils_Request::retrieve('limit', 'Positive', $this, FALSE); - $urlParams = "reset=1&rgid={$this->_rgid}&gid={$this->_gid}&limit=" . $this->limit; + + $urlParams = ['reset' => 1, 'rgid' => $this->_rgid, 'gid' => $this->_gid, 'limit' => $this->limit]; $this->bounceIfInvalid($this->_cid, $this->_oid); @@ -83,7 +84,7 @@ public function preProcess() { 'return' => 'contact_type', )); - $browseUrl = CRM_Utils_System::url('civicrm/contact/dedupefind', $urlParams . '&action=browse'); + $browseUrl = CRM_Utils_System::url('civicrm/contact/dedupefind', array_merge($urlParams, ['action' => 'browse'])); if (!$this->_rgid) { // Unset browse URL as we have come from the search screen. @@ -124,13 +125,13 @@ public function preProcess() { $this->assign('mainUfId', $mainUfId); $this->assign('mainUfName', $mainUser ? $mainUser->name : NULL); } - - $flipUrl = CRM_Utils_System::url('civicrm/contact/merge', - "reset=1&action=update&cid={$this->_oid}&oid={$this->_cid}&rgid={$this->_rgid}&gid={$gid}" - ); + $flipParams = array_merge($urlParams, ['action' => 'update', 'cid' => $this->_oid, 'oid' => $this->_cid]); if (!$flip) { - $flipUrl .= '&flip=1'; + $flipParams['flip'] = '1'; } + $flipUrl = CRM_Utils_System::url('civicrm/contact/merge', + $flipParams + ); $this->assign('flip', $flipUrl); $this->prev = $this->next = NULL; @@ -140,8 +141,13 @@ public function preProcess() { ) as $position) { if (!empty($pos[$position])) { if ($pos[$position]['id1'] && $pos[$position]['id2']) { - $urlParams .= "&cid={$pos[$position]['id1']}&oid={$pos[$position]['id2']}&mergeId={$pos[$position]['mergeId']}&action=update"; - $this->$position = CRM_Utils_System::url('civicrm/contact/merge', $urlParams); + $rowParams = array_merge($urlParams, [ + 'action' => 'update', + 'cid' => $pos[$position]['id1'], + 'oid' => $pos[$position]['id2'], + 'mergeId' => $pos[$position]['mergeId'], + ]); + $this->$position = CRM_Utils_System::url('civicrm/contact/merge', $rowParams); $this->assign($position, $this->$position); } } @@ -294,18 +300,17 @@ public function postProcess() { $message = ''; CRM_Core_Session::setStatus($message, ts('Contacts Merged'), 'success'); - $url = CRM_Utils_System::url('civicrm/contact/view', "reset=1&cid={$this->_cid}"); - $urlParams = "reset=1&gid={$this->_gid}&rgid={$this->_rgid}&limit={$this->limit}"; + $urlParams = ['reset' => 1, 'cid' => $this->_cid, 'rgid' => $this->_rgid, 'gid' => $this->_gid, 'limit' => $this->limit]; + $contactViewUrl = CRM_Utils_System::url('civicrm/contact/view', ['reset' => 1, 'cid' => $this->_cid]); if (!empty($formValues['_qf_Merge_submit'])) { - $urlParams .= "&action=update"; - $lisitingURL = CRM_Utils_System::url('civicrm/contact/dedupefind', + $urlParams['action'] = "update"; + CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contact/dedupefind', $urlParams - ); - CRM_Utils_System::redirect($lisitingURL); + )); } if (!empty($formValues['_qf_Merge_done'])) { - CRM_Utils_System::redirect($url); + CRM_Utils_System::redirect($contactViewUrl); } if ($this->next && $this->_mergeId) { @@ -321,12 +326,16 @@ public function postProcess() { $pos['next']['id2'] ) { - $urlParams .= "&cid={$pos['next']['id1']}&oid={$pos['next']['id2']}&mergeId={$pos['next']['mergeId']}&action=update"; - $url = CRM_Utils_System::url('civicrm/contact/merge', $urlParams); + $urlParams['cid'] = $pos['next']['id1']; + $urlParams['oid'] = $pos['next']['id2']; + $urlParams['mergeId'] = $pos['next']['mergeId']; + $urlParams['action'] = 'update'; + CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contact/merge', $urlParams)); } } - CRM_Utils_System::redirect($url); + // Perhaps never reached. + CRM_Utils_System::redirect($contactViewUrl); } /** diff --git a/CRM/Contact/Page/AJAX.php b/CRM/Contact/Page/AJAX.php index 6f9123fac68d..09f396b4da2a 100644 --- a/CRM/Contact/Page/AJAX.php +++ b/CRM/Contact/Page/AJAX.php @@ -808,9 +808,16 @@ public static function getDedupes() { $searchRows[$count]['weight'] = CRM_Utils_Array::value('weight', $pair); if (!empty($pairInfo['data']['canMerge'])) { - $mergeParams = "reset=1&cid={$pairInfo['entity_id1']}&oid={$pairInfo['entity_id2']}&action=update&rgid={$rgid}&limit=" . CRM_Utils_Request::retrieve('limit', 'Integer'); + $mergeParams = [ + 'reset' => 1, + 'cid' => $pairInfo['entity_id1'], + 'oid' => $pairInfo['entity_id2'], + 'action' => 'update', + 'rgid' => $rgid, + 'limit' => CRM_Utils_Request::retrieve('limit', 'Integer'), + ]; if ($gid) { - $mergeParams .= "&gid={$gid}"; + $mergeParams['gid'] = $gid; } $searchRows[$count]['actions'] = "" . ts('flip') . " | "; diff --git a/CRM/Contact/Page/DedupeFind.php b/CRM/Contact/Page/DedupeFind.php index 99a91201debd..285c5bc820d1 100644 --- a/CRM/Contact/Page/DedupeFind.php +++ b/CRM/Contact/Page/DedupeFind.php @@ -142,7 +142,7 @@ public function run() { $urlQry['selected'] = 1; } - $this->assign('sourceUrl', CRM_Utils_System::url('civicrm/ajax/dedupefind', $urlQry, FALSE, NULL, FALSE)); + $this->assign('sourceUrl', CRM_Utils_System::url('civicrm/ajax/dedupefind', $urlQry)); //reload from cache table $cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, $criteria);