Skip to content

Commit

Permalink
Fix MIME guessing of extension from type
Browse files Browse the repository at this point in the history
  • Loading branch information
paulbalandan committed Jun 1, 2022
1 parent c740fbf commit 8a11ced
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 18 deletions.
19 changes: 8 additions & 11 deletions app/Config/Mimes.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,6 @@ class Mimes
],
'pptx' => [
'application/vnd.openxmlformats-officedocument.presentationml.presentation',
'application/x-zip',
'application/zip',
],
'wbxml' => 'application/wbxml',
'wmlc' => 'application/wmlc',
Expand Down Expand Up @@ -512,20 +510,19 @@ public static function guessExtensionFromType(string $type, ?string $proposedExt

$proposedExtension = trim(strtolower($proposedExtension ?? ''));

if ($proposedExtension !== '') {
if (array_key_exists($proposedExtension, static::$mimes) && in_array($type, is_string(static::$mimes[$proposedExtension]) ? [static::$mimes[$proposedExtension]] : static::$mimes[$proposedExtension], true)) {
// The detected mime type matches with the proposed extension.
return $proposedExtension;
}

// An extension was proposed, but the media type does not match the mime type list.
return null;
if (
$proposedExtension !== ''
&& array_key_exists($proposedExtension, static::$mimes)
&& in_array($type, (array) static::$mimes[$proposedExtension], true)
) {
// The detected mime type matches with the proposed extension.
return $proposedExtension;
}

// Reverse check the mime type list if no extension was proposed.
// This search is order sensitive!
foreach (static::$mimes as $ext => $types) {
if ((is_string($types) && $types === $type) || (is_array($types) && in_array($type, $types, true))) {
if (in_array($type, (array) $types, true)) {
return $ext;
}
}
Expand Down
7 changes: 6 additions & 1 deletion system/Files/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,12 @@ public function getSizeByUnit(string $unit = 'b')
*/
public function guessExtension(): ?string
{
return Mimes::guessExtensionFromType($this->getMimeType());
// naively get the path extension using pathinfo
$pathinfo = pathinfo($this->getRealPath() ?: $this->__toString()) + ['extension' => ''];

$proposedExtension = $pathinfo['extension'];

return Mimes::guessExtensionFromType($this->getMimeType(), $proposedExtension);
}

/**
Expand Down
25 changes: 25 additions & 0 deletions tests/system/Files/FileTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use CodeIgniter\Files\Exceptions\FileNotFoundException;
use CodeIgniter\Test\CIUnitTestCase;
use ZipArchive;

/**
* @internal
Expand Down Expand Up @@ -44,10 +45,34 @@ public function testGuessExtension()
{
$file = new File(SYSTEMPATH . 'Common.php');
$this->assertSame('php', $file->guessExtension());

$file = new File(SYSTEMPATH . 'index.html');
$this->assertSame('html', $file->guessExtension());

$file = new File(ROOTPATH . 'phpunit.xml.dist');
$this->assertSame('xml', $file->guessExtension());

$tmp = tempnam(SUPPORTPATH, 'foo');
$file = new File($tmp, true);
$this->assertNull($file->guessExtension());
unlink($tmp);
}

/**
* @see https://github.com/codeigniter4/CodeIgniter4/issues/6046
*/
public function testGuessExtensionOnZip(): void
{
$tmp = SUPPORTPATH . 'foobar.zip';

$zip = new ZipArchive();
$zip->open($tmp, ZipArchive::CREATE | ZipArchive::CHECKCONS | ZipArchive::EXCL);
$zip->addFile(SYSTEMPATH . 'Common.php');
$zip->close();

$file = new File($tmp, true);
$this->assertSame('zip', $file->guessExtension());
unlink($tmp);
}

public function testRandomName()
Expand Down
10 changes: 4 additions & 6 deletions tests/system/HTTP/Files/FileCollectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public function testExtensionGuessing()
$this->assertInstanceOf(UploadedFile::class, $file);
$this->assertSame('txt', $file->getExtension());
// but not client mime type
$this->assertNull(Mimes::guessExtensionFromType($file->getClientMimeType(), $file->getClientExtension()));
$this->assertSame('csv', Mimes::guessExtensionFromType($file->getClientMimeType(), $file->getClientExtension()));

// proposed extension does not match finfo_open mime type (text/plain)
// but can be resolved by reverse searching
Expand All @@ -208,14 +208,12 @@ public function testExtensionGuessing()
$this->assertSame('zip', $file->getExtension());

// proposed extension matches client mime type, but not finfo_open mime type (application/zip)
// this is a zip file (userFile4) but hat been renamed to 'rar'
// this is a zip file (userFile4) but has been renamed to 'rar'
$file = $collection->getFile('userfile5');
$this->assertInstanceOf(UploadedFile::class, $file);
// getExtension falls back to clientExtension (insecure)
$this->assertSame('rar', $file->getExtension());
$this->assertSame('zip', $file->getExtension());
$this->assertSame('rar', Mimes::guessExtensionFromType($file->getClientMimeType(), $file->getClientExtension()));
// guessExtension is secure and does not returns empty
$this->assertSame('', $file->guessExtension());
$this->assertSame('zip', $file->guessExtension());
}

/**
Expand Down
1 change: 1 addition & 0 deletions user_guide_src/source/changelogs/v4.2.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Behavior Changes
- There is a possible backward compatibility break for those users extending the History Collector and they should probably update ``History::setFiles()`` method.
- The :php:func:`dot_array_search`'s unexpected behavior has been fixed. Now ``dot_array_search('foo.bar.baz', ['foo' => ['bar' => 23]])`` returns ``null``. The previous versions returned ``23``.
- The ``CodeIgniter::storePreviousURL()`` has been changed to store only the URLs whose Content-Type was ``text/html``. It also affects the behavior of :php:func:`previous_url` and :php:func:`redirect()->back() <redirect>`.
- Guessing the file extension from the MIME type has been changed if the proposed extension is not valid. Previously, the guessing will early terminate and return ``null``. Now, if a proposed extension is given and is invalid, the MIME guessing will continue checking using the mapping of extension to MIME types.

Enhancements
************
Expand Down

0 comments on commit 8a11ced

Please sign in to comment.