Skip to content

Commit

Permalink
MDL-59960 core_files: improve mimetype detection
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
polothy committed Aug 28, 2017
1 parent 9eb3c17 commit ee8bce2
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 26 deletions.
18 changes: 3 additions & 15 deletions lib/filestorage/file_system.php
Original file line number Diff line number Diff line change
Expand Up @@ -514,14 +514,13 @@ protected static function get_file_handle_for_path($path, $type = stored_file::F
* @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);
}

Expand All @@ -539,18 +538,7 @@ public function mimetype_from_storedfile($file) {
// 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());
}

/**
Expand Down
19 changes: 8 additions & 11 deletions lib/filestorage/tests/file_system_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -951,14 +951,13 @@ public function test_mimetype_from_hash_using_filename() {
* 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);
Expand Down Expand Up @@ -1023,8 +1022,8 @@ public function test_mimetype_from_storedfile_using_filename() {
*/
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');

Expand All @@ -1040,14 +1039,12 @@ public function test_mimetype_from_storedfile_using_file_content_remote() {
$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');

Expand Down

0 comments on commit ee8bce2

Please sign in to comment.