fix(storage): ensure LocalTempFileTrait::toTmpFile always closes streams

A refactor for clarity and general robustness

Signed-off-by: Josh <josh.t.richards@gmail.com>

lib/private/Files/Storage/LocalTempFileTrait.php

@@ -1,7 +1,7 @@
 <?php
 
 /**
- * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2016-2026 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
@@ -11,20 +11,26 @@ use OCP\ITempManager;
 use OCP\Server;
 
 /**
- * Storage backend class for providing common filesystem operation methods
- * which are not storage-backend specific.
+ * Helper methods for temporary local file handling for storage paths.
  *
- * \OC\Files\Storage\Common is never used directly; it is extended by all other
- * storage backends, where its methods may be overridden, and additional
- * (backend-specific) methods are defined.
+ * Intended for use by storage implementations such as \OC\Files\Storage\Common.
  *
- * Some \OC\Files\Storage\Common methods call functions which are first defined
- * in classes which extend it, e.g. $this->stat() .
+ * This trait caches per-path temporary local copies created from storage streams,
+ * so repeated local-file access can reuse the same temp file during the instance
+ * lifetime.
  */
 trait LocalTempFileTrait {
 	/** @var array<string,string|false> */
 	protected array $cachedFiles = [];
 
+	/**
+	 * Returns the temporary local file path associated with the specified storage file.
+	 *
+	 * Creates a temp file on first use if necessary. Caches for repeated (instance-level) access.
+	 *
+	 * @param string $path Storage-internal path
+	 * @return string|false Local temp file path, or false if the source cannot be opened/copied
+	 */
 	protected function getCachedFile(string $path): string|false {
 		if (!isset($this->cachedFiles[$path])) {
 			$this->cachedFiles[$path] = $this->toTmpFile($path);
@@ -32,28 +38,49 @@ trait LocalTempFileTrait {
 		return $this->cachedFiles[$path];
 	}
 
+	/**
+	 * Invalidate the cached temp local file entry for the specified storage file.
+	 *
+	 * @param string $path Storage-internal path
+	 */
 	protected function removeCachedFile(string $path): void {
 		unset($this->cachedFiles[$path]);
 	}
 
-	protected function toTmpFile(string $path): string|false { //no longer in the storage api, still useful here
+	/**
+	 * Copies a storage file stream into a temporary local file.
+	 *
+	 * The temporary local file keeps the same extension (if any) as the source file.
+	 *
+	 * @param string $path Storage-internal path
+	 * @return string|false Local temp file path, or false on failure
+	 */
+	protected function toTmpFile(string $path): string|false {
 		$source = $this->fopen($path, 'r');
 		if (!$source) {
 			return false;
 		}
-		if ($pos = strrpos($path, '.')) {
-			$extension = substr($path, $pos);
-		} else {
-			$extension = '';
-		}
+
+		$ext = pathinfo($path, PATHINFO_EXTENSION);
+		$extension = $ext !== '' ? '.' . $ext : '';
+
 		$tmpFile = Server::get(ITempManager::class)->getTemporaryFile($extension);
 		$target = fopen($tmpFile, 'w');
-		$result = stream_copy_to_stream($source, $target);
-		fclose($target);
-		if ($result === false) {
+		if ($target === false) {
+			fclose($source);
 			return false;
 		}
 
+		try {
+			$result = stream_copy_to_stream($source, $target);
+			if ($result === false) {
+				return false;
+			}
+		} finally {
+			fclose($target);
+			fclose($source);
+		}
+
 		return $tmpFile;
 	}
 }