refactor(ObjectStore/S3): improve multipart upload validation and strengthen S3 typing/docs

lib/private/Files/ObjectStore/S3.php

@@ -1,7 +1,7 @@
 <?php
 
 /**
- * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2016-2026 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
@@ -13,6 +13,15 @@
 use OCP\Files\ObjectStore\IObjectStoreMetaData;
 use OCP\Files\ObjectStore\IObjectStoreMultiPartUpload;
 
+/**
+ * Nextcloud's S3-backed primary object storage implementation.
+ *
+ * This class provides the concrete AWS S3 integration for Nextcloud's generic
+ * object store interfaces.
+ *
+ * Note: This is not the S3-backed External Storage backend (though some
+ * lower-level S3 components are shared).
+ */
 class S3 implements IObjectStore, IObjectStoreMultiPartUpload, IObjectStoreMetaData {
 	use S3ConnectionTrait;
 	use S3ObjectTrait;
@@ -22,24 +31,24 @@ public function __construct(array $parameters) {
 		$this->parseParams($parameters);
 	}
 
-	/**
-	 * @return string the container or bucket name where objects are stored
-	 * @since 7.0.0
-	 */
 	public function getStorageId() {
 		return $this->id;
 	}
 
 	public function initiateMultipartUpload(string $urn): string {
-		$upload = $this->getConnection()->createMultipartUpload([
+		$request = [
 			'Bucket' => $this->bucket,
 			'Key' => $urn,
-		] + $this->getSSECParameters());
-		$uploadId = $upload->get('UploadId');
-		if ($uploadId === null) {
-			throw new Exception('No upload id returned');
+		] + $this->getSSECParameters();
+
+		$result = $this->getConnection()->createMultipartUpload($request);		
+		$uploadId = $result->get('UploadId');
+
+		if (!is_string($uploadId) || $uploadId === '') {
+			throw new Exception("Failed to initiate multipart upload for key '{$urn}': missing UploadId");
 		}
-		return (string)$uploadId;
+
+		return $uploadId;
 	}
 
 	public function uploadMultipartPart(string $urn, string $uploadId, int $partId, $stream, $size): Result {

lib/private/Files/ObjectStore/S3ConfigTrait.php

@@ -1,43 +1,70 @@
 <?php
 
 declare(strict_types=1);
+
 /**
- * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2024-2026 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
+
 namespace OC\Files\ObjectStore;
 
 /**
- * Shared configuration between ConnectionTrait and ObjectTrait to ensure both to be in sync
+ * Shared configuration parameters, used by both S3 connection and object logic, to keep them in sync.
+ *
+ * S3ConnectionTrait primarily uses the `$params` property as a container for all connection
+ * configuration, which may include raw config from user input, normalized and merged values,
+ * and details for advanced connection handling (hostname, region, credentials, etc.).
+ *
+ * S3ObjectTrait and related object-level code primarily use the dedicated, typed properties
+ * defined below (e.g., $bucket, $uploadPartSize, $timeout) for object-specific features.
+ *
+ * @todo: There is overlap between `$params` and the individual properties. It is currently
+ * unclear whether this distinction is the result of deliberate separation of concerns
+ * (connection-wide vs object-specific settings), or if it is technical debt.
+ *
+ * @todo: Some of the default values assigned below are currently - generally unnecessarily -
+ * overridden elsewhere.
  */
 trait S3ConfigTrait {
-	protected array $params;
+	 // S3 connection configuration parameters.
+	protected array $params = [];
+
+	/* 
+	 * Overlap between the above array and the individual properties is currently expected
+	 * (see above). Most object related operations use the individual properties below.
+	 */
 
-	protected string $bucket;
+	// Bucket name
+	protected string $bucket = '';
 
-	/** Maximum number of concurrent multipart uploads */
-	protected int $concurrency;
+	// Max concurrent multipart uploads
+	protected int $concurrency = 5;
 
-	/** Timeout, in seconds, for the connection to S3 server, not for the
-	 *  request. */
-	protected float $connectTimeout;
+	// Server connection timeout in seconds (not total request timeout).
+	protected float $connectTimeout = 5;
 
-	protected int $timeout;
+	// Total request timeout in seconds
+	protected int $timeout = 15;
 
-	protected string|false $proxy;
+	// Proxy URL string or false if not in use.
+	protected string|false $proxy = false;
 
-	protected string $storageClass;
+	// Storage class for new objects
+	protected string $storageClass = 'STANDARD';
 
-	/** @var int Part size in bytes (float is added for 32bit support) */
-	protected int|float $uploadPartSize;
+	// Multipart upload part size in bytes (int|float for 32-bit compatibility).
+	protected int|float $uploadPartSize = 524288000; // 500 MiB default
 
-	/** @var int Limit on PUT in bytes (float is added for 32bit support) */
-	private int|float $putSizeLimit;
+	// Maximum allowed PUT size in bytes (int|float for 32-bit compatibility).
+	private int|float $putSizeLimit = 104857600; // 100 MiB default
 
-	/** @var int Limit on COPY in bytes (float is added for 32bit support) */
-	private int|float $copySizeLimit;
+	// Maximum allowed COPY size in bytes (int|float for 32-bit compatibility).
+	private int|float $copySizeLimit = 5242880000; // ~5 GiB default
 
+	// Should multipart copy be used when copying large objects?
 	private bool $useMultipartCopy = true;
 
-	protected int $retriesMaxAttempts;
+	// Max retry attempts for S3 API calls.
+	protected int $retriesMaxAttempts = 5;
 }

lib/public/Files/ObjectStore/IObjectStoreMultiPartUpload.php

@@ -10,30 +10,81 @@
 use Aws\Result;
 
 /**
+ * Multipart upload capabilities for object stores.
+ *
+ * Implementations are expected to support the standard multipart lifecycle:
+ * initiate -> uploadMultipartPart (1..n) -> completeMultipartUpload | abortMultipartUpload.
+ *
+ * Notes:
+ * - Part IDs are expected to be positive integers starting at 1.
+ * - Callers should pass parts to completion in ascending part order unless an implementation documents otherwise.
+ * - Re-uploading the same part ID for the same uploadId may overwrite the previously uploaded part,
+ *   depending on backend semantics.
+ *
  * @since 26.0.0
  */
 interface IObjectStoreMultiPartUpload {
 	/**
+	 * Start a multipart upload for the object identified by $urn.
+	 *
+	 * @param string $urn Object identifier in the object store namespace.
+	 * @return string Backend upload identifier to be used for subsequent part operations.
+	 *
 	 * @since 26.0.0
 	 */
 	public function initiateMultipartUpload(string $urn): string;
 
 	/**
+	 * Upload one multipart chunk for an active multipart upload.
+	 *
+	 * @param string $urn Object identifier in the object store namespace.
+	 * @param string $uploadId Upload identifier previously returned by initiateMultipartUpload().
+	 * @param int $partId Part number.
+	 * @param resource|object $stream Stream payload for the part. Implementations may accept
+	 * 								  stream resources or stream-like objects.
+	 * @param int $size Size of the part payload in bytes.
+	 * @return Result Backend result metadata for the uploaded part (e.g. ETag/checksum fields if provided).
+	 *
 	 * @since 26.0.0
 	 */
 	public function uploadMultipartPart(string $urn, string $uploadId, int $partId, $stream, $size): Result;
 
 	/**
+	 * Complete an active multipart upload by assembling uploaded parts.
+	 *
+	 * May take a long time!
+	 *
+	 * @param string $urn Object identifier in the object store namespace.
+	 * @param string $uploadId Upload identifier previously returned by initiateMultipartUpload().
+	 * @param array<int, array<string, mixed>> $result Part metadata used for final assembly.
+	 *        Expected to contain backend-specific per-part information returned from uploadMultipartPart(),
+	 *        commonly including part number and ETag/checksum fields.
+	 * @return int Size in bytes of the assembled object as stored after upload completion.
+	 *
 	 * @since 26.0.0
 	 */
 	public function completeMultipartUpload(string $urn, string $uploadId, array $result): int;
 
 	/**
+	 * Abort an active multipart upload.
+	 *
+	 * After aborting, uploaded parts associated with the uploadId are expected to be discarded by backend
+	 * cleanup semantics.
+	 *
+	 * @param string $urn Object identifier in the object store namespace.
+	 * @param string $uploadId Upload identifier previously returned by initiateMultipartUpload().
+	 *
 	 * @since 26.0.0
 	 */
 	public function abortMultipartUpload(string $urn, string $uploadId): void;
 
 	/**
+	 * Retrieve already uploaded parts for a given multipart upload.
+	 *
+	 * @param string $urn Object identifier in the object store namespace.
+	 * @param string $uploadId Upload identifier previously returned by initiateMultipartUpload().
+	 * @return array<int, array<string, mixed>> Backend-specific list of uploaded part descriptors.
+	 *
 	 * @since 26.0.0
 	 */
 	public function getMultipartUploads(string $urn, string $uploadId): array;