MDL-58297 filestorage: New functions for hash calculation
authorAndrew Nicols <andrew@nicols.co.uk>
Fri, 17 Mar 2017 07:19:10 +0000 (15:19 +0800)
committerAndrew Nicols <andrew@nicols.co.uk>
Fri, 7 Apr 2017 02:40:23 +0000 (10:40 +0800)
lib/filestorage/file_storage.php
lib/filestorage/file_system.php
lib/filestorage/file_system_filedir.php
lib/filestorage/stored_file.php
lib/filestorage/tests/file_storage_test.php
lib/filestorage/tests/file_system_filedir_test.php
lib/filestorage/tests/file_system_test.php
lib/filestorage/tests/tgz_packer_test.php

index 1cf24fc..8158bc5 100644 (file)
@@ -982,7 +982,7 @@ class file_storage {
         static $contenthash = null;
         if (!$contenthash) {
             $this->add_string_to_pool('');
-            $contenthash = sha1('');
+            $contenthash = self::hash_from_string('');
         }
 
         $now = time();
@@ -2323,4 +2323,24 @@ class file_storage {
         $data = array('id' => $referencefileid, 'lastsync' => $lastsync);
         $DB->update_record('files_reference', (object)$data);
     }
+
+    /**
+     * Calculate and return the contenthash of the supplied file.
+     *
+     * @param   string $filepath The path to the file on disk
+     * @return  string The file's content hash
+     */
+    public static function hash_from_path($filepath) {
+        return sha1_file($filepath);
+    }
+
+    /**
+     * Calculate and return the contenthash of the supplied content.
+     *
+     * @param   string $content The file content
+     * @return  string The file's content hash
+     */
+    public static function hash_from_string($content) {
+        return sha1($content);
+    }
 }
index bfee09c..3da4171 100644 (file)
@@ -182,7 +182,7 @@ abstract class file_system {
      * @return bool
      */
     public function is_file_readable_locally_by_hash($contenthash, $fetchifnotfound = false) {
-        if ($contenthash === sha1('')) {
+        if ($contenthash === file_storage::hash_from_string('')) {
             // Files with empty size are either directories or empty.
             // We handle these virtually.
             return true;
@@ -203,7 +203,7 @@ abstract class file_system {
      * @return bool
      */
     public function is_file_readable_remotely_by_hash($contenthash) {
-        if ($contenthash === sha1('')) {
+        if ($contenthash === file_storage::hash_from_string('')) {
             // Files with empty size are either directories or empty.
             // We handle these virtually.
             return true;
@@ -247,8 +247,8 @@ abstract class file_system {
     protected static function is_file_removable($contenthash) {
         global $DB;
 
-        if ($contenthash === sha1('')) {
-            // No need to delete empty content file with sha1('') content hash.
+        if ($contenthash === file_storage::hash_from_string('')) {
+            // No need to delete files without content.
             return false;
         }
 
index ff7a440..82f6b5c 100644 (file)
@@ -262,7 +262,7 @@ class file_system_filedir extends file_system {
             $trashfile = $alttrashfile;
         }
 
-        if (filesize($trashfile) != $file->get_filesize() or sha1_file($trashfile) != $contenthash) {
+        if (filesize($trashfile) != $file->get_filesize() or file_storage::hash_from_path($trashfile) != $contenthash) {
             // The files are different. Leave this one in trash - something seems to be wrong with it.
             return false;
         }
@@ -356,9 +356,9 @@ class file_system_filedir extends file_system {
         }
 
         if (is_null($contenthash)) {
-            $contenthash = sha1_file($pathname);
+            $contenthash = file_storage::hash_from_path($pathname);
         } else if ($CFG->debugdeveloper) {
-            $filehash = sha1_file($pathname);
+            $filehash = file_storage::hash_from_path($pathname);
             if ($filehash === false) {
                 throw new file_exception('storedfilecannotread', '', $pathname);
             }
@@ -372,16 +372,16 @@ class file_system_filedir extends file_system {
             throw new file_exception('storedfilecannotread', '', $pathname);
         }
 
-        if ($filesize > 0 and $contenthash === sha1('')) {
-            // Did the file change or is sha1_file() borked for this file?
+        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 = sha1_file($pathname);
+            $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 === sha1('')) {
+            if ($filesize > 0 and $contenthash === file_storage::hash_from_string('')) {
                 // This is very weird...
                 throw new file_exception('storedfilecannotread', '', $pathname);
             }
@@ -396,8 +396,8 @@ class file_system_filedir extends file_system {
             if (filesize($hashfile) === $filesize) {
                 return array($contenthash, $filesize, false);
             }
-            if (sha1_file($hashfile) === $contenthash) {
-                // Jackpot! We have a sha1 collision.
+            if (file_storage::hash_from_path($hashfile) === $contenthash) {
+                // Jackpot! We have a hash collision.
                 mkdir("$this->filedir/jackpot/", $this->dirpermissions, true);
                 copy($pathname, "$this->filedir/jackpot/{$contenthash}_1");
                 copy($hashfile, "$this->filedir/jackpot/{$contenthash}_2");
@@ -425,7 +425,7 @@ class file_system_filedir extends file_system {
             ignore_user_abort($prev);
             throw new file_exception('storedfilecannotcreatefile');
         }
-        if (sha1_file($hashfile.'.tmp') !== $contenthash) {
+        if (file_storage::hash_from_path($hashfile.'.tmp') !== $contenthash) {
             // Highly unlikely edge case, but this can happen on an NFS volume with no space remaining.
             @unlink($hashfile.'.tmp');
             ignore_user_abort($prev);
@@ -452,7 +452,7 @@ class file_system_filedir extends file_system {
     public function add_file_from_string($content) {
         global $CFG;
 
-        $contenthash = sha1($content);
+        $contenthash = file_storage::hash_from_string($content);
         // Binary length.
         $filesize = strlen($content);
 
@@ -465,8 +465,8 @@ class file_system_filedir extends file_system {
             if (filesize($hashfile) === $filesize) {
                 return array($contenthash, $filesize, false);
             }
-            if (sha1_file($hashfile) === $contenthash) {
-                // Jackpot! We have a sha1 collision.
+            if (file_storage::hash_from_path($hashfile) === $contenthash) {
+                // Jackpot! We have a hash collision.
                 mkdir("$this->filedir/jackpot/", $this->dirpermissions, true);
                 copy($hashfile, "$this->filedir/jackpot/{$contenthash}_1");
                 file_put_contents("$this->filedir/jackpot/{$contenthash}_2", $content);
index 192e7b8..789a2c8 100644 (file)
@@ -991,4 +991,24 @@ class stored_file {
         // Generate the resized image.
         return resize_image_from_image($original, $imageinfo, $width, $height);
     }
+
+    /**
+     * Check whether the supplied file is the same as this file.
+     *
+     * @param   string $path The path to the file on disk
+     * @return  boolean
+     */
+    public function compare_to_path($path) {
+        return $this->get_contenthash() === file_storage::hash_from_path($path);
+    }
+
+    /**
+     * Check whether the supplied content is the same as this file.
+     *
+     * @param   string $content The file content
+     * @return  boolean
+     */
+    public function compare_to_string($content) {
+        return $this->get_contenthash() === file_storage::hash_from_string($content);
+    }
 }
index 46e97fc..08febf3 100644 (file)
@@ -59,7 +59,7 @@ class core_files_file_storage_testcase extends advanced_testcase {
         $file = $fs->create_file_from_string($filerecord, $content);
 
         $this->assertInstanceOf('stored_file', $file);
-        $this->assertSame(sha1($content), $file->get_contenthash());
+        $this->assertTrue($file->compare_to_string($content));
         $this->assertSame($pathhash, $file->get_pathnamehash());
 
         $this->assertTrue($DB->record_exists('files', array('pathnamehash'=>$pathhash)));
@@ -132,7 +132,7 @@ class core_files_file_storage_testcase extends advanced_testcase {
         $file = $fs->create_file_from_pathname($filerecord, $filepath);
 
         $this->assertInstanceOf('stored_file', $file);
-        $this->assertSame(sha1_file($filepath), $file->get_contenthash());
+        $this->assertTrue($file->compare_to_path($filepath));
 
         $this->assertTrue($DB->record_exists('files', array('pathnamehash'=>$pathhash)));
 
index 1f65099..6f4ee64 100644 (file)
@@ -99,7 +99,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
      * @return stored_file
      */
     protected function get_stored_file($filecontent, $filename = null, $mockedmethods = null) {
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         if (empty($filename)) {
             $filename = $contenthash;
         }
@@ -199,7 +199,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
      */
     public function test_get_remote_path_from_hash() {
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $expectedresult = (object) [];
 
         $fs = $this->get_testable_mock([
@@ -408,7 +408,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         global $CFG;
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $filedircontent = [
             $contenthash => $filecontent,
         ];
@@ -445,7 +445,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         // Setup the filedir.
         // This contains a virtual file which has a cache mismatch.
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $trashdircontent = [
             '0f' => [
@@ -483,7 +483,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         // Setup the filedir.
         // This contains a virtual file which has a cache mismatch.
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $filedircontent = $trashdircontent = [
             '0f' => [
@@ -520,7 +520,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         // Setup the filedir.
         // This contains a virtual file which has a cache mismatch.
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $trashdircontent = [
             '0f' => [
@@ -555,7 +555,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         // Setup the filedir.
         // This contains a virtual file which has a cache mismatch.
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $trashdircontent = [
             '0f' => [
@@ -591,7 +591,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         // Setup the filedir.
         // This contains a virtual file which has a cache mismatch.
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $trashdircontent = [
             $contenthash => $filecontent,
@@ -622,7 +622,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         $this->resetAfterTest();
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $filedircontent = [
             '0f' => [],
         ];
@@ -660,7 +660,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         // Setup the filedir.
         // This contains a virtual file which has a cache mismatch.
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $sourcedircontent = [
             'file' => $filecontent,
         ];
@@ -708,7 +708,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         $this->resetAfterTest();
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $sourcedir = [
             'file' => $filecontent,
         ];
@@ -728,7 +728,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         $this->resetAfterTest();
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $filedircontent = [
             '0f' => [
                 'f3' => [
@@ -771,7 +771,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         $this->resetAfterTest();
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $filedircontent = [
             '0f' => [],
         ];
@@ -804,7 +804,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         global $CFG;
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $vfileroot = $this->setup_vfile_root();
 
         // Note, the vfs file system does not support locks - prevent file locking here.
@@ -828,7 +828,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         $this->resetAfterTest();
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $filedircontent = [
             '0f' => [],
@@ -859,7 +859,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         global $CFG;
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $filedircontent = [
             '0f' => [
                 'f3' => [
@@ -890,7 +890,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         $this->resetAfterTest();
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $vfileroot = $this->setup_vfile_root();
 
         $fs = new file_system_filedir();
@@ -911,7 +911,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         $this->resetAfterTest();
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $filedircontent = $trashdircontent = [
             '0f' => [
@@ -947,7 +947,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
 
         $fs = new file_system_filedir();
 
-        $result = $fs->remove_file(sha1(''));
+        $result = $fs->remove_file(file_storage::hash_from_string(''));
         $this->assertNull($result);
     }
 
@@ -960,7 +960,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         global $DB;
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $filedircontent = [
             '0f' => [
                 'f3' => [
@@ -991,7 +991,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         global $DB;
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $filedircontent = [
             '0f' => [
                 'f3' => [
@@ -1020,7 +1020,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase {
         $this->resetAfterTest();
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $filedircontent = $trashdircontent = [
             '0f' => [
index 2aeff76..89162f4 100644 (file)
@@ -70,7 +70,7 @@ class core_files_file_system_testcase extends advanced_testcase {
      * @return stored_file
      */
     protected function get_stored_file($filecontent, $filename = null, $mockedmethods = null) {
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         if (empty($filename)) {
             $filename = $contenthash;
         }
@@ -229,7 +229,7 @@ class core_files_file_system_testcase extends advanced_testcase {
         ]);
         $fs->expects($this->once())
             ->method('get_local_path_from_hash')
-            ->with($this->equalTo(sha1($filecontent)), $this->equalTo($fetch))
+            ->with($this->equalTo(file_storage::hash_from_string($filecontent)), $this->equalTo($fetch))
             ->willReturn($filepath);
 
         $file = $this->get_stored_file($filecontent);
@@ -256,7 +256,7 @@ class core_files_file_system_testcase extends advanced_testcase {
 
         $fs->expects($this->once())
             ->method('get_remote_path_from_hash')
-            ->with($this->equalTo(sha1($filecontent)), $this->equalTo(false))
+            ->with($this->equalTo(file_storage::hash_from_string($filecontent)), $this->equalTo(false))
             ->willReturn($filepath);
 
         $file = $this->get_stored_file($filecontent);
@@ -278,7 +278,7 @@ class core_files_file_system_testcase extends advanced_testcase {
      */
     public function test_is_file_readable_locally_by_hash() {
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $filepath = __FILE__;
 
         $fs = $this->get_testable_mock([
@@ -297,7 +297,7 @@ class core_files_file_system_testcase extends advanced_testcase {
      */
     public function test_is_file_readable_locally_by_hash_empty() {
         $filecontent = '';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $fs = $this->get_testable_mock([
             'get_local_path_from_hash',
@@ -314,7 +314,7 @@ class core_files_file_system_testcase extends advanced_testcase {
      */
     public function test_is_file_readable_remotely_by_hash() {
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $fs = $this->get_testable_mock([
             'get_remote_path_from_hash',
@@ -332,7 +332,7 @@ class core_files_file_system_testcase extends advanced_testcase {
      */
     public function test_is_file_readable_remotely_by_hash_empty() {
         $filecontent = '';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $fs = $this->get_testable_mock([
             'get_remote_path_from_hash',
@@ -349,7 +349,7 @@ class core_files_file_system_testcase extends advanced_testcase {
      */
     public function test_is_file_readable_remotely_by_hash_not_found() {
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $fs = $this->get_testable_mock([
             'get_remote_path_from_hash',
@@ -461,7 +461,7 @@ class core_files_file_system_testcase extends advanced_testcase {
      */
     public function test_is_file_removable_empty() {
         $filecontent = '';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $method = new ReflectionMethod(file_system::class, 'is_file_removable');
         $method->setAccessible(true);
@@ -477,7 +477,7 @@ class core_files_file_system_testcase extends advanced_testcase {
         global $DB;
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $DB = $this->getMockBuilder(\moodle_database::class)
             ->setMethods(['record_exists'])
@@ -499,7 +499,7 @@ class core_files_file_system_testcase extends advanced_testcase {
         global $DB;
 
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $DB = $this->getMockBuilder(\moodle_database::class)
             ->setMethods(['record_exists'])
@@ -937,7 +937,7 @@ class core_files_file_system_testcase extends advanced_testcase {
         $filepath = '/path/to/file/not/currently/on/disk';
         $filecontent = 'example content';
         $filename = 'test.jpg';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
 
         $fs = $this->get_testable_mock(['get_remote_path_from_hash']);
         $fs->method('get_remote_path_from_hash')->willReturn($filepath);
@@ -953,7 +953,7 @@ class core_files_file_system_testcase extends advanced_testcase {
     public function test_mimetype_from_hash_using_file_content() {
         $filepath = '/path/to/file/not/currently/on/disk';
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $filename = 'example';
 
         $filepath = __DIR__ . DIRECTORY_SEPARATOR . 'fixtures' . DIRECTORY_SEPARATOR . 'testimage.jpg';
@@ -971,7 +971,7 @@ class core_files_file_system_testcase extends advanced_testcase {
     public function test_mimetype_from_hash_using_file_content_remote() {
         $filepath = '/path/to/file/not/currently/on/disk';
         $filecontent = 'example content';
-        $contenthash = sha1($filecontent);
+        $contenthash = file_storage::hash_from_string($filecontent);
         $filename = 'example';
 
         $filepath = __DIR__ . DIRECTORY_SEPARATOR . 'fixtures' . DIRECTORY_SEPARATOR . 'testimage.jpg';
index 74dba64..3d66b75 100644 (file)
@@ -355,7 +355,7 @@ class core_files_tgz_packer_testcase extends advanced_testcase implements file_p
         $packer = get_file_packer('application/x-gzip');
         $result = $packer->archive_to_pathname($filelist, $archive, true, $this);
         $this->assertTrue($result);
-        $hashwith = sha1_file($archive);
+        $hashwith = file_storage::hash_from_path($archive);
 
         // List files.
         $files = $packer->list_files($archive);
@@ -382,7 +382,7 @@ class core_files_tgz_packer_testcase extends advanced_testcase implements file_p
         $packer->set_include_index(false);
         $result = $packer->archive_to_pathname($filelist, $archive, true, $this);
         $this->assertTrue($result);
-        $hashwithout = sha1_file($archive);
+        $hashwithout = file_storage::hash_from_path($archive);
         $files = $packer->list_files($archive);
         $this->assertEquals($expectedinfo, self::convert_info_for_assert($files));