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

Prevent copy and move to same destinations as the source. #1715

Merged
merged 5 commits into from
Oct 20, 2023
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
2 changes: 2 additions & 0 deletions src/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

class Config
{
public const OPTION_COPY_IDENTICAL_PATH = 'copy_destination_same_as_source';
public const OPTION_MOVE_IDENTICAL_PATH = 'move_destination_same_as_source';
public const OPTION_VISIBILITY = 'visibility';
public const OPTION_DIRECTORY_VISIBILITY = 'directory_visibility';

Expand Down
40 changes: 30 additions & 10 deletions src/Filesystem.php
Original file line number Diff line number Diff line change
Expand Up @@ -119,20 +119,40 @@ private function pipeListing(string $location, bool $deep, iterable $listing): G

public function move(string $source, string $destination, array $config = []): void
{
$this->adapter->move(
$this->pathNormalizer->normalizePath($source),
$this->pathNormalizer->normalizePath($destination),
$this->config->extend($config)
);
$config = $this->config->extend($config);
$from = $this->pathNormalizer->normalizePath($source);
$to = $this->pathNormalizer->normalizePath($destination);

if ($from === $to) {
$resolutionStrategy = $config->get(Config::OPTION_MOVE_IDENTICAL_PATH, ResolveIdenticalPathConflict::TRY);

if ($resolutionStrategy === ResolveIdenticalPathConflict::FAIL) {
throw UnableToMoveFile::sourceAndDestinationAreTheSame($source, $destination);
} elseif ($resolutionStrategy === ResolveIdenticalPathConflict::IGNORE) {
return;
}
}

$this->adapter->move($from, $to, $config);
}

public function copy(string $source, string $destination, array $config = []): void
{
$this->adapter->copy(
$this->pathNormalizer->normalizePath($source),
$this->pathNormalizer->normalizePath($destination),
$this->config->extend($config)
);
$config = $this->config->extend($config);
$from = $this->pathNormalizer->normalizePath($source);
$to = $this->pathNormalizer->normalizePath($destination);

if ($from === $to) {
$resolutionStrategy = $config->get(Config::OPTION_COPY_IDENTICAL_PATH, ResolveIdenticalPathConflict::TRY);

if ($resolutionStrategy === ResolveIdenticalPathConflict::FAIL) {
throw UnableToCopyFile::sourceAndDestinationAreTheSame($source, $destination);
} elseif ($resolutionStrategy === ResolveIdenticalPathConflict::IGNORE) {
return;
}
}

$this->adapter->copy($from, $to, $config);
}

public function lastModified(string $path): int
Expand Down
71 changes: 71 additions & 0 deletions src/FilesystemTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,26 @@ public function publicUrl(string $path, Config $config): string
self::assertSame('custom/file.txt', $filesystem->publicUrl('file.txt'));
}

/**
* @test
*/
public function copying_from_and_to_the_same_location_fails(): void
{
$this->expectExceptionObject(UnableToCopyFile::fromLocationTo('from.txt', 'from.txt'));

$this->filesystem->copy('from.txt', 'from.txt');
}

/**
* @test
*/
public function moving_from_and_to_the_same_location_fails(): void
{
$this->expectExceptionObject(UnableToMoveFile::fromLocationTo('from.txt', 'from.txt'));

$this->filesystem->move('from.txt', 'from.txt');
}

/**
* @test
*/
Expand Down Expand Up @@ -591,6 +611,57 @@ public function not_being_able_to_generate_temporary_urls(): void
$filesystem->temporaryUrl('some/file.txt', new DateTimeImmutable());
}

/**
* @test
*/
public function ignoring_same_paths_for_move_and_copy(): void
{
$this->expectNotToPerformAssertions();

$filesystem = new Filesystem(
new InMemoryFilesystemAdapter(),
[
Config::OPTION_COPY_IDENTICAL_PATH => ResolveIdenticalPathConflict::IGNORE,
Config::OPTION_MOVE_IDENTICAL_PATH => ResolveIdenticalPathConflict::IGNORE,
]
);

$filesystem->move('from.txt', 'from.txt');
$filesystem->copy('from.txt', 'from.txt');
}

/**
* @test
*/
public function failing_same_paths_for_move(): void
{
$filesystem = new Filesystem(
new InMemoryFilesystemAdapter(),
[
Config::OPTION_MOVE_IDENTICAL_PATH => ResolveIdenticalPathConflict::FAIL,
]
);

$this->expectExceptionObject(UnableToMoveFile::fromLocationTo('from.txt', 'from.txt'));
$filesystem->move('from.txt', 'from.txt');
}

/**
* @test
*/
public function failing_same_paths_for_copy(): void
{
$filesystem = new Filesystem(
new InMemoryFilesystemAdapter(),
[
Config::OPTION_COPY_IDENTICAL_PATH => ResolveIdenticalPathConflict::FAIL,
]
);

$this->expectExceptionObject(UnableToCopyFile::fromLocationTo('from.txt', 'from.txt'));
$filesystem->copy('from.txt', 'from.txt');
}

/**
* @test
*/
Expand Down
8 changes: 4 additions & 4 deletions src/InMemory/InMemoryFilesystemAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -221,15 +221,15 @@ public function listContents(string $path, bool $deep): iterable

public function move(string $source, string $destination, Config $config): void
{
$source = $this->preparePath($source);
$destination = $this->preparePath($destination);
$sourcePath = $this->preparePath($source);
$destinationPath = $this->preparePath($destination);

if ( ! $this->fileExists($source) || $this->fileExists($destination)) {
throw UnableToMoveFile::fromLocationTo($source, $destination);
}

$this->files[$destination] = $this->files[$source];
unset($this->files[$source]);
$this->files[$destinationPath] = $this->files[$sourcePath];
unset($this->files[$sourcePath]);
}

public function copy(string $source, string $destination, Config $config): void
Expand Down
4 changes: 2 additions & 2 deletions src/Local/LocalFilesystemAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ public function move(string $source, string $destination, Config $config): void
);

if ( ! @rename($sourcePath, $destinationPath)) {
throw UnableToMoveFile::fromLocationTo($sourcePath, $destinationPath);
throw UnableToMoveFile::because(error_get_last()['message'] ?? 'unknown reason', $source, $destination);
}
}

Expand All @@ -263,7 +263,7 @@ public function copy(string $source, string $destination, Config $config): void
);

if ( ! @copy($sourcePath, $destinationPath)) {
throw UnableToCopyFile::fromLocationTo($sourcePath, $destinationPath);
throw UnableToCopyFile::because(error_get_last()['message'] ?? 'unknown', $source, $destination);
}
}

Expand Down
11 changes: 11 additions & 0 deletions src/ResolveIdenticalPathConflict.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php
declare(strict_types=1);

namespace League\Flysystem;

class ResolveIdenticalPathConflict
{
public const IGNORE = 'ignore';
public const FAIL = 'fail';
public const TRY = 'try';
}
14 changes: 14 additions & 0 deletions src/UnableToCopyFile.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,20 @@ public static function fromLocationTo(
return $e;
}

public static function sourceAndDestinationAreTheSame(string $source, string $destination): UnableToCopyFile
{
return UnableToCopyFile::because('Source and destination are the same', $source, $destination);
}

public static function because(string $reason, string $sourcePath, string $destinationPath): UnableToCopyFile
{
$e = new static("Unable to copy file from $sourcePath to $destinationPath, because $reason");
$e->source = $sourcePath;
$e->destination = $destinationPath;

return $e;
}

public function operation(): string
{
return FilesystemOperationFailed::OPERATION_COPY;
Expand Down
18 changes: 18 additions & 0 deletions src/UnableToMoveFile.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ final class UnableToMoveFile extends RuntimeException implements FilesystemOpera
*/
private $destination;

public static function sourceAndDestinationAreTheSame(string $source, string $destination): UnableToMoveFile
{
return UnableToMoveFile::because('Source and destination are the same', $source, $destination);
}

public function source(): string
{
return $this->source;
Expand All @@ -42,6 +47,19 @@ public static function fromLocationTo(
return $e;
}

public static function because(
string $reason,
string $sourcePath,
string $destinationPath,
): UnableToMoveFile {
$message = "Unable to move file from $sourcePath to $destinationPath, because $reason";
$e = new static($message);
$e->source = $sourcePath;
$e->destination = $destinationPath;

return $e;
}

public function operation(): string
{
return FilesystemOperationFailed::OPERATION_MOVE;
Expand Down
Loading