Merge pull request #1 from codeigniter4/develop

Update from parent repo.

Author: NPWSamarasinghe

CHANGELOG.md

@@ -10,6 +10,12 @@
 - `CodeIgniter\Config\Config` is now deprecated in favor of `CodeIgniter\Config\Factories::config()`
 - HTTP Layer Refactor: Numerous deprecations have been made towards a transition to a PSR-compliant HTTP layer. [See the User Guide](user_guide_src/source/installation/upgrade_405.rst)
 
+**Mime Type Detection**
+
+- `Config\Mimes::guessExtensionFromType` now only reverse searches the `$mimes` array if no extension is proposed (i.e., usually not for uploaded files).
+- The fallback values of `UploadedFile->getExtension()` and `UploadedFile->guessExtension()` have been changed. `UploadedFile->getExtension()` now returns `$this->getClientExtension()` instead of `''`; `UploadedFile->guessExtension()` now returns `''` instead of `$this->getClientExtension()`.
+These changes increase security when handling uploaded files as the client can no longer force a wrong mime type on the application. However, these might affect how file extensions are detected in your application.
+
 ## [v4.0.4](https://github.com/codeigniter4/CodeIgniter4/tree/v4.0.4) (2020-07-15)
 
 [Full Changelog](https://github.com/codeigniter4/CodeIgniter4/compare/v4.0.3...v4.0.4)

admin/framework/composer.json

@@ -22,6 +22,9 @@
 		"predis/predis": "^1.1",
 		"squizlabs/php_codesniffer": "^3.3"
 	},
+	"suggest": {
+		"ext-fileinfo": "Improves mime type detection for files"
+	},
 	"autoload": {
 		"psr-4": {
 			"CodeIgniter\\": "system/"

admin/module/composer.json

@@ -12,6 +12,9 @@
 		"mikey179/vfsstream": "^1.6",
 		"phpunit/phpunit": "^8.5"
 	},
+	"suggest": {
+		"ext-fileinfo": "Improves mime type detection for files"
+	},
 	"autoload-dev": {
 		"psr-4": {
 			"Tests\\Support\\": "tests/_support"

admin/starter/composer.json

@@ -13,6 +13,9 @@
 		"mikey179/vfsstream": "^1.6",
 		"phpunit/phpunit": "^8.5"
 	},
+	"suggest": {
+		"ext-fileinfo": "Improves mime type detection for files"
+	},
 	"autoload": {
 		"psr-4": {
 			"App\\": "app",

app/Config/Mimes.php

@@ -12,6 +12,9 @@
  * the most common one should be first in the array to aid the guess*
  * methods. The same applies when more than one mime-type exists for a
  * single extension.
+ *
+ * When working with mime types, please make sure you have the ´fileinfo´
+ * extension enabled to reliably detect the media types.
  */
 
 class Mimes
@@ -33,7 +36,6 @@ class Mimes
 			'text/csv',
 			'text/x-comma-separated-values',
 			'text/comma-separated-values',
-			'application/octet-stream',
 			'application/vnd.ms-excel',
 			'application/x-csv',
 			'text/x-csv',
@@ -69,7 +71,6 @@ class Mimes
 			'application/pdf',
 			'application/force-download',
 			'application/x-download',
-			'binary/octet-stream',
 		],
 		'ai'    => [
 			'application/pdf',
@@ -462,12 +463,10 @@ class Mimes
 		'srt'   => [
 			'text/srt',
 			'text/plain',
-			'application/octet-stream',
 		],
 		'vtt'   => [
 			'text/vtt',
 			'text/plain',
-			'application/octet-stream',
 		],
 		'ico'   => [
 			'image/x-icon',
@@ -509,11 +508,20 @@ public static function guessExtensionFromType(string $type, string $proposedExte
 
 		$proposedExtension = trim(strtolower($proposedExtension));
 
-		if ($proposedExtension !== '' && array_key_exists($proposedExtension, static::$mimes) && in_array($type, is_string(static::$mimes[$proposedExtension]) ? [static::$mimes[$proposedExtension]] : static::$mimes[$proposedExtension], true))
+		if ($proposedExtension !== '')
 		{
-			return $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;
 		}
 
+		// 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)))

composer.json

@@ -18,12 +18,15 @@
 		"codeigniter4/codeigniter4-standard": "^1.0",
 		"fakerphp/faker": "^1.9",
 		"mikey179/vfsstream": "^1.6",
-		"phpstan/phpstan": "^0.12",
+		"phpstan/phpstan": "0.12.65",
 		"phpunit/phpunit": "^8.5 || ^9.1",
 		"predis/predis": "^1.1",
 		"rector/rector": "^0.8",
 		"squizlabs/php_codesniffer": "^3.3"
 	},
+	"suggest": {
+		"ext-fileinfo": "Improves mime type detection for files"
+	},
 	"config": {
 		"optimize-autoloader": true,
 		"preferred-install": "dist",

system/BaseModel.php

@@ -20,6 +20,7 @@
 use CodeIgniter\Pager\Pager;
 use CodeIgniter\Validation\ValidationInterface;
 use Config\Services;
+use InvalidArgumentException;
 use ReflectionClass;
 use ReflectionException;
 use ReflectionProperty;
@@ -715,31 +716,7 @@ public function insert($data = null, bool $returnID = true)
 	{
 		$this->insertID = 0;
 
-		if (empty($data))
-		{
-			throw DataException::forEmptyDataset('insert');
-		}
-
-		// If $data is using a custom class with public or protected
-		// properties representing the collection elements, we need to grab
-		// them as an array.
-		if (is_object($data) && ! $data instanceof stdClass)
-		{
-			$data = $this->objectToArray($data, false, true);
-		}
-
-		// If it's still a stdClass, go ahead and convert to
-		// an array so doProtectFields and other model methods
-		// don't have to do special checks.
-		if (is_object($data))
-		{
-			$data = (array) $data;
-		}
-
-		if (empty($data))
-		{
-			throw DataException::forEmptyDataset('insert');
-		}
+		$data = $this->transformDataToArray($data, 'insert');
 
 		// Validate data before saving.
 		if (! $this->skipValidation && ! $this->cleanRules()->validate($data))
@@ -877,32 +854,7 @@ public function update($id = null, $data = null): bool
 			$id = [$id];
 		}
 
-		if (empty($data))
-		{
-			throw DataException::forEmptyDataset('update');
-		}
-
-		// If $data is using a custom class with public or protected
-		// properties representing the collection elements, we need to grab
-		// them as an array.
-		if (is_object($data) && ! $data instanceof stdClass)
-		{
-			$data = $this->objectToArray($data, true, true);
-		}
-
-		// If it's still a stdClass, go ahead and convert to
-		// an array so doProtectFields and other model methods
-		// don't have to do special checks.
-		if (is_object($data))
-		{
-			$data = (array) $data;
-		}
-
-		// If it's still empty here, means $data is no change or is empty object
-		if (empty($data))
-		{
-			throw DataException::forEmptyDataset('update');
-		}
+		$data = $this->transformDataToArray($data, 'update');
 
 		// Validate data before saving.
 		if (! $this->skipValidation && ! $this->cleanRules(true)->validate($data))
@@ -1692,6 +1644,55 @@ protected function objectToRawArray($data, bool $onlyChanged = true, bool $recur
 		return $properties;
 	}
 
+	/**
+	 * Transform data to array
+	 *
+	 * @param array|object|null $data Data
+	 * @param string            $type Type of data (insert|update)
+	 *
+	 * @return array
+	 *
+	 * @throws DataException
+	 * @throws InvalidArgumentException
+	 * @throws ReflectionException
+	 */
+	protected function transformDataToArray($data, string $type): array
+	{
+		if (! in_array($type, ['insert', 'update'], true))
+		{
+			throw new InvalidArgumentException(sprintf('Invalid type "%s" used upon transforming data to array.', $type));
+		}
+
+		if (empty($data))
+		{
+			throw DataException::forEmptyDataset($type);
+		}
+
+		// If $data is using a custom class with public or protected
+		// properties representing the collection elements, we need to grab
+		// them as an array.
+		if (is_object($data) && ! $data instanceof stdClass)
+		{
+			$data = $this->objectToArray($data, true, true);
+		}
+
+		// If it's still a stdClass, go ahead and convert to
+		// an array so doProtectFields and other model methods
+		// don't have to do special checks.
+		if (is_object($data))
+		{
+			$data = (array) $data;
+		}
+
+		// If it's still empty here, means $data is no change or is empty object
+		if (empty($data))
+		{
+			throw DataException::forEmptyDataset($type);
+		}
+
+		return $data;
+	}
+
 	// endregion
 
 	// region Magic

system/Database/BaseConnection.php

@@ -289,6 +289,13 @@ abstract class BaseConnection implements ConnectionInterface
 	 */
 	protected $aliasedTables = [];
 
+	/**
+	 * Query Class
+	 *
+	 * @var string
+	 */
+	protected $queryClass = 'CodeIgniter\\Database\\Query';
+
 	//--------------------------------------------------------------------
 
 	/**
@@ -302,6 +309,13 @@ public function __construct(array $params)
 		{
 			$this->$key = $value;
 		}
+
+		$queryClass = str_replace('Connection', 'Query', static::class);
+
+		if (class_exists($queryClass))
+		{
+			$this->queryClass = $queryClass;
+		}
 	}
 
 	//--------------------------------------------------------------------
@@ -594,9 +608,13 @@ abstract protected function execute(string $sql);
 	 * @param string  $queryClass
 	 *
 	 * @return BaseResult|Query|false
+	 *
+	 * @todo BC set $queryClass default as null in 4.1
 	 */
-	public function query(string $sql, $binds = null, bool $setEscapeFlags = true, string $queryClass = 'CodeIgniter\\Database\\Query')
+	public function query(string $sql, $binds = null, bool $setEscapeFlags = true, string $queryClass = '')
 	{
+		$queryClass = $queryClass ?: $this->queryClass;
+
 		if (empty($this->connID))
 		{
 			$this->initialize();

system/HTTP/Files/UploadedFile.php

@@ -308,24 +308,29 @@ public function getTempName(): string
 	 * Overrides SPLFileInfo's to work with uploaded files, since
 	 * the temp file that's been uploaded doesn't have an extension.
 	 *
-	 * Is simply an alias for guessExtension for a safer method
-	 * than simply relying on the provided extension.
-	 * Additionally it will return clientExtension in case if there are
-	 * other extensions with the same mime type.
+	 * This method tries to guess the extension from the files mime
+	 * type but will return the clientExtension if it fails to do so.
+	 *
+	 * This method will always return a more or less helpfull extension
+	 * but might be insecure if the mime type is not machted. Consider
+	 * using guessExtension for a more safe version.
 	 */
 	public function getExtension(): string
 	{
-		return $this->guessExtension();
+		return $this->guessExtension() ?: $this->getClientExtension();
 	}
 
 	/**
-	 * Attempts to determine the best file extension.
+	 * Attempts to determine the best file extension from the file's
+	 * mime type. In contrast to getExtension, this method will return
+	 * an empty string if it fails to determine an extension instead of
+	 * falling back to the unsecure clientExtension.
 	 *
 	 * @return string
 	 */
 	public function guessExtension(): string
 	{
-		return Mimes::guessExtensionFromType($this->getClientMimeType(), $this->getClientExtension()) ?? $this->getClientExtension();
+		return Mimes::guessExtensionFromType($this->getMimeType(), $this->getClientExtension()) ?? '';
 	}
 
 	//--------------------------------------------------------------------

system/I18n/Exceptions/I18nException.php

@@ -18,6 +18,19 @@
  */
 class I18nException extends FrameworkException
 {
+	/**
+	 * Thrown when createFromFormat fails to receive a valid
+	 * DateTime back from DateTime::createFromFormat.
+	 *
+	 * @param string $format
+	 *
+	 * @return static
+	 */
+	public static function forInvalidFormat(string $format)
+	{
+		return new static(lang('Time.invalidFormat', [$format]));
+	}
+
 	/**
 	 * Thrown when the numeric representation of the month falls
 	 * outside the range of allowed months.