Merge branch '1' into 2.0

silverstripe · Aug 19, 2023 · 3b2f72d · 3b2f72d
2 parents 20b6bff + c0d5a19
commit 3b2f72d
Show file tree

Hide file tree

Showing 11 changed files with 120 additions and 14 deletions.
diff --git a/src/AssetControlExtension.php b/src/AssetControlExtension.php
@@ -52,6 +52,10 @@ class AssetControlExtension extends DataExtension
      */
     public function onAfterDelete()
     {
+        if (!$this->hasAssets()) {
+            return;
+        }
+
         // Prepare blank manipulation
         $manipulations = new AssetManipulationList();
 
@@ -71,6 +75,10 @@ public function onAfterDelete()
      */
     public function onBeforeWrite()
     {
+        if (!$this->hasAssets()) {
+            return;
+        }
+
         // Prepare blank manipulation
         $manipulations = new AssetManipulationList();
 
@@ -209,6 +217,18 @@ protected function addAssetsFromRecord(AssetManipulationList $manipulation, Data
         }
     }
 
+    private function hasAssets(): bool
+    {
+        $fields = DataObject::getSchema()->fieldSpecs($this->owner);
+        foreach ($fields as $field => $db) {
+            $fieldObj = $this->owner->dbObject($field);
+            if ($fieldObj instanceof DBFile) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     /**
      * Return a list of all tuples attached to this dataobject
      * Note: Variants are excluded

diff --git a/src/File.php b/src/File.php
@@ -680,7 +680,7 @@ protected function onBeforeWrite()
         $currentname = $name = $this->getField('Name');
         $title = $this->getField('Title');
 
-        $changed = $this->isChanged('Name');
+        $changed = $this->isChanged('Name') || $this->isChanged('ParentID');
 
         // Name can't be blank, default to Title or singular name
         if (!$name) {

diff --git a/src/InterventionBackend.php b/src/InterventionBackend.php
@@ -172,7 +172,7 @@ public function getAssetContainer()
      */
     public function setAssetContainer($assetContainer)
     {
-        $this->image = null;
+        $this->setImageResource(null);
         $this->container = $assetContainer;
         return $this;
     }
@@ -293,9 +293,6 @@ public function getImageResource()
             if ($error) {
                 $this->markFailed($hash, $variant, $error);
             }
-            if (isset($path) && file_exists($path)) {
-                unlink($path);
-            }
         }
         return null;
     }
@@ -340,6 +337,12 @@ public function loadFrom($path)
     public function setImageResource($image)
     {
         $this->image = $image;
+        if ($image === null) {
+            // remove our temp file if it exists
+            if (file_exists($this->getTempPath() ?? '')) {
+                unlink($this->getTempPath());
+            }
+        }
         return $this;
     }
 

diff --git a/src/Shortcodes/FileLinkTracking.php b/src/Shortcodes/FileLinkTracking.php
@@ -120,15 +120,22 @@ public function augmentSyncLinkTracking()
         $allFields = DataObject::getSchema()->fieldSpecs($this->owner);
         $linkedPages = [];
         $anyBroken = false;
+        $hasTrackedFields = false;
         foreach ($allFields as $field => $fieldSpec) {
             $fieldObj = $this->owner->dbObject($field);
             if ($fieldObj instanceof DBHTMLText) {
+                $hasTrackedFields = true;
                 // Merge links in this field with global list.
                 $linksInField = $this->trackLinksInField($field, $anyBroken);
                 $linkedPages = array_merge($linkedPages, $linksInField);
             }
         }
 
+        // We cannot rely on linkedPages being empty, because we need to remove them if there was any
+        if (!$hasTrackedFields) {
+            return;
+        }
+
         // Soft support for HasBrokenFile db field (e.g. SiteTree)
         if ($this->owner->hasField('HasBrokenFile')) {
             $this->owner->HasBrokenFile = $anyBroken;

diff --git a/src/Shortcodes/ImageShortcodeProvider.php b/src/Shortcodes/ImageShortcodeProvider.php
@@ -5,8 +5,6 @@
 use Psr\SimpleCache\CacheInterface;
 use SilverStripe\Assets\File;
 use SilverStripe\Assets\Image;
-use SilverStripe\Assets\Storage\AssetStore;
-use SilverStripe\Core\Convert;
 use SilverStripe\Core\Flushable;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\View\HTML;
@@ -119,7 +117,7 @@ public static function handle_shortcode($args, $content, $parser, $shortcode, $e
             return in_array($k, $whitelist) && (strlen(trim($v ?? '')) || $k === 'alt');
         }, ARRAY_FILTER_USE_BOTH);
 
-        $markup = HTML::createTag('img', $attrs);
+        $markup = self::createImageTag($attrs);
 
         // cache it for future reference
         if ($fileFound) {
@@ -133,6 +131,25 @@ public static function handle_shortcode($args, $content, $parser, $shortcode, $e
         return $markup;
     }
 
+    /**
+     * Construct and return HTML image tag.
+     */
+    public static function createImageTag(array $attributes) : string
+    {
+        $preparedAttributes = '';
+        foreach ($attributes as $attributeKey => $attributeValue) {
+            if (strlen($attributeValue ?? '') > 0 || $attributeKey === 'alt') {
+                $preparedAttributes .= sprintf(
+                    ' %s="%s"',
+                    $attributeKey,
+                    htmlspecialchars($attributeValue ?? '', ENT_QUOTES, 'UTF-8', false)
+                );
+            }
+        }
+
+        return "<img{$preparedAttributes} />";
+    }
+
     /**
      * Regenerates "[image id=n]" shortcode with new src attribute prior to being edited within the CMS.
      *
@@ -156,7 +173,7 @@ public static function regenerate_shortcode($args, $content, $parser, $shortcode
         // Rebuild shortcode
         $parts = [];
         foreach ($args as $name => $value) {
-            $htmlValue = Convert::raw2att($value);
+            $htmlValue = htmlspecialchars($value ?? '', ENT_QUOTES, 'UTF-8', false);
             $parts[] = sprintf('%s="%s"', $name, $htmlValue);
         }
         return sprintf("[%s %s]", $shortcode, implode(' ', $parts));

diff --git a/tests/php/FileTest.php b/tests/php/FileTest.php
@@ -1257,4 +1257,27 @@ public function testArchivingModifiedKeepArchivedBothPhysicalFilesWithDifferentF
         $this->assertTrue($store->exists('file-changed.txt', $secondHash));
         $this->assertSame(AssetStore::VISIBILITY_PROTECTED, $store->getVisibility('file-changed.txt', $secondHash));
     }
+
+    public function testMoveFileRenamesDuplicateFilename()
+    {
+        $folder1 = $this->objFromFixture(Folder::class, 'folder1');
+        $folder2 = $this->objFromFixture(Folder::class, 'folder2');
+        $file1 = $this->objFromFixture(File::class, 'pdf');
+        $file1->ParentID = $folder1->ID;
+        $file1->write();
+        $file2 = File::create([
+            'FileFilename' => $file1->FileFilename,
+            'FileHash' => $file1->FileHash,
+            'Name' => $file1->Name,
+            'ParentID' => $folder2->ID,
+        ]);
+        $file2->write();
+        $this->assertTrue(strpos($file1->getFilename(), 'FileTest.pdf') !== false);
+        $this->assertTrue(strpos($file2->getFilename(), 'FileTest.pdf') !== false);
+        // Move file1 to folder2 and ensure it gets renamed as it would have a duplicate filename
+        $file1->ParentID = $folder2->ID;
+        $file1->write();
+        $this->assertTrue(strpos($file1->getFilename(), 'FileTest-v2.pdf') !== false);
+        $this->assertTrue(strpos($file2->getFilename(), 'FileTest.pdf') !== false);
+    }
 }
diff --git a/tests/php/ImageTest.php b/tests/php/ImageTest.php
@@ -89,7 +89,7 @@ public function testGetTagWithoutTitle()
         Config::modify()->set(DBFile::class, 'force_resample', false);
 
         $image = $this->objFromFixture(Image::class, 'imageWithoutTitle');
-        $expected = '<img width="300" height="300" alt="test image" src="/assets/ImageTest/folder/test-image.png" loading="lazy" />';
+        $expected = '<img width="300" height="300" alt="test image without title" src="/assets/ImageTest/folder/test-image-without-title.png" loading="lazy" />';
         $actual = trim($image->getTag() ?? '');
 
         $this->assertEquals($expected, $actual);

diff --git a/tests/php/ImageTest.yml b/tests/php/ImageTest.yml
@@ -9,21 +9,21 @@ SilverStripe\Assets\Image:
     Parent: =>SilverStripe\Assets\Folder.folder1
     Name: test-image.png
   imageWithoutTitle:
-    FileFilename: folder/test-image.png
+    FileFilename: folder/test-image-without-title.png
     FileHash: 444065542b5dd5187166d8e1cd684e0d724c5a97
     Parent: =>SilverStripe\Assets\Folder.folder1
-    Name: test-image.png
+    Name: test-image-without-title.png
   imageWithoutTitleContainingDots:
     FileFilename: folder/test.image.with.dots.png
     FileHash: 46affab7043cfd9f1ded919dd24affd08e926eca
     Parent: =>SilverStripe\Assets\Folder.folder1
     Name: test.image.with.dots.png
   imageWithMetacharacters:
     Title: This is a/an image Title
-    FileFilename: folder/test-image.png
+    FileFilename: folder/test-image-metacharacters.png
     FileHash: 444065542b5dd5187166d8e1cd684e0d724c5a97
     Parent: =>SilverStripe\Assets\Folder.folder1
-    Name: test-image.png
+    Name: test-image-metacharacters.png
   lowQualityJPEG:
     Title: This is a low quality JPEG
     FileFilename: folder/test-image-low-quality.jpg

diff --git a/tests/php/ImageTest/test-image-metacharacters.png b/tests/php/ImageTest/test-image-metacharacters.png
diff --git a/tests/php/ImageTest/test-image-without-title.png b/tests/php/ImageTest/test-image-without-title.png
diff --git a/tests/php/Shortcodes/ImageShortcodeProviderTest.php b/tests/php/Shortcodes/ImageShortcodeProviderTest.php
@@ -344,4 +344,40 @@ public function testWhiteIsConfigurable()
             ))
         );
     }
+
+    public function gettersAndSettersProvider(): array
+    {
+        return [
+                    'image without special characters' => [
+                        '<img src="http://example.com/image.jpg" alt="My alt text" title="My Title" width="300" height="200" class="leftAlone ss-htmleditorfield-file image" />',
+                        [
+                            'src' => 'http://example.com/image.jpg',
+                            'alt' => 'My alt text',
+                            'title' => 'My Title',
+                            'width' => '300',
+                            'height' => '200',
+                            'class' => 'leftAlone ss-htmleditorfield-file image',
+                        ],
+                    ],
+                    'image with special characters' => [
+                        '<img src="http://example.com/image.jpg" alt="My alt text &amp; special character" title="My Title &amp; special character" width="300" height="200" class="leftAlone ss-htmleditorfield-file image" />',
+                        [
+                            'src' => 'http://example.com/image.jpg',
+                            'alt' => 'My alt text &amp; special character',
+                            'title' => 'My Title & special character',
+                            'width' => '300',
+                            'height' => '200',
+                            'class' => 'leftAlone ss-htmleditorfield-file image',
+                        ]
+                    ]
+                ];
+    }
+
+    /**
+     * @dataProvider gettersAndSettersProvider
+     */
+    public function testCreateImageTag(string $expected, array $attributes)
+    {
+        $this->assertEquals($expected, ImageShortcodeProvider::createImageTag($attributes));
+    }
 }