From aa24a4ad5df494ee0fbe0a7c986919156208d083 Mon Sep 17 00:00:00 2001
From: Git'Fellow <12234510+solracsf@users.noreply.github.com>
Date: Fri, 5 Jul 2024 10:49:10 +0200
Subject: [PATCH] fix(s3): Don't wait indefinitely for S3 to return

Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>

fix: lint

Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>

fix: use AwsException

Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>

fix: Throw on connection failure

Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>

fix: Wrap all in try catch block

Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>

fix: use RequestTimeout error message

Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>

log: use OCP Server class

Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>

fix: Handle connect timeout only

Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>

fix: Handle errors more generically

Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
---
 .../Files/ObjectStore/S3ConnectionTrait.php   | 66 +++++++++++--------
 1 file changed, 37 insertions(+), 29 deletions(-)

diff --git a/lib/private/Files/ObjectStore/S3ConnectionTrait.php b/lib/private/Files/ObjectStore/S3ConnectionTrait.php
index 61fee13aecfdc..ea9bc50ee4f57 100644
--- a/lib/private/Files/ObjectStore/S3ConnectionTrait.php
+++ b/lib/private/Files/ObjectStore/S3ConnectionTrait.php
@@ -41,6 +41,7 @@
 use GuzzleHttp\Promise;
 use GuzzleHttp\Promise\RejectedPromise;
 use OCP\ICertificateManager;
+use OCP\Server;
 use Psr\Log\LoggerInterface;
 
 trait S3ConnectionTrait {
@@ -125,7 +126,11 @@ public function getConnection() {
 			'signature_provider' => \Aws\or_chain([self::class, 'legacySignatureProvider'], ClientResolver::_default_signature_provider()),
 			'csm' => false,
 			'use_arn_region' => false,
-			'http' => ['verify' => $this->getCertificateBundlePath()],
+			'http' => [
+				'verify' => $this->getCertificateBundlePath(),
+				// Timeout for the connection to S3 server, not for the request.
+				'connect_timeout' => 5
+			],
 			'use_aws_shared_config_files' => false,
 		];
 
@@ -143,35 +148,38 @@ public function getConnection() {
 		}
 		$this->connection = new S3Client($options);
 
-		if (!$this->connection::isBucketDnsCompatible($this->bucket)) {
-			$logger = \OC::$server->get(LoggerInterface::class);
-			$logger->debug('Bucket "' . $this->bucket . '" This bucket name is not dns compatible, it may contain invalid characters.',
-				['app' => 'objectstore']);
-		}
-
-		if ($this->params['verify_bucket_exists'] && !$this->connection->doesBucketExist($this->bucket)) {
-			$logger = \OC::$server->get(LoggerInterface::class);
-			try {
-				$logger->info('Bucket "' . $this->bucket . '" does not exist - creating it.', ['app' => 'objectstore']);
-				if (!$this->connection::isBucketDnsCompatible($this->bucket)) {
-					throw new \Exception("The bucket will not be created because the name is not dns compatible, please correct it: " . $this->bucket);
-				}
-				$this->connection->createBucket(['Bucket' => $this->bucket]);
-				$this->testTimeout();
-			} catch (S3Exception $e) {
-				$logger->debug('Invalid remote storage.', [
-					'exception' => $e,
-					'app' => 'objectstore',
-				]);
-				if ($e->getAwsErrorCode() !== "BucketAlreadyOwnedByYou") {
-					throw new \Exception('Creation of bucket "' . $this->bucket . '" failed. ' . $e->getMessage());
+		try {
+			$logger = Server::get(LoggerInterface::class);
+			if (!$this->connection::isBucketDnsCompatible($this->bucket)) {
+				$logger->debug('Bucket "' . $this->bucket . '" This bucket name is not dns compatible, it may contain invalid characters.',
+					['app' => 'objectstore']);
+			}
+	
+			if ($this->params['verify_bucket_exists'] && !$this->connection->doesBucketExist($this->bucket)) {
+				try {
+					$logger->info('Bucket "' . $this->bucket . '" does not exist - creating it.', ['app' => 'objectstore']);
+					if (!$this->connection::isBucketDnsCompatible($this->bucket)) {
+						throw new \Exception("The bucket will not be created because the name is not dns compatible, please correct it: " . $this->bucket);
+					}
+					$this->connection->createBucket(['Bucket' => $this->bucket]);
+					$this->testTimeout();
+				} catch (S3Exception $e) {
+					$logger->debug('Invalid remote storage.', [
+						'exception' => $e,
+						'app' => 'objectstore',
+					]);
+					if ($e->getAwsErrorCode() !== 'BucketAlreadyOwnedByYou') {
+						throw new \Exception('Creation of bucket "' . $this->bucket . '" failed. ' . $e->getMessage());
+					}
 				}
 			}
-		}
-
-		// google cloud's s3 compatibility doesn't like the EncodingType parameter
-		if (strpos($base_url, 'storage.googleapis.com')) {
-			$this->connection->getHandlerList()->remove('s3.auto_encode');
+	
+			// google cloud's s3 compatibility doesn't like the EncodingType parameter
+			if (strpos($base_url, 'storage.googleapis.com')) {
+				$this->connection->getHandlerList()->remove('s3.auto_encode');
+			}
+		} catch (S3Exception $e) {
+			throw new \Exception('S3 service is unable to handle request: ' . $e->getMessage());
 		}
 
 		return $this->connection;
@@ -220,7 +228,7 @@ protected function getCertificateBundlePath(): ?string {
 			// since we store the certificate bundles on the primary storage, we can't get the bundle while setting up the primary storage
 			if (!isset($this->params['primary_storage'])) {
 				/** @var ICertificateManager $certManager */
-				$certManager = \OC::$server->get(ICertificateManager::class);
+				$certManager = Server::get(ICertificateManager::class);
 				return $certManager->getAbsoluteBundlePath();
 			} else {
 				return \OC::$SERVERROOT . '/resources/config/ca-bundle.crt';