diff --git a/lib/private/Http/Client/Client.php b/lib/private/Http/Client/Client.php index f1b81df396477..44217de6f2099 100644 --- a/lib/private/Http/Client/Client.php +++ b/lib/private/Http/Client/Client.php @@ -21,6 +21,7 @@ use OCP\ServerVersion; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\UriInterface; use Psr\Log\LoggerInterface; use function parse_url; @@ -41,67 +42,125 @@ public function __construct( } private function buildRequestOptions(array $options): array { - $proxy = $this->getProxyUri(); - $defaults = [ RequestOptions::VERIFY => $this->getCertBundle(), RequestOptions::TIMEOUT => IClient::DEFAULT_REQUEST_TIMEOUT, - // Prefer HTTP/2 globally (PSR-7 request version) - RequestOptions::VERSION => '2.0', ]; - $defaults['curl'][\CURLOPT_HTTP_VERSION] = \CURL_HTTP_VERSION_2TLS; - - $options['nextcloud']['allow_local_address'] = $this->isLocalAddressAllowed($options); - if ($options['nextcloud']['allow_local_address'] === false) { - $onRedirectFunction = function ( - \Psr\Http\Message\RequestInterface $request, - \Psr\Http\Message\ResponseInterface $response, - \Psr\Http\Message\UriInterface $uri, - ) use ($options): void { - $this->preventLocalAddress($uri->__toString(), $options); - }; - $defaults[RequestOptions::ALLOW_REDIRECTS] = [ - 'on_redirect' => $onRedirectFunction - ]; + $this->applyHttp2Defaults($defaults); + $this->applyLocalAddressProtection($defaults, $options); + $this->applyProxyOption($defaults); + + $options = array_merge($defaults, $options); + $options[RequestOptions::HEADERS] ??= []; + + $this->ensureDefaultUserAgent($options); + $this->ensureDefaultAcceptEncoding($options); + $this->normalizeLegacySaveToOption($options); + + return $options; + } + + /** + * Propagate local-address policy into request options and validate redirect targets. + */ + private function applyLocalAddressProtection(array &$defaults, array &$options): void { + $allowLocalAddress = $this->isLocalAddressAllowed($options); + $options['nextcloud']['allow_local_address'] = $allowLocalAddress; + + if ($allowLocalAddress) { + return; } + $defaults[RequestOptions::ALLOW_REDIRECTS] = [ + 'on_redirect' => function ( + RequestInterface $_request, + ResponseInterface $_response, + UriInterface $uri, + ) use ($options): void { + $this->preventLocalAddress((string)$uri, $options); + }, + ]; + } + + private function applyProxyOption(array &$defaults): void { + $proxy = $this->getProxyUri(); + // Only add RequestOptions::PROXY if Nextcloud is explicitly // configured to use a proxy. This is needed in order not to override // Guzzle default values. if ($proxy !== null) { $defaults[RequestOptions::PROXY] = $proxy; } + } - $options = array_merge($defaults, $options); + private function ensureDefaultUserAgent(array &$options): void { + if (isset($options[RequestOptions::HEADERS]['User-Agent'])) { + return; + } + + $userAgent = 'Nextcloud-Server-Crawler/' . $this->serverVersion->getVersionString(); + $overwriteCliUrl = $this->config->getSystemValueString('overwrite.cli.url'); + $addUrl = !empty($overwriteCliUrl) && $this->config->getSystemValueBool('http_client_add_user_agent_url', false); - if (!isset($options[RequestOptions::HEADERS]['User-Agent'])) { - $userAgent = 'Nextcloud-Server-Crawler/' . $this->serverVersion->getVersionString(); - $overwriteCliUrl = $this->config->getSystemValueString('overwrite.cli.url'); - if ($this->config->getSystemValueBool('http_client_add_user_agent_url') && !empty($overwriteCliUrl)) { - $userAgent .= '; +' . rtrim($overwriteCliUrl, '/'); - } - $options[RequestOptions::HEADERS]['User-Agent'] = $userAgent; + // Optionally append the instance URL to the crawler user agent. + if ($addUrl) { + $userAgent .= '; +' . rtrim($overwriteCliUrl, '/'); } - // Ensure headers array exists and set Accept-Encoding only if not present - $headers = $options[RequestOptions::HEADERS] ?? []; - if (!isset($headers['Accept-Encoding'])) { - $acceptEnc = 'gzip'; - if (function_exists('brotli_uncompress')) { - $acceptEnc = 'br, ' . $acceptEnc; - } - $options[RequestOptions::HEADERS] = $headers; // ensure headers are present - $options[RequestOptions::HEADERS]['Accept-Encoding'] = $acceptEnc; + $options[RequestOptions::HEADERS]['User-Agent'] = $userAgent; + } + + private function ensureDefaultAcceptEncoding(array &$options): void { + if (isset($options[RequestOptions::HEADERS]['Accept-Encoding'])) { + return; } - // Fallback for save_to - if (isset($options['save_to'])) { - $options['sink'] = $options['save_to']; - unset($options['save_to']); + $acceptEncoding = 'gzip'; + if (function_exists('brotli_uncompress')) { + $acceptEncoding = 'br, ' . $acceptEncoding; } - return $options; + $options[RequestOptions::HEADERS]['Accept-Encoding'] = $acceptEncoding; + } + + private function normalizeLegacySaveToOption(array &$options): void { + if (!isset($options['save_to'])) { + return; + } + + // Support legacy 'save_to' by mapping it to Guzzle's 'sink'. + $options['sink'] = $options['save_to']; + unset($options['save_to']); + } + + private function applyHttp2Defaults(array &$defaults): void { + if (!$this->supportsHttp2()) { + return; + } + + // Prefer HTTP/2 when supported by the local libcurl build. + $defaults[RequestOptions::VERSION] = '2.0'; + + if (\defined('CURL_HTTP_VERSION_2TLS')) { + $defaults['curl'][\CURLOPT_HTTP_VERSION] = \CURL_HTTP_VERSION_2TLS; + return; + } + + if (\defined('CURL_HTTP_VERSION_2_0')) { + $defaults['curl'][\CURLOPT_HTTP_VERSION] = \CURL_HTTP_VERSION_2_0; + } + } + + protected function supportsHttp2(): bool { + if (!\function_exists('curl_version') || !\defined('CURL_VERSION_HTTP2')) { + return false; + } + + $curlVersion = \curl_version(); + $features = $curlVersion['features'] ?? 0; + + return ($features & \CURL_VERSION_HTTP2) === \CURL_VERSION_HTTP2; } private function getCertBundle(): string { @@ -155,7 +214,7 @@ private function getProxyUri(): ?array { return $proxy; } - private function isLocalAddressAllowed(array $options) : bool { + private function isLocalAddressAllowed(array $options): bool { if (($options['nextcloud']['allow_local_address'] ?? false) || $this->config->getSystemValueBool('allow_local_remote_servers', false)) { return true; diff --git a/tests/lib/Http/Client/ClientTest.php b/tests/lib/Http/Client/ClientTest.php index ed732cdddc3f6..a36a7f97d0459 100644 --- a/tests/lib/Http/Client/ClientTest.php +++ b/tests/lib/Http/Client/ClientTest.php @@ -51,14 +51,37 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->serverVersion = $this->createMock(ServerVersion::class); - $this->client = new Client( - $this->config, - $this->certificateManager, - $this->guzzleClient, - $this->remoteHostValidator, - $this->logger, - $this->serverVersion, - ); + $this->client = $this->newClientWithHttp2Support(true); + } + + private function newClientWithHttp2Support(bool $supportsHttp2): Client { + $client = $this->getMockBuilder(Client::class) + ->setConstructorArgs([ + $this->config, + $this->certificateManager, + $this->guzzleClient, + $this->remoteHostValidator, + $this->logger, + $this->serverVersion, + ]) + ->onlyMethods(['supportsHttp2']) + ->getMock(); + + $client->method('supportsHttp2') + ->willReturn($supportsHttp2); + + return $client; + } + + private function addExpectedHttp2Options(array $options): array { + $options['version'] = '2.0'; + $options['curl'] = [ + \CURLOPT_HTTP_VERSION => \defined('CURL_HTTP_VERSION_2TLS') + ? \CURL_HTTP_VERSION_2TLS + : \CURL_HTTP_VERSION_2_0, + ]; + + return $options; } public function testGetProxyUri(): void { @@ -257,7 +280,9 @@ public function testPreventLocalAddressOnDelete(string $uri): void { $this->client->delete($uri); } - private function setUpDefaultRequestOptions(): void { + private function setUpDefaultRequestOptions(bool $supportsHttp2 = true): void { + $this->client = $this->newClientWithHttp2Support($supportsHttp2); + $this->config ->method('getSystemValue') ->willReturnMap([ @@ -288,6 +313,7 @@ private function setUpDefaultRequestOptions(): void { ->willReturn('123.45.6'); $acceptEnc = function_exists('brotli_uncompress') ? 'br, gzip' : 'gzip'; + $this->defaultRequestOptions = [ 'verify' => '/my/path.crt', 'proxy' => [ @@ -298,17 +324,16 @@ private function setUpDefaultRequestOptions(): void { 'User-Agent' => 'Nextcloud-Server-Crawler/123.45.6', 'Accept-Encoding' => $acceptEnc, - ], 'timeout' => 30, 'nextcloud' => [ 'allow_local_address' => true, ], - 'version' => '2.0', - 'curl' => [ - \CURLOPT_HTTP_VERSION => \CURL_HTTP_VERSION_2TLS, - ], ]; + + if ($supportsHttp2) { + $this->defaultRequestOptions = $this->addExpectedHttp2Options($this->defaultRequestOptions); + } } public function testGet(): void { @@ -468,8 +493,9 @@ public function testHeadWithOptions(): void { } public function testSetDefaultOptionsWithNotInstalled(): void { + $this->client = $this->newClientWithHttp2Support(true); + $this->config - ->expects($this->exactly(3)) ->method('getSystemValueBool') ->willReturnMap([ ['installed', false, false], @@ -483,6 +509,7 @@ public function testSetDefaultOptionsWithNotInstalled(): void { ['proxy', '', ''], ['overwrite.cli.url', '', ''], ]); + $this->certificateManager ->expects($this->never()) ->method('listCertificates'); @@ -496,7 +523,7 @@ public function testSetDefaultOptionsWithNotInstalled(): void { $acceptEnc = function_exists('brotli_uncompress') ? 'br, gzip' : 'gzip'; - $this->assertEquals([ + $expected = [ 'verify' => \OC::$SERVERROOT . '/resources/config/ca-bundle.crt', 'headers' => [ 'User-Agent' => 'Nextcloud-Server-Crawler/123.45.6', @@ -514,16 +541,17 @@ public function testSetDefaultOptionsWithNotInstalled(): void { ): void { }, ], - 'version' => '2.0', - 'curl' => [ - \CURLOPT_HTTP_VERSION => \CURL_HTTP_VERSION_2TLS, - ], - ], self::invokePrivate($this->client, 'buildRequestOptions', [[]])); + ]; + + $expected = $this->addExpectedHttp2Options($expected); + + $this->assertEquals($expected, self::invokePrivate($this->client, 'buildRequestOptions', [[]])); } public function testSetDefaultOptionsWithProxy(): void { + $this->client = $this->newClientWithHttp2Support(true); + $this->config - ->expects($this->exactly(3)) ->method('getSystemValueBool') ->willReturnMap([ ['installed', false, true], @@ -543,6 +571,7 @@ public function testSetDefaultOptionsWithProxy(): void { ['proxyuserpwd', '', ''], ['overwrite.cli.url', '', ''], ]); + $this->certificateManager ->expects($this->once()) ->method('getAbsoluteBundlePath') @@ -554,7 +583,7 @@ public function testSetDefaultOptionsWithProxy(): void { $acceptEnc = function_exists('brotli_uncompress') ? 'br, gzip' : 'gzip'; - $this->assertEquals([ + $expected = [ 'verify' => '/my/path.crt', 'proxy' => [ 'http' => 'foo', @@ -576,16 +605,17 @@ public function testSetDefaultOptionsWithProxy(): void { ): void { }, ], - 'version' => '2.0', - 'curl' => [ - \CURLOPT_HTTP_VERSION => \CURL_HTTP_VERSION_2TLS, - ], - ], self::invokePrivate($this->client, 'buildRequestOptions', [[]])); + ]; + + $expected = $this->addExpectedHttp2Options($expected); + + $this->assertEquals($expected, self::invokePrivate($this->client, 'buildRequestOptions', [[]])); } public function testSetDefaultOptionsWithProxyAndExclude(): void { + $this->client = $this->newClientWithHttp2Support(true); + $this->config - ->expects($this->exactly(3)) ->method('getSystemValueBool') ->willReturnMap([ ['installed', false, true], @@ -605,6 +635,7 @@ public function testSetDefaultOptionsWithProxyAndExclude(): void { ['proxyuserpwd', '', ''], ['overwrite.cli.url', '', ''], ]); + $this->certificateManager ->expects($this->once()) ->method('getAbsoluteBundlePath') @@ -616,7 +647,7 @@ public function testSetDefaultOptionsWithProxyAndExclude(): void { $acceptEnc = function_exists('brotli_uncompress') ? 'br, gzip' : 'gzip'; - $this->assertEquals([ + $expected = [ 'verify' => '/my/path.crt', 'proxy' => [ 'http' => 'foo', @@ -639,11 +670,62 @@ public function testSetDefaultOptionsWithProxyAndExclude(): void { ): void { }, ], - 'version' => '2.0', - 'curl' => [ - \CURLOPT_HTTP_VERSION => \CURL_HTTP_VERSION_2TLS, + ]; + + $expected = $this->addExpectedHttp2Options($expected); + + $this->assertEquals($expected, self::invokePrivate($this->client, 'buildRequestOptions', [[]])); + } + + public function testSetDefaultOptionsWithoutHttp2Support(): void { + $this->client = $this->newClientWithHttp2Support(false); + + $this->config + ->method('getSystemValueBool') + ->willReturnMap([ + ['installed', false, true], + ['allow_local_remote_servers', false, false], + ['http_client_add_user_agent_url', false, false], + ]); + $this->config + ->expects($this->exactly(2)) + ->method('getSystemValueString') + ->willReturnMap([ + ['proxy', '', ''], + ['overwrite.cli.url', '', ''], + ]); + + $this->certificateManager + ->expects($this->once()) + ->method('getAbsoluteBundlePath') + ->willReturn('/my/path.crt'); + + $this->serverVersion->method('getVersionString') + ->willReturn('123.45.6'); + + $acceptEnc = function_exists('brotli_uncompress') ? 'br, gzip' : 'gzip'; + + $expected = [ + 'verify' => '/my/path.crt', + 'headers' => [ + 'User-Agent' => 'Nextcloud-Server-Crawler/123.45.6', + 'Accept-Encoding' => $acceptEnc, + ], + 'timeout' => 30, + 'nextcloud' => [ + 'allow_local_address' => false, ], - ], self::invokePrivate($this->client, 'buildRequestOptions', [[]])); + 'allow_redirects' => [ + 'on_redirect' => function ( + \Psr\Http\Message\RequestInterface $request, + \Psr\Http\Message\ResponseInterface $response, + \Psr\Http\Message\UriInterface $uri, + ): void { + }, + ], + ]; + + $this->assertEquals($expected, self::invokePrivate($this->client, 'buildRequestOptions', [[]])); } public static function dataForTestSetServerUrlInUserAgent(): array { @@ -655,8 +737,9 @@ public static function dataForTestSetServerUrlInUserAgent(): array { #[DataProvider('dataForTestSetServerUrlInUserAgent')] public function testSetServerUrlInUserAgent(string $url, string $userAgent): void { + $this->client = $this->newClientWithHttp2Support(true); + $this->config - ->expects($this->exactly(3)) ->method('getSystemValueBool') ->willReturnMap([ ['installed', false, true], @@ -670,6 +753,7 @@ public function testSetServerUrlInUserAgent(string $url, string $userAgent): voi ['proxy', '', ''], ['overwrite.cli.url', '', $url], ]); + $this->certificateManager ->expects($this->once()) ->method('getAbsoluteBundlePath') @@ -679,11 +763,13 @@ public function testSetServerUrlInUserAgent(string $url, string $userAgent): voi $this->serverVersion->method('getVersionString') ->willReturn('123.45.6'); - $this->assertEquals([ + $acceptEnc = function_exists('brotli_uncompress') ? 'br, gzip' : 'gzip'; + + $expected = [ 'verify' => '/my/path.crt', 'headers' => [ 'User-Agent' => $userAgent, - 'Accept-Encoding' => 'gzip', + 'Accept-Encoding' => $acceptEnc, ], 'timeout' => 30, 'nextcloud' => [ @@ -697,10 +783,10 @@ public function testSetServerUrlInUserAgent(string $url, string $userAgent): voi ): void { }, ], - 'version' => '2.0', - 'curl' => [ - \CURLOPT_HTTP_VERSION => \CURL_HTTP_VERSION_2TLS, - ], - ], self::invokePrivate($this->client, 'buildRequestOptions', [[]])); + ]; + + $expected = $this->addExpectedHttp2Options($expected); + + $this->assertEquals($expected, self::invokePrivate($this->client, 'buildRequestOptions', [[]])); } }