diff --git a/lib/classes/formatting.php b/lib/classes/formatting.php new file mode 100644 index 00000000000..048891986a1 --- /dev/null +++ b/lib/classes/formatting.php @@ -0,0 +1,135 @@ +. + +namespace core; + +/** + * Content formatting methods for Moodle. + * + * @package core + * @copyright 2023 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class formatting { + /** + * Given a simple string, this function returns the string + * processed by enabled string filters if $CFG->filterall is enabled + * + * This function should be used to print short strings (non html) that + * need filter processing e.g. activity titles, post subjects, + * glossary concepts. + * + * @staticvar bool $strcache + * @param string $string The string to be filtered. Should be plain text, expect + * possibly for multilang tags. + * @param boolean $striplinks To strip any link in the result text. Moodle 1.8 default changed from false to true! MDL-8713 + * @param array $options options array/object or courseid + * @return string + */ + public function format_string( + $string, + $striplinks = true, + $options = null, + ): string { + global $CFG, $PAGE; + + if ($string === '' || is_null($string)) { + // No need to do any filters and cleaning. + return ''; + } + + // We'll use a in-memory cache here to speed up repeated strings. + static $strcache = false; + + if (empty($CFG->version) or $CFG->version < 2013051400 or during_initial_install()) { + // Do not filter anything during installation or before upgrade completes. + return $string = strip_tags($string); + } + + if ($strcache === false or count($strcache) > 2000) { + // This number might need some tuning to limit memory usage in cron. + $strcache = array(); + } + + if (is_numeric($options)) { + // Legacy courseid usage. + $options = array('context' => context_course::instance($options)); + } else { + // Detach object, we can not modify it. + $options = (array)$options; + } + + if (empty($options['context'])) { + // Fallback to $PAGE->context this may be problematic in CLI and other non-standard pages :-(. + $options['context'] = $PAGE->context; + } else if (is_numeric($options['context'])) { + $options['context'] = context::instance_by_id($options['context']); + } + if (!isset($options['filter'])) { + $options['filter'] = true; + } + + $options['escape'] = !isset($options['escape']) || $options['escape']; + + if (!$options['context']) { + // We did not find any context? weird. + return $string = strip_tags($string); + } + + // Calculate md5. + $cachekeys = array( + $string, $striplinks, $options['context']->id, + $options['escape'], current_language(), $options['filter'] + ); + $md5 = md5(implode('<+>', $cachekeys)); + + // Fetch from cache if possible. + if (isset($strcache[$md5])) { + return $strcache[$md5]; + } + + // First replace all ampersands not followed by html entity code + // Regular expression moved to its own method for easier unit testing. + $string = $options['escape'] ? replace_ampersands_not_followed_by_entity($string) : $string; + + if (!empty($CFG->filterall) && $options['filter']) { + $filtermanager = \filter_manager::instance(); + $filtermanager->setup_page_for_filters($PAGE, $options['context']); // Setup global stuff filters may have. + $string = $filtermanager->filter_string($string, $options['context']); + } + + // If the site requires it, strip ALL tags from this string. + if (!empty($CFG->formatstringstriptags)) { + if ($options['escape']) { + $string = str_replace(array('<', '>'), array('<', '>'), strip_tags($string)); + } else { + $string = strip_tags($string); + } + } else { + // Otherwise strip just links if that is required (default). + if ($striplinks) { + // Strip links in string. + $string = strip_links($string); + } + $string = clean_text($string); + } + + // Store to cache. + $strcache[$md5] = $string; + + return $string; + } +} diff --git a/lib/tests/formatting_test.php b/lib/tests/formatting_test.php new file mode 100644 index 00000000000..35d5f954df5 --- /dev/null +++ b/lib/tests/formatting_test.php @@ -0,0 +1,150 @@ +. + +namespace core; + +/** + * Tests for Moodle's String Formatter. + * + * @package core + * @copyright 2023 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \core\formatting + * @coversDefaultClass \core\formatting + */ +class formatting_test extends \advanced_testcase { + /** + * @covers ::format_string + */ + public function test_format_string_striptags(): void { + global $CFG; + + $this->resetAfterTest(); + + $formatting = new formatting(); + + // < and > signs. + $CFG->formatstringstriptags = false; + $this->assertSame('x < 1', $formatting->format_string('x < 1')); + $this->assertSame('x > 1', $formatting->format_string('x > 1')); + $this->assertSame('x < 1 and x > 0', $formatting->format_string('x < 1 and x > 0')); + + $CFG->formatstringstriptags = true; + $this->assertSame('x < 1', $formatting->format_string('x < 1')); + $this->assertSame('x > 1', $formatting->format_string('x > 1')); + $this->assertSame('x < 1 and x > 0', $formatting->format_string('x < 1 and x > 0')); + } + + /** + * @covers ::format_string + * @dataProvider format_string_provider + * @param string $expected + * @param mixed $input + * @param array $options + */ + public function test_format_string_values( + string $expected, + mixed $input, + array $options = [], + ): void { + $formatting = new formatting(); + $this->assertSame( + $expected, + $formatting->format_string($input, ...$options), + ); + } + + /** + * Data provider for format_string tests. + * + * @return array + */ + public static function format_string_provider(): array { + return [ + // Ampersands. + ["& &&&&& &&", "& &&&&& &&"], + ["ANother & &&&&& Category", "ANother & &&&&& Category"], + ["ANother & &&&&& Category", "ANother & &&&&& Category", [true]], + ["Nick's Test Site & Other things", "Nick's Test Site & Other things", [true]], + ["& < > \" '", "& < > \" '", [true, ['escape' => false]]], + + // String entities. + [""", """], + + // Digital entities. + ["&11234;", "&11234;"], + + // Unicode entities. + ["ᅻ", "ᅻ"], + + // Nulls. + ['', null], + ['', null, [true, ['escape' => false]]], + ]; + } + + /** + * The format string static caching should include the filters option to make + * sure filters are correctly applied when requested. + */ + public function test_format_string_static_caching_with_filters(): void { + global $CFG; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $generator = $this->getDataGenerator(); + $course = $generator->create_course(); + $user = $generator->create_user(); + + $rawstring = 'EnglishCatalan'; + $expectednofilter = strip_tags($rawstring); + $expectedfilter = 'English'; + $striplinks = true; + $context = \core\context\course::instance($course->id); + $options = [ + 'context' => $context, + 'escape' => true, + 'filter' => false, + ]; + + $this->setUser($user); + + $formatting = new formatting(); + + // Format the string without filters. It should just strip the + // links. + $nofilterresult = $formatting->format_string($rawstring, $striplinks, $options); + $this->assertEquals($expectednofilter, $nofilterresult); + + // Add the multilang filter. Make sure it's enabled globally. + $CFG->filterall = true; + $CFG->stringfilters = 'multilang'; + filter_set_global_state('multilang', TEXTFILTER_ON); + filter_set_local_state('multilang', $context->id, TEXTFILTER_ON); + // This time we want to apply the filters. + $options['filter'] = true; + $filterresult = $formatting->format_string($rawstring, $striplinks, $options); + $this->assertMatchesRegularExpression("/$expectedfilter/", $filterresult); + + filter_set_local_state('multilang', $context->id, TEXTFILTER_OFF); + + // Confirm that we get back the cached string. The result should be + // the same as the filtered text above even though we've disabled the + // multilang filter in between. + $cachedresult = $formatting->format_string($rawstring, $striplinks, $options); + $this->assertMatchesRegularExpression("/$expectedfilter/", $cachedresult); + } +} diff --git a/lib/tests/weblib_test.php b/lib/tests/weblib_test.php index f553acaf397..263e5a42005 100644 --- a/lib/tests/weblib_test.php +++ b/lib/tests/weblib_test.php @@ -24,97 +24,6 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU Public License */ class weblib_test extends advanced_testcase { - /** - * @covers ::format_string - */ - public function test_format_string() { - global $CFG; - - // Ampersands. - $this->assertSame("& &&&&& &&", format_string("& &&&&& &&")); - $this->assertSame("ANother & &&&&& Category", format_string("ANother & &&&&& Category")); - $this->assertSame("ANother & &&&&& Category", format_string("ANother & &&&&& Category", true)); - $this->assertSame("Nick's Test Site & Other things", format_string("Nick's Test Site & Other things", true)); - $this->assertSame("& < > \" '", format_string("& < > \" '", true, ['escape' => false])); - - // String entities. - $this->assertSame(""", format_string(""")); - - // Digital entities. - $this->assertSame("&11234;", format_string("&11234;")); - - // Unicode entities. - $this->assertSame("ᅻ", format_string("ᅻ")); - - // Nulls. - $this->assertSame('', format_string(null)); - $this->assertSame('', format_string(null, true, ['escape' => false])); - - // < and > signs. - $originalformatstringstriptags = $CFG->formatstringstriptags; - - $CFG->formatstringstriptags = false; - $this->assertSame('x < 1', format_string('x < 1')); - $this->assertSame('x > 1', format_string('x > 1')); - $this->assertSame('x < 1 and x > 0', format_string('x < 1 and x > 0')); - - $CFG->formatstringstriptags = true; - $this->assertSame('x < 1', format_string('x < 1')); - $this->assertSame('x > 1', format_string('x > 1')); - $this->assertSame('x < 1 and x > 0', format_string('x < 1 and x > 0')); - - $CFG->formatstringstriptags = $originalformatstringstriptags; - } - - /** - * The format string static caching should include the filters option to make - * sure filters are correctly applied when requested. - */ - public function test_format_string_static_caching_with_filters() { - global $CFG; - - $this->resetAfterTest(true); - $this->setAdminUser(); - $generator = $this->getDataGenerator(); - $course = $generator->create_course(); - $user = $generator->create_user(); - $rawstring = 'EnglishCatalan'; - $expectednofilter = strip_tags($rawstring); - $expectedfilter = 'English'; - $striplinks = true; - $context = context_course::instance($course->id); - $options = [ - 'context' => $context, - 'escape' => true, - 'filter' => false - ]; - - $this->setUser($user); - - // Format the string without filters. It should just strip the - // links. - $nofilterresult = format_string($rawstring, $striplinks, $options); - $this->assertEquals($expectednofilter, $nofilterresult); - - // Add the multilang filter. Make sure it's enabled globally. - $CFG->filterall = true; - $CFG->stringfilters = 'multilang'; - filter_set_global_state('multilang', TEXTFILTER_ON); - filter_set_local_state('multilang', $context->id, TEXTFILTER_ON); - // This time we want to apply the filters. - $options['filter'] = true; - $filterresult = format_string($rawstring, $striplinks, $options); - $this->assertMatchesRegularExpression("/$expectedfilter/", $filterresult); - - filter_set_local_state('multilang', $context->id, TEXTFILTER_OFF); - - // Confirm that we get back the cached string. The result should be - // the same as the filtered text above even though we've disabled the - // multilang filter in between. - $cachedresult = format_string($rawstring, $striplinks, $options); - $this->assertMatchesRegularExpression("/$expectedfilter/", $cachedresult); - } - /** * @covers ::s */ diff --git a/lib/upgrade.txt b/lib/upgrade.txt index bf4d288cbd4..b5fd18365c3 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -40,6 +40,8 @@ information provided here is intended especially for developers. * New Behat `heading` named selector to more easily assert the presence of H1-H6 elements on the page * Login can now utilise new param 'loginredirect' to indicate when to use value set for $CFG->alternateloginurl. * \action_menu_link::$instance has been deprecated as it is no longer used. +* The `format_string()` method has moved to `\core\formatting::format_string()`. + The old method will be maintained, but new code should use the new method with first-class parameters. === 4.3 === diff --git a/lib/weblib.php b/lib/weblib.php index 8415fe0a90b..49fd74283fe 100644 --- a/lib/weblib.php +++ b/lib/weblib.php @@ -1505,36 +1505,21 @@ function reset_text_filters_cache($phpunitreset = false) { * @return string */ function format_string($string, $striplinks = true, $options = null) { - global $CFG, $PAGE; + global $CFG; - if ($string === '' || is_null($string)) { - // No need to do any filters and cleaning. - return ''; - } + // Manually include the formatting class for now until after the release after 4.5 LTS. + require_once("{$CFG->libdir}/classes/formatting.php"); - // We'll use a in-memory cache here to speed up repeated strings. - static $strcache = false; - - if (empty($CFG->version) or $CFG->version < 2013051400 or during_initial_install()) { - // Do not filter anything during installation or before upgrade completes. - return $string = strip_tags($string); - } - - if ($strcache === false or count($strcache) > 2000) { - // This number might need some tuning to limit memory usage in cron. - $strcache = array(); - } + $params = [ + 'string' => $string, + 'striplinks' => (bool) $striplinks, + ]; // This method only expects either: // - an array of options; // - a stdClass of options to be cast to an array; or // - an integer courseid. - if ($options === null) { - $options = []; - } else if (is_numeric($options)) { - // Legacy courseid usage. - $options = ['context' => \core\context\course::instance($options)]; - } else if ($options instanceof \core\context) { + if ($options instanceof \core\context) { // A common mistake has been to call this function with a context object. // This has never been expected, or nor supported. debugging( @@ -1543,9 +1528,10 @@ function format_string($string, $striplinks = true, $options = null) { DEBUG_DEVELOPER, ); $options = ['context' => $options]; - } else if (is_array($options) || is_a($options, \stdClass::class)) { - // Re-cast to array to prevent modifications to the original object. - $options = (array) $options; + // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedIf + } else if ($options === null || is_numeric($options) || is_array($options) || is_a($options, \stdClass::class)) { + // Do nothing. These are accepted options. + // Pass to the new method. } else { // Something else was passed, so we'll just use an empty array. // Attempt to cast to array since we always used to, but throw in some debugging. @@ -1556,63 +1542,13 @@ function format_string($string, $striplinks = true, $options = null) { $options = (array) $options; } - if (empty($options['context'])) { - // Fallback to $PAGE->context this may be problematic in CLI and other non-standard pages :-(. - $options['context'] = $PAGE->context; - } else if (is_numeric($options['context'])) { - $options['context'] = context::instance_by_id($options['context']); - } - if (!isset($options['filter'])) { - $options['filter'] = true; + if ($options !== null) { + $params['options'] = $options; } - $options['escape'] = !isset($options['escape']) || $options['escape']; - - if (!$options['context']) { - // We did not find any context? weird. - return $string = strip_tags($string); - } - - // Calculate md5. - $cachekeys = array($string, $striplinks, $options['context']->id, - $options['escape'], current_language(), $options['filter']); - $md5 = md5(implode('<+>', $cachekeys)); - - // Fetch from cache if possible. - if (isset($strcache[$md5])) { - return $strcache[$md5]; - } - - // First replace all ampersands not followed by html entity code - // Regular expression moved to its own method for easier unit testing. - $string = $options['escape'] ? replace_ampersands_not_followed_by_entity($string) : $string; - - if (!empty($CFG->filterall) && $options['filter']) { - $filtermanager = filter_manager::instance(); - $filtermanager->setup_page_for_filters($PAGE, $options['context']); // Setup global stuff filters may have. - $string = $filtermanager->filter_string($string, $options['context']); - } - - // If the site requires it, strip ALL tags from this string. - if (!empty($CFG->formatstringstriptags)) { - if ($options['escape']) { - $string = str_replace(array('<', '>'), array('<', '>'), strip_tags($string)); - } else { - $string = strip_tags($string); - } - } else { - // Otherwise strip just links if that is required (default). - if ($striplinks) { - // Strip links in string. - $string = strip_links($string); - } - $string = clean_text($string); - } - - // Store to cache. - $strcache[$md5] = $string; - - return $string; + return \core\di::get(\core\formatting::class)->format_string( + ...$params, + ); } /**