Merge pull request #50436 from nextcloud/fix/encoding-wrapper-scanner

fix: Harden files scanner for invalid null access

Author: susnux

apps/files_sharing/lib/Cache.php

@@ -63,12 +63,12 @@ protected function getRoot() {
 				/** @var Jail $currentStorage */
 				$absoluteRoot = $currentStorage->getJailedPath($absoluteRoot);
 			}
-			$this->root = $absoluteRoot;
+			$this->root = $absoluteRoot ?? '';
 		}
 		return $this->root;
 	}
 
-	protected function getGetUnjailedRoot() {
+	protected function getGetUnjailedRoot(): string {
 		return $this->sourceRootInfo->getPath();
 	}
 

apps/files_sharing/lib/External/Scanner.php

@@ -29,9 +29,10 @@ public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $loc
 	 * @param string $file file to scan
 	 * @param int $reuseExisting
 	 * @param int $parentId
-	 * @param array | null $cacheData existing data in the cache for the file to be scanned
+	 * @param \OC\Files\Cache\CacheEntry|array|null|false $cacheData existing data in the cache for the file to be scanned
 	 * @param bool $lock set to false to disable getting an additional read lock during scanning
-	 * @return array | null an array of metadata of the scanned file
+	 * @param array|null $data the metadata for the file, as returned by the storage
+	 * @return array|null an array of metadata of the scanned file
 	 */
 	public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true, $data = null) {
 		try {

apps/files_sharing/lib/Scanner.php

@@ -57,7 +57,7 @@ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData =
 		$sourceScanner = $this->getSourceScanner();
 		if ($sourceScanner instanceof ObjectStoreScanner) {
 			// ObjectStoreScanner doesn't scan
-			return [];
+			return null;
 		} else {
 			return parent::scanFile($file, $reuseExisting, $parentId, $cacheData, $lock);
 		}

build/psalm-baseline.xml

@@ -873,11 +873,6 @@
       <code><![CDATA[Mount]]></code>
     </MoreSpecificReturnType>
   </file>
-  <file src="apps/files_sharing/lib/External/Scanner.php">
-    <MoreSpecificImplementedParamType>
-      <code><![CDATA[$cacheData]]></code>
-    </MoreSpecificImplementedParamType>
-  </file>
   <file src="apps/files_sharing/lib/MountProvider.php">
     <RedundantFunctionCall>
       <code><![CDATA[array_values]]></code>
@@ -1758,14 +1753,10 @@
     </MoreSpecificReturnType>
   </file>
   <file src="lib/private/Files/Cache/Cache.php">
-    <InvalidArgument>
-      <code><![CDATA[$parentData]]></code>
-    </InvalidArgument>
     <InvalidNullableReturnType>
       <code><![CDATA[array]]></code>
     </InvalidNullableReturnType>
     <InvalidScalarArgument>
-      <code><![CDATA[$path]]></code>
       <code><![CDATA[\OC_Util::normalizeUnicode($path)]]></code>
     </InvalidScalarArgument>
     <NullableReturnStatement>
@@ -1796,12 +1787,6 @@
     <InvalidArgument>
       <code><![CDATA[self::SCAN_RECURSIVE_INCOMPLETE]]></code>
     </InvalidArgument>
-    <InvalidReturnStatement>
-      <code><![CDATA[$existingChildren]]></code>
-    </InvalidReturnStatement>
-    <InvalidReturnType>
-      <code><![CDATA[array[]]]></code>
-    </InvalidReturnType>
   </file>
   <file src="lib/private/Files/Cache/Storage.php">
     <InvalidNullableReturnType>

lib/private/Files/Cache/Cache.php

@@ -109,7 +109,7 @@ public function getNumericStorageId() {
 	/**
 	 * get the stored metadata of a file or folder
 	 *
-	 * @param string | int $file either the path of a file or folder or the file id for a file or folder
+	 * @param string|int $file either the path of a file or folder or the file id for a file or folder
 	 * @return ICacheEntry|false the cache entry as array or false if the file is not found in the cache
 	 */
 	public function get($file) {
@@ -131,15 +131,17 @@ public function get($file) {
 		$data = $result->fetch();
 		$result->closeCursor();
 
-		//merge partial data
-		if (!$data && is_string($file) && isset($this->partial[$file])) {
-			return $this->partial[$file];
-		} elseif (!$data) {
-			return $data;
-		} else {
+		if ($data !== false) {
 			$data['metadata'] = $metadataQuery->extractMetadata($data)->asArray();
 			return self::cacheEntryFromData($data, $this->mimetypeLoader);
+		} else {
+			//merge partial data
+			if (is_string($file) && isset($this->partial[$file])) {
+				return $this->partial[$file];
+			}
 		}
+
+		return false;
 	}
 
 	/**
@@ -886,19 +888,23 @@ public function searchQuery(ISearchQuery $query) {
 	/**
 	 * Re-calculate the folder size and the size of all parent folders
 	 *
-	 * @param string|boolean $path
-	 * @param array $data (optional) meta data of the folder
+	 * @param array|ICacheEntry|null $data (optional) meta data of the folder
 	 */
-	public function correctFolderSize($path, $data = null, $isBackgroundScan = false) {
+	public function correctFolderSize(string $path, $data = null, bool $isBackgroundScan = false): void {
 		$this->calculateFolderSize($path, $data);
+
 		if ($path !== '') {
 			$parent = dirname($path);
 			if ($parent === '.' || $parent === '/') {
 				$parent = '';
 			}
+
 			if ($isBackgroundScan) {
 				$parentData = $this->get($parent);
-				if ($parentData['size'] !== -1 && $this->getIncompleteChildrenCount($parentData['fileid']) === 0) {
+				if ($parentData !== false
+					&& $parentData['size'] !== -1
+					&& $this->getIncompleteChildrenCount($parentData['fileid']) === 0
+				) {
 					$this->correctFolderSize($parent, $parentData, $isBackgroundScan);
 				}
 			} else {
@@ -1009,8 +1015,8 @@ protected function calculateFolderSizeInner(string $path, $entry = null, bool $i
 			}
 
 			// only set unencrypted size for a folder if any child entries have it set, or the folder is empty
-			$shouldWriteUnEncryptedSize = $unencryptedMax > 0 || $totalSize === 0 || $entry['unencrypted_size'] > 0;
-			if ($entry['size'] !== $totalSize || ($entry['unencrypted_size'] !== $unencryptedTotal && $shouldWriteUnEncryptedSize)) {
+			$shouldWriteUnEncryptedSize = $unencryptedMax > 0 || $totalSize === 0 || ($entry['unencrypted_size'] ?? 0) > 0;
+			if ($entry['size'] !== $totalSize || (($entry['unencrypted_size'] ?? 0) !== $unencryptedTotal && $shouldWriteUnEncryptedSize)) {
 				if ($shouldWriteUnEncryptedSize) {
 					// if all children have an unencrypted size of 0, just set the folder unencrypted size to 0 instead of summing the sizes
 					if ($unencryptedMax === 0) {