fix: make deleteOldFiles / moveNewVersionInPlace more robust

Signed-off-by: Josh Richards <[email protected]>
nextcloud · Dec 19, 2024 · ef4c5c1 · ef4c5c1
1 parent e73aa39
commit ef4c5c1
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 52 deletions.
diff --git a/index.php b/index.php
@@ -7,6 +7,7 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
+
 class UpdateException extends \Exception {
 
 	/** @param list<string> $data */
@@ -927,10 +928,13 @@ public function deleteOldFiles(): void {
 		 * @var string $path
 		 * @var \SplFileInfo $fileInfo
 		 */
+		$files = [];
+		$directories = [];
 		foreach ($this->getRecursiveDirectoryIterator() as $path => $fileInfo) {
 			$currentDir = $this->baseDir . '/../';
 			$fileName = explode($currentDir, $path)[1];
 			$folderStructure = explode('/', $fileName, -1);
+
 			// Exclude the exclusions
 			if (isset($folderStructure[0])) {
 				if (array_search($folderStructure[0], $excludedElements) !== false) {
@@ -942,18 +946,30 @@ public function deleteOldFiles(): void {
 				}
 			}
 			if ($fileInfo->isFile() || $fileInfo->isLink()) {
-				$state = unlink($path);
-				if ($state === false) {
-					throw new \Exception('Could not unlink: '.$path);
-				}
+				$files[] = $path;
 			} elseif ($fileInfo->isDir()) {
-				$state = rmdir($path);
-				if ($state === false) {
-					throw new \Exception('Could not rmdir: '.$path);
-				}
+				$directories[] = $path;
+			}
+		}
+
+		//
+		// Do the actual writes (outside the RDI to avoid problems on FreeBSD/etc)
+		//
+
+		foreach ($files as $file) {
+			$state = unlink($file);
+			if ($state === false) {
+				throw new \Exception('Could not unlink: '.$path);
 			}
 		}
 
+		foreach ($directories as $dir) {
+			$state = rmdir($dir);
+			if ($state === false) {
+				throw new \Exception('Could not rmdir: '.$path);
+			}
+		}
+
 		$this->silentLog('[info] end of deleteOldFiles()');
 	}
 
@@ -967,6 +983,8 @@ private function moveWithExclusions(string $dataLocation, array $excludedElement
 		 * @var string $path
 		 * @var \SplFileInfo $fileInfo
 		 */
+		$files = [];
+		$directories = [];
 		foreach ($this->getRecursiveDirectoryIterator($dataLocation) as $path => $fileInfo) {
 			$fileName = explode($dataLocation, $path)[1];
 			$folderStructure = explode('/', $fileName, -1);
@@ -983,29 +1001,41 @@ private function moveWithExclusions(string $dataLocation, array $excludedElement
 			}
 
 			if ($fileInfo->isFile()) {
-				if (!file_exists($this->baseDir . '/../' . dirname($fileName))) {
-					$state = mkdir($this->baseDir . '/../' . dirname($fileName), 0755, true);
-					if ($state === false) {
-						throw new \Exception('Could not mkdir ' . $this->baseDir  . '/../' . dirname($fileName));
-					}
-				}
-				$state = rename($path, $this->baseDir  . '/../' . $fileName);
-				if ($state === false) {
-					throw new \Exception(
-						sprintf(
-							'Could not rename %s to %s',
-							$path,
-							$this->baseDir . '/../' . $fileName
-						)
-					);
-				}
+				$files[$path] = $fileName;
 			}
 			if ($fileInfo->isDir()) {
-				$state = rmdir($path);
+				$directories[] = $path;
+			}
+		}
+
+		//
+		// Do the actual writes (outside the RDI to avoid problems on FreeBSD/etc)
+		//
+
+		foreach ($files as $file => $fileName) {
+			if (!file_exists($this->baseDir . '/../' . dirname($fileName))) {
+				$state = mkdir($this->baseDir . '/../' . dirname($fileName), 0755, true);
 				if ($state === false) {
-					throw new \Exception('Could not rmdir ' . $path);
+					throw new \Exception('Could not mkdir ' . $this->baseDir  . '/../' . dirname($fileName));
 				}
 			}
+			$state = rename($file, $this->baseDir  . '/../' . $fileName);
+			if ($state === false) {
+				throw new \Exception(
+					sprintf(
+						'Could not rename %s to %s',
+						$file,
+						$this->baseDir . '/../' . $fileName
+					)
+				);
+			}
+		}
+
+		foreach ($directories as $dir) {
+			$state = rmdir($dir);
+			if ($state === false) {
+				throw new \Exception('Could not rmdir ' . $dir);
+			}
 		}
 	}
 

diff --git a/lib/Updater.php b/lib/Updater.php
@@ -890,10 +890,13 @@ public function deleteOldFiles(): void {
 		 * @var string $path
 		 * @var \SplFileInfo $fileInfo
 		 */
+		$files = [];
+		$directories = [];
 		foreach ($this->getRecursiveDirectoryIterator() as $path => $fileInfo) {
 			$currentDir = $this->baseDir . '/../';
 			$fileName = explode($currentDir, $path)[1];
 			$folderStructure = explode('/', $fileName, -1);
+
 			// Exclude the exclusions
 			if (isset($folderStructure[0])) {
 				if (array_search($folderStructure[0], $excludedElements) !== false) {
@@ -905,18 +908,30 @@ public function deleteOldFiles(): void {
 				}
 			}
 			if ($fileInfo->isFile() || $fileInfo->isLink()) {
-				$state = unlink($path);
-				if ($state === false) {
-					throw new \Exception('Could not unlink: '.$path);
-				}
+				$files[] = $path;
 			} elseif ($fileInfo->isDir()) {
-				$state = rmdir($path);
-				if ($state === false) {
-					throw new \Exception('Could not rmdir: '.$path);
-				}
+				$directories[] = $path;
+			}
+		}
+
+		//
+		// Do the actual writes (outside the RDI to avoid problems on FreeBSD/etc)
+		//
+
+		foreach ($files as $file) {
+			$state = unlink($file);
+			if ($state === false) {
+				throw new \Exception('Could not unlink: '.$path);
 			}
 		}
 
+		foreach ($directories as $dir) {
+			$state = rmdir($dir);
+			if ($state === false) {
+				throw new \Exception('Could not rmdir: '.$path);
+			}
+		}
+
 		$this->silentLog('[info] end of deleteOldFiles()');
 	}
 
@@ -930,6 +945,8 @@ private function moveWithExclusions(string $dataLocation, array $excludedElement
 		 * @var string $path
 		 * @var \SplFileInfo $fileInfo
 		 */
+		$files = [];
+		$directories = [];
 		foreach ($this->getRecursiveDirectoryIterator($dataLocation) as $path => $fileInfo) {
 			$fileName = explode($dataLocation, $path)[1];
 			$folderStructure = explode('/', $fileName, -1);
@@ -946,29 +963,41 @@ private function moveWithExclusions(string $dataLocation, array $excludedElement
 			}
 
 			if ($fileInfo->isFile()) {
-				if (!file_exists($this->baseDir . '/../' . dirname($fileName))) {
-					$state = mkdir($this->baseDir . '/../' . dirname($fileName), 0755, true);
-					if ($state === false) {
-						throw new \Exception('Could not mkdir ' . $this->baseDir  . '/../' . dirname($fileName));
-					}
-				}
-				$state = rename($path, $this->baseDir  . '/../' . $fileName);
-				if ($state === false) {
-					throw new \Exception(
-						sprintf(
-							'Could not rename %s to %s',
-							$path,
-							$this->baseDir . '/../' . $fileName
-						)
-					);
-				}
+				$files[$path] = $fileName;
 			}
 			if ($fileInfo->isDir()) {
-				$state = rmdir($path);
+				$directories[] = $path;
+			}
+		}
+
+		//
+		// Do the actual writes (outside the RDI to avoid problems on FreeBSD/etc)
+		//
+
+		foreach ($files as $file => $fileName) {
+			if (!file_exists($this->baseDir . '/../' . dirname($fileName))) {
+				$state = mkdir($this->baseDir . '/../' . dirname($fileName), 0755, true);
 				if ($state === false) {
-					throw new \Exception('Could not rmdir ' . $path);
+					throw new \Exception('Could not mkdir ' . $this->baseDir  . '/../' . dirname($fileName));
 				}
 			}
+			$state = rename($file, $this->baseDir  . '/../' . $fileName);
+			if ($state === false) {
+				throw new \Exception(
+					sprintf(
+						'Could not rename %s to %s',
+						$file,
+						$this->baseDir . '/../' . $fileName
+					)
+				);
+			}
+		}
+
+		foreach ($directories as $dir) {
+			$state = rmdir($dir);
+			if ($state === false) {
+				throw new \Exception('Could not rmdir ' . $dir);
+			}
 		}
 	}
 

diff --git a/updater.phar b/updater.phar