diff --git a/src/controllers/AssetsController.php b/src/controllers/AssetsController.php index e791dfb..cccca7f 100644 --- a/src/controllers/AssetsController.php +++ b/src/controllers/AssetsController.php @@ -11,12 +11,9 @@ use craft\fields\Assets as AssetsField; use craft\helpers\Assets; use craft\helpers\Db; -use craft\helpers\FileHelper; -use craft\helpers\Image; use craft\models\Volume; use craft\web\Controller; use DateTime; -use Throwable; use yii\base\Event; use yii\base\Exception; use yii\base\Model; @@ -97,7 +94,6 @@ public function actionCreateAsset(): Response $filename = $this->request->getRequiredBodyParam('filename'); $originalFilename = $this->request->getRequiredBodyParam('originalFilename'); $targetFilename = $this->request->getRequiredBodyParam('targetFilename'); - $size = $this->request->getBodyParam('size'); $elementsService = Craft::$app->getElements(); $lastModifiedMs = (int) $this->request->getBodyParam('lastModified'); $dateModified = $lastModifiedMs @@ -160,7 +156,6 @@ public function actionCreateAsset(): Response $asset->uploaderId = Craft::$app->getUser()->getId(); $asset->avoidFilenameConflicts = true; $asset->dateModified = $dateModified; - $asset->size = $size; // Setting newFolderId, so that extension validation on newLocation occurs $asset->newFolderId = $folder->id; @@ -171,7 +166,11 @@ public function actionCreateAsset(): Response // Handle special characters that have been encoded from the presigned URL $asset->folderPath = is_string($folder->path) ? Fs::urlEncodePathSegments($folder->path) : $asset->folderPath; - [$asset->width, $asset->height] = $this->uploadedImageDimensions($asset, $filename); + $asset->size = $this->uploadedAssetSize($asset, $filename); + $fs = $asset->getVolume()->getFs(); + [$asset->width, $asset->height] = $fs instanceof Fs + ? $fs->getImageDimensions($asset->getPath()) ?? [null, null] + : [null, null]; if (!$selectionCondition) { $asset->newFilename = $targetFilename; @@ -240,7 +239,6 @@ public function actionReplaceFile(): Response $sourceAssetId = $this->request->getBodyParam('sourceAssetId'); $filename = $this->request->getBodyParam('filename'); $targetFilename = $this->request->getBodyParam('targetFilename'); - $size = $this->request->getBodyParam('size'); $lastModifiedMs = (int) $this->request->getBodyParam('lastModified'); $dateModified = $lastModifiedMs ? DateTime::createFromFormat('U', (string) floor($lastModifiedMs / 1000)) @@ -271,7 +269,7 @@ public function actionReplaceFile(): Response // Handle the Element Action if ($assetToReplace !== null && $filename) { - $assetToReplace->size = $size; + $assetToReplace->size = $this->uploadedAssetSize($assetToReplace, $filename); $assetToReplace->dateModified = $dateModified; if (!$this->replaceAssetFile($assetToReplace, $filename, $targetFilename)) { throw new Exception('Unable to replace asset.'); @@ -354,7 +352,10 @@ public function replaceAssetFile(Asset $asset, string $filename, string $targetF $asset->avoidFilenameConflicts = true; $asset->setScenario(Asset::SCENARIO_REPLACE); $asset->setFilename($filename); - [$asset->width, $asset->height] = $this->uploadedImageDimensions($asset, $filename); + $fs = $asset->getVolume()->getFs(); + [$asset->width, $asset->height] = $fs instanceof Fs + ? $fs->getImageDimensions($asset->getPath()) ?? [null, null] + : [null, null]; $asset->newFilename = $targetFilename; $saved = $this->saveAsset($asset); @@ -395,46 +396,16 @@ private function volumeSubpath(Volume $volume): string return method_exists($volume, 'getSubpath') ? $volume->getSubpath() : ''; } - protected function uploadedImageDimensions(Asset $asset, string $filename): array + protected function uploadedAssetSize(Asset $asset, string $filename): int { - if (Assets::getFileKindByExtension($filename) !== Asset::KIND_IMAGE) { - return [null, null]; - } - - return $this->readUploadedImageDimensions($asset) ?? [null, null]; - } - - protected function readUploadedImageDimensions(Asset $asset): ?array - { - $stream = null; - $tempPath = null; - - try { - $stream = $asset->getVolume()->getFs()->getFileStream($asset->getPath()); - $imageSize = Image::imageSizeByStream($stream); + $size = $asset->getVolume()->getFileSize($asset->getPath($filename)); - if ($imageSize === false || !isset($imageSize[0], $imageSize[1])) { - fclose($stream); - $stream = null; - - $tempPath = $asset->getCopyOfFile(); - $imageSize = Image::imageSize($tempPath); - } - - return [ - (int)$imageSize[0] ?: null, - (int)$imageSize[1] ?: null, - ]; - } catch (Throwable) { - return null; - } finally { - if (is_resource($stream)) { - fclose($stream); - } - - if ($tempPath !== null) { - FileHelper::unlink($tempPath); - } + if ($size > Assets::getMaxUploadSize()) { + throw new BadRequestHttpException(Craft::t('app', '“{filename}” is too large.', [ + 'filename' => $filename, + ])); } + + return $size; } } diff --git a/src/fs/Fs.php b/src/fs/Fs.php index 8c05a06..559bdf7 100644 --- a/src/fs/Fs.php +++ b/src/fs/Fs.php @@ -10,12 +10,14 @@ use craft\cloud\Module; use craft\cloud\StaticCache; use craft\cloud\StaticCacheTag; +use craft\elements\Asset; use craft\errors\FsException; use craft\flysystem\base\FlysystemFs; use craft\fs\Local; use craft\helpers\App; use craft\helpers\Assets; use craft\helpers\DateTimeHelper; +use craft\helpers\Image; use DateTime; use DateTimeInterface; use Generator; @@ -37,6 +39,8 @@ */ abstract class Fs extends FlysystemFs { + private const IMAGE_DIMENSION_HEADER_BYTES = 1048576; + protected static bool $showUrlSetting = false; protected ?string $expires = null; protected ?Local $localFs = null; @@ -519,6 +523,101 @@ public function getFileStream(string $uriPath) return parent::getFileStream($uriPath); } + public function getImageDimensions(string $uriPath): ?array + { + if (Assets::getFileKindByExtension($uriPath) !== Asset::KIND_IMAGE) { + return null; + } + + $stream = $this->getFileStreamRange($uriPath, 0, self::IMAGE_DIMENSION_HEADER_BYTES - 1); + + if ($stream === null) { + return null; + } + + try { + $imageSize = Image::imageSizeByStream($stream); + + if ($imageSize === false || !isset($imageSize[0], $imageSize[1])) { + return null; + } + + return [ + (int)$imageSize[0] ?: null, + (int)$imageSize[1] ?: null, + ]; + } catch (Throwable) { + return null; + } finally { + if (is_resource($stream)) { + fclose($stream); + } + } + } + + /** + * @return resource|null + */ + public function getFileStreamRange(string $uriPath, int $start, int $end) + { + if ($start < 0 || $end < $start) { + return null; + } + + $sourceStream = null; + + try { + if (!$this->useLocalFs) { + $bucket = $this->getBucketName(); + + if ($bucket === null) { + return null; + } + + $object = $this->getClient()->getObject([ + 'Bucket' => $bucket, + 'Key' => $this->createBucketPath($uriPath)->toString(), + 'Range' => "bytes=$start-$end", + ]); + + return $this->stringStream((string)$object->get('Body')); + } + + $sourceStream = $this->getFileStream($uriPath); + + if (fseek($sourceStream, $start) === -1) { + return null; + } + + $data = stream_get_contents($sourceStream, $end - $start + 1); + + return is_string($data) ? $this->stringStream($data) : null; + } catch (Throwable) { + return null; + } finally { + if (is_resource($sourceStream)) { + fclose($sourceStream); + } + } + } + + /** + * @return resource|null + */ + private function stringStream(string $contents) + { + $stream = fopen('php://temp', 'r+'); + + if ($stream === false) { + return null; + } + + fwrite($stream, $contents); + rewind($stream); + + return $stream; + } + /** * @inheritDoc */ diff --git a/tests/unit/AssetsControllerTest.php b/tests/unit/AssetsControllerTest.php index f6756e9..cd42362 100644 --- a/tests/unit/AssetsControllerTest.php +++ b/tests/unit/AssetsControllerTest.php @@ -5,9 +5,12 @@ use Codeception\Test\Unit; use Craft; use craft\cloud\controllers\AssetsController; +use craft\cloud\fs\Fs; use craft\elements\Asset; +use craft\helpers\Assets as AssetsHelper; use craft\models\Volume; use ReflectionMethod; +use yii\web\BadRequestHttpException; class AssetsControllerTest extends Unit { @@ -40,54 +43,54 @@ public function testVolumeSubpathReturnsVolumeSubpathOnCraft5(): void $this->assertSame('volume-prefix/', $this->invokeVolumeSubpath($volume)); } - public function testImageUploadsUseUploadedImageDimensions(): void + public function testImageDimensionsUseNullWhenUploadedImageDimensionsCannotBeRead(): void { - $controller = new DimensionTestAssetsController('cloud-assets', Craft::$app); - $controller->uploadedImageDimensions = [2139, 3020]; + $fs = new HeaderTestFs(); + $fs->header = 'not image data'; + + $this->assertNull($fs->getImageDimensions('upload.jpeg')); + $this->assertSame(1, $fs->readCount); + } + + public function testNonImageUploadsUseNullDimensions(): void + { + $fs = new HeaderTestFs(); + $fs->header = 'not image data'; - $this->assertSame([2139, 3020], $controller->uploadedImageDimensionsForTest( - new Asset(), - 'upload.jpeg', - )); - $this->assertSame(1, $controller->readCount); + $this->assertNull($fs->getImageDimensions('document.pdf')); + $this->assertSame(0, $fs->readCount); } - public function testImageUploadsUseNullDimensionsWhenUploadedImageDimensionsCannotBeRead(): void + public function testUploadedAssetSizeUsesActualVolumeSize(): void { $controller = new DimensionTestAssetsController('cloud-assets', Craft::$app); + $asset = new SizeTestAsset(123); + $asset->setFilename('upload.jpeg'); - $this->assertSame([null, null], $controller->uploadedImageDimensionsForTest( - new Asset(), - 'upload.jpeg', - )); - $this->assertSame(1, $controller->readCount); + $this->assertSame(123, $controller->uploadedAssetSizeForTest($asset, 'upload.jpeg')); } - public function testNonImageUploadsUseNullDimensions(): void + public function testUploadedAssetSizeRejectsOversizedActualFile(): void { $controller = new DimensionTestAssetsController('cloud-assets', Craft::$app); - $controller->uploadedImageDimensions = [2139, 3020]; + $asset = new SizeTestAsset((int)AssetsHelper::getMaxUploadSize() + 1); + $asset->setFilename('upload.jpeg'); + + $this->expectException(BadRequestHttpException::class); - $this->assertSame([null, null], $controller->uploadedImageDimensionsForTest( - new Asset(), - 'document.pdf', - )); - $this->assertSame(0, $controller->readCount); + $controller->uploadedAssetSizeForTest($asset, 'upload.jpeg'); } - public function testReplacementUploadsUseServerDimensionsForUploadedFile(): void + public function testImageDimensionsCanBeReadFromBoundedJpegHeader(): void { - $asset = new Asset(); - $asset->folderPath = 'uploads'; + $fs = new HeaderTestFs(); + $jpeg = "\xFF\xD8" + . "\xFF\xE1" . pack('n', 4) . 'xx' + . "\xFF\xC0" . pack('n', 17) . "\x08" . pack('n', 3020) . pack('n', 2139) . str_repeat("\0", 10); - $controller = new DimensionTestAssetsController('cloud-assets', Craft::$app); - $controller->uploadedImageDimensions = [1080, 1440]; + $fs->header = $jpeg; - $this->assertSame([1080, 1440], $controller->uploadedImageDimensionsForTest( - $asset, - 'upload-replacement.jpeg', - )); - $this->assertSame(1, $controller->readCount); + $this->assertSame([2139, 3020], $fs->getImageDimensions('upload.jpeg')); } private function invokeVolumeSubpath(Volume $volume): string @@ -102,20 +105,66 @@ private function invokeVolumeSubpath(Volume $volume): string class DimensionTestAssetsController extends AssetsController { - public ?array $uploadedImageDimensions = null; + public function uploadedAssetSizeForTest(Asset $asset, string $filename): int + { + return $this->uploadedAssetSize($asset, $filename); + } +} + +class HeaderTestFs extends Fs +{ + public string $header; public int $readCount = 0; - public function uploadedImageDimensionsForTest(Asset $asset, string $filename): array + public static function displayName(): string { - $asset->setFilename($filename); - - return $this->uploadedImageDimensions($asset, $filename); + return 'Header Test'; } - protected function readUploadedImageDimensions(Asset $asset): ?array + public function getFileStreamRange(string $uriPath, int $start, int $end) { $this->readCount++; - return $this->uploadedImageDimensions; + $stream = fopen('php://temp', 'r+'); + + if ($stream === false) { + return null; + } + + fwrite($stream, $this->header); + rewind($stream); + + return $stream; + } +} + +class SizeTestAsset extends Asset +{ + private int $fileSize; + + public function __construct(int $fileSize) + { + $this->fileSize = $fileSize; + + parent::__construct(); + } + + public function getVolume(): Volume + { + return new class($this->fileSize) extends Volume { + private int $fileSize; + + public function __construct(int $fileSize) + { + $this->fileSize = $fileSize; + + parent::__construct(); + } + + public function getFileSize(string $uri): int + { + return $this->fileSize; + } + }; } }