Fallback for cross-device link error during rename call

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,19 +276,19 @@ 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))) {
 				throw new ForbiddenException('Invalid path', false);
 			}
 		}
 
-		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;
+	}
 }

lib/private/Files/Storage/Local/CrossDeviceLinkException.php

@@ -0,0 +1,31 @@
+<?php
+/**
+ * @copyright Copyright (c) 2020, Jakub Gawron <[email protected]>
+ *
+ * @author Jakub Gawron <[email protected]>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\Files\Storage\Local;
+
+/**
+ * Exception thrown due to the "cross-device link not permitted" error. 
+ */
+class CrossDeviceLinkException extends \Exception {
+
+}

lib/private/Files/Storage/Local/DirectoryRenamer.php

@@ -0,0 +1,131 @@
+<?php
+/**
+ * @copyright Copyright (c) 2020, Jakub Gawron <[email protected]>
+ *
+ * @author Jakub Gawron <[email protected]>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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;
+	}
+}

tests/lib/Files/Storage/Local/DirectoryRenamerTest.php

@@ -0,0 +1,186 @@
+<?php
+/**
+ * @copyright Copyright (c) 2020, Jakub Gawron <[email protected]>
+ *
+ * @author Jakub Gawron <[email protected]>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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',
+				],
+			];
+		}
+	}
+}