From a09d45f767024cae24af64d0e14e8d7d6bb795f3 Mon Sep 17 00:00:00 2001
From: Matthew Hilton <matthewhilton@catalyst-au.net>
Date: Fri, 8 Mar 2024 10:50:29 +1000
Subject: [PATCH] drag and drop handler registration performance fix

---
 course/dnduploadlib.php      | 110 ++++++++++++++++++++---------
 lib/classes/filetypes.php    | 131 ++++++++++++++++++++++++++---------
 lib/filelib.php              |  23 ++----
 lib/form/filemanager.php     |   8 +--
 lib/form/filepicker.php      |   9 +--
 lib/tests/filetypes_test.php |  54 +++++++--------
 repository/lib.php           |   4 +-
 7 files changed, 211 insertions(+), 128 deletions(-)

diff --git a/course/dnduploadlib.php b/course/dnduploadlib.php
index 5320c02d676..17d6bdaea45 100644
--- a/course/dnduploadlib.php
+++ b/course/dnduploadlib.php
@@ -128,6 +128,7 @@ class dndupload_handler {
                         get_string('nameforpage', 'moodle'), get_string('whatforpage', 'moodle'), 30);
 
         $this->context = context_course::instance($course->id);
+        $filehandlerstoregister = [];
 
         // Loop through all modules to find handlers.
         $mods = get_plugin_list_with_function('mod', 'dndupload_register');
@@ -145,7 +146,10 @@ class dndupload_handler {
             }
             if (isset($resp['files'])) {
                 foreach ($resp['files'] as $file) {
-                    $this->register_file_handler($file['extension'], $modname, $file['message']);
+                    // Collect and build them all at once at the end of the loop.
+                    $toregister = (object) $file;
+                    $toregister->module = $modname;
+                    $filehandlerstoregister[] = $toregister;
                 }
             }
             if (isset($resp['addtypes'])) {
@@ -170,6 +174,77 @@ class dndupload_handler {
             }
             $PAGE->requires->string_for_js('pluginname', $modname);
         }
+        $this->filehandlers = $this->build_all_handlers($filehandlerstoregister);
+    }
+
+    /**
+     * Given the raw definitions as defined by the modules,
+     * apply any site-wide restrictions and return
+     *
+     * @param array $rawdefinitions array of stdClass with extension, message and modname
+     * @return array of stdClass with the site wide restrictions applied and the extensions normalisd
+     */
+    protected function build_all_handlers($rawdefinitions) {
+        $util = new core_form\filetypes_util();
+
+        $definitions = [];
+
+        foreach ($rawdefinitions as $definition) {
+            // Normalise, e.g. get rid of whitespace.
+            $extension = $util->normalize_file_types([$definition->extension]);
+
+            // Split any types e.g. web_audio into their extensions e.g. .mp3, .ogg, etc...
+            $extensions = core_filetypes::file_split_types_to_exts($extension);
+
+            foreach ($extensions as $extension) {
+                // For each, duplicate it into the raw extension.
+                $duplicated = $definition;
+                $duplicated->extension = $extension;
+                $definitions[] = $duplicated;
+            }
+        }
+
+        // Apply site wide filter (if enabled).
+        $sitewidetypes = array_unique(core_filetypes::file_split_types_to_exts(core_filetypes::file_get_site_restrictions()));
+
+        if (!in_array('*', $sitewidetypes)) {
+            // First we need to check for a special case where an definition accepts '*' but the filter is enabled.
+            // We must replace '*' instead with every allowed file type.
+            $expandeddefinitions = array_map(function($definition) use ($sitewidetypes) {
+                // Not affected, return as-is.
+                if ($definition->extension != '*') {
+                    return [$definition];
+                }
+
+                // Else definition is '*',
+                // Return a copy of the message, but with each allowed extension.
+                return array_map(function($allowedextension) use ($definition) {
+                    $duplicated = clone $definition;
+                    $duplicated->extension = $allowedextension;
+                    return $duplicated;
+                }, $sitewidetypes);
+
+            }, $definitions);
+
+            // Flatten.
+            $definitions = array_merge(...$expandeddefinitions);
+
+            // Apply the site wide filter to each extension.
+            $definitions = array_filter($definitions, function($definition) use ($sitewidetypes) {
+                return in_array($definition->extension, $sitewidetypes);
+            });
+        }
+
+        // Before returning, remove the . in the extension.
+        $definitions = array_map(function($definition) {
+            $definition->extension = str_replace('.', '', $definition->extension);
+            return $definition;
+        }, $definitions);
+
+        // Use array_values to remove indexing on the array.
+        $definitions = array_values($definitions);
+
+        return $definitions;
     }
 
     /**
@@ -229,39 +304,6 @@ class dndupload_handler {
         $this->types[$type]->handlers[] = $add;
     }
 
-    /**
-     * Used to declare that a particular module will handle a particular type
-     * of dropped file
-     *
-     * @param string $extension The file extension to handle ('*' for all types)
-     * @param string $module The name of the module to handle this type
-     * @param string $message The message to show the user if more than one handler is registered
-     *                        for a type and the user needs to make a choice between them
-     */
-    protected function register_file_handler($extension, $module, $message) {
-        $extension = strtolower($extension);
-
-        // If filetype restrictions exist, apply to extension.
-        // If type is all, split to allowed extensions, and register a handler for each one.
-        // Else, only apply handler if extension after restrictions is not empty.
-        $allowed = core_filetypes::file_apply_siterestrictions($extension, true);
-        if ($extension === '*' && $allowed !== '*') {
-            foreach ($allowed as $allowedfile) {
-                // Remove . from filename.
-                $allowedfile = preg_replace('/\./', '', $allowedfile);
-                // Recursively register filehandler for individual extensions.
-                $this->register_file_handler($allowedfile, $module, $message);
-            }
-            return;
-        } else if (!empty($allowed)) {
-            $add = new stdClass;
-            $add->extension = $extension;
-            $add->module = $module;
-            $add->message = $message;
-            $this->filehandlers[] = $add;
-        }
-    }
-
     /**
      * Check to see if the type has been registered
      *
diff --git a/lib/classes/filetypes.php b/lib/classes/filetypes.php
index 456e77c197f..d5f46964c2e 100644
--- a/lib/classes/filetypes.php
+++ b/lib/classes/filetypes.php
@@ -753,7 +753,7 @@ abstract class core_filetypes {
     public static function file_split_types_to_exts($types) {
         $util = new core_form\filetypes_util();
         $mimetypes = get_mimetypes_array();
-        $return = array();
+        $return = [];
 
         foreach ($types as $type) {
             // Filter for where group matches type, only keep extension.
@@ -781,48 +781,111 @@ abstract class core_filetypes {
     }
 
     /**
-     * Returns an array of file extensions for use with filemanager and filepicker.
-     * This function is used to apply sitewide file restrictions, and breaks filetype groups
-     * apart to allow only approved filetypes.
-     *
-     * @param array|string $allowedtypes an array of allowed file extensions or groups, or '*' for all.
-     * @param bool $splittypes whether the parent type restrictions (e.g. web_image) should always be split to extensions.
-     * @return array|string the array of all allowed filetypes, following site restriction application , or '*' for all.
+     * Returns the site-wide file restriction filters after normalisation.
+     * @return array Site wide file extension filters. May be '*', types such as 'web_image' or extensions such as '.mp3'
      */
-    public static function file_apply_siterestrictions($allowedtypes, $splittypes = false) {
+    public static function file_get_site_restrictions() {
         global $CFG;
+        $rawvalue = $CFG->allowedfiletypes;
 
-        $util = new core_form\filetypes_util();
-        $sitewidetypes = $util->normalize_file_types($CFG->allowedfiletypes);
-        if (empty($sitewidetypes)) {
-            $sitewidetypes = '*';
+        if (empty($rawvalue) || strpos($rawvalue, '*') !== false) {
+            return ['*'];
         }
 
-        // If parent should always be split to extensions.
-        if ($splittypes) {
-            if (is_array($sitewidetypes)) {
-                // Remove any dupes if types is an array.
-                $sitewidetypes = self::file_split_types_to_exts(array_unique($sitewidetypes));
-            }
+        $util = new core_form\filetypes_util();
+        $sitewidetypes = $util->normalize_file_types($rawvalue);
+
+        return $sitewidetypes;
+    }
+
+    /**
+     * Determines if a given extension is allowed by the given filters
+     * @param array $allowedextensionsandtypes file extensions or types, e.g. '.mp3', '*', or 'web_image'.
+     * If empty, everything is allowed.
+     * @return bool true if allowed, else false.
+     */
+    public static function is_allowed($extension, $allowedextensionsandtypes) {
+        // Everything allowed - no need to check any further.
+        if (in_array('*', $allowedextensionsandtypes) || empty($allowedextensionsandtypes)) {
+            return true;
         }
 
-        // Cast single case to array, in case it's not already.
-        $allowedtypes = (array) $allowedtypes;
+        // Normalise the extension.
+        $util = new core_form\filetypes_util();
+        $extension = current($util->normalize_file_types([$extension]));
 
-        if ($allowedtypes === array('*') || $allowedtypes === array()) {
-            // If no restrictions exist on this repository type.
-            return $sitewidetypes;
-        } else if ($sitewidetypes === '*') {
-            // If no sitewide restrictions are imposed, return allowedtypes.
-            return $allowedtypes;
-        } else {
-            // If file type restrictions already exist.
-            // Convert sitewide and allowed to extensions only, remove any duplicates.
-            $sitewidetypes = self::file_split_types_to_exts(array_unique($sitewidetypes));
-            $allowedtypes = self::file_split_types_to_exts(array_unique($allowedtypes));
+        // Expand any types to extensions.
+        $allextsallowed = array_unique(self::file_split_types_to_exts($allowedextensionsandtypes));
+
+        return in_array($extension, $allextsallowed);
+    }
+
+    /**
+     * Filters the given extensions by the given filters
+     * @param array $rawextensionsandtypes file extensions or types, e.g. '.mp3', '*', or 'web_image'
+     * @param array $filterextensionsandtypes file extensions or types, e.g. '.mp3', '*', or 'web_image'
+     * @return array file extensions available after filtering. May be '*', types such as 'web_image' or extensions such as '.mp3'
+     */
+    public static function filter_extensions_and_types($rawextensionsandtypes, $filterextensionsandtypes) {
+        // Normalise the incoming rawextensionsandtypes and filterextensionsandtypes.
+        $util = new core_form\filetypes_util();
+        $rawextensionsandtypes = $util->normalize_file_types($rawextensionsandtypes);
+        $filterextensionsandtypes = $util->normalize_file_types($filterextensionsandtypes);
 
-            // Filter extensions for types allowed by sitewide restrictions then reset indices for order.
-            return array_values(array_intersect($allowedtypes, $sitewidetypes));
+        // Filter is set to allow all, so return everything as-is.
+        if (in_array('*', $filterextensionsandtypes) || empty($filterextensionsandtypes)) {
+            return $rawextensionsandtypes;
         }
+
+        // If raw allows all (filtering is active), only return what filter allows.
+        if (in_array('*', $rawextensionsandtypes)) {
+            return $filterextensionsandtypes;
+        }
+
+        // Generate a list of every extension possible by the given filter.
+        $allextsallowed = array_map(fn($val) => self::file_split_types_to_exts([$val]), $filterextensionsandtypes);
+        $allextsallowed = array_unique(array_merge(...array_values($allextsallowed)));
+
+        // Find the intersect taking into account types and extensions.
+        $result = array_map(function($extensionortype) use ($allextsallowed, $filterextensionsandtypes) {
+            $isextension = strpos($extensionortype, '.') === 0;
+
+            // If is extension, simply check if it is in the list of all extensions allowed.
+            if ($isextension && in_array($extensionortype, $allextsallowed)) {
+                return [$extensionortype];
+            }
+
+            // If is a type, and the type is entirely allowed by the filter, return as-is.
+            if (!$isextension && in_array($extensionortype, $filterextensionsandtypes)) {
+                return [$extensionortype];
+            }
+
+            // If is a type, and the filters partially allow its types, return what is partially allowed.
+            if (!$isextension) {
+                // Find the intersection of what this type maps to, and what extensions are allowed in the filter.
+                $allowedbytype = self::file_split_types_to_exts([$extensionortype]);
+                $allowedfromgiven = array_intersect($allowedbytype, $allextsallowed);
+                return array_values($allowedfromgiven);
+            }
+
+            // Else not allowed - return nothing.
+            return [];
+        }, $rawextensionsandtypes);
+
+        // Flatten.
+        $result = array_merge(...$result);
+
+        // Use array-values to zero index the array.
+        return array_values($result);
+    }
+
+    /**
+     * Applies site file restrictions to given list of extensions and types
+     * @param array $rawextensionsandtypes file extensions or types, e.g. '.mp3', '*', or 'web_image'
+     * @return array file extensions available after filtering. May be '*', types such as 'web_image' or extensions such as '.mp3'
+     */
+    public static function file_apply_siterestrictions($rawextensionsortypes) {
+        $sitefilter = self::file_get_site_restrictions();
+        return self::filter_extensions_and_types($rawextensionsortypes, $sitefilter);
     }
 }
diff --git a/lib/filelib.php b/lib/filelib.php
index 7345abdf8c8..7eb46061b5b 100644
--- a/lib/filelib.php
+++ b/lib/filelib.php
@@ -1147,9 +1147,7 @@ function file_save_draft_area_files($draftitemid, $contextid, $component, $filea
     }
 
     // Sitewide filetype check.
-    if (!empty($CFG->allowedfiletypes)) {
-        $options['accepted_types'] = core_filetypes::file_apply_siterestrictions($options['accepted_types'], true);
-    }
+    $options['accepted_types'] = core_filetypes::file_apply_siterestrictions($options['accepted_types']);
 
     // Check if the user has copy-pasted from other draft areas. Those files will be located in different draft
     // areas and need to be copied into the current draft area.
@@ -1194,19 +1192,12 @@ function file_save_draft_area_files($draftitemid, $contextid, $component, $filea
                     continue;
                 }
 
-                // Only delete bad files if allowed filetypes is setup.
-                if (!empty($CFG->allowedfiletypes) && isset($options['accepted_types']) && $options['accepted_types'] != '*') {
-                    $allowedtypes = $options['accepted_types'];
-                    $name = $file->get_filename();
-                    // Check for directories.
-                    if ($name != '.') {
-                        // If extension isn't in allowed list, skip saving file.
-                        $extension = strtolower('.' . pathinfo($name, PATHINFO_EXTENSION));
-                        if (!in_array($extension, $allowedtypes)) {
-                            $filedeleted = true;
-                            continue;
-                        }
-                    }
+                // Check if allowed via whitelist.
+                $name = $file->get_filename();
+                $extension = strtolower('.' . pathinfo($name, PATHINFO_EXTENSION));
+                if (!core_filetypes::is_allowed($extension, $options['accepted_types'] ?? ['*'])) {
+                    $filedeleted = true;
+                    continue;
                 }
 
                 $filecount++;
diff --git a/lib/form/filemanager.php b/lib/form/filemanager.php
index 1cf035e0cba..17900a976ba 100644
--- a/lib/form/filemanager.php
+++ b/lib/form/filemanager.php
@@ -75,9 +75,7 @@ class MoodleQuickForm_filemanager extends HTML_QuickForm_element implements temp
             }
         }
 
-        if (!empty($CFG->allowedfiletypes)) {
-            $this->_options['accepted_types'] = \core_filetypes::file_apply_siterestrictions($this->_options['accepted_types']);
-        }
+        $this->_options['accepted_types'] = \core_filetypes::file_apply_siterestrictions($this->_options['accepted_types']);
 
         if (!empty($options['maxbytes'])) {
             $this->_options['maxbytes'] = get_user_max_upload_file_size($PAGE->context, $CFG->maxbytes, $options['maxbytes']);
@@ -440,9 +438,7 @@ class form_filemanager implements renderable {
         $this->options->filecount = $filecount;
 
         // Sitewide file restriction check.
-        if (!empty($CFG->allowedfiletypes)) {
-            $options->accepted_types = \core_filetypes::file_apply_siterestrictions($options->accepted_types);
-        }
+        $options->accepted_types = \core_filetypes::file_apply_siterestrictions($options->accepted_types);
 
         // copying other options
         foreach ($options as $name=>$value) {
diff --git a/lib/form/filepicker.php b/lib/form/filepicker.php
index 9d29f80311f..0db757324dc 100644
--- a/lib/form/filepicker.php
+++ b/lib/form/filepicker.php
@@ -85,14 +85,7 @@ class MoodleQuickForm_filepicker extends HTML_QuickForm_input implements templat
         }
 
         // Sitewide filecheck.
-        if (!empty($CFG->allowedfiletypes)) {
-            // Check for empty options array, treat as all.
-            if ($options === array ()) {
-                $this->_options['accepted_types'] = \core_filetypes::file_apply_siterestrictions('*');
-            } else {
-                $this->_options['accepted_types'] = \core_filetypes::file_apply_siterestrictions($options['accepted_types']);
-            }
-        }
+        $this->_options['accepted_types'] = \core_filetypes::file_apply_siterestrictions($options['accepted_types'] ?? ['*']);
 
         $this->_options['maxbytes'] = get_user_max_upload_file_size($PAGE->context, $CFG->maxbytes, $coursemaxbytes, $fpmaxbytes);
         $this->_type = 'filepicker';
diff --git a/lib/tests/filetypes_test.php b/lib/tests/filetypes_test.php
index 1e8603bea29..d93627449db 100644
--- a/lib/tests/filetypes_test.php
+++ b/lib/tests/filetypes_test.php
@@ -289,62 +289,62 @@ class filetypes_test extends \advanced_testcase {
         // Set 2 types of files allowed, PDF and PNG.
         $CFG->allowedfiletypes = 'pdf, png';
         // Test default options,  '*'.
-        $expected = array('.pdf', '.png');
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions('*'));
+        $expected = ['.pdf', '.png'];
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['*']));
         // Test default as an array.
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(array('*')));
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['*']));
 
         // Test filegroup 'web_image'.
         // This will split the filegroups only extensions inside the filegroup,
         // AND inside allowedfiletypes.
         // PDF won't be allowed here.
-        $expected = array('.png');
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions('web_image'));
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(array('web_image')));
+        $expected = ['.png'];
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['web_image']));
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['web_image']));
 
         // Test multiple file groups.
-        $expected = array('.png', '.pdf');
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(array('web_image', 'document')));
+        $expected = ['.png', '.pdf'];
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['web_image', 'document']));
 
         // Test that calling twice doesn't produce garbage (e.g. inherited options filemanager->filepicker).
-        $expected = array('.png', '.pdf');
-        $firstrun = core_filetypes::file_apply_siterestrictions(array('web_image', 'document'));
+        $expected = ['.png', '.pdf'];
+        $firstrun = core_filetypes::file_apply_siterestrictions(['web_image', 'document']);
         $this->assertEquals($expected, $firstrun);
         $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions($firstrun));
 
         // Test that with no available filetypes, empty array returned.
         $CFG->allowedfiletypes = 'pdf';
-        $expected = array();
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(array('web_image')));
+        $expected = [];
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['web_image']));
 
         // Test empty array returned for empty input.
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(array('')));
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(''));
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['']));
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['']));
 
         // Test config with groups.
         $CFG->allowedfiletypes = 'html_track';
-        $expected = array('.vtt');
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(array('pdf', 'vtt', 'mp3')));
+        $expected = ['.vtt'];
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['pdf', 'vtt', 'mp3']));
 
         // Test no split of groups for all filetypes, as there is nothing to filter.
-        $expected = array('html_track');
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(array('*')));
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions('*'));
+        $expected = ['html_track'];
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['*']));
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['*']));
 
         // Multiple type splits.
         $CFG->allowedfiletypes = 'document';
-        $expected = array('.doc', '.docx');
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(array('doc', 'docx')));
-        $expected = array('.doc');
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions('doc'));
+        $expected = ['.doc', '.docx'];
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['doc', 'docx']));
+        $expected = ['.doc'];
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['doc']));
 
         // Check duplicate extensions are handled correctly.
-        $expected = array('.doc', '.docx');
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(array('doc', 'doc', 'docx')));
+        $expected = ['.doc', '.docx'];
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['doc', 'doc', 'docx']));
 
         // Unset allowedfiletypes, and check that splitting is handled correctly.
         $CFG->allowedfiletypes = '';
-        $expected = '*';
-        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions('*', true));
+        $expected = ['*'];
+        $this->assertEquals($expected, core_filetypes::file_apply_siterestrictions(['*']));
     }
 }
diff --git a/repository/lib.php b/repository/lib.php
index 6da86bbf6ea..b50e06f2d9c 100644
--- a/repository/lib.php
+++ b/repository/lib.php
@@ -3157,9 +3157,7 @@ function initialise_filepicker($args) {
     }
 
     // Sitewide filetype check.
-    if (!empty($CFG->allowedfiletypes)) {
-        $args->accepted_types = core_filetypes::file_apply_siterestrictions($args->accepted_types);
-    }
+    $args->accepted_types = core_filetypes::file_apply_siterestrictions($args->accepted_types);
 
     $user_context = context_user::instance($USER->id);
 
-- 
2.34.1

