]> BookStack Code Mirror - bookstack/commitdiff
Fixed issue where more images than expected could be deleted
authorDan Brown <redacted>
Fri, 24 Jul 2020 22:41:59 +0000 (23:41 +0100)
committerDan Brown <redacted>
Fri, 24 Jul 2020 22:41:59 +0000 (23:41 +0100)
When deleting images, images within the same directory, that have
a suffix of the delete image name, would also be deleted.

Added test to cover.

app/Uploads/ImageService.php
tests/Uploads/ImageTest.php

index 756149fe7a1bacdc2f11a2f4ee3672ceb50c8676..78458dc3968b6ad16828ab00dbe212b08aed512b 100644 (file)
@@ -223,6 +223,7 @@ class ImageService extends UploadService
         $storage->setVisibility($thumbFilePath, 'public');
         $this->cache->put('images-' . $image->id . '-' . $thumbFilePath, $thumbFilePath, 60 * 60 * 72);
 
+
         return $this->getPublicUrl($thumbFilePath);
     }
 
@@ -292,11 +293,9 @@ class ImageService extends UploadService
 
     /**
      * Destroys an image at the given path.
-     * Searches for image thumbnails in addition to main provided path..
-     * @param string $path
-     * @return bool
+     * Searches for image thumbnails in addition to main provided path.
      */
-    protected function destroyImagesFromPath(string $path)
+    protected function destroyImagesFromPath(string $path): bool
     {
         $storage = $this->getStorage();
 
@@ -306,8 +305,7 @@ class ImageService extends UploadService
 
         // Delete image files
         $imagesToDelete = $allImages->filter(function ($imagePath) use ($imageFileName) {
-            $expectedIndex = strlen($imagePath) - strlen($imageFileName);
-            return strpos($imagePath, $imageFileName) === $expectedIndex;
+            return basename($imagePath) === $imageFileName;
         });
         $storage->delete($imagesToDelete->all());
 
index 416927ac93170bb1649429d7fdd354932befb6af..3cdcdf3fd6cf658bfc84b31fc0607d6b64514980 100644 (file)
@@ -262,10 +262,11 @@ class ImageTest extends TestCase
         $page = Page::first();
         $this->asAdmin();
         $imageName = 'first-image.png';
+        $relPath = $this->getTestImagePath('gallery', $imageName);
+        $this->deleteImage($relPath);
 
         $this->uploadImage($imageName, $page->id);
         $image = Image::first();
-        $relPath = $this->getTestImagePath('gallery', $imageName);
 
         $delete = $this->delete( '/images/' . $image->id);
         $delete->assertStatus(200);
@@ -278,6 +279,31 @@ class ImageTest extends TestCase
         $this->assertFalse(file_exists(public_path($relPath)), 'Uploaded image has not been deleted as expected');
     }
 
+    public function test_image_delete_does_not_delete_similar_images()
+    {
+        $page = Page::first();
+        $this->asAdmin();
+        $imageName = 'first-image.png';
+
+        $relPath = $this->getTestImagePath('gallery', $imageName);
+        $this->deleteImage($relPath);
+
+        $this->uploadImage($imageName, $page->id);
+        $this->uploadImage($imageName, $page->id);
+        $this->uploadImage($imageName, $page->id);
+
+        $image = Image::first();
+        $folder = public_path(dirname($relPath));
+        $imageCount = count(glob($folder . '/*'));
+
+        $delete = $this->delete( '/images/' . $image->id);
+        $delete->assertStatus(200);
+
+        $newCount = count(glob($folder . '/*'));
+        $this->assertEquals($imageCount - 1, $newCount, 'More files than expected have been deleted');
+        $this->assertFalse(file_exists(public_path($relPath)), 'Uploaded image has not been deleted as expected');
+    }
+
     protected function getTestProfileImage()
     {
         $imageName = 'profile.png';