Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.x] Sanitize asset folder name on creation #10577

Merged
merged 6 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Actions/RenameAssetFolder.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Statamic\Actions;

use Statamic\Contracts\Assets\AssetFolder;
use Statamic\Rules\AlphaDashSpace;

class RenameAssetFolder extends Action
{
Expand Down Expand Up @@ -43,7 +44,7 @@ protected function fieldItems()
return [
'name' => [
'type' => 'text',
'validate' => 'required|alpha_dash',
'validate' => ['required', 'string', new AlphaDashSpace],
'classes' => 'mousetrap',
'focus' => true,
'debounce' => false,
Expand Down
18 changes: 2 additions & 16 deletions src/Assets/AssetFolder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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), '/');

Expand All @@ -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}
*/
Expand Down
8 changes: 6 additions & 2 deletions src/Http/Controllers/CP/Assets/FoldersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

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\AlphaDashSpace;

class FoldersController extends CpController
{
Expand All @@ -15,10 +17,12 @@ public function store(Request $request, $container)

$request->validate([
'path' => 'required',
'directory' => 'required|alpha_dash',
'directory' => ['required', 'string', new AlphaDashSpace],
]);

$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([
Expand Down
17 changes: 17 additions & 0 deletions src/Rules/AlphaDashSpace.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

namespace Statamic\Rules;

use Closure;
use Illuminate\Contracts\Validation\ValidationRule;

class AlphaDashSpace implements ValidationRule
{
public function validate(string $attribute, mixed $value, Closure $fail): void
{
// Same as `alpha_dash`, but allows spaces
if (! preg_match('/\A[ \pL\pM\pN_-]+\z/u', $value)) {
$fail('statamic::validation.alpha_dash')->translate();
}
}
}
85 changes: 85 additions & 0 deletions tests/Assets/AssetFolderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
Loading