Thumbnail image size improvements

code/Forms/UploadField.php

@@ -42,13 +42,13 @@ class UploadField extends FormField implements FileHandleField
      * @config
      * @var int
      */
-    private static $thumbnail_width = 60;
+    private static $thumbnail_width = 120;
 
     /**
      * @config
      * @var int
      */
-    private static $thumbnail_height = 60;
+    private static $thumbnail_height = 120;
 
     /**
      * Set if uploading new files is enabled.

code/GraphQL/Resolvers/FileTypeResolver.php

@@ -90,12 +90,27 @@ public static function resolveFileSmallThumbnail($object)
      */
     public static function resolveFileThumbnail($object)
     {
+        // If we're allowed to generate thumbnails for this file, tell the generator it's allowed to do it.
+        $generator = static::singleton()->getThumbnailGenerator();
+        $idsAllowed = FolderTypeResolver::getIdsAllowedToGenerateThumbnails();
+        $shouldGenerateThumbnail = !empty($idsAllowed) && in_array($object->ID, $idsAllowed);
+        if ($shouldGenerateThumbnail) {
+            $origGenerates = $generator->getGenerates();
+            $generator->setGenerates(true);
+        }
+
         // Make large thumbnail
         $width = AssetAdmin::config()->uninherited('thumbnail_width');
         $height = AssetAdmin::config()->uninherited('thumbnail_height');
-        return static::singleton()
-            ->getThumbnailGenerator()
-            ->generateThumbnailLink($object, $width, $height);
+
+        try {
+            return $generator->generateThumbnailLink($object, $width, $height);
+        } finally {
+            // Make sure to set the generates value back to what it was, regardless of what happens
+            if ($shouldGenerateThumbnail) {
+                $generator->setGenerates($origGenerates);
+            }
+        }
     }
 
     /**

code/GraphQL/Resolvers/FolderTypeResolver.php

@@ -18,10 +18,20 @@
 use InvalidArgumentException;
 use Exception;
 use Closure;
+use SilverStripe\AssetAdmin\Controller\AssetAdmin;
 use SilverStripe\ORM\DataQuery;
 
 class FolderTypeResolver
 {
+    /**
+     * Any IDs of files which we're allowed to generate thumbnails for.
+     * The intention is to (as much as is feasible) only allow generating thumbnails
+     * during asset admin graphQL requests and not for requests that could be performed
+     * by an attacker.
+     * @internal
+     */
+    private static array $idsAllowedToGenerateThumbnails = [];
+
     /**
      * @param Folder $object
      * @param array $args
@@ -74,6 +84,17 @@ public static function resolveFolderChildren(
         $canViewList = $list->filter('ID', $canViewIDs ?: 0)
             ->limit(null);
 
+        // If we haven't already marked IDs as being okay to generate thumbnails,
+        // and we have a safe limit for the number of children,
+        // mark these children as being okay to generate thumbnails for.
+        if (empty(self::$idsAllowedToGenerateThumbnails)
+            && !empty($args['limit'])
+            && is_numeric($args['limit'])
+            && (int)$args['limit'] <= AssetAdmin::config()->get('page_length')
+        ) {
+            self::$idsAllowedToGenerateThumbnails = $canViewIDs;
+        }
+
         return $canViewList;
     }
 
@@ -182,4 +203,9 @@ public static function sortChildren(array $context): Closure
             return $list;
         };
     }
+
+    public static function getIdsAllowedToGenerateThumbnails(): array
+    {
+        return self::$idsAllowedToGenerateThumbnails;
+    }
 }

code/Model/ThumbnailGenerator.php

@@ -63,7 +63,7 @@ class ThumbnailGenerator
      * @var string
      * @config
      */
-    private static $method = 'FitMax';
+    private static $method = 'Fill';
 
     /**
      * Generate thumbnail and return the "src" property for this thumbnail

tests/php/GraphQL/FileTypeCreatorTest.php

@@ -58,19 +58,19 @@ public function testThumbnail()
 
         // protected image should have inline thumbnail
         $thumbnail = FileTypeResolver::resolveFileThumbnail($image, [], [], null);
-        $this->assertStringStartsWith('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAWAAAADr', $thumbnail);
+        $this->assertStringStartsWith('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAWAAAAEI', $thumbnail);
 
         // public image should have url
         $image->publishSingle();
         $thumbnail = FileTypeResolver::resolveFileThumbnail($image, [], [], null);
-        $this->assertEquals('/assets/FileTypeCreatorTest/TestImage__FitMaxWzM1MiwyNjRd.png', $thumbnail);
+        $this->assertEquals('/assets/FileTypeCreatorTest/TestImage__FillWzM1MiwyNjRd.png', $thumbnail);
 
         // Public assets can be set to inline
         ThumbnailGenerator::config()->merge('thumbnail_links', [
             AssetStore::VISIBILITY_PUBLIC => ThumbnailGenerator::INLINE,
         ]);
         $thumbnail = FileTypeResolver::resolveFileThumbnail($image, [], [], null);
-        $this->assertStringStartsWith('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAWAAAADr', $thumbnail);
+        $this->assertStringStartsWith('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAWAAAAEI', $thumbnail);
 
         // Protected assets can be set to url
         // This uses protected asset adapter, so not direct asset link
@@ -79,6 +79,6 @@ public function testThumbnail()
         ]);
         $image->doUnpublish();
         $thumbnail = FileTypeResolver::resolveFileThumbnail($image, [], [], null);
-        $this->assertEquals('/assets/8cf6c65fa7/TestImage__FitMaxWzM1MiwyNjRd.png', $thumbnail);
+        $this->assertEquals('/assets/8cf6c65fa7/TestImage__FillWzM1MiwyNjRd.png', $thumbnail);
     }
 }

tests/php/GraphQL/Legacy/FileTypeCreatorTest.php

@@ -49,7 +49,6 @@ public function testThumbnail()
         $image = new Image();
         $image->setFromLocalFile(__DIR__.'/../../Forms/fixtures/largeimage.png', 'TestImage.png');
         $image->write();
-
         // Image original is unset
         $thumbnail = $type->resolveThumbnailField($image, [], [], null);
         $this->assertNull($thumbnail);
@@ -59,19 +58,19 @@ public function testThumbnail()
 
         // protected image should have inline thumbnail
         $thumbnail = $type->resolveThumbnailField($image, [], [], null);
-        $this->assertStringStartsWith('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAWAAAADr', $thumbnail);
+        $this->assertStringStartsWith('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAWAAAAEI', $thumbnail);
 
         // public image should have url
         $image->publishSingle();
         $thumbnail = $type->resolveThumbnailField($image, [], [], null);
-        $this->assertEquals('/assets/FileTypeCreatorTest/TestImage__FitMaxWzM1MiwyNjRd.png', $thumbnail);
+        $this->assertEquals('/assets/FileTypeCreatorTest/TestImage__FillWzM1MiwyNjRd.png', $thumbnail);
 
         // Public assets can be set to inline
         ThumbnailGenerator::config()->merge('thumbnail_links', [
             AssetStore::VISIBILITY_PUBLIC => ThumbnailGenerator::INLINE,
         ]);
         $thumbnail = $type->resolveThumbnailField($image, [], [], null);
-        $this->assertStringStartsWith('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAWAAAADr', $thumbnail);
+        $this->assertStringStartsWith('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAWAAAAEI', $thumbnail);
 
         // Protected assets can be set to url
         // This uses protected asset adapter, so not direct asset link
@@ -80,6 +79,6 @@ public function testThumbnail()
         ]);
         $image->doUnpublish();
         $thumbnail = $type->resolveThumbnailField($image, [], [], null);
-        $this->assertEquals('/assets/8cf6c65fa7/TestImage__FitMaxWzM1MiwyNjRd.png', $thumbnail);
+        $this->assertEquals('/assets/8cf6c65fa7/TestImage__FillWzM1MiwyNjRd.png', $thumbnail);
     }
 }

tests/php/Model/ThumbnailGeneratorTest.php

@@ -39,7 +39,7 @@ public function testGenerateThumbnail()
         // protected image should have inline thumbnail
         $thumbnail = $generator->generateThumbnail($image, 100, 200);
         $this->assertEquals(100, $thumbnail->getWidth());
-        $this->assertEquals(50, $thumbnail->getHeight()); // Note: Aspect ratio of original image retained
+        $this->assertEquals(200, $thumbnail->getHeight());
 
         // Build non-image
         $file = new File();
@@ -57,6 +57,20 @@ public function testGenerateThumbnail()
         $this->assertNull($thumbnail);
     }
 
+    public function testGenerateThumbnailFitMax()
+    {
+        ThumbnailGenerator::config()->set('method', 'FitMax');
+        $generator = new ThumbnailGenerator();
+        // Build image
+        $image = new Image();
+        $image->setFromLocalFile(__DIR__ . '/../Forms/fixtures/testimage.png', 'TestImage.png');
+        $image->write();
+
+        $thumbnail = $generator->generateThumbnail($image, 100, 200);
+        $this->assertEquals(100, $thumbnail->getWidth());
+        $this->assertEquals(50, $thumbnail->getHeight()); // Note: Aspect ratio of original image retained
+    }
+
     public function testGenerateLink()
     {
         $generator = new ThumbnailGenerator();
@@ -82,7 +96,7 @@ public function testGenerateLink()
 
         // protected image should have inline thumbnail
         $thumbnail = $generator->generateThumbnailLink($image, 100, 200);
-        $this->assertStringStartsWith('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAGQAAAAy', $thumbnail);
+        $this->assertStringStartsWith('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAGQAAADI', $thumbnail);
 
         // but not if it's too big
         Config::nest();
@@ -92,20 +106,20 @@ public function testGenerateLink()
         $this->assertNull($thumbnail);
         // With graceful thumbnails, it should come back as a URL
         $thumbnail = $generator->generateThumbnailLink($image, 100, 200, true);
-        $this->assertMatchesRegularExpression('#/assets/[A-Za-z0-9]+/TestImage__FitMaxWzEwMCwyMDBd\.png$#', $thumbnail);
+        $this->assertMatchesRegularExpression('#/assets/[A-Za-z0-9]+/TestImage__FillWzEwMCwyMDBd\.png$#', $thumbnail);
         Config::unnest();
 
         // public image should have url
         $image->publishSingle();
         $thumbnail = $generator->generateThumbnailLink($image, 100, 200);
-        $this->assertEquals('/assets/ThumbnailGeneratorTest/TestImage__FitMaxWzEwMCwyMDBd.png', $thumbnail);
+        $this->assertEquals('/assets/ThumbnailGeneratorTest/TestImage__FillWzEwMCwyMDBd.png', $thumbnail);
 
         // Public assets can be set to inline
         ThumbnailGenerator::config()->merge('thumbnail_links', [
             AssetStore::VISIBILITY_PUBLIC => ThumbnailGenerator::INLINE,
         ]);
         $thumbnail = $generator->generateThumbnailLink($image, 100, 200);
-        $this->assertStringStartsWith('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAGQAAAAy', $thumbnail);
+        $this->assertStringStartsWith('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAGQAAADI', $thumbnail);
 
         // Protected assets can be set to url
         // This uses protected asset adapter, so not direct asset link
@@ -114,6 +128,6 @@ public function testGenerateLink()
         ]);
         $image->doUnpublish();
         $thumbnail = $generator->generateThumbnailLink($image, 100, 200);
-        $this->assertEquals('/assets/906835357d/TestImage__FitMaxWzEwMCwyMDBd.png', $thumbnail);
+        $this->assertEquals('/assets/906835357d/TestImage__FillWzEwMCwyMDBd.png', $thumbnail);
     }
 }