Skip to content

Commit dbe4acb

Browse files
solracsfsolracsf
committed
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>
1 parent a98a0eb commit dbe4acb

File tree

1 file changed

+41
-29
lines changed

1 file changed

+41
-29
lines changed

‎lib/private/Files/ObjectStore/S3ConnectionTrait.php‎

Lines changed: 41 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use GuzzleHttp\Promise\Create;
1515
use GuzzleHttp\Promise\RejectedPromise;
1616
use OCP\ICertificateManager;
17+
use OCP\Server;
1718
use Psr\Log\LoggerInterface;
1819

1920
trait S3ConnectionTrait {
@@ -98,7 +99,11 @@ public function getConnection() {
9899
'signature_provider' => \Aws\or_chain([self::class, 'legacySignatureProvider'], ClientResolver::_default_signature_provider()),
99100
'csm' => false,
100101
'use_arn_region' => false,
101-
'http' => ['verify' => $this->getCertificateBundlePath()],
102+
'http' => [
103+
'verify' => $this->getCertificateBundlePath(),
104+
// Timeout for the connection to S3 server, not for the request.
105+
'connect_timeout' => 5
106+
],
102107
'use_aws_shared_config_files' => false,
103108
];
104109

@@ -116,35 +121,42 @@ public function getConnection() {
116121
}
117122
$this->connection = new S3Client($options);
118123

119-
if (!$this->connection::isBucketDnsCompatible($this->bucket)) {
120-
$logger = \OC::$server->get(LoggerInterface::class);
121-
$logger->debug('Bucket "' . $this->bucket . '" This bucket name is not dns compatible, it may contain invalid characters.',
122-
['app' => 'objectstore']);
123-
}
124-
125-
if ($this->params['verify_bucket_exists'] && !$this->connection->doesBucketExist($this->bucket)) {
126-
$logger = \OC::$server->get(LoggerInterface::class);
127-
try {
128-
$logger->info('Bucket "' . $this->bucket . '" does not exist - creating it.', ['app' => 'objectstore']);
129-
if (!$this->connection::isBucketDnsCompatible($this->bucket)) {
130-
throw new \Exception("The bucket will not be created because the name is not dns compatible, please correct it: " . $this->bucket);
131-
}
132-
$this->connection->createBucket(['Bucket' => $this->bucket]);
133-
$this->testTimeout();
134-
} catch (S3Exception $e) {
135-
$logger->debug('Invalid remote storage.', [
136-
'exception' => $e,
137-
'app' => 'objectstore',
138-
]);
139-
if ($e->getAwsErrorCode() !== "BucketAlreadyOwnedByYou") {
140-
throw new \Exception('Creation of bucket "' . $this->bucket . '" failed. ' . $e->getMessage());
124+
try {
125+
$logger = Server::get(LoggerInterface::class);
126+
if (!$this->connection::isBucketDnsCompatible($this->bucket)) {
127+
$logger->debug('Bucket "' . $this->bucket . '" This bucket name is not dns compatible, it may contain invalid characters.',
128+
['app' => 'objectstore']);
129+
}
130+
131+
if ($this->params['verify_bucket_exists'] && !$this->connection->doesBucketExist($this->bucket)) {
132+
try {
133+
$logger->info('Bucket "' . $this->bucket . '" does not exist - creating it.', ['app' => 'objectstore']);
134+
if (!$this->connection::isBucketDnsCompatible($this->bucket)) {
135+
throw new \Exception("The bucket will not be created because the name is not dns compatible, please correct it: " . $this->bucket);
136+
}
137+
$this->connection->createBucket(['Bucket' => $this->bucket]);
138+
$this->testTimeout();
139+
} catch (S3Exception $e) {
140+
$logger->debug('Invalid remote storage.', [
141+
'exception' => $e,
142+
'app' => 'objectstore',
143+
]);
144+
if ($e->getAwsErrorCode() !== 'BucketAlreadyOwnedByYou') {
145+
throw new \Exception('Creation of bucket "' . $this->bucket . '" failed. ' . $e->getMessage());
146+
}
141147
}
142148
}
143-
}
144-
145-
// google cloud's s3 compatibility doesn't like the EncodingType parameter
146-
if (strpos($base_url, 'storage.googleapis.com')) {
147-
$this->connection->getHandlerList()->remove('s3.auto_encode');
149+
150+
// google cloud's s3 compatibility doesn't like the EncodingType parameter
151+
if (strpos($base_url, 'storage.googleapis.com')) {
152+
$this->connection->getHandlerList()->remove('s3.auto_encode');
153+
}
154+
} catch (S3Exception $e) {
155+
if ($e->getAwsErrorCode() === 'RequestTimeout' || $e->getAwsErrorCode() === 'NetworkingError') {
156+
throw new \Exception('S3 connection failed due to timeout or connection issues: ' . $e->getMessage());
157+
} else {
158+
throw new \Exception('S3 service is unable to handle request: ' . $e->getMessage());
159+
}
148160
}
149161

150162
return $this->connection;
@@ -193,7 +205,7 @@ protected function getCertificateBundlePath(): ?string {
193205
// since we store the certificate bundles on the primary storage, we can't get the bundle while setting up the primary storage
194206
if (!isset($this->params['primary_storage'])) {
195207
/** @var ICertificateManager $certManager */
196-
$certManager = \OC::$server->get(ICertificateManager::class);
208+
$certManager = Server::get(ICertificateManager::class);
197209
return $certManager->getAbsoluteBundlePath();
198210
} else {
199211
return \OC::$SERVERROOT . '/resources/config/ca-bundle.crt';

0 commit comments

Comments
 (0)