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 7, 2022
1 parent 6cab49d commit f48f749
Show file tree
Hide file tree
Showing 8 changed files with 116 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/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ See all the changes.
.. toctree::
:titlesonly:

v4.2.1
v4.2.0
v4.1.9
v4.1.8
Expand Down
18 changes: 18 additions & 0 deletions user_guide_src/source/changelogs/v4.2.1.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
Version 4.2.1
#############

Release Date: Unreleased

**4.2.1 release of CodeIgniter4**

.. contents::
:local:
:depth: 2

BREAKING
********

Behavior Changes
================

- 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.
53 changes: 53 additions & 0 deletions user_guide_src/source/installation/upgrade_421.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#############################
Upgrading from 4.2.0 to 4.2.1
#############################

Please refer to the upgrade instructions corresponding to your installation method.

- :ref:`Composer Installation App Starter Upgrading <app-starter-upgrading>`
- :ref:`Composer Installation Adding CodeIgniter4 to an Existing Project Upgrading <adding-codeigniter4-upgrading>`
- :ref:`Manual Installation Upgrading <installing-manual-upgrading>`

.. contents::
:local:
:depth: 2

Mandatory File Changes
**********************

app/Config/Mimes.php
===================

- The mapping of file extensions to MIME types in **app/Config/Mimes.php** was updated to fix a bug. Also, the logic of ``Mimes::getExtensionFromType()`` was changed.

Breaking Changes
****************


Breaking Enhancements
*********************


Project Files
*************

Numerous files in the **project space** (root, app, public, writable) received updates. Due to
these files being outside of the **system** scope they will not be changed without your intervention.
There are some third-party CodeIgniter modules available to assist with merging changes to
the project space: `Explore on Packagist <https://packagist.org/explore/?query=codeigniter4%20updates>`_.

.. note:: Except in very rare cases for bug fixes, no changes made to files for the project space
will break your application. All changes noted here are optional until the next major version,
and any mandatory changes will be covered in the sections above.

Content Changes
===============


All Changes
===========

This is a list of all files in the **project space** that received changes;
many will be simple comments or formatting that have no effect on the runtime:

* app/Config/Mimes.php
1 change: 1 addition & 0 deletions user_guide_src/source/installation/upgrading.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ upgrading from.
.. toctree::
:titlesonly:

upgrade_421
upgrade_420
upgrade_418
upgrade_417
Expand Down

0 comments on commit f48f749

Please sign in to comment.