From 57d795d23b9ee19273ebd37c3f264d34e56c70d5 Mon Sep 17 00:00:00 2001 From: Sugeng Sulistiyawan Date: Sun, 20 Oct 2024 10:28:37 +0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=EF=B8=8F=20Fixed=20Best=20Practice?= =?UTF-8?q?,=20Bug,=20and=20Security=20Issues?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/AbstractComponent.php | 17 +++++++++ src/AsyncAwsS3Component.php | 15 +++----- src/AwsS3Component.php | 14 +++---- src/GoogleCloudStorageComponent.php | 7 ++-- src/GoogleDriveComponent.php | 50 +++++++++++++------------ src/LocalComponent.php | 11 +++--- src/SftpComponent.php | 11 +++--- src/WebDavComponent.php | 7 ++-- src/ZipArchiveComponent.php | 11 +++--- src/actions/FileAction.php | 2 +- src/adapter/ZipArchiveAdapter.php | 10 +++-- src/traits/UrlGeneratorAdapterTrait.php | 14 +++---- 12 files changed, 91 insertions(+), 78 deletions(-) diff --git a/src/AbstractComponent.php b/src/AbstractComponent.php index aa67966..91eeb4f 100644 --- a/src/AbstractComponent.php +++ b/src/AbstractComponent.php @@ -9,6 +9,7 @@ use League\Flysystem\PathNormalizer; use League\Flysystem\WhitespacePathNormalizer; use yii\base\Component; +use yii\base\InvalidConfigException; /** * Class AbstractComponent @@ -132,4 +133,20 @@ public function convertToDateTime($dateValue) * @return FilesystemAdapter */ abstract protected function initAdapter(); + + /** + * Validate property + * + * @param array $properties + * @return void + * @throws InvalidConfigException + */ + public function validateProperties(array $properties = []) + { + foreach ($properties as $property) { + if (empty($this->$property)) { + throw new InvalidConfigException("The \"$property\" property must be set."); + } + } + } } diff --git a/src/AsyncAwsS3Component.php b/src/AsyncAwsS3Component.php index e21c6f3..f9a0275 100644 --- a/src/AsyncAwsS3Component.php +++ b/src/AsyncAwsS3Component.php @@ -110,18 +110,15 @@ class AsyncAwsS3Component extends AbstractComponent /** * @inheritdoc + * @throws InvalidConfigException */ public function init() { - if (empty($this->accessKeyId)) { - throw new InvalidConfigException('The "accessKeyId" property must be set.'); - } - if (empty($this->accessKeySecret)) { - throw new InvalidConfigException('The "accessKeySecret" property must be set.'); - } - if (empty($this->bucket)) { - throw new InvalidConfigException('The "bucket" property must be set.'); - } + $this->validateProperties([ + 'bucket', + 'accessKeyId', + 'accessKeySecret', + ]); parent::init(); } diff --git a/src/AwsS3Component.php b/src/AwsS3Component.php index 8919ef4..472877a 100644 --- a/src/AwsS3Component.php +++ b/src/AwsS3Component.php @@ -104,20 +104,16 @@ class AwsS3Component extends AbstractComponent /** * @inheritdoc + * @throws InvalidConfigException */ public function init() { + $properties = ['bucket']; if (empty($this->credentials)) { - if (empty($this->key)) { - throw new InvalidConfigException('The "key" property must be set.'); - } - if (empty($this->secret)) { - throw new InvalidConfigException('The "secret" property must be set.'); - } - } - if (empty($this->bucket)) { - throw new InvalidConfigException('The "bucket" property must be set.'); + $properties[] = 'key'; + $properties[] = 'secret'; } + $this->validateProperties($properties); parent::init(); } diff --git a/src/GoogleCloudStorageComponent.php b/src/GoogleCloudStorageComponent.php index e121876..b9a0c2f 100644 --- a/src/GoogleCloudStorageComponent.php +++ b/src/GoogleCloudStorageComponent.php @@ -140,12 +140,13 @@ class GoogleCloudStorageComponent extends AbstractComponent /** * @inheritdoc + * @throws InvalidConfigException */ public function init() { - if (empty($this->bucket)) { - throw new InvalidConfigException('The "bucket" property must be set.'); - } + $this->validateProperties([ + 'bucket', + ]); parent::init(); } diff --git a/src/GoogleDriveComponent.php b/src/GoogleDriveComponent.php index 47b3d0a..5fc998e 100644 --- a/src/GoogleDriveComponent.php +++ b/src/GoogleDriveComponent.php @@ -83,34 +83,26 @@ class GoogleDriveComponent extends AbstractComponent */ protected $client; + /** + * @var string[] + */ + protected $_availableOptions = [ + 'teamDriveId', + 'sharedFolderId', + ]; + /** * @inheritdoc + * @throws InvalidConfigException */ public function init() { - if (empty($this->clientId)) { - throw new InvalidConfigException('The "clientId" property must be set.'); - } - - if (empty($this->clientSecret)) { - throw new InvalidConfigException('The "clientSecret" property must be set.'); - } - - if (empty($this->refreshToken)) { - throw new InvalidConfigException('The "refreshToken" property must be set.'); - } - - if (!empty($this->teamDriveId)) { - $this->options['teamDriveId'] = $this->teamDriveId; - } - - if (!empty($this->sharedFolderId)) { - $this->options['sharedFolderId'] = $this->sharedFolderId; - } - - if (empty($this->secret)) { - throw new InvalidConfigException('The "secret" property must be set.'); - } + $this->validateProperties([ + 'clientId', + 'clientSecret', + 'refreshToken', + 'secret', + ]); $this->initEncrypter($this->secret); @@ -131,9 +123,19 @@ protected function initAdapter() $client->setApplicationName($this->applicationName); } + foreach ($this->_availableOptions as $property) { + if (!empty($this->$property)) { + $this->options[$property] = $this->$property; + } + } + $this->client = $client; $service = new Drive($this->client); - return new GoogleDriveAdapter($service, $this->prefix, $this->options); + $adapter = new GoogleDriveAdapter($service, $this->prefix, $this->options); + // for UrlGeneratorAdapterTrait + $adapter->component = $this; + + return $adapter; } } diff --git a/src/LocalComponent.php b/src/LocalComponent.php index 52a6b19..16a4873 100644 --- a/src/LocalComponent.php +++ b/src/LocalComponent.php @@ -44,15 +44,14 @@ class LocalComponent extends AbstractComponent /** * @inheritdoc + * @throws InvalidConfigException */ public function init() { - if (empty($this->path)) { - throw new InvalidConfigException('The "path" property must be set.'); - } - if (empty($this->secret)) { - throw new InvalidConfigException('The "secret" property must be set.'); - } + $this->validateProperties([ + 'path', + 'secret', + ]); $this->initEncrypter($this->secret); diff --git a/src/SftpComponent.php b/src/SftpComponent.php index c6b090e..5596449 100644 --- a/src/SftpComponent.php +++ b/src/SftpComponent.php @@ -134,15 +134,14 @@ class SftpComponent extends AbstractComponent /** * @inheritdoc + * @throws InvalidConfigException */ public function init() { - if (empty($this->host)) { - throw new InvalidConfigException('The "host" property must be set.'); - } - if (empty($this->username)) { - throw new InvalidConfigException('The "username" property must be set.'); - } + $this->validateProperties([ + 'host', + 'username', + ]); $this->passphrase = $this->passphrase ?: ($this->password ?: ($this->username ?: Yii::$app->id)); $this->initEncrypter($this->passphrase); diff --git a/src/WebDavComponent.php b/src/WebDavComponent.php index ec1117a..229a6e4 100644 --- a/src/WebDavComponent.php +++ b/src/WebDavComponent.php @@ -91,12 +91,13 @@ class WebDavComponent extends AbstractComponent /** * @inheritdoc + * @throws InvalidConfigException */ public function init() { - if (empty($this->baseUri)) { - throw new InvalidConfigException('The "baseUri" property must be set.'); - } + $this->validateProperties([ + 'baseUri', + ]); parent::init(); } diff --git a/src/ZipArchiveComponent.php b/src/ZipArchiveComponent.php index 840c8b6..b3446fe 100644 --- a/src/ZipArchiveComponent.php +++ b/src/ZipArchiveComponent.php @@ -44,15 +44,14 @@ class ZipArchiveComponent extends AbstractComponent /** * @inheritdoc + * @throws InvalidConfigException */ public function init() { - if (empty($this->pathToZip)) { - throw new InvalidConfigException('The "pathToZip" property must be set.'); - } - if (empty($this->secret)) { - throw new InvalidConfigException('The "secret" property must be set.'); - } + $this->validateProperties([ + 'pathToZip', + 'secret', + ]); $this->initEncrypter($this->secret); diff --git a/src/actions/FileAction.php b/src/actions/FileAction.php index c42a66f..58c3169 100644 --- a/src/actions/FileAction.php +++ b/src/actions/FileAction.php @@ -52,7 +52,7 @@ public function run($data = null) $filesystem = Yii::$app->get($this->component); try { - $params = Json::decode($filesystem->decrypt($data)); + $params = Json::decode($filesystem?->decrypt($data)); $now = (int) (new DateTimeImmutable())->getTimestamp(); $expires = (int) $params['expires']; diff --git a/src/adapter/ZipArchiveAdapter.php b/src/adapter/ZipArchiveAdapter.php index 01694a2..cebaaaa 100644 --- a/src/adapter/ZipArchiveAdapter.php +++ b/src/adapter/ZipArchiveAdapter.php @@ -207,6 +207,8 @@ public function setVisibility(string $path, string $visibility): void { $archive = $this->zipArchiveProvider->createZipArchive(); $location = $this->pathPrefixer->prefixPath($path); + + /** @var array|false $stats */ $stats = $archive->statName($location) ?: $archive->statName($location . '/'); if ($stats === false) { @@ -377,7 +379,7 @@ public function copy(string $source, string $destination, Config $config): void $this->writeStream($destination, $readStream, $config); } catch (Throwable $exception) { if (isset($readStream)) { - @fclose($readStream); + /** @scrutinizer ignore-unhandled */ @fclose($readStream); } throw UnableToCopyFile::fromLocationTo($source, $destination, $exception); @@ -452,7 +454,7 @@ private function setVisibilityAttribute(string $statsName, string $visibility, Z // ================================================= /** - * @var UrlGeneratorComponentTrait|AbstractComponent + * @var AbstractComponent|UrlGeneratorComponentTrait */ public $component; @@ -464,7 +466,7 @@ public function publicUrl(string $path, Config $config): string 'expires' => 0, ]; - return Url::toRoute([$this->component->action, 'data' => $this->component->encrypt(Json::encode($params))], true); + return Url::toRoute([$this->component?->action, 'data' => $this->component?->encrypt(Json::encode($params))], true); } public function temporaryUrl(string $path, DateTimeInterface $expiresAt, Config $config): string @@ -475,6 +477,6 @@ public function temporaryUrl(string $path, DateTimeInterface $expiresAt, Config 'expires' => (int) $expiresAt->getTimestamp(), ]; - return Url::toRoute([$this->component->action, 'data' => $this->component->encrypt(Json::encode($params))], true); + return Url::toRoute([$this->component?->action, 'data' => $this->component?->encrypt(Json::encode($params))], true); } } diff --git a/src/traits/UrlGeneratorAdapterTrait.php b/src/traits/UrlGeneratorAdapterTrait.php index f866d09..a6ac2d9 100644 --- a/src/traits/UrlGeneratorAdapterTrait.php +++ b/src/traits/UrlGeneratorAdapterTrait.php @@ -19,15 +19,15 @@ trait UrlGeneratorAdapterTrait { /** - * @var UrlGeneratorComponentTrait|AbstractComponent + * @var AbstractComponent|UrlGeneratorComponentTrait */ public $component; public function publicUrl(string $path, /** @scrutinizer ignore-unused */Config $config): string { // TODO: Use absolute path and don't encrypt - if ($this->component->prefix) { - $prefixer = new PathPrefixer($this->component->prefix); + if ($this->component?->prefix) { + $prefixer = new PathPrefixer((string) $this->component?->prefix); $path = $prefixer->stripPrefix($path); } $params = [ @@ -35,14 +35,14 @@ public function publicUrl(string $path, /** @scrutinizer ignore-unused */Config 'expires' => 0, ]; - return Url::toRoute([$this->component->action, 'data' => $this->component->encrypt(Json::encode($params))], true); + return Url::toRoute([$this->component?->action, 'data' => $this->component?->encrypt(Json::encode($params))], true); } public function temporaryUrl(string $path, DateTimeInterface $expiresAt, /** @scrutinizer ignore-unused */Config $config): string { // TODO: Use absolute path and don't encrypt - if ($this->component->prefix) { - $prefixer = new PathPrefixer($this->component->prefix); + if ($this->component?->prefix) { + $prefixer = new PathPrefixer((string) $this->component?->prefix); $path = $prefixer->stripPrefix($path); } $params = [ @@ -50,6 +50,6 @@ public function temporaryUrl(string $path, DateTimeInterface $expiresAt, /** @sc 'expires' => (int) $expiresAt->getTimestamp(), ]; - return Url::toRoute([$this->component->action, 'data' => $this->component->encrypt(Json::encode($params))], true); + return Url::toRoute([$this->component?->action, 'data' => $this->component?->encrypt(Json::encode($params))], true); } }