diff --git a/Civi/Token/TokenCompatSubscriber.php b/Civi/Token/TokenCompatSubscriber.php index 92e20b780680..1a311cd33048 100644 --- a/Civi/Token/TokenCompatSubscriber.php +++ b/Civi/Token/TokenCompatSubscriber.php @@ -411,29 +411,17 @@ protected function getFieldMetadata(): array { * Apply the various CRM_Utils_Token helpers. * * @param \Civi\Token\Event\TokenRenderEvent $e - * - * @throws \CRM_Core_Exception */ public function onRender(TokenRenderEvent $e): void { - $isHtml = ($e->message['format'] === 'text/html'); $useSmarty = !empty($e->context['smarty']); - - if (!empty($e->context['contact'])) { - // @todo - remove this - it simply removes the last unresolved tokens before - // they break smarty. - // historically it was only called when context['contact'] so that is - // retained but it only works because it's almost always true. - $remainingTokens = array_keys(\CRM_Utils_Token::getTokens($e->string)); - if (!empty($remainingTokens)) { - $e->string = \CRM_Utils_Token::replaceHookTokens($e->string, $e->context['contact'], $remainingTokens); - } - } + $e->string = $e->getTokenProcessor()->visitTokens($e->string, function() { + // For historical consistency, we filter out unrecognized tokens. + return ''; + }); if ($useSmarty) { $smartyVars = []; foreach ($e->context['smartyTokenAlias'] ?? [] as $smartyName => $tokenName) { - // Note: $e->row->tokens resolves event-based tokens (eg CRM_*_Tokens). But if the target token relies on the - // above bits (replaceGreetingTokens=>replaceContactTokens=>replaceHookTokens) then this lookup isn't sufficient. $smartyVars[$smartyName] = \CRM_Utils_Array::pathGet($e->row->tokens, explode('.', $tokenName)); } \CRM_Core_Smarty::singleton()->pushScope($smartyVars); diff --git a/Civi/Token/TokenProcessor.php b/Civi/Token/TokenProcessor.php index bb066183a3b3..cc079ddaa39e 100644 --- a/Civi/Token/TokenProcessor.php +++ b/Civi/Token/TokenProcessor.php @@ -386,15 +386,37 @@ public function render($name, $row) { return $event->string; } - private function visitTokens(string $expression, callable $callback): string { + /** + * Examine a token string and filter each token expression. + * + * @internal + * This function is only intended for use within civicrm-core. The name/location/callback-signature may change. + * @param string $expression + * Ex: 'Hello {foo.bar} and {whiz.bang|filter:"arg"}!' + * @param callable $callback + * A function which visits (and substitutes) each token. + * function(?string $fullToken, ?string $entity, ?string $field, ?array $modifier) + * @return string + */ + public function visitTokens(string $expression, callable $callback): string { // Regex examples: '{foo.bar}', '{foo.bar|whiz}', '{foo.bar|whiz:"bang"}', '{foo.bar|whiz:"bang":"bang"}' // Regex counter-examples: '{foobar}', '{foo bar}', '{$foo.bar}', '{$foo.bar|whiz}', '{foo.bar|whiz{bang}}' // Key observations: Civi tokens MUST have a `.` and MUST NOT have a `$`. Civi filters MUST NOT have `{}`s or `$`s. - $tokRegex = '([\w]+)\.([\w:\.]+)'; /* EX: 'foo.bar' in '{foo.bar|whiz:"bang":"bang"}' */ - $argRegex = ':[\w": %\-_()\[\]\+/#@!,\.\?]*'; /* EX: ':"bang":"bang"' in '{foo.bar|whiz:"bang":"bang"}' */ - // Debatable: Maybe relax to this: $argRegex = ':[^{}\n]*'; /* EX: ':"bang":"bang"' in '{foo.bar|whiz:"bang":"bang"}' */ - $filterRegex = "(\w+(?:$argRegex)?)"; /* EX: 'whiz:"bang"' in '{foo.bar|whiz:"bang"' */ - return preg_replace_callback(";\{$tokRegex(?:\|$filterRegex)?\};", function($m) use ($callback) { + + static $fullRegex = NULL; + if ($fullRegex === NULL) { + // The regex is a bit complicated, we so break it down into fragments. + // Consider the example '{foo.bar|whiz:"bang":"bang"}'. Each fragment matches the following: + + $tokenRegex = '([\w]+)\.([\w:\.]+)'; /* MATCHES: 'foo.bar' */ + $filterArgRegex = ':[\w": %\-_()\[\]\+/#@!,\.\?]*'; /* MATCHES: ':"bang":"bang"' */ + // Key rule of filterArgRegex is to prohibit '{}'s because they may parse ambiguously. So you *might* relax it to: + // $filterArgRegex = ':[^{}\n]*'; /* MATCHES: ':"bang":"bang"' */ + $filterNameRegex = "\w+"; /* MATCHES: 'whiz' */ + $filterRegex = "\|($filterNameRegex(?:$filterArgRegex)?)"; /* MATCHES: '|whiz:"bang":"bang"' */ + $fullRegex = ";\{$tokenRegex(?:$filterRegex)?\};"; + } + return preg_replace_callback($fullRegex, function($m) use ($callback) { $filterParts = NULL; if (isset($m[3])) { $filterParts = []; diff --git a/tests/phpunit/CRM/Activity/Form/Task/PDFLetterCommonTest.php b/tests/phpunit/CRM/Activity/Form/Task/PDFLetterCommonTest.php index 4e920f45f539..97c07607974f 100644 --- a/tests/phpunit/CRM/Activity/Form/Task/PDFLetterCommonTest.php +++ b/tests/phpunit/CRM/Activity/Form/Task/PDFLetterCommonTest.php @@ -140,15 +140,16 @@ public function testCreateDocumentSpecialTokens(): void { } /** + * Unknown tokens are removed at the very end. + * * @throws \CRM_Core_Exception * @throws \CiviCRM_API3_Exception */ public function testCreateDocumentUnknownTokens(): void { $activity = $this->activityCreate(); - $html_message = 'Unknown token: {activity.something_unknown}'; + $html_message = 'Unknown token: '; $form = $this->getFormObject('CRM_Activity_Form_Task_PDF'); $output = $form->createDocument([$activity['id']], $html_message, ['is_unit_test' => TRUE]); - // Unknown tokens should be left alone $this->assertEquals($html_message, $output[0]); } diff --git a/tests/phpunit/CRM/Core/BAO/MessageTemplateTest.php b/tests/phpunit/CRM/Core/BAO/MessageTemplateTest.php index ed303f27341f..3b5454cb6604 100644 --- a/tests/phpunit/CRM/Core/BAO/MessageTemplateTest.php +++ b/tests/phpunit/CRM/Core/BAO/MessageTemplateTest.php @@ -397,6 +397,20 @@ public function hookTokenValues(array &$details): void { } } + /** + * Test that unresolved tokens are not causing a fatal error in smarty. + * + * @throws \API_Exception + * @throws \CRM_Core_Exception + */ + public function testUnresolvedTokens(): void { + CRM_Core_BAO_MessageTemplate::renderTemplate([ + 'messageTemplate' => [ + 'msg_text' => '{contact.blah}', + ], + ])['text']; + } + /** * Hook to advertise tokens. * diff --git a/tests/phpunit/Civi/Token/TokenProcessorTest.php b/tests/phpunit/Civi/Token/TokenProcessorTest.php index 3c9890c420b7..1769c7006a8f 100644 --- a/tests/phpunit/Civi/Token/TokenProcessorTest.php +++ b/tests/phpunit/Civi/Token/TokenProcessorTest.php @@ -1,7 +1,6 @@ ['foo', 'bar', NULL], '{foo.bar|whiz}' => ['foo', 'bar', ['whiz']], '{foo.bar|whiz:"bang"}' => ['foo', 'bar', ['whiz', 'bang']], - '{love.shack|place:"bang":"b@ng, on +he/([do0r])?!"}' => ['love', 'shack', ['place', 'bang', 'b@ng, on +he/([do0r])?!']], + '{FoO.bAr|whiz:"bang"}' => ['FoO', 'bAr', ['whiz', 'bang']], + '{oo_f.ra_b|b_52:"bang":"b@ng, on +he/([do0r])?!"}' => ['oo_f', 'ra_b', ['b_52', 'bang', 'b@ng, on +he/([do0r])?!']], + '{foo.bar.whiz}' => ['foo', 'bar.whiz', NULL], + '{foo.bar.whiz|bang}' => ['foo', 'bar.whiz', ['bang']], + '{foo.bar:label}' => ['foo', 'bar:label', NULL], + '{foo.bar:label|truncate:"10"}' => ['foo', 'bar:label', ['truncate', '10']], ]; foreach ($examples as $input => $expected) { array_unshift($expected, $input); $log = []; - Invasive::call([$p, 'visitTokens'], [ - $input, - function (?string $fullToken, ?string $entity, ?string $field, ?array $modifier) use (&$log) { - $log[] = [$fullToken, $entity, $field, $modifier]; - }, - ]); + $filtered = $p->visitTokens($input, function (?string $fullToken, ?string $entity, ?string $field, ?array $modifier) use (&$log) { + $log[] = [$fullToken, $entity, $field, $modifier]; + return 'Replaced!'; + }); $this->assertEquals(1, count($log), "Should receive one callback on expression: $input"); $this->assertEquals($expected, $log[0]); + $this->assertEquals('Replaced!', $filtered); } }