diff --git a/lib/private/Files/Storage/Local.php b/lib/private/Files/Storage/Local.php index 13a3ca87097fa..ee6d120b08eea 100644 --- a/lib/private/Files/Storage/Local.php +++ b/lib/private/Files/Storage/Local.php @@ -42,6 +42,7 @@ use OC\Files\Filesystem; use OC\Files\Storage\Wrapper\Jail; +use OC\Files\Storage\Local\DirectoryRenamer; use OCP\Files\ForbiddenException; use OCP\Files\Storage\IStorage; use OCP\ILogger; @@ -275,11 +276,7 @@ public function rename($path1, $path2) { $stat1 = stat(dirname($this->getSourcePath($path1))); $stat2 = stat(dirname($this->getSourcePath($path2))); if ($stat1['dev'] !== $stat2['dev']) { - $result = $this->copy($path1, $path2); - if ($result) { - $result &= $this->rmdir($path1); - } - return $result; + return $this->copyAndUnlink($path1, $path2); } if ($this->treeContainsBlacklistedFile($this->getSourcePath($path1))) { @@ -287,7 +284,11 @@ public function rename($path1, $path2) { } } - return rename($this->getSourcePath($path1), $this->getSourcePath($path2)); + $renamer = new DirectoryRenamer(function () use ($path1, $path2) { + return $this->copyAndUnlink($path1, $path2); + }); + + return $renamer->rename($this->getSourcePath($path1), $this->getSourcePath($path2)); } public function copy($path1, $path2) { @@ -502,4 +503,12 @@ public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $t public function writeStream(string $path, $stream, int $size = null): int { return (int)file_put_contents($this->getSourcePath($path), $stream); } + + private function copyAndUnlink($path1, $path2) { + $result = $this->copy($path1, $path2); + if ($result) { + $result &= $this->rmdir($path1); + } + return $result; + } } diff --git a/lib/private/Files/Storage/Local/CrossDeviceLinkException.php b/lib/private/Files/Storage/Local/CrossDeviceLinkException.php new file mode 100644 index 0000000000000..83d5dccf81bcf --- /dev/null +++ b/lib/private/Files/Storage/Local/CrossDeviceLinkException.php @@ -0,0 +1,31 @@ + + * + * @author Jakub Gawron + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Files\Storage\Local; + +/** + * Exception thrown due to the "cross-device link not permitted" error. + */ +class CrossDeviceLinkException extends \Exception { + +} diff --git a/lib/private/Files/Storage/Local/DirectoryRenamer.php b/lib/private/Files/Storage/Local/DirectoryRenamer.php new file mode 100644 index 0000000000000..ea210687eee7d --- /dev/null +++ b/lib/private/Files/Storage/Local/DirectoryRenamer.php @@ -0,0 +1,131 @@ + + * + * @author Jakub Gawron + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Files\Storage\Local; + +/** + * Support class for Local Storage, responsible only for handling directory renames. + * It attempts to perform normal rename, + * but fallbacks to copy and unlink strategy when it fails due to "cross-device link not permitted" error. + */ +class DirectoryRenamer { + const STATE_NO_ERROR = 0; + const STATE_CAUGHT_RENAME_WARNING = 1; + + /** + * @var callable + */ + private $fallbackHandler; + /** + * @var bool + */ + private $shouldRestoreHandler; + + /** + * @param callable $fallbackHandler Copy and unlink strategy to use when normal rename fails + */ + public function __construct(callable $fallbackHandler) { + $this->fallbackHandler = $fallbackHandler; + $this->shouldRestoreHandler = true; + } + + /** + * Rename a file or directory + * + * @param string $oldname + * @param string $newname + * @return bool + */ + public function rename(string $oldname, string $newname): bool { + $this->setupErrorHandler(); + + try { + return rename($oldname, $newname); + } catch (CrossDeviceLinkException $e) { + return ($this->fallbackHandler)(); + } finally { + if ($this->shouldRestoreHandler) { + restore_error_handler(); + } + } + + return false; + } + + /** + * Sets a temporary error handler that covers the cross-device link error with a state machine. + */ + private function setupErrorHandler(): void { + $state = self::STATE_NO_ERROR; + $previousHandler = set_error_handler(function($errno, $errstr) use (&$state, &$previousHandler) { + // when we hit first warning, we'll check if it's what we expect + // if it is, we transition to next state and return + // otherwise we continue + switch ($state) { + case self::STATE_NO_ERROR: + if ($errstr === 'rename(): The first argument to copy() function cannot be a directory') { + $state = self::STATE_CAUGHT_RENAME_WARNING; + return true; + } + break; + } + + // when we hit second warning, or a first warning that wasn't the above expected warning + // we'll check if it is the cross-device link and if so, we throw the exception + // to let the caller know to use the fallback strategy + switch ($state) { + case self::STATE_NO_ERROR: + case self::STATE_CAUGHT_RENAME_WARNING: + if (static::endsWith($errstr, 'cross-device link')) { + restore_error_handler(); + $this->shouldRestoreHandler = false; + throw new CrossDeviceLinkException(); + } + break; + } + + // if we get to this point, we got called with warnings which we can't handle (or not in the order anticipated) + // so we restore the previous error handler and let it handle this warning + restore_error_handler(); + $this->shouldRestoreHandler = false; + if ($previousHandler) { + return call_user_func_array($previousHandler, func_get_args()); + } + + return false; + }, E_WARNING | E_USER_WARNING); + } + + /** + * Check if $haystack ends with $needle (case insensitive) + * @param string $haystack + * @param string $needle + * @return bool + */ + private static function endsWith(string $haystack, string $needle): bool { + return strcasecmp( + substr($haystack, -strlen($needle)), + $needle + ) === 0; + } +} diff --git a/tests/lib/Files/Storage/Local/DirectoryRenamerTest.php b/tests/lib/Files/Storage/Local/DirectoryRenamerTest.php new file mode 100644 index 0000000000000..17cbc602d6aca --- /dev/null +++ b/tests/lib/Files/Storage/Local/DirectoryRenamerTest.php @@ -0,0 +1,186 @@ + + * + * @author Jakub Gawron + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Files\Storage\Local { + + class ImplHolder { + /** + * @var callable|null + */ + public static $impl = null; + } + + function rename(...$args) { + if (!is_callable(ImplHolder::$impl)) { + throw new \RuntimeException('Mock rename implementation has to be set on ImplHolder::$impl'); + } + return call_user_func_array(ImplHolder::$impl, $args); + } +} + +namespace Test\Files\Storage\Local { + + use OC\Files\Storage\Local\DirectoryRenamer; + use OC\Files\Storage\Local\ImplHolder; + + class DirectoryRenamerTest extends \Test\TestCase { + + /** + * @dataProvider renameImplFailsSoFallbackIsCalledProvider + * @param callable $impl + */ + public function testFallbackIsCalledWhenRenameFails(callable $impl) { + ImplHolder::$impl = $impl; + + $fallbackCalled = false; + + $renamer = new DirectoryRenamer(function() use (&$fallbackCalled) { + $fallbackCalled = true; + return true; + }); + + $this->assertTrue( + $renamer->rename('//mnt/nfs/foo/y', '//mnt/nfs/bar/x') + ); + + $this->assertTrue($fallbackCalled, 'Fallback handler wasn\'t called'); + } + + /** + * @dataProvider renameImplPassesSoFallbackIsNotCalledProvider + * @param callable $impl + */ + public function testFallbackIsNotCalledWhenRenamePasses(callable $impl) { + ImplHolder::$impl = $impl; + + $fallbackCalled = false; + + $renamer = new DirectoryRenamer(function() use (&$fallbackCalled) { + $fallbackCalled = true; + return false; + }); + + $this->assertTrue( + $renamer->rename('//mnt/nfs/foo/y', '//mnt/nfs/bar/x') + ); + + $this->assertFalse($fallbackCalled, 'Fallback handler was called'); + } + + /** + * @dataProvider renameImplFailsUnexpectedlySoFallbackIsNotCalledProvider + * @param callable $impl + * @param null|string $expectedWarning + */ + public function testFallbackIsNotCalledWhenRenameFailsUnexpectedly(callable $impl, ?string $expectedWarning) { + ImplHolder::$impl = $impl; + + $fallbackCalled = false; + + $renamer = new DirectoryRenamer(function() use (&$fallbackCalled) { + $fallbackCalled = true; + return false; + }); + + $warnings = []; + + if ($expectedWarning) { + set_error_handler(function($_, $errmsg) use (&$warnings) { + $warnings[] = $errmsg; + }, E_USER_WARNING); + } + + $this->assertFalse( + $renamer->rename('//mnt/nfs/foo/y', '//mnt/nfs/bar/x') + ); + + $this->assertFalse($fallbackCalled, 'Fallback handler was called'); + + if ($expectedWarning) { + $this->assertContains($expectedWarning, $warnings); + } + } + + public function renameImplFailsSoFallbackIsCalledProvider() { + return [ + [ + function ($oldname, $newname) { + trigger_error('rename(): The first argument to copy() function cannot be a directory', E_USER_WARNING); + trigger_error('rename('.$oldname.','.$newname.'): Invalid cross-device link', E_USER_WARNING); + return false; + }, + ], + [ + function ($oldname, $newname) { + trigger_error('rename('.$oldname.','.$newname.'): Invalid cross-device link', E_USER_WARNING); + return false; + }, + ], + [ + function ($oldname, $newname) { + trigger_error('rename('.$oldname.','.$newname.'): Cross-device link', E_USER_WARNING); + return false; + }, + ], + ]; + } + + public function renameImplPassesSoFallbackIsNotCalledProvider() { + return [ + [ + function () { + return true; + }, + ], + ]; + } + + public function renameImplFailsUnexpectedlySoFallbackIsNotCalledProvider() { + return [ + [ + function () { + trigger_error('rename(): The first argument to copy() function cannot be a directory', E_USER_WARNING); + trigger_error('unable to rename, destination directory is not writable', E_USER_WARNING); + return false; + }, + 'unable to rename, destination directory is not writable', + ], + [ + function () { + trigger_error('unable to rename, destination directory is not writable', E_USER_WARNING); + return false; + }, + 'unable to rename, destination directory is not writable', + ], + [ + function ($oldname, $newname) { + trigger_error('unable to rename, destination directory is not writable', E_USER_WARNING); + trigger_error('rename('.$oldname.','.$newname.'): Invalid cross-device link', E_USER_WARNING); + return false; + }, + 'unable to rename, destination directory is not writable', + ], + ]; + } + } +}