MDL-59961 core_files: make content hash validation reusable

Other file systems could make use of this code as they all need to validate the content has and be able to read the file from disk.
gjb2048 · Aug 28, 2017 · 9eb1a2c · 9eb1a2c
1 parent 9eb3c17
commit 9eb1a2c
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 41 deletions.
diff --git a/lib/filestorage/file_system.php b/lib/filestorage/file_system.php
@@ -443,6 +443,60 @@ public function xsendfile($contenthash) {
         return xsendfile($this->get_remote_path_from_hash($contenthash));
     }
 
+    /**
+     * Validate that the content hash matches the content hash of the file on disk.
+     *
+     * @param string $contenthash The current content hash to validate
+     * @param string $pathname The path to the file on disk
+     * @return array The content hash (it might change) and file size
+     */
+    protected function validate_hash_and_file_size($contenthash, $pathname) {
+        global $CFG;
+
+        if (!is_readable($pathname)) {
+            throw new file_exception('storedfilecannotread', '', $pathname);
+        }
+
+        $filesize = filesize($pathname);
+        if ($filesize === false) {
+            throw new file_exception('storedfilecannotread', '', $pathname);
+        }
+
+        if (is_null($contenthash)) {
+            $contenthash = file_storage::hash_from_path($pathname);
+        } else if ($CFG->debugdeveloper) {
+            $filehash = file_storage::hash_from_path($pathname);
+            if ($filehash === false) {
+                throw new file_exception('storedfilecannotread', '', $pathname);
+            }
+            if ($filehash !== $contenthash) {
+                // Hopefully this never happens, if yes we need to fix calling code.
+                debugging("Invalid contenthash submitted for file $pathname", DEBUG_DEVELOPER);
+                $contenthash = $filehash;
+            }
+        }
+        if ($contenthash === false) {
+            throw new file_exception('storedfilecannotread', '', $pathname);
+        }
+
+        if ($filesize > 0 and $contenthash === file_storage::hash_from_string('')) {
+            // Did the file change or is file_storage::hash_from_path() borked for this file?
+            clearstatcache();
+            $contenthash = file_storage::hash_from_path($pathname);
+            $filesize    = filesize($pathname);
+
+            if ($contenthash === false or $filesize === false) {
+                throw new file_exception('storedfilecannotread', '', $pathname);
+            }
+            if ($filesize > 0 and $contenthash === file_storage::hash_from_string('')) {
+                // This is very weird...
+                throw new file_exception('storedfilecannotread', '', $pathname);
+            }
+        }
+
+        return [$contenthash, $filesize];
+    }
+
     /**
      * Add the supplied file to the file system.
      *

diff --git a/lib/filestorage/file_system_filedir.php b/lib/filestorage/file_system_filedir.php
@@ -344,48 +344,8 @@ protected function empty_trash() {
      * @return array (contenthash, filesize, newfile)
      */
     public function add_file_from_path($pathname, $contenthash = null) {
-        global $CFG;
-
-        if (!is_readable($pathname)) {
-            throw new file_exception('storedfilecannotread', '', $pathname);
-        }
-
-        $filesize = filesize($pathname);
-        if ($filesize === false) {
-            throw new file_exception('storedfilecannotread', '', $pathname);
-        }
-
-        if (is_null($contenthash)) {
-            $contenthash = file_storage::hash_from_path($pathname);
-        } else if ($CFG->debugdeveloper) {
-            $filehash = file_storage::hash_from_path($pathname);
-            if ($filehash === false) {
-                throw new file_exception('storedfilecannotread', '', $pathname);
-            }
-            if ($filehash !== $contenthash) {
-                // Hopefully this never happens, if yes we need to fix calling code.
-                debugging("Invalid contenthash submitted for file $pathname", DEBUG_DEVELOPER);
-                $contenthash = $filehash;
-            }
-        }
-        if ($contenthash === false) {
-            throw new file_exception('storedfilecannotread', '', $pathname);
-        }
-
-        if ($filesize > 0 and $contenthash === file_storage::hash_from_string('')) {
-            // Did the file change or is file_storage::hash_from_path() borked for this file?
-            clearstatcache();
-            $contenthash = file_storage::hash_from_path($pathname);
-            $filesize = filesize($pathname);
 
-            if ($contenthash === false or $filesize === false) {
-                throw new file_exception('storedfilecannotread', '', $pathname);
-            }
-            if ($filesize > 0 and $contenthash === file_storage::hash_from_string('')) {
-                // This is very weird...
-                throw new file_exception('storedfilecannotread', '', $pathname);
-            }
-        }
+        list($contenthash, $filesize) = $this->validate_hash_and_file_size($contenthash, $pathname);
 
         $hashpath = $this->get_fulldir_from_hash($contenthash);
         $hashfile = $this->get_local_path_from_hash($contenthash, false);