fix(client): Use shared default options for HTTP client requests

Makes sure we always set a sane timeout and `allow_local_address`. Fixes: #3255 Fixes: #3435 Fixes: nextcloud/server#44190 Signed-off-by: Jonas <[email protected]>
nextcloud · Jul 24, 2024 · 00f61ba · 00f61ba
1 parent 0980182
commit 00f61ba
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 28 deletions.
diff --git a/composer/composer/autoload_classmap.php b/composer/composer/autoload_classmap.php
@@ -75,6 +75,7 @@
     'OCA\\Richdocuments\\Service\\FileTargetService' => $baseDir . '/../lib/Service/FileTargetService.php',
     'OCA\\Richdocuments\\Service\\FontService' => $baseDir . '/../lib/Service/FontService.php',
     'OCA\\Richdocuments\\Service\\InitialStateService' => $baseDir . '/../lib/Service/InitialStateService.php',
+    'OCA\\Richdocuments\\Service\\RemoteOptionsService' => $baseDir . '/../lib/Service/RemoteOptionsService.php',
     'OCA\\Richdocuments\\Service\\RemoteService' => $baseDir . '/../lib/Service/RemoteService.php',
     'OCA\\Richdocuments\\Service\\UserScopeService' => $baseDir . '/../lib/Service/UserScopeService.php',
     'OCA\\Richdocuments\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php',

diff --git a/composer/composer/autoload_static.php b/composer/composer/autoload_static.php
@@ -90,6 +90,7 @@ class ComposerStaticInitRichdocuments
         'OCA\\Richdocuments\\Service\\FileTargetService' => __DIR__ . '/..' . '/../lib/Service/FileTargetService.php',
         'OCA\\Richdocuments\\Service\\FontService' => __DIR__ . '/..' . '/../lib/Service/FontService.php',
         'OCA\\Richdocuments\\Service\\InitialStateService' => __DIR__ . '/..' . '/../lib/Service/InitialStateService.php',
+        'OCA\\Richdocuments\\Service\\RemoteOptionsService' => __DIR__ . '/..' . '/../lib/Service/RemoteOptionsService.php',
         'OCA\\Richdocuments\\Service\\RemoteService' => __DIR__ . '/..' . '/../lib/Service/RemoteService.php',
         'OCA\\Richdocuments\\Service\\UserScopeService' => __DIR__ . '/..' . '/../lib/Service/UserScopeService.php',
         'OCA\\Richdocuments\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php',

diff --git a/lib/Preview/Office.php b/lib/Preview/Office.php
@@ -7,23 +7,30 @@
 
 use OCA\Richdocuments\AppConfig;
 use OCA\Richdocuments\Capabilities;
+use OCA\Richdocuments\Service\RemoteOptionsService;
 use OCP\Files\File;
+use OCP\Files\FileInfo;
 use OCP\Http\Client\IClientService;
 use OCP\IImage;
 use OCP\Image;
 use OCP\Preview\IProviderV2;
 use Psr\Log\LoggerInterface;
 
 abstract class Office implements IProviderV2 {
-	private array $capabilities = [];
-
-	public function __construct(private IClientService $clientService, private AppConfig $config, Capabilities $capabilities, private LoggerInterface $logger) {
-		$this->capabilitites = $capabilities->getCapabilities()['richdocuments'] ?? [];
+	private array $capabilities;
+
+	public function __construct(
+		private IClientService $clientService,
+		private AppConfig $config,
+		Capabilities $capabilities,
+		private LoggerInterface $logger,
+	) {
+		$this->capabilities = $capabilities->getCapabilities()['richdocuments'] ?? [];
 	}
 
-	public function isAvailable(\OCP\Files\FileInfo $file): bool {
-		if (isset($this->capabilitites['collabora']['convert-to']['available'])) {
-			return (bool)$this->capabilitites['collabora']['convert-to']['available'];
+	public function isAvailable(FileInfo $file): bool {
+		if (isset($this->capabilities['collabora']['convert-to']['available'])) {
+			return (bool)$this->capabilities['collabora']['convert-to']['available'];
 		}
 		return false;
 	}
@@ -45,11 +52,9 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 		}
 
 		$client = $this->clientService->newClient();
-		$options = [
-			'timeout' => 25,
-			// FIXME: Can be removed once https://github.com/CollaboraOnline/online/issues/6983 is fixed upstream
-			'expect' => false,
-		];
+		$options = RemoteOptionsService::getDefaultOptions();
+		// FIXME: can be removed once https://github.com/CollaboraOnline/online/issues/6983 is fixed upstream
+		$options['expect'] = false;
 
 		if ($this->config->getAppValue('richdocuments', 'disable_certificate_verification') === 'yes') {
 			$options['verify'] = false;

diff --git a/lib/Service/CachedRequestService.php b/lib/Service/CachedRequestService.php
@@ -19,7 +19,6 @@
 use Psr\Log\LoggerInterface;
 
 abstract class CachedRequestService {
-
 	public function __construct(
 		private IClientService $clientService,
 		private ICacheFactory $cacheFactory,
@@ -91,12 +90,7 @@ public function resetCache(): void {
 	}
 
 	protected function getDefaultRequestOptions(): array {
-		$options = [
-			'timeout' => 5,
-			'nextcloud' => [
-				'allow_local_address' => true
-			]
-		];
+		$options = RemoteOptionsService::getDefaultOptions();
 
 		if ($this->appConfig->getValueString('richdocuments', 'disable_certificate_verification') === 'yes') {
 			$options['verify'] = false;

diff --git a/lib/Service/RemoteOptionsService.php b/lib/Service/RemoteOptionsService.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+namespace OCA\Richdocuments\Service;
+
+class RemoteOptionsService {
+	public const REMOTE_TIMEOUT_DEFAULT = 5;
+
+	public static function getDefaultOptions(int $timeout = self::REMOTE_TIMEOUT_DEFAULT): array {
+		return [
+			'timeout' => $timeout,
+			'nextcloud' => [
+				'allow_local_address' => true,
+			]
+		];
+	}
+
+}
diff --git a/lib/Service/RemoteService.php b/lib/Service/RemoteService.php
@@ -13,9 +13,6 @@
 use Psr\Log\LoggerInterface;
 
 class RemoteService {
-
-	public const REMOTE_TIMEOUT_DEFAULT = 25;
-
 	public function __construct(
 		private AppConfig $appConfig,
 		private IClientService $clientService,
@@ -69,12 +66,10 @@ private function getRequestOptionsForFile(File $file, ?string $target = null): a
 			$stream = $file->fopen('rb');
 		}
 
-		$options = [
-			'timeout' => self::REMOTE_TIMEOUT_DEFAULT,
-			'multipart' => [
-				['name' => $file->getName(), 'contents' => $stream],
-				['name' => 'target', 'contents' => $target]
-			]
+		$options = RemoteOptionsService::getDefaultOptions(25);
+		$options['multipart'] = [
+			['name' => $file->getName(), 'contents' => $stream],
+			['name' => 'target', 'contents' => $target]
 		];
 
 		if ($this->appConfig->getDisableCertificateValidation()) {