From 9df3628eea77c5e4486cd959da60e4ffe9a82314 Mon Sep 17 00:00:00 2001 From: Andrei Mondoc Date: Thu, 10 Oct 2019 10:52:36 +0100 Subject: [PATCH 1/3] add externUrl method and civicrm_alterExternUrl hook --- .../Form/ContributionPage/Widget.php | 2 + CRM/Core/Payment/PaymentExpress.php | 2 +- CRM/Cxn/BAO/Cxn.php | 17 +---- CRM/Mailing/BAO/Mailing.php | 4 +- CRM/Mailing/BAO/TrackableURL.php | 2 +- CRM/Utils/System.php | 35 ++++++++++ templates/CRM/Contribute/Page/Widget.tpl | 2 +- tests/phpunit/CRM/Utils/SystemTest.php | 70 +++++++++++++++++++ 8 files changed, 113 insertions(+), 21 deletions(-) diff --git a/CRM/Contribute/Form/ContributionPage/Widget.php b/CRM/Contribute/Form/ContributionPage/Widget.php index 2a88bc06b5df..9d08453b0c00 100644 --- a/CRM/Contribute/Form/ContributionPage/Widget.php +++ b/CRM/Contribute/Form/ContributionPage/Widget.php @@ -55,6 +55,8 @@ public function preProcess() { $this->assign('cpageId', $this->_id); + $this->assign('widgetExternUrl', CRM_Utils_System::externUrl('extern/widget', "cpageId={$this->_id}&widgetId={$this->_widget->id}&format=3")); + $config = CRM_Core_Config::singleton(); $title = CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_ContributionPage', $this->_id, diff --git a/CRM/Core/Payment/PaymentExpress.php b/CRM/Core/Payment/PaymentExpress.php index 28552293feed..47db6a690ca1 100644 --- a/CRM/Core/Payment/PaymentExpress.php +++ b/CRM/Core/Payment/PaymentExpress.php @@ -121,7 +121,7 @@ public function doTransferCheckout(&$params, $component) { CRM_Core_Error::fatal(ts('Component is invalid')); } - $url = $config->userFrameworkResourceURL . "extern/pxIPN.php"; + $url = CRM_Utils_System::externUrl('extern/pxIPN'); if ($component == 'event') { $cancelURL = CRM_Utils_System::url('civicrm/event/register', diff --git a/CRM/Cxn/BAO/Cxn.php b/CRM/Cxn/BAO/Cxn.php index 658bd61fc6e8..e304c99b3278 100644 --- a/CRM/Cxn/BAO/Cxn.php +++ b/CRM/Cxn/BAO/Cxn.php @@ -45,22 +45,7 @@ class CRM_Cxn_BAO_Cxn extends CRM_Cxn_DAO_Cxn { * @return string */ public static function getSiteCallbackUrl() { - $config = CRM_Core_Config::singleton(); - - if (preg_match('/^(http|https):/', $config->resourceBase)) { - $civiUrl = $config->resourceBase; - } - else { - $civiUrl = rtrim(CRM_Utils_System::baseURL(), '/') . '/' . ltrim($config->resourceBase, '/'); - } - - // In practice, this may not be necessary, but we want to prevent - // edge-cases that downgrade security-level below system policy. - if (Civi::settings()->get('enableSSL')) { - $civiUrl = preg_replace('/^http:/', 'https:', $civiUrl); - } - - return rtrim($civiUrl, '/') . '/extern/cxn.php'; + return CRM_Utils_System::externUrl('extern/cxn', NULL, NULL, TRUE, TRUE); } /** diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php index 9a24f8a9a821..28164b65abe3 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -1163,8 +1163,8 @@ public function compose( // push the tracking url on to the html email if necessary if ($this->open_tracking && $html) { - array_push($html, "\n" . '" + array_push($html, "\n" . '' ); } diff --git a/CRM/Mailing/BAO/TrackableURL.php b/CRM/Mailing/BAO/TrackableURL.php index da57a134c3eb..87d02927797c 100644 --- a/CRM/Mailing/BAO/TrackableURL.php +++ b/CRM/Mailing/BAO/TrackableURL.php @@ -93,7 +93,7 @@ public static function getTrackerURL($url, $mailing_id, $queue_id) { $urlCache[$mailing_id . $url] = $redirect; } - $returnUrl = "{$urlCache[$mailing_id . $url]}&qid={$queue_id}"; + $returnUrl = CRM_Utils_System::externUrl('extern/url', "u=$id&qid=$queue_id"); if ($hrefExists) { $returnUrl = "href='{$returnUrl}' rel='nofollow'"; diff --git a/CRM/Utils/System.php b/CRM/Utils/System.php index b52932368805..cdb6ffbd3f8a 100644 --- a/CRM/Utils/System.php +++ b/CRM/Utils/System.php @@ -300,6 +300,41 @@ public static function url( return $url; } + /** + * Generates an extern url. + * + * @param string $path + * The extern path, such as "extern/url". + * @param string $query + * A query string to append to the link. + * @param string $fragment + * A fragment identifier (named anchor) to append to the link. + * @param bool $absolute + * Whether to force the output to be an absolute link (beginning with a + * URI-scheme such as 'http:'). + * @param bool $isSSL + * NULL to autodetect. TRUE to force to SSL. + */ + public static function externUrl($path = NULL, $query = NULL, $fragment = NULL, $absolute = TRUE, $isSSL = NULL) { + $query = self::makeQueryString($query); + + $url = Civi::paths()->getUrl("[civicrm.root]/{$path}.php", $absolute ? 'absolute' : 'relative', $isSSL) + . ($query ? "?$query" : "") + . ($fragment ? "#$fragment" : ""); + + $parsedUrl = CRM_Utils_Url::parseUrl($url); + $event = \Civi\Core\Event\GenericHookEvent::create([ + 'url' => &$parsedUrl, + 'path' => $path, + 'query' => $query, + 'fragment' => $fragment, + 'absolute' => $absolute, + 'isSSL' => $isSSL, + ]); + Civi::service('dispatcher')->dispatch('hook_civicrm_alterExternUrl', $event); + return CRM_Utils_Url::unparseUrl($event->url); + } + /** * Path of the current page e.g. 'civicrm/contact/view' * diff --git a/templates/CRM/Contribute/Page/Widget.tpl b/templates/CRM/Contribute/Page/Widget.tpl index 1074e2e1a71f..125d07d34029 100644 --- a/templates/CRM/Contribute/Page/Widget.tpl +++ b/templates/CRM/Contribute/Page/Widget.tpl @@ -211,4 +211,4 @@ function onReady( ) { } {/literal} - + diff --git a/tests/phpunit/CRM/Utils/SystemTest.php b/tests/phpunit/CRM/Utils/SystemTest.php index ea129b0fbae3..74222baa7d43 100644 --- a/tests/phpunit/CRM/Utils/SystemTest.php +++ b/tests/phpunit/CRM/Utils/SystemTest.php @@ -114,6 +114,76 @@ public function getURLs() { ]; } + /** + * Test extern url. + */ + public function testExternUrl() { + $siteKey = mt_rand(); + $apiKey = mt_rand(); + $restUrl = CRM_Utils_System::externUrl('extern/rest', "entity=Contact&action=get&key=$siteKey&api_key=$apiKey"); + $this->assertContains('extern/rest.php', $restUrl); + $this->assertContains('?', $restUrl); + $this->assertContains('entity=Contact', $restUrl); + $this->assertContains('action=get', $restUrl); + $this->assertContains("key=$siteKey", $restUrl); + $this->assertContains("api_key=$apiKey", $restUrl); + } + + /** + * Test the alterExternUrl hook. + * + * @param string $path + * @param array $expected + * + * @dataProvider getExternURLs + */ + public function testAlterExternUrlHook($path, $expected) { + Civi::service('dispatcher')->addListener('hook_civicrm_alterExternUrl', [$this, 'hook_civicrm_alterExternUrl']); + $externUrl = CRM_Utils_System::externUrl($path, $expected['query']); + $this->assertContains($path, $externUrl); + $this->assertContains($expected['query'], $externUrl); + } + + /** + * Hook for alterExternUrl. + * + * @param \Civi\Core\Event\GenericHookEvent $event + * @param string $hookName + */ + public function hook_civicrm_alterExternUrl(\Civi\Core\Event\GenericHookEvent $event, $hookName) { + $this->assertEquals('hook_civicrm_alterExternUrl', $hookName); + $this->assertTrue($event->hasField('url')); + $this->assertTrue($event->hasField('path')); + $this->assertTrue($event->hasField('query')); + $this->assertTrue($event->hasField('fragment')); + $this->assertTrue($event->hasField('absolute')); + $this->assertTrue($event->hasField('isSSL')); + } + + /** + * Get extern url params for testing. + * + * @return array + */ + public function getExternURLs() { + return [ + [ + 'extern/url', + [ + 'path' => 'extern/url', + 'query' => 'u=1&qid=1', + ], + ], + [ + 'extern/open', + [ + 'path' => 'extern/open', + 'query' => 'q=1', + ], + ], + ]; + } + /** * Demonstrate the, um, "flexibility" of isNull */ From f8274a1c2f153b4d56e2870439cb8b7073933d11 Mon Sep 17 00:00:00 2001 From: Andrei Mondoc Date: Fri, 11 Oct 2019 12:09:19 +0200 Subject: [PATCH 2/3] cache the hooked url value --- CRM/Mailing/BAO/TrackableURL.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CRM/Mailing/BAO/TrackableURL.php b/CRM/Mailing/BAO/TrackableURL.php index 87d02927797c..759c1053a8ca 100644 --- a/CRM/Mailing/BAO/TrackableURL.php +++ b/CRM/Mailing/BAO/TrackableURL.php @@ -73,7 +73,6 @@ public static function getTrackerURL($url, $mailing_id, $queue_id) { else { $hrefExists = FALSE; - $config = CRM_Core_Config::singleton(); $tracker = new CRM_Mailing_BAO_TrackableURL(); if (preg_match('/^href/i', $url)) { @@ -89,7 +88,7 @@ public static function getTrackerURL($url, $mailing_id, $queue_id) { } $id = $tracker->id; - $redirect = $config->userFrameworkResourceURL . "extern/url.php?u=$id"; + $redirect = CRM_Utils_System::externUrl('extern/url', "u=$id"); $urlCache[$mailing_id . $url] = $redirect; } From 095e7f1c3d4121346c9a278552004ff59e616fc8 Mon Sep 17 00:00:00 2001 From: Andrei Mondoc Date: Fri, 11 Oct 2019 13:34:14 +0200 Subject: [PATCH 3/3] alter url path and query and assert it --- tests/phpunit/CRM/Utils/SystemTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/CRM/Utils/SystemTest.php b/tests/phpunit/CRM/Utils/SystemTest.php index 74222baa7d43..1e29254278b9 100644 --- a/tests/phpunit/CRM/Utils/SystemTest.php +++ b/tests/phpunit/CRM/Utils/SystemTest.php @@ -140,8 +140,8 @@ public function testExternUrl() { public function testAlterExternUrlHook($path, $expected) { Civi::service('dispatcher')->addListener('hook_civicrm_alterExternUrl', [$this, 'hook_civicrm_alterExternUrl']); $externUrl = CRM_Utils_System::externUrl($path, $expected['query']); - $this->assertContains($path, $externUrl); - $this->assertContains($expected['query'], $externUrl); + $this->assertContains('path/altered/by/hook', $externUrl, 'Hook failed to alter URL path'); + $this->assertContains($expected['query'] . '&thisWas=alteredByHook', $externUrl, 'Hook failed to alter URL query'); } /** @@ -158,6 +158,8 @@ public function hook_civicrm_alterExternUrl(\Civi\Core\Event\GenericHookEvent $e $this->assertTrue($event->hasField('fragment')); $this->assertTrue($event->hasField('absolute')); $this->assertTrue($event->hasField('isSSL')); + $event->url = $event->url->withPath('path/altered/by/hook'); + $event->url = $event->url->withQuery($event->query . '&thisWas=alteredByHook'); } /**