MDL-65308 core: change a couple of unsafe preg_quote() cases
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Tue, 9 Apr 2019 01:13:57 +0000 (03:13 +0200)
committerEloy Lafuente (stronk7) <stronk7@moodle.org>
Mon, 15 Apr 2019 23:40:47 +0000 (01:40 +0200)
With PHP 7.3, the hash (#) is being escaped by preg_quote().

While normally that doesnt have much impact and normal operations
continue working perfectly... when the results of the function are
used to match against the same string... they don't match anymore.

Have found a couple of there double-uses in core and this
commit fixes them. Covered with tests.

backup/util/helper/restore_log_rule.class.php
backup/util/helper/tests/restore_log_rule_test.php
mod/lesson/pagetypes/shortanswer.php

index 6e182c0..9d3f613 100644 (file)
@@ -238,12 +238,12 @@ class restore_log_rule implements processable {
     protected function build_regexp($expression, $tokens) {
         // Replace to temp (and preg_quote() safe) placeholders
         foreach ($tokens as $token) {
-            $expression = preg_replace('~' . preg_quote($token, '~') . '~', '#@@#@@#', $expression, 1);
+            $expression = preg_replace('~' . preg_quote($token, '~') . '~', '%@@%@@%', $expression, 1);
         }
         // quote the expression
         $expression = preg_quote($expression, '~');
         // Replace all the placeholders
-        $expression = preg_replace('~#@@#@@#~', '(.*)', $expression);
+        $expression = preg_replace('~%@@%@@%~', '(.*)', $expression);
         return '~' . $expression . '~';
     }
 }
index 03d016b..24d549b 100644 (file)
@@ -49,4 +49,20 @@ class backup_restore_log_rule_testcase extends basic_testcase {
         // But the original log has been kept unmodified by the process() call.
         $this->assertEquals($originallog, $log);
     }
+
+    public function test_build_regexp() {
+        $original = 'Any (string) with [placeholders] like {this} and {this}. [end].';
+        $expectation = '~Any \(string\) with (.*) like (.*) and (.*)\. (.*)\.~';
+
+        $lr = new restore_log_rule('this', 'doesnt', 'matter', 'here');
+        $class = new ReflectionClass('restore_log_rule');
+
+        $method = $class->getMethod('extract_tokens');
+        $method->setAccessible(true);
+        $tokens = $method->invoke($lr, $original);
+
+        $method = $class->getMethod('build_regexp');
+        $method->setAccessible(true);
+        $this->assertSame($expectation, $method->invoke($lr, $original, $tokens));
+    }
 }
index 62abed9..f59d94b 100644 (file)
@@ -141,9 +141,9 @@ class lesson_page_type_shortanswer extends lesson_page {
                     $ignorecase = 'i';
                 }
             } else {
-                $expectedanswer = str_replace('*', '#####', $expectedanswer);
+                $expectedanswer = str_replace('*', '%@@%@@%', $expectedanswer);
                 $expectedanswer = preg_quote($expectedanswer, '/');
-                $expectedanswer = str_replace('#####', '.*', $expectedanswer);
+                $expectedanswer = str_replace('%@@%@@%', '.*', $expectedanswer);
             }
             // see if user typed in any of the correct answers
             if ((!$this->lesson->custom && $this->lesson->jumpto_is_correct($this->properties->id, $answer->jumpto)) or ($this->lesson->custom && $answer->score > 0) ) {