diff --git a/CRM/Admin/Form/Setting/Url.php b/CRM/Admin/Form/Setting/Url.php index 09b1dd1fc45f..fa854247eaf4 100644 --- a/CRM/Admin/Form/Setting/Url.php +++ b/CRM/Admin/Form/Setting/Url.php @@ -21,6 +21,7 @@ class CRM_Admin_Form_Setting_Url extends CRM_Admin_Form_Setting { protected $_settings = [ 'disable_core_css' => CRM_Core_BAO_Setting::SYSTEM_PREFERENCES_NAME, + 'defaultExternUrl' => CRM_Core_BAO_Setting::SYSTEM_PREFERENCES_NAME, 'userFrameworkResourceURL' => CRM_Core_BAO_Setting::URL_PREFERENCES_NAME, 'imageUploadURL' => CRM_Core_BAO_Setting::URL_PREFERENCES_NAME, 'customCSSURL' => CRM_Core_BAO_Setting::URL_PREFERENCES_NAME, diff --git a/CRM/Mailing/BAO/TrackableURL.php b/CRM/Mailing/BAO/TrackableURL.php index 86650e3afe3d..0113e53bb9b7 100644 --- a/CRM/Mailing/BAO/TrackableURL.php +++ b/CRM/Mailing/BAO/TrackableURL.php @@ -76,7 +76,9 @@ public static function getTrackerURL($url, $mailing_id, $queue_id) { $urlCache[$mailing_id . $url] = $redirect; } - $returnUrl = CRM_Utils_System::externUrl('extern/url', "u=$id&qid=$queue_id"); + // This looks silly - calling the hook twice. This smells like an accident. Restoring old cache-based lookup. + // $returnUrl = CRM_Utils_System::externUrl('extern/url', "u=$id&qid=$queue_id"); + $returnUrl = "{$urlCache[$mailing_id . $url]}&qid={$queue_id}"; if ($hrefExists) { $returnUrl = "href='{$returnUrl}' rel='nofollow'"; diff --git a/CRM/Mailing/Page/Open.php b/CRM/Mailing/Page/Open.php new file mode 100644 index 000000000000..ac26fbfff8f6 --- /dev/null +++ b/CRM/Mailing/Page/Open.php @@ -0,0 +1,58 @@ +getPath('[civicrm.root]/i/tracker.gif'); + + header('Cache-Control: must-revalidate, post-check=0, pre-check=0'); + header('Content-Description: File Transfer'); + header('Content-type: image/gif'); + header('Content-Length: ' . filesize($filename)); + header('Content-Disposition: inline; filename=tracker.gif'); + + readfile($filename); + + CRM_Utils_System::civiExit(); + } + +} diff --git a/CRM/Mailing/Page/Url.php b/CRM/Mailing/Page/Url.php new file mode 100644 index 000000000000..29be68c66110 --- /dev/null +++ b/CRM/Mailing/Page/Url.php @@ -0,0 +1,104 @@ +extractPassthroughParameters(); + + if (strlen($query_string) > 0) { + // Parse the url to preserve the fragment. + $pieces = parse_url($url); + + if (isset($pieces['fragment'])) { + $url = str_replace('#' . $pieces['fragment'], '', $url); + } + + // Handle additional query string params. + if ($query_string) { + if (stristr($url, '?')) { + $url .= '&' . $query_string; + } + else { + $url .= '?' . $query_string; + } + } + + // slap the fragment onto the end per URL spec + if (isset($pieces['fragment'])) { + $url .= '#' . $pieces['fragment']; + } + } + CRM_Utils_System::redirect($url, [ + 'for' => 'civicrm/mailing/url', + 'queue_id' => $queue_id, + 'url_id' => $url_id, + ]); + } + + /** + * Determine if this request has any valid pass-through parameters. + * + * Under CRM-7103 (v3.3), all unrecognized query-parameters (besides qid/u) are passed + * through as part of the redirect. This mechanism is relevant to certain + * customizations (eg using `hook_alterMailParams` to append extra URL args) + * but does not matter for normal URLs. + * + * The functionality seems vaguely problematic (IMHO) - especially now that + * 'extern/url.php' is moving into the CMS/Civi router ('civicrm/mailing/url'). + * But it's the current protocol. + * + * A better design might be to support `hook_alterRedirect` in the CiviMail + * click-through tracking. Then you don't have to take any untrusted inputs + * and you can fix URL mistakes in realtime. + * + * @return string + * @link https://issues.civicrm.org/jira/browse/CRM-7103 + */ + protected function extractPassthroughParameters():string { + $config = CRM_Core_Config::singleton(); + + $query_param = $_GET; + unset($query_param['qid']); + unset($query_param['u']); + unset($query_param[$config->userFrameworkURLVar]); + if ($config->userFramework === 'WordPress') { + // Ugh + unset($query_param['page']); + unset($query_param['noheader']); + } + + $query_string = http_build_query($query_param); + return $query_string; + } + +} diff --git a/CRM/Mailing/xml/Menu/Mailing.xml b/CRM/Mailing/xml/Menu/Mailing.xml index 2df7db36229d..d4cc7d274ae7 100644 --- a/CRM/Mailing/xml/Menu/Mailing.xml +++ b/CRM/Mailing/xml/Menu/Mailing.xml @@ -202,4 +202,14 @@ CRM_Mailing_Page_AJAX::getContactMailings access CiviCRM + + civicrm/mailing/url + CRM_Mailing_Page_Url + *always allow* + + + civicrm/mailing/open + CRM_Mailing_Page_Open + *always allow* + diff --git a/CRM/Utils/System.php b/CRM/Utils/System.php index 8713cf27fad5..11e9eb1710a3 100644 --- a/CRM/Utils/System.php +++ b/CRM/Utils/System.php @@ -318,6 +318,41 @@ public static function externUrl($path = NULL, $query = NULL, $fragment = NULL, return urldecode(CRM_Utils_Url::unparseUrl($event->url)); } + /** + * Perform any current conversions/migrations on the extern URL. + * + * @param \Civi\Core\Event\GenericHookEvent $e + * @see CRM_Utils_Hook::alterExternUrl + */ + public static function migrateExternUrl(\Civi\Core\Event\GenericHookEvent $e) { + + /** + * $mkRouteUri is a small adapter to return generated URL as a "UriInterface". + * @param string $path + * @param string $query + * @return \Psr\Http\Message\UriInterface + */ + $mkRouteUri = function ($path, $query) use ($e) { + $urlTxt = CRM_Utils_System::url($path, $query, $e->absolute, $e->fragment, FALSE); + if ($e->isSSL || ($e->isSSL === NULL && \CRM_Utils_System::isSSL())) { + $urlTxt = str_replace('http://', 'https://', $urlTxt); + } + return CRM_Utils_Url::parseUrl($urlTxt); + }; + + switch (Civi::settings()->get('defaultExternUrl') . ':' . $e->path) { + case 'router:extern/open': + $e->url = $mkRouteUri('civicrm/mailing/open', preg_replace('/(^|&)q=/', '\1qid=', $e->query)); + break; + + case 'router:extern/url': + $e->url = $mkRouteUri('civicrm/mailing/url', $e->query); + break; + + // Otherwise, keep the default. + } + } + /** * @deprecated * @see \CRM_Utils_System::currentPath diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php index 57ee30916408..5e064cae4450 100644 --- a/Civi/Core/Container.php +++ b/Civi/Core/Container.php @@ -354,6 +354,7 @@ public function createEventDispatcher($container) { $dispatcher->addListener('hook_civicrm_buildAsset', ['\CRM_Core_Resources', 'renderMenubarStylesheet']); $dispatcher->addListener('hook_civicrm_coreResourceList', ['\CRM_Utils_System', 'appendCoreResources']); $dispatcher->addListener('hook_civicrm_getAssetUrl', ['\CRM_Utils_System', 'alterAssetUrl']); + $dispatcher->addListener('hook_civicrm_alterExternUrl', ['\CRM_Utils_System', 'migrateExternUrl'], 1000); $dispatcher->addListener('civi.dao.postInsert', ['\CRM_Core_BAO_RecurringEntity', 'triggerInsert']); $dispatcher->addListener('civi.dao.postUpdate', ['\CRM_Core_BAO_RecurringEntity', 'triggerUpdate']); $dispatcher->addListener('civi.dao.postDelete', ['\CRM_Core_BAO_RecurringEntity', 'triggerDelete']); diff --git a/settings/Core.setting.php b/settings/Core.setting.php index 349f266bde10..114158509dd8 100644 --- a/settings/Core.setting.php +++ b/settings/Core.setting.php @@ -239,6 +239,28 @@ 'description' => NULL, 'help_text' => NULL, ], + 'defaultExternUrl' => [ + 'group_name' => 'CiviCRM Preferences', + 'group' => 'core', + 'name' => 'defaultExternUrl', + 'type' => 'String', + 'quick_form_type' => 'Select', + 'html_type' => 'Select', + 'html_attributes' => [ + 'class' => 'crm-select2', + ], + 'default' => 'router', + 'add' => '5.27', + 'title' => ts('Extern URL Style'), + 'is_domain' => 1, + 'is_contact' => 0, + 'description' => ts('This setting provides transitional support. It should be set to "Prefer normal router." If your deployment requires "Prefer standalone script", then please ensure that the issue is tracked in lab.civicrm.org.'), + 'help_text' => NULL, + 'options' => [ + 'standalone' => ts('Prefer standalone scripts'), + 'router' => ts('Prefer normal router'), + ], + ], 'activity_assignee_notification' => [ 'group_name' => 'CiviCRM Preferences', 'group' => 'core', diff --git a/templates/CRM/Admin/Form/Setting/Url.hlp b/templates/CRM/Admin/Form/Setting/Url.hlp index a2d1b1a86b24..e01986593a77 100644 --- a/templates/CRM/Admin/Form/Setting/Url.hlp +++ b/templates/CRM/Admin/Form/Setting/Url.hlp @@ -70,6 +70,16 @@

{ts}You can modify the look and feel of CiviCRM by adding your own stylesheet. For small to medium sized modifications, use your css file to override some of the styles in civicrm.css. Or if you need to make drastic changes, you can choose to disable civicrm.css completely.{/ts}

{/htxt} +{htxt id='id-defaultExternUrl'} +

{ts}CiviCRM generates callback URLs for external services.{/ts}

+

{ts}Some callback URLs are being migrated to a different style. During the transition, you may indicate a preferred style, such as:{/ts}

+ +

{ts}This setting only affects the default URL produced by "civicrm-core". Extensions and add-ons may override specific URLs.{/ts}

+{/htxt} + {htxt id='id-url_vars'} {ts}URL Variables{/ts} diff --git a/templates/CRM/Admin/Form/Setting/Url.tpl b/templates/CRM/Admin/Form/Setting/Url.tpl index 3ecb7699b760..c6df140ff65e 100644 --- a/templates/CRM/Admin/Form/Setting/Url.tpl +++ b/templates/CRM/Admin/Form/Setting/Url.tpl @@ -77,6 +77,15 @@

{ts}{$verifySSL_description}{/ts}

+ + + +
+ {$form.defaultExternUrl.label} {help id='id-defaultExternUrl'} + + {$form.defaultExternUrl.html}
+

{ts}{$settings_fields.defaultExternUrl.description}{/ts}

+
{include file="CRM/common/formButtons.tpl" location="bottom"}
diff --git a/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php b/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php index fa637eb0cdf3..3e3a9c540277 100644 --- a/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php +++ b/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php @@ -151,7 +151,7 @@ public function testHtmlWithOpenTracking() { // Default footer "Sample Footer for HTML formatted content" . ".*\n" . - "text ); @@ -200,13 +200,13 @@ public function testHtmlWithOpenAndUrlTracking() { $this->assertRegExp( ";" . // body_html - "

You can go to Google" . + "

You can go to Google" . " or opt out.

\n" . // Default footer "Sample Footer for HTML formatted content" . ".*\n" . // Open-tracking code - "text ); @@ -219,7 +219,7 @@ public function testHtmlWithOpenAndUrlTracking() { "\n" . "Links:\n" . "------\n" . - "\\[1\\] .*extern/url\.php\?u=\d+&qid=\d+\n" . + "\\[1\\] http.*(extern/url.php|civicrm/mailing/url)(\?|&)u=\d+&qid=\d+\n" . "\\[2\\] http.*civicrm/mailing/optout.*\n" . "\n" . // Default footer @@ -243,32 +243,32 @@ public function urlTrackingExamples() { $cases = []; // Tracking disabled - $cases[] = [ + $cases[0] = [ '

Foo

', ';

Foo

;', ';\\[1\\] http://example\.net/;', ['url_tracking' => 0], ]; - $cases[] = [ + $cases[1] = [ '

Foo

', // FIXME: Legacy tracker adds extra quote after URL ';

Foo

;', ';\\[1\\] http://example\.net/\?id=\d+;', ['url_tracking' => 0], ]; - $cases[] = [ + $cases[2] = [ '

Foo

', ';

Foo

;', ';\\[1\\] http.*civicrm/mailing/optout.*;', ['url_tracking' => 0], ]; - $cases[] = [ + $cases[3] = [ '

Look at .

', ';

Look at \.

;', ';Look at \.;', ['url_tracking' => 0], ]; - $cases[] = [ + $cases[4] = [ // Plain-text URL's are tracked in plain-text emails... // but not in HTML emails. "

Please go to: http://example.net/

", @@ -278,13 +278,13 @@ public function urlTrackingExamples() { ]; // Tracking enabled - $cases[] = [ + $cases[5] = [ '

Foo

', - ';

Foo

;', - ';\\[1\\] .*extern/url\.php\?u=\d+.*;', + ';

Foo

;', + ';\\[1\\] .*(extern/url.php|civicrm/mailing/url)[\?&]u=\d+.*;', ['url_tracking' => 1], ]; - $cases[] = [ + $cases[6] = [ // FIXME: CiviMail URL tracking doesn't track tokenized links. '

Foo

', // FIXME: Legacy tracker adds extra quote after URL @@ -292,26 +292,26 @@ public function urlTrackingExamples() { ';\\[1\\] http://example\.net/\?id=\d+;', ['url_tracking' => 1], ]; - $cases[] = [ + $cases[7] = [ // It would be redundant/slow to track the action URLs? '

Foo

', ';

Foo

;', ';\\[1\\] http.*civicrm/mailing/optout.*;', ['url_tracking' => 1], ]; - $cases[] = [ + $cases[8] = [ // It would be excessive/slow to track every embedded image. '

Look at .

', ';

Look at \.

;', ';Look at \.;', ['url_tracking' => 1], ]; - $cases[] = [ + $cases[9] = [ // Plain-text URL's are tracked in plain-text emails... // but not in HTML emails. "

Please go to: http://example.net/

", ";

Please go to: http://example\.net/

;", - ';Please go to: .*extern/url.php\?u=\d+&qid=\d+;', + ';Please go to: .*(extern/url.php|civicrm/mailing/url)[\?&]u=\d+&qid=\d+;', ['url_tracking' => 1], ]; @@ -322,6 +322,7 @@ public function urlTrackingExamples() { * Generate a fully-formatted mailing (with body_html content). * * @dataProvider urlTrackingExamples + * @throws \CRM_Core_Exception */ public function testUrlTracking($inputHtml, $htmlUrlRegex, $textUrlRegex, $params) { $caseName = print_r(['inputHtml' => $inputHtml, 'params' => $params], 1);