From 9a00017629e4b5c99a998c5b97efa3315310fe33 Mon Sep 17 00:00:00 2001 From: Philipp Daun Date: Tue, 6 Aug 2024 13:54:57 +0200 Subject: [PATCH 1/6] Create rule for allowed folder name --- src/Rules/AllowedFolder.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 src/Rules/AllowedFolder.php diff --git a/src/Rules/AllowedFolder.php b/src/Rules/AllowedFolder.php new file mode 100644 index 0000000000..d2062a21c7 --- /dev/null +++ b/src/Rules/AllowedFolder.php @@ -0,0 +1,17 @@ +translate(); + } + } +} From e45a9ad829765d31ce184dde96dc26ce377073fa Mon Sep 17 00:00:00 2001 From: Philipp Daun Date: Tue, 6 Aug 2024 13:55:32 +0200 Subject: [PATCH 2/6] Use folder name rule instead of alpha_dash --- src/Actions/RenameAssetFolder.php | 3 ++- src/Http/Controllers/CP/Assets/FoldersController.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Actions/RenameAssetFolder.php b/src/Actions/RenameAssetFolder.php index bbdefb70ea..a90078c82f 100644 --- a/src/Actions/RenameAssetFolder.php +++ b/src/Actions/RenameAssetFolder.php @@ -3,6 +3,7 @@ namespace Statamic\Actions; use Statamic\Contracts\Assets\AssetFolder; +use Statamic\Rules\AllowedFolder; class RenameAssetFolder extends Action { @@ -43,7 +44,7 @@ protected function fieldItems() return [ 'name' => [ 'type' => 'text', - 'validate' => 'required|alpha_dash', + 'validate' => ['required', 'string', new AllowedFolder], 'classes' => 'mousetrap', 'focus' => true, 'debounce' => false, diff --git a/src/Http/Controllers/CP/Assets/FoldersController.php b/src/Http/Controllers/CP/Assets/FoldersController.php index 8dbff9f44a..6e18e9619b 100644 --- a/src/Http/Controllers/CP/Assets/FoldersController.php +++ b/src/Http/Controllers/CP/Assets/FoldersController.php @@ -6,6 +6,7 @@ use Illuminate\Validation\ValidationException; use Statamic\Facades\Path; use Statamic\Http\Controllers\CP\CpController; +use Statamic\Rules\AllowedFolder; class FoldersController extends CpController { @@ -15,7 +16,7 @@ public function store(Request $request, $container) $request->validate([ 'path' => 'required', - 'directory' => 'required|alpha_dash', + 'directory' => ['required', 'string', new AllowedFolder], ]); $path = ltrim(Path::assemble($request->path, $request->directory), '/'); From 3fd568f9542d05fa004b76d7c710835a6108cf53 Mon Sep 17 00:00:00 2001 From: Philipp Daun Date: Tue, 6 Aug 2024 13:56:05 +0200 Subject: [PATCH 3/6] Re-use filename sanitizer from Uploader --- src/Assets/AssetFolder.php | 18 ++---------------- .../CP/Assets/FoldersController.php | 5 ++++- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/Assets/AssetFolder.php b/src/Assets/AssetFolder.php index c43f194c53..c4a300799b 100644 --- a/src/Assets/AssetFolder.php +++ b/src/Assets/AssetFolder.php @@ -3,6 +3,7 @@ namespace Statamic\Assets; use Illuminate\Contracts\Support\Arrayable; +use Statamic\Assets\AssetUploader as Uploader; use Statamic\Contracts\Assets\AssetFolder as Contract; use Statamic\Events\AssetFolderDeleted; use Statamic\Events\AssetFolderSaved; @@ -165,7 +166,7 @@ public function move($parent, $name = null) throw new \Exception('Folder cannot be moved to its own subfolder.'); } - $name = $this->getSafeBasename($name ?? $this->basename()); + $name = Uploader::getSafeFilename($name ?? $this->basename()); $oldPath = $this->path(); $newPath = Str::removeLeft(Path::tidy($parent.'/'.$name), '/'); @@ -185,21 +186,6 @@ public function move($parent, $name = null) return $folder; } - /** - * Ensure safe basename string. - * - * @param string $string - * @return string - */ - private function getSafeBasename($string) - { - if (config('statamic.assets.lowercase')) { - $string = strtolower($string); - } - - return (string) $string; - } - /** * {@inheritdoc} */ diff --git a/src/Http/Controllers/CP/Assets/FoldersController.php b/src/Http/Controllers/CP/Assets/FoldersController.php index 6e18e9619b..5274934c9c 100644 --- a/src/Http/Controllers/CP/Assets/FoldersController.php +++ b/src/Http/Controllers/CP/Assets/FoldersController.php @@ -4,6 +4,7 @@ use Illuminate\Http\Request; use Illuminate\Validation\ValidationException; +use Statamic\Assets\AssetUploader; use Statamic\Facades\Path; use Statamic\Http\Controllers\CP\CpController; use Statamic\Rules\AllowedFolder; @@ -19,7 +20,9 @@ public function store(Request $request, $container) 'directory' => ['required', 'string', new AllowedFolder], ]); - $path = ltrim(Path::assemble($request->path, $request->directory), '/'); + $name = AssetUploader::getSafeFilename($request->directory); + + $path = ltrim(Path::assemble($request->path, $name), '/'); if ($container->disk()->exists($path)) { throw ValidationException::withMessages([ From 44c7fb985c05e1dad1e8b1e510218e1f461d35ff Mon Sep 17 00:00:00 2001 From: Philipp Daun Date: Tue, 6 Aug 2024 14:27:53 +0200 Subject: [PATCH 4/6] Add test for sanitized folder names --- tests/Assets/AssetFolderTest.php | 85 ++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/tests/Assets/AssetFolderTest.php b/tests/Assets/AssetFolderTest.php index 740d62c20a..e287be9843 100644 --- a/tests/Assets/AssetFolderTest.php +++ b/tests/Assets/AssetFolderTest.php @@ -708,6 +708,91 @@ public function it_doesnt_lowercase_moved_folders_when_configured() } } + #[Test] + public function it_sanitizes_when_moving_to_another_folder_with_a_new_folder_name() + { + $container = $this->containerWithDisk(); + $disk = $container->disk()->filesystem(); + + $paths = collect([ + 'move/one.txt', + 'move/two.txt', + 'move/sub/three.txt', + 'move/sub/subsub/four.txt', + 'destination/folder/five.txt', + ]); + + $paths->each(function ($path) use ($disk, $container) { + $disk->put($path, ''); + $container->makeAsset($path)->save(); + }); + + $folder = (new Folder)->container($container)->path('move'); + + Event::fake(); + + $folder->move('destination/folder', 'new: move'); + + $disk->assertMissing('move'); + $disk->assertMissing('move/sub'); + $disk->assertMissing('move/sub/subsub'); + + $disk->assertExists('destination/folder/new-move'); + $disk->assertExists('destination/folder/new-move/sub'); + $disk->assertExists('destination/folder/new-move/sub/subsub'); + + $this->assertEquals([ + 'destination', + 'destination/folder', + 'destination/folder/new-move', + 'destination/folder/new-move/sub', + 'destination/folder/new-move/sub/subsub', + ], $container->folders()->all()); + + $this->assertEquals([ + 'destination', + 'destination/folder', + 'destination/folder/five.txt', + 'destination/folder/new-move', + 'destination/folder/new-move/sub', + 'destination/folder/new-move/sub/subsub', + 'destination/folder/new-move/sub/subsub/four.txt', + 'destination/folder/new-move/sub/three.txt', + 'destination/folder/new-move/one.txt', + 'destination/folder/new-move/two.txt', + ], $container->contents()->cached()->keys()->all()); + + // Assert asset folder events. + $paths = ['move', 'move/sub', 'move/sub/subsub']; + Event::assertDispatchedTimes(AssetFolderDeleted::class, count($paths)); + Event::assertDispatchedTimes(AssetFolderSaved::class, count($paths)); + foreach ($paths as $path) { + Event::assertDispatched(AssetFolderDeleted::class, function (AssetFolderDeleted $event) use ($path) { + return $event->folder->path() === $path; + }); + } + $paths = ['new-move', 'new-move/sub', 'new-move/sub/subsub']; + foreach ($paths as $path) { + Event::assertDispatched(AssetFolderSaved::class, function (AssetFolderSaved $event) use ($path) { + return $event->folder->path() === 'destination/folder/'.$path; + }); + } + + // Assert asset events. + $paths = [ + 'destination/folder/new-move/one.txt', + 'destination/folder/new-move/two.txt', + 'destination/folder/new-move/sub/three.txt', + 'destination/folder/new-move/sub/subsub/four.txt', + ]; + Event::assertDispatchedTimes(AssetSaved::class, count($paths)); + foreach ($paths as $path) { + Event::assertDispatched(AssetSaved::class, function (AssetSaved $event) use ($path) { + return $event->asset->path() === $path; + }); + } + } + #[Test] public function it_cannot_be_moved_to_its_own_subfolder() { From 3135367b15a968cf28936d601699483ed66ea517 Mon Sep 17 00:00:00 2001 From: Philipp Daun Date: Tue, 6 Aug 2024 15:14:43 +0200 Subject: [PATCH 5/6] Use simplified test folder name --- tests/Assets/AssetFolderTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Assets/AssetFolderTest.php b/tests/Assets/AssetFolderTest.php index e287be9843..3e181ea3bc 100644 --- a/tests/Assets/AssetFolderTest.php +++ b/tests/Assets/AssetFolderTest.php @@ -731,7 +731,7 @@ public function it_sanitizes_when_moving_to_another_folder_with_a_new_folder_nam Event::fake(); - $folder->move('destination/folder', 'new: move'); + $folder->move('destination/folder', 'new move'); $disk->assertMissing('move'); $disk->assertMissing('move/sub'); From bce49361af96fb757940f5f73375c133f10bce8e Mon Sep 17 00:00:00 2001 From: Philipp Daun Date: Tue, 6 Aug 2024 16:58:37 +0200 Subject: [PATCH 6/6] Rename alpha-dash-space validation rule --- src/Actions/RenameAssetFolder.php | 4 ++-- src/Http/Controllers/CP/Assets/FoldersController.php | 4 ++-- src/Rules/{AllowedFolder.php => AlphaDashSpace.php} | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) rename src/Rules/{AllowedFolder.php => AlphaDashSpace.php} (89%) diff --git a/src/Actions/RenameAssetFolder.php b/src/Actions/RenameAssetFolder.php index a90078c82f..bc61c2f677 100644 --- a/src/Actions/RenameAssetFolder.php +++ b/src/Actions/RenameAssetFolder.php @@ -3,7 +3,7 @@ namespace Statamic\Actions; use Statamic\Contracts\Assets\AssetFolder; -use Statamic\Rules\AllowedFolder; +use Statamic\Rules\AlphaDashSpace; class RenameAssetFolder extends Action { @@ -44,7 +44,7 @@ protected function fieldItems() return [ 'name' => [ 'type' => 'text', - 'validate' => ['required', 'string', new AllowedFolder], + 'validate' => ['required', 'string', new AlphaDashSpace], 'classes' => 'mousetrap', 'focus' => true, 'debounce' => false, diff --git a/src/Http/Controllers/CP/Assets/FoldersController.php b/src/Http/Controllers/CP/Assets/FoldersController.php index 5274934c9c..031ac13901 100644 --- a/src/Http/Controllers/CP/Assets/FoldersController.php +++ b/src/Http/Controllers/CP/Assets/FoldersController.php @@ -7,7 +7,7 @@ use Statamic\Assets\AssetUploader; use Statamic\Facades\Path; use Statamic\Http\Controllers\CP\CpController; -use Statamic\Rules\AllowedFolder; +use Statamic\Rules\AlphaDashSpace; class FoldersController extends CpController { @@ -17,7 +17,7 @@ public function store(Request $request, $container) $request->validate([ 'path' => 'required', - 'directory' => ['required', 'string', new AllowedFolder], + 'directory' => ['required', 'string', new AlphaDashSpace], ]); $name = AssetUploader::getSafeFilename($request->directory); diff --git a/src/Rules/AllowedFolder.php b/src/Rules/AlphaDashSpace.php similarity index 89% rename from src/Rules/AllowedFolder.php rename to src/Rules/AlphaDashSpace.php index d2062a21c7..4af5013114 100644 --- a/src/Rules/AllowedFolder.php +++ b/src/Rules/AlphaDashSpace.php @@ -5,7 +5,7 @@ use Closure; use Illuminate\Contracts\Validation\ValidationRule; -class AllowedFolder implements ValidationRule +class AlphaDashSpace implements ValidationRule { public function validate(string $attribute, mixed $value, Closure $fail): void {