MDL-65349 xhprof: Removed greedy matching from wildcard parsing
authorBrendan Heywood <brendan@catalyst-au.net>
Wed, 16 Oct 2019 00:34:24 +0000 (11:34 +1100)
committerBrendan Heywood <brendan@catalyst-au.net>
Wed, 23 Oct 2019 00:08:27 +0000 (11:08 +1100)
lang/en/admin.php
lib/tests/xhprof_test.php [new file with mode: 0644]
lib/upgrade.txt
lib/xhprof/readme_moodle.txt
lib/xhprof/xhprof_moodle.php

index b49c0a3..0552e93 100644 (file)
@@ -989,11 +989,11 @@ $string['profilingautofrec_help'] = 'By configuring this setting, some request (
 $string['profilingenabled'] = 'Enable profiling';
 $string['profilingenabled_help'] = 'If you enable this setting, then profiling will be available in this site and you will be able to define its behavior by configuring the next options.';
 $string['profilingexcluded'] = 'Exclude profiling';
-$string['profilingexcluded_help'] = 'List of (comma separated, absolute skipping wwwroot, callable) URLs that will be excluded from being profiled from the ones defined by \'Profile these\' setting.';
+$string['profilingexcluded_help'] = 'List of (comma or newline separated, absolute skipping wwwroot, callable) URLs that will be excluded from being profiled from the ones defined by \'Profile these\' setting.';
 $string['profilingimportprefix'] = 'Profiling import prefix';
 $string['profilingimportprefix_desc'] = 'For easier detection, all the imported profiling runs will be prefixed with the value specified here.';
 $string['profilingincluded'] = 'Profile these';
-$string['profilingincluded_help'] = 'List of (comma separated, absolute skipping wwwroot, callable) URLs that will be automatically profiled. Examples: /index.php, /course/view.php. Also accepts the * wildchar at any position. Examples: /mod/forum/*, /mod/*/view.php.';
+$string['profilingincluded_help'] = 'List of (comma or newline separated, absolute skipping wwwroot, callable) URLs that will be automatically profiled. Examples: /index.php, /course/view.php. Also accepts the * wildchar at any position. Examples: /mod/forum/*, /mod/*/view.php.';
 $string['profilinglifetime'] = 'Keep profiling runs';
 $string['profilinglifetime_help'] = 'Specify the time you want to keep information about old profiling runs. Older ones will be pruned periodically. Note that this excludes any profiling run marked as \'reference run\'.';
 $string['profilingslow'] = 'Profile slow pages';
diff --git a/lib/tests/xhprof_test.php b/lib/tests/xhprof_test.php
new file mode 100644 (file)
index 0000000..0a546eb
--- /dev/null
@@ -0,0 +1,85 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Unit tests for xhprof.
+ *
+ * @package   core
+ * @copyright 2019 Brendan Heywood <brendan@catalyst-au.net>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later (5)
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Unit tests for the xhprof class.
+ *
+ * @copyright 2019 Brendan Heywood <brendan@catalyst-au.net>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class core_xhprof_testcase extends advanced_testcase {
+
+    /**
+     * Data provider for string matches
+     *
+     * @return  array
+     */
+    public function profiling_string_matches_provider() {
+        return [
+            ['/index.php',              '/index.php',           true],
+            ['/some/dir/index.php',     '/index.php',           false],
+            ['/course/view.php',        '/course/view.php',     true],
+            ['/view.php',               '/course/view.php',     false],
+            ['/mod/forum',              '/mod/forum/*',         false],
+            ['/mod/forum/',             '/mod/forum/*',         true],
+            ['/mod/forum/index.php',    '/mod/forum/*',         true],
+            ['/mod/forum/foo.php',      '/mod/forum/*',         true],
+            ['/mod/forum/view.php',     '/mod/*/view.php',      true],
+            ['/mod/one/two/view.php',   '/mod/*/view.php',      true],
+            ['/view.php',               '*/view.php',           true],
+            ['/mod/one/two/view.php',   '*/view.php',           true],
+            ['/foo.php',                '/foo.php,/bar.php',    true],
+            ['/bar.php',                '/foo.php,/bar.php',    true],
+            ['/foo/bar.php',            "/foo.php,/bar.php",    false],
+            ['/foo/bar.php',            "/foo.php,*/bar.php",   true],
+            ['/foo/bar.php',            "/foo*.php,/bar.php",   true],
+            ['/foo.php',                "/foo.php\n/bar.php",   true],
+            ['/bar.php',                "/foo.php\n/bar.php",   true],
+            ['/foo/bar.php',            "/foo.php\n/bar.php",   false],
+            ['/foo/bar.php',            "/foo.php\n*/bar.php",  true],
+            ['/foo/bar.php',            "/foo*.php\n/bar.php",  true],
+        ];
+    }
+
+    /**
+     * Test the matching syntax
+     *
+     * @dataProvider profiling_string_matches_provider
+     * @param   string $string
+     * @param   string $patterns
+     * @param   bool   $expected
+     */
+    public function test_profiling_string_matches($string, $patterns, $expected) {
+
+        global $CFG;
+        require_once($CFG->libdir . '/xhprof/xhprof_moodle.php');
+
+        $result = profiling_string_matches($string, $patterns);
+        $this->assertSame($result, $expected);
+    }
+
+}
+
index 2f785c6..130980f 100644 (file)
@@ -68,6 +68,7 @@ validation against and defaults to null (so, no user needed) if not provided.
   and should be set to either false or true. Developers will probably want to set this to false.
 * The core_enrol_edit_user_enrolment webservice has been deprecated. Please use core_enrol_submit_user_enrolment_form instead.
 * \single_button constructor has a new attributes param to add attributes to the button HTML tag.
+* Improved url matching behaviour for profiled urls and excluded urls
 
 === 3.7 ===
 
index 8bd8bc5..58c61f7 100644 (file)
@@ -40,4 +40,5 @@ TODO:
 20160721 - MDL-55292 - Russell Smith (mr-russ): Add support for tideways profiler collection for PHP7
 20171002 - MDL-60313 - Marina Glancy (marinaglancy): Upgrade to 0.9.4 release; patched for PHP7.2
 20190314 - MDL-64543 - Brendan Heywood (brendanheywood): Add support for conditional slow profiling
+20191016 - MDL-65349 - Brendan Heywood (brendanheywood): Improved url matching behaviour
 
index 36548ea..9a3c6a2 100644 (file)
@@ -465,7 +465,7 @@ function profiling_list_controls($listurl) {
  * against an array of * wildchar patterns
  */
 function profiling_string_matches($string, $patterns) {
-    $patterns = explode(',', $patterns);
+   $patterns = preg_split("/\n|,/", $patterns);
     foreach ($patterns as $pattern) {
         // Trim and prepare pattern
         $pattern = str_replace('\*', '.*', preg_quote(trim($pattern), '~'));
@@ -473,7 +473,7 @@ function profiling_string_matches($string, $patterns) {
         if (empty($pattern)) {
             continue;
         }
-        if (preg_match('~' . $pattern . '~', $string)) {
+        if (preg_match('~^' . $pattern . '$~', $string)) {
             return true;
         }
     }