MDL-69050 lang: Stop using the term blacklist in mustache output engine
authorDavid Mudrák <david@moodle.com>
Wed, 16 Sep 2020 11:17:04 +0000 (13:17 +0200)
committerDavid Mudrák <david@moodle.com>
Thu, 24 Sep 2020 17:39:36 +0000 (19:39 +0200)
lib/amd/build/templates.min.js.map
lib/amd/src/templates.js
lib/classes/output/mustache_engine.php
lib/classes/output/mustache_helper_collection.php
lib/outputrenderers.php
lib/tests/output_mustache_helper_collection_test.php
lib/upgrade.txt

index b75e37f..028dd34 100644 (file)
Binary files a/lib/amd/build/templates.min.js.map and b/lib/amd/build/templates.min.js.map differ
index 8ba0e8b..adb2528 100644 (file)
@@ -65,8 +65,8 @@ define([
     /** @var {Bool} isLoadingTemplates - Whether templates are currently being loaded */
     var isLoadingTemplates = false;
 
-    /** @var {Array} blacklistedNestedHelpers - List of helpers that can't be called within other helpers */
-    var blacklistedNestedHelpers = ['js'];
+    /** @var {Array} disallowedNestedHelpers - List of helpers that can't be called within other helpers */
+    var disallowedNestedHelpers = ['js'];
 
     /**
      * Search the various caches for a template promise for the given search key.
@@ -609,7 +609,7 @@ define([
      * template.
      *
      * This will parse the provided text before giving it to the helper function
-     * in order to remove any blacklisted nested helpers to prevent one helper
+     * in order to remove any disallowed nested helpers to prevent one helper
      * from calling another.
      *
      * In particular to prevent the JS helper from being called from within another
@@ -623,12 +623,12 @@ define([
     Renderer.prototype.addHelperFunction = function(helperFunction, context) {
         return function() {
             return function(sectionText, helper) {
-                // Override the blacklisted helpers in the template context with
+                // Override the disallowed helpers in the template context with
                 // a function that returns an empty string for use when executing
                 // other helpers. This is to prevent these helpers from being
                 // executed as part of the rendering of another helper in order to
                 // prevent any potential security issues.
-                var originalHelpers = blacklistedNestedHelpers.reduce(function(carry, name) {
+                var originalHelpers = disallowedNestedHelpers.reduce(function(carry, name) {
                     if (context.hasOwnProperty(name)) {
                         carry[name] = context[name];
                     }
@@ -636,14 +636,14 @@ define([
                     return carry;
                 }, {});
 
-                blacklistedNestedHelpers.forEach(function(helperName) {
+                disallowedNestedHelpers.forEach(function(helperName) {
                     context[helperName] = function() {
                         return '';
                     };
                 });
 
                 // Execute the helper with the modified context that doesn't include
-                // the blacklisted nested helpers. This prevents the blacklisted
+                // the disallowed nested helpers. This prevents the disallowed
                 // helpers from being called from within other helpers.
                 var result = helperFunction.apply(this, [context, sectionText, helper]);
 
index 4dd2b90..ada43c2 100644 (file)
@@ -38,7 +38,7 @@ class mustache_engine extends \Mustache_Engine {
     /**
      * @var string[] Names of helpers that aren't allowed to be called within other helpers.
      */
-    private $blacklistednestedhelpers = [];
+    private $disallowednestedhelpers = [];
 
     /**
      * Mustache engine constructor.
@@ -47,13 +47,19 @@ class mustache_engine extends \Mustache_Engine {
      * $options = [
      *      // A list of helpers (by name) to prevent from executing within the rendering
      *      // of other helpers.
-     *      'blacklistednestedhelpers' => ['js']
+     *      'disallowednestedhelpers' => ['js']
      * ];
      * @param array $options [description]
      */
     public function __construct(array $options = []) {
+
         if (isset($options['blacklistednestedhelpers'])) {
-            $this->blacklistednestedhelpers = $options['blacklistednestedhelpers'];
+            debugging('blacklistednestedhelpers option is deprecated. Use disallowednestedhelpers instead.', DEBUG_DEVELOPER);
+            $this->disallowednestedhelpers = $options['blacklistednestedhelpers'];
+        }
+
+        if (isset($options['disallowednestedhelpers'])) {
+            $this->disallowednestedhelpers = $options['disallowednestedhelpers'];
         }
 
         parent::__construct($options);
@@ -69,7 +75,7 @@ class mustache_engine extends \Mustache_Engine {
     public function getHelpers()
     {
         if (!isset($this->helpers)) {
-            $this->helpers = new mustache_helper_collection(null, $this->blacklistednestedhelpers);
+            $this->helpers = new mustache_helper_collection(null, $this->disallowednestedhelpers);
         }
 
         return $this->helpers;
index 233d741..0c72ff6 100644 (file)
@@ -34,7 +34,7 @@ class mustache_helper_collection extends \Mustache_HelperCollection {
     /**
      * @var string[] Names of helpers that aren't allowed to be called within other helpers.
      */
-    private $blacklistednestedhelpers = [];
+    private $disallowednestedhelpers = [];
 
     /**
      * Helper Collection constructor.
@@ -44,43 +44,43 @@ class mustache_helper_collection extends \Mustache_HelperCollection {
      * @throws \Mustache_Exception_InvalidArgumentException if the $helpers argument isn't an array or Traversable
      *
      * @param array|\Traversable $helpers (default: null)
-     * @param string[] $blacklistednestedhelpers Names of helpers that aren't allowed to be called within other helpers.
+     * @param string[] $disallowednestedhelpers Names of helpers that aren't allowed to be called within other helpers.
      */
-    public function __construct($helpers = null, array $blacklistednestedhelpers = []) {
-        $this->blacklistednestedhelpers = $blacklistednestedhelpers;
+    public function __construct($helpers = null, array $disallowednestedhelpers = []) {
+        $this->disallowednestedhelpers = $disallowednestedhelpers;
         parent::__construct($helpers);
     }
 
     /**
      * Add a helper to this collection.
      *
-     * This function has overridden the parent implementation to provide blacklist
+     * This function has overridden the parent implementation to provide disallowing
      * functionality for certain helpers to prevent them being called from within
      * other helpers. This is because the JavaScript helper can be used in a
      * security exploit if it can be nested.
      *
      * The function will wrap callable helpers in an anonymous function that strips
-     * out the blacklisted helpers from the source string before giving it to the
-     * helper function. This prevents the blacklisted helper functions from being
+     * out the disallowed helpers from the source string before giving it to the
+     * helper function. This prevents the disallowed helper functions from being
      * called by nested render functions from within other helpers.
      *
      * @see \Mustache_HelperCollection::add()
      * @param string $name
      * @param mixed  $helper
      */
-    public function add($name, $helper)
-    {
-        $blacklist = $this->blacklistednestedhelpers;
+    public function add($name, $helper) {
 
-        if (is_callable($helper) && !empty($blacklist)) {
-            $helper = function($source, \Mustache_LambdaHelper $lambdahelper) use ($helper, $blacklist) {
+        $disallowedlist = $this->disallowednestedhelpers;
 
-                // Temporarily override the blacklisted helpers to return nothing
+        if (is_callable($helper) && !empty($disallowedlist)) {
+            $helper = function($source, \Mustache_LambdaHelper $lambdahelper) use ($helper, $disallowedlist) {
+
+                // Temporarily override the disallowed helpers to return nothing
                 // so that they can't be executed from within other helpers.
-                $disabledhelpers = $this->disable_helpers($blacklist);
+                $disabledhelpers = $this->disable_helpers($disallowedlist);
                 // Call the original function with the modified sources.
                 $result = call_user_func($helper, $source, $lambdahelper);
-                // Restore the original blacklisted helper implementations now
+                // Restore the original disallowed helper implementations now
                 // that this helper has finished executing so that the rest of
                 // the rendering process continues to work correctly.
                 $this->restore_helpers($disabledhelpers);
@@ -89,7 +89,7 @@ class mustache_helper_collection extends \Mustache_HelperCollection {
                 // This is done because a secondary render is called on the result
                 // of a helper function if it still includes mustache tags. See
                 // the section function of Mustache_Compiler for details.
-                return $this->strip_blacklisted_helpers($blacklist, $result);
+                return $this->strip_disallowed_helpers($disallowedlist, $result);
             };
         }
 
@@ -137,18 +137,18 @@ class mustache_helper_collection extends \Mustache_HelperCollection {
     }
 
     /**
-     * Parse the given string and remove any reference to blacklisted helpers.
+     * Parse the given string and remove any reference to disallowed helpers.
      *
      * E.g.
-     * $blacklist = ['js'];
+     * $disallowedlist = ['js'];
      * $string = "core, move, {{#js}} some nasty JS hack {{/js}}"
      * result: "core, move, {{}}"
      *
-     * @param  string[] $blacklist List of helper names to strip
+     * @param  string[] $disallowedlist List of helper names to strip
      * @param  string $string String to parse
      * @return string Parsed string
      */
-    public function strip_blacklisted_helpers($blacklist, $string) {
+    public function strip_disallowed_helpers($disallowedlist, $string) {
         $starttoken = \Mustache_Tokenizer::T_SECTION;
         $endtoken = \Mustache_Tokenizer::T_END_SECTION;
         if ($endtoken == '/') {
@@ -160,7 +160,7 @@ class mustache_helper_collection extends \Mustache_HelperCollection {
             // the user is able to change the delimeters on a per template
             // basis so they may not be curly braces.
             return '/\s*' . $starttoken . '\s*'. $name . '\W+.*' . $endtoken . '\s*' . $name . '\s*/';
-        }, $blacklist);
+        }, $disallowedlist);
 
         // This will strip out unwanted helpers from the $source string
         // before providing it to the original helper function.
@@ -168,9 +168,25 @@ class mustache_helper_collection extends \Mustache_HelperCollection {
         // Before:
         // "core, move, {{#js}} some nasty JS hack {{/js}}"
         // After:
-        // "core, move, {{}}"
+        // "core, move, {{}}".
         return preg_replace_callback($regexes, function() {
             return '';
         }, $string);
     }
+
+    /**
+     * Parse the given string and remove any reference to disallowed helpers.
+     *
+     * @deprecated Deprecated since Moodle 3.10 (MDL-69050) - use {@see self::strip_disallowed_helpers()}
+     * @param  string[] $disallowedlist List of helper names to strip
+     * @param  string $string String to parse
+     * @return string Parsed string
+     */
+    public function strip_blacklisted_helpers($disallowedlist, $string) {
+
+        debugging('mustache_helper_collection::strip_blacklisted_helpers() is deprecated. ' .
+            'Please use mustache_helper_collection::strip_disallowed_helpers() instead.', DEBUG_DEVELOPER);
+
+        return $this->strip_disallowed_helpers($disallowedlist, $string);
+    }
 }
index dd9bd24..d81e43a 100644 (file)
@@ -129,7 +129,7 @@ class renderer_base {
                 // Don't allow the JavaScript helper to be executed from within another
                 // helper. If it's allowed it can be used by users to inject malicious
                 // JS into the page.
-                'blacklistednestedhelpers' => ['js']));
+                'disallowednestedhelpers' => ['js']));
 
         }
 
index aaac1e9..52e3976 100644 (file)
@@ -30,94 +30,94 @@ use core\output\mustache_helper_collection;
  */
 class core_output_mustache_helper_collection_testcase extends advanced_testcase {
     /**
-     * Test cases to confirm that blacklisted helpers are stripped from the source
+     * Test cases to confirm that disallowed helpers are stripped from the source
      * text by the helper before being passed to other another helper. This prevents
      * nested calls to helpers.
      */
-    public function get_strip_blacklisted_helpers_testcases() {
+    public function get_strip_disallowed_helpers_testcases() {
         return [
-            'no blacklist' => [
-                'blacklist' => [],
+            'no disallowed' => [
+                'disallowed' => [],
                 'input' => 'core, move, {{#js}} some nasty JS {{/js}}',
                 'expected' => 'core, move, {{#js}} some nasty JS {{/js}}'
             ],
-            'blacklist no match' => [
-                'blacklist' => ['foo'],
+            'disallowed no match' => [
+                'disallowed' => ['foo'],
                 'input' => 'core, move, {{#js}} some nasty JS {{/js}}',
                 'expected' => 'core, move, {{#js}} some nasty JS {{/js}}'
             ],
-            'blacklist partial match 1' => [
-                'blacklist' => ['js'],
+            'disallowed partial match 1' => [
+                'disallowed' => ['js'],
                 'input' => 'core, move, {{#json}} some nasty JS {{/json}}',
                 'expected' => 'core, move, {{#json}} some nasty JS {{/json}}'
             ],
-            'blacklist partial match 2' => [
-                'blacklist' => ['js'],
+            'disallowed partial match 2' => [
+                'disallowed' => ['js'],
                 'input' => 'core, move, {{#onjs}} some nasty JS {{/onjs}}',
                 'expected' => 'core, move, {{#onjs}} some nasty JS {{/onjs}}'
             ],
-            'single blacklist 1' => [
-                'blacklist' => ['js'],
+            'single disallowed 1' => [
+                'disallowed' => ['js'],
                 'input' => 'core, move, {{#js}} some nasty JS {{/js}}',
                 'expected' => 'core, move, {{}}'
             ],
-            'single blacklist 2' => [
-                'blacklist' => ['js'],
+            'single disallowed 2' => [
+                'disallowed' => ['js'],
                 'input' => 'core, move, {{ # js }} some nasty JS {{ /  js }}',
                 'expected' => 'core, move, {{}}'
             ],
-            'single blacklist 3' => [
-                'blacklist' => ['js'],
+            'single disallowed 3' => [
+                'disallowed' => ['js'],
                 'input' => 'core, {{#js}} some nasty JS {{/js}}, test',
                 'expected' => 'core, {{}}, test'
             ],
-            'single blacklist 3' => [
-                'blacklist' => ['js'],
+            'single disallowed 3' => [
+                'disallowed' => ['js'],
                 'input' => 'core, {{#ok}} this is ok {{/ok}}, {{#js}} some nasty JS {{/js}}',
                 'expected' => 'core, {{#ok}} this is ok {{/ok}}, {{}}'
             ],
-            'single blacklist multiple matches 1' => [
-                'blacklist' => ['js'],
+            'single disallowed multiple matches 1' => [
+                'disallowed' => ['js'],
                 'input' => 'core, {{#js}} some nasty JS {{/js}}, {{#js}} some nasty JS {{/js}}',
                 'expected' => 'core, {{}}'
             ],
-            'single blacklist multiple matches 2' => [
-                'blacklist' => ['js'],
+            'single disallowed multiple matches 2' => [
+                'disallowed' => ['js'],
                 'input' => 'core, {{ # js }} some nasty JS {{ /  js }}, {{ # js }} some nasty JS {{ /  js }}',
                 'expected' => 'core, {{}}'
             ],
-            'single blacklist multiple matches nested 1' => [
-                'blacklist' => ['js'],
+            'single disallowed multiple matches nested 1' => [
+                'disallowed' => ['js'],
                 'input' => 'core, move, {{#js}} some nasty JS {{#js}} some nasty JS {{/js}} {{/js}}',
                 'expected' => 'core, move, {{}}'
             ],
-            'single blacklist multiple matches nested 2' => [
-                'blacklist' => ['js'],
+            'single disallowed multiple matches nested 2' => [
+                'disallowed' => ['js'],
                 'input' => 'core, move, {{ # js }} some nasty JS {{ # js }} some nasty JS {{ /  js }}{{ /  js }}',
                 'expected' => 'core, move, {{}}'
             ],
-            'multiple blacklist 1' => [
-                'blacklist' => ['js', 'foo'],
+            'multiple disallowed 1' => [
+                'disallowed' => ['js', 'foo'],
                 'input' => 'core, move, {{#js}} some nasty JS {{/js}}',
                 'expected' => 'core, move, {{}}'
             ],
-            'multiple blacklist 2' => [
-                'blacklist' => ['js', 'foo'],
+            'multiple disallowed 2' => [
+                'disallowed' => ['js', 'foo'],
                 'input' => 'core, {{#foo}} blah {{/foo}}, {{#js}} js {{/js}}',
                 'expected' => 'core, {{}}, {{}}'
             ],
-            'multiple blacklist 3' => [
-                'blacklist' => ['js', 'foo'],
+            'multiple disallowed 3' => [
+                'disallowed' => ['js', 'foo'],
                 'input' => '{{#foo}} blah {{/foo}}, {{#foo}} blah {{/foo}}, {{#js}} js {{/js}}',
                 'expected' => '{{}}, {{}}'
             ],
-            'multiple blacklist 4' => [
-                'blacklist' => ['js', 'foo'],
+            'multiple disallowed 4' => [
+                'disallowed' => ['js', 'foo'],
                 'input' => '{{#foo}} blah {{/foo}}, {{#js}} js {{/js}}, {{#foo}} blah {{/foo}}',
                 'expected' => '{{}}'
             ],
-            'multiple blacklist 4' => [
-                'blacklist' => ['js', 'foo'],
+            'multiple disallowed 4' => [
+                'disallowed' => ['js', 'foo'],
                 'input' => 'core, move, {{#js}} JS {{#foo}} blah {{/foo}} {{/js}}',
                 'expected' => 'core, move, {{}}'
             ],
@@ -126,29 +126,29 @@ class core_output_mustache_helper_collection_testcase extends advanced_testcase
 
     /**
      * Test that the mustache_helper_collection class correctly strips
-     * @dataProvider get_strip_blacklisted_helpers_testcases()
-     * @param string[] $blacklist The list of helpers to strip
+     * @dataProvider get_strip_disallowed_helpers_testcases()
+     * @param string[] $disallowed The list of helpers to strip
      * @param string $input The input string for the helper
-     * @param string $expected The expected output of the string after blacklist strip
+     * @param string $expected The expected output of the string after disallowed strip
      */
-    public function test_strip_blacklisted_helpers($blacklist, $input, $expected) {
-        $collection = new mustache_helper_collection(null, $blacklist);
-        $this->assertEquals($expected, $collection->strip_blacklisted_helpers($blacklist, $input));
+    public function test_strip_disallowed_helpers($disallowed, $input, $expected) {
+        $collection = new mustache_helper_collection(null, $disallowed);
+        $this->assertEquals($expected, $collection->strip_disallowed_helpers($disallowed, $input));
     }
 
     /**
-     * Test that the blacklisted helpers are disabled during the execution of other
+     * Test that the disallowed helpers are disabled during the execution of other
      * helpers.
      *
-     * Any non-blacklisted helper should still be available to call during the
+     * Any allowed helper should still be available to call during the
      * execution of a helper.
      */
-    public function test_blacklisted_helpers_disabled_during_execution() {
+    public function test_disallowed_helpers_disabled_during_execution() {
         $engine = new \Mustache_Engine();
         $context = new \Mustache_Context();
         $lambdahelper = new \Mustache_LambdaHelper($engine, $context);
-        $blacklist = ['bad'];
-        $collection = new mustache_helper_collection(null, $blacklist);
+        $disallowed = ['bad'];
+        $collection = new mustache_helper_collection(null, $disallowed);
         $badcalled = false;
         $goodcalled = false;
 
@@ -174,4 +174,16 @@ class core_output_mustache_helper_collection_testcase extends advanced_testcase
         $this->assertTrue($goodcalled);
         $this->assertFalse($badcalled);
     }
+
+    /**
+     * Test that calling deprecated method strip_blacklisted_helpers() still works and shows developer debugging.
+     */
+    public function test_deprecated_strip_blacklisted_helpers() {
+
+        $collection = new mustache_helper_collection(null, ['js']);
+        $stripped = $collection->strip_blacklisted_helpers(['js'], '{{#js}} JS {{/js}}');
+        $this->assertEquals('{{}}', $stripped);
+        $this->assertDebuggingCalled('mustache_helper_collection::strip_blacklisted_helpers() is deprecated. ' .
+            'Please use mustache_helper_collection::strip_disallowed_helpers() instead.', DEBUG_DEVELOPER);
+    }
 }
index 699f5c6..c5fe41f 100644 (file)
@@ -50,6 +50,8 @@ information provided here is intended especially for developers.
 * The http-message library has been added to Moodle core in /lib/http-message.
 * Methods `filetypes_util::is_whitelisted()` and `filetypes_util::get_not_whitelisted()` have been deprecated and
   renamed to `is_listed()` and `get_not_listed()` respectively.
+* Method `mustache_helper_collection::strip_blacklisted_helpers()` has been deprecated and renamed to
+  `strip_disallowed_helpers()`.
 
 === 3.9 ===
 * Following function has been deprecated, please use \core\task\manager::run_from_cli().