From b6aa232205d1f889be95a57e76ac391023e00bfd Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Fri, 24 Jul 2020 23:41:59 +0100
Subject: [PATCH] Fixed issue where more images than expected could be deleted
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 | 10 ++++------
tests/Uploads/ImageTest.php | 28 +++++++++++++++++++++++++++-
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php
index 756149fe7..78458dc39 100644
--- a/app/Uploads/ImageService.php
+++ b/app/Uploads/ImageService.php
@@ -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());
diff --git a/tests/Uploads/ImageTest.php b/tests/Uploads/ImageTest.php
index 416927ac9..3cdcdf3fd 100644
--- a/tests/Uploads/ImageTest.php
+++ b/tests/Uploads/ImageTest.php
@@ -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';