From ee8bce2d3bee326bf1f96035c27275c839fb2a17 Mon Sep 17 00:00:00 2001
From: Mark Nielsen <mark@moodlerooms.com>
Date: Mon, 28 Aug 2017 10:29:54 -0700
Subject: [PATCH] MDL-59960 core_files: improve mimetype detection

Improve mimetype detection for remote files that
have no file extension.  The mimetype detection
that makes use of the file, only works with local
files, so do not use the remote path which can be
a URL or stream.
---
 lib/filestorage/file_system.php            | 18 +++---------------
 lib/filestorage/tests/file_system_test.php | 19 ++++++++-----------
 2 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/lib/filestorage/file_system.php b/lib/filestorage/file_system.php
index 3da41716ef6..fb9898ff157 100644
--- a/lib/filestorage/file_system.php
+++ b/lib/filestorage/file_system.php
@@ -514,14 +514,13 @@ abstract class file_system {
      * @return string The MIME type.
      */
     public function mimetype_from_hash($contenthash, $filename) {
-        $pathname = $this->get_remote_path_from_hash($contenthash);
+        $pathname = $this->get_local_path_from_hash($contenthash);
         $mimetype = file_storage::mimetype($pathname, $filename);
 
-        if (!$this->is_file_readable_locally_by_hash($contenthash, false) && $mimetype === 'document/unknown') {
+        if ($mimetype === 'document/unknown' && !$this->is_file_readable_locally_by_hash($contenthash)) {
             // The type is unknown, but the full checks weren't completed because the file isn't locally available.
             // Ensure we have a local copy and try again.
             $pathname = $this->get_local_path_from_hash($contenthash, true);
-
             $mimetype = file_storage::mimetype_from_file($pathname);
         }
 
@@ -539,18 +538,7 @@ abstract class file_system {
             // Files with an empty filesize are treated as directories and have no mimetype.
             return null;
         }
-        $pathname = $this->get_remote_path_from_storedfile($file);
-        $mimetype = file_storage::mimetype($pathname, $file->get_filename());
-
-        if (!$this->is_file_readable_locally_by_storedfile($file) && $mimetype === 'document/unknown') {
-            // The type is unknown, but the full checks weren't completed because the file isn't locally available.
-            // Ensure we have a local copy and try again.
-            $pathname = $this->get_local_path_from_storedfile($file, true);
-
-            $mimetype = file_storage::mimetype_from_file($pathname);
-        }
-
-        return $mimetype;
+        return $this->mimetype_from_hash($file->get_contenthash(), $file->get_filename());
     }
 
     /**
diff --git a/lib/filestorage/tests/file_system_test.php b/lib/filestorage/tests/file_system_test.php
index adf539f5f5d..038cad1a317 100644
--- a/lib/filestorage/tests/file_system_test.php
+++ b/lib/filestorage/tests/file_system_test.php
@@ -951,14 +951,13 @@ class core_files_file_system_testcase extends advanced_testcase {
      * a locally available file whose filename does not suggest mimetype.
      */
     public function test_mimetype_from_hash_using_file_content() {
-        $filepath = '/path/to/file/not/currently/on/disk';
         $filecontent = 'example content';
         $contenthash = file_storage::hash_from_string($filecontent);
         $filename = 'example';
 
         $filepath = __DIR__ . "/fixtures/testimage.jpg";
-        $fs = $this->get_testable_mock(['get_remote_path_from_hash']);
-        $fs->method('get_remote_path_from_hash')->willReturn($filepath);
+        $fs = $this->get_testable_mock(['get_local_path_from_hash']);
+        $fs->method('get_local_path_from_hash')->willReturn($filepath);
 
         $result = $fs->mimetype_from_hash($contenthash, $filename);
         $this->assertEquals('image/jpeg', $result);
@@ -1023,8 +1022,8 @@ class core_files_file_system_testcase extends advanced_testcase {
      */
     public function test_mimetype_from_storedfile_using_file_content() {
         $filepath = __DIR__ . "/fixtures/testimage.jpg";
-        $fs = $this->get_testable_mock(['get_remote_path_from_storedfile']);
-        $fs->method('get_remote_path_from_storedfile')->willReturn($filepath);
+        $fs = $this->get_testable_mock(['get_local_path_from_hash']);
+        $fs->method('get_local_path_from_hash')->willReturn($filepath);
 
         $file = $this->get_stored_file('example content');
 
@@ -1040,14 +1039,12 @@ class core_files_file_system_testcase extends advanced_testcase {
         $filepath = __DIR__ . "/fixtures/testimage.jpg";
 
         $fs = $this->get_testable_mock([
-            'get_remote_path_from_storedfile',
-            'is_file_readable_locally_by_storedfile',
-            'get_local_path_from_storedfile',
+            'is_file_readable_locally_by_hash',
+            'get_local_path_from_hash',
         ]);
 
-        $fs->method('get_remote_path_from_storedfile')->willReturn('/path/to/remote/file');
-        $fs->method('is_file_readable_locally_by_storedfile')->willReturn(false);
-        $fs->method('get_local_path_from_storedfile')->willReturn($filepath);
+        $fs->method('is_file_readable_locally_by_hash')->willReturn(false);
+        $fs->method('get_local_path_from_hash')->will($this->onConsecutiveCalls('/path/to/remote/file', $filepath));
 
         $file = $this->get_stored_file('example content');