diff --git a/config/config.sample.php b/config/config.sample.php index 2894bc5deb..ea91bb83bb 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -520,6 +520,12 @@ $CONFIG = array( /** * The URL of your proxy server, for example ``proxy.example.com:8081``. * + * Note: Guzzle (the http library used by Nextcloud) is reading the environment + * variables HTTP_PROXY (only for cli request), HTTPS_PROXY, and NO_PROXY by default. + * + * If you configure proxy with Nextcloud any default configuration by Guzzle + * is overwritten. Make sure to set ``proxyexclude`` accordingly if necessary. + * * Defaults to ``''`` (empty string) */ 'proxy' => '', @@ -532,6 +538,16 @@ $CONFIG = array( */ 'proxyuserpwd' => '', +/** + * List of host names that should not be proxied to. + * For example: ``['.mit.edu', 'foo.com']``. + * + * Hint: Use something like ``explode(',', getenv('NO_PROXY'))`` to sync this + * value with the global NO_PROXY option. + * + * Defaults to empty array. + */ +'proxyexclude' => [], /** * Deleted Items (trash bin) diff --git a/lib/private/Http/Client/Client.php b/lib/private/Http/Client/Client.php index c52d90ff8e..657bfb8d53 100644 --- a/lib/private/Http/Client/Client.php +++ b/lib/private/Http/Client/Client.php @@ -65,12 +65,20 @@ class Client implements IClient { } private function buildRequestOptions(array $options): array { + $proxy = $this->getProxyUri(); + $defaults = [ - RequestOptions::PROXY => $this->getProxyUri(), RequestOptions::VERIFY => $this->getCertBundle(), RequestOptions::TIMEOUT => 30, ]; + // 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); if (!isset($options[RequestOptions::HEADERS]['User-Agent'])) { @@ -96,11 +104,21 @@ class Client implements IClient { } /** - * Get the proxy URI + * Returns a null or an associative array specifiying the proxy URI for + * 'http' and 'https' schemes, in addition to a 'no' key value pair + * providing a list of host names that should not be proxied to. + * + * @return array|null + * + * The return array looks like: + * [ + * 'http' => 'username:password@proxy.example.com', + * 'https' => 'username:password@proxy.example.com', + * 'no' => ['foo.com', 'bar.com'] + * ] * - * @return string|null */ - private function getProxyUri(): ?string { + private function getProxyUri(): ?array { $proxyHost = $this->config->getSystemValue('proxy', ''); if ($proxyHost === '' || $proxyHost === null) { @@ -108,12 +126,21 @@ class Client implements IClient { } $proxyUserPwd = $this->config->getSystemValue('proxyuserpwd', ''); - - if ($proxyUserPwd === '' || $proxyUserPwd === null) { - return $proxyHost; + if ($proxyUserPwd !== '' && $proxyUserPwd !== null) { + $proxyHost = $proxyUserPwd . '@' . $proxyHost; } - return $proxyUserPwd . '@' . $proxyHost; + $proxy = [ + 'http' => $proxyHost, + 'https' => $proxyHost, + ]; + + $proxyExclude = $this->config->getSystemValue('proxyexclude', []); + if ($proxyExclude !== [] && $proxyExclude !== null) { + $proxy['no'] = $proxyExclude; + } + + return $proxy; } /** diff --git a/tests/lib/Http/Client/ClientTest.php b/tests/lib/Http/Client/ClientTest.php index 73cfb0bb6c..2f9e70a8bb 100644 --- a/tests/lib/Http/Client/ClientTest.php +++ b/tests/lib/Http/Client/ClientTest.php @@ -63,7 +63,15 @@ class ClientTest extends \Test\TestCase { ->method('getSystemValue') ->with('proxyuserpwd', null) ->willReturn(null); - $this->assertSame('foo', self::invokePrivate($this->client, 'getProxyUri')); + $this->config + ->expects($this->at(2)) + ->method('getSystemValue') + ->with('proxyexclude', []) + ->willReturn([]); + $this->assertEquals([ + 'http' => 'foo', + 'https' => 'foo' + ], self::invokePrivate($this->client, 'getProxyUri')); } public function testGetProxyUriProxyHostWithPassword(): void { @@ -87,7 +95,58 @@ class ClientTest extends \Test\TestCase { }) ) ->willReturn('username:password'); - $this->assertSame('username:password@foo', self::invokePrivate($this->client, 'getProxyUri')); + $this->config + ->expects($this->at(2)) + ->method('getSystemValue') + ->with( + $this->equalTo('proxyexclude'), + $this->callback(function ($input) { + return $input === []; + }) + ) + ->willReturn([]); + $this->assertEquals([ + 'http' => 'username:password@foo', + 'https' => 'username:password@foo' + ], self::invokePrivate($this->client, 'getProxyUri')); + } + + public function testGetProxyUriProxyHostWithPasswordAndExclude(): void { + $this->config + ->expects($this->at(0)) + ->method('getSystemValue') + ->with( + $this->equalTo('proxy'), + $this->callback(function ($input) { + return $input === ''; + }) + ) + ->willReturn('foo'); + $this->config + ->expects($this->at(1)) + ->method('getSystemValue') + ->with( + $this->equalTo('proxyuserpwd'), + $this->callback(function ($input) { + return $input === ''; + }) + ) + ->willReturn('username:password'); + $this->config + ->expects($this->at(2)) + ->method('getSystemValue') + ->with( + $this->equalTo('proxyexclude'), + $this->callback(function ($input) { + return $input === []; + }) + ) + ->willReturn(['bar']); + $this->assertEquals([ + 'http' => 'username:password@foo', + 'https' => 'username:password@foo', + 'no' => ['bar'] + ], self::invokePrivate($this->client, 'getProxyUri')); } private function setUpDefaultRequestOptions(): void { @@ -101,6 +160,11 @@ class ClientTest extends \Test\TestCase { ->method('getSystemValue') ->with('proxyuserpwd', null) ->willReturn(null); + $this->config + ->expects($this->at(2)) + ->method('getSystemValue') + ->with('proxyexclude', []) + ->willReturn([]); $this->certificateManager ->expects($this->once()) ->method('getAbsoluteBundlePath') @@ -109,7 +173,10 @@ class ClientTest extends \Test\TestCase { $this->defaultRequestOptions = [ 'verify' => '/my/path.crt', - 'proxy' => 'foo', + 'proxy' => [ + 'http' => 'foo', + 'https' => 'foo' + ], 'headers' => [ 'User-Agent' => 'Nextcloud Server Crawler', ], @@ -131,7 +198,10 @@ class ClientTest extends \Test\TestCase { $options = array_merge($this->defaultRequestOptions, [ 'verify' => false, - 'proxy' => 'bar', + 'proxy' => [ + 'http' => 'bar', + 'https' => 'bar' + ], ]); $this->guzzleClient->method('request') @@ -154,7 +224,10 @@ class ClientTest extends \Test\TestCase { $options = array_merge($this->defaultRequestOptions, [ 'verify' => false, - 'proxy' => 'bar', + 'proxy' => [ + 'http' => 'bar', + 'https' => 'bar' + ], ]); $this->guzzleClient->method('request') @@ -177,7 +250,10 @@ class ClientTest extends \Test\TestCase { $options = array_merge($this->defaultRequestOptions, [ 'verify' => false, - 'proxy' => 'bar', + 'proxy' => [ + 'http' => 'bar', + 'https' => 'bar' + ], ]); $this->guzzleClient->method('request') @@ -200,7 +276,10 @@ class ClientTest extends \Test\TestCase { $options = array_merge($this->defaultRequestOptions, [ 'verify' => false, - 'proxy' => 'bar', + 'proxy' => [ + 'http' => 'bar', + 'https' => 'bar' + ], ]); $this->guzzleClient->method('request') @@ -223,7 +302,10 @@ class ClientTest extends \Test\TestCase { $options = array_merge($this->defaultRequestOptions, [ 'verify' => false, - 'proxy' => 'bar', + 'proxy' => [ + 'http' => 'bar', + 'https' => 'bar' + ], ]); $this->guzzleClient->method('request') @@ -246,7 +328,10 @@ class ClientTest extends \Test\TestCase { $options = array_merge($this->defaultRequestOptions, [ 'verify' => false, - 'proxy' => 'bar', + 'proxy' => [ + 'http' => 'bar', + 'https' => 'bar' + ], ]); $this->guzzleClient->method('request') @@ -268,7 +353,6 @@ class ClientTest extends \Test\TestCase { $this->assertEquals([ 'verify' => \OC::$SERVERROOT . '/resources/config/ca-bundle.crt', - 'proxy' => null, 'headers' => [ 'User-Agent' => 'Nextcloud Server Crawler' ], @@ -287,6 +371,11 @@ class ClientTest extends \Test\TestCase { ->method('getSystemValue') ->with('proxyuserpwd', null) ->willReturn(null); + $this->config + ->expects($this->at(2)) + ->method('getSystemValue') + ->with('proxyexclude', []) + ->willReturn([]); $this->certificateManager ->expects($this->once()) ->method('getAbsoluteBundlePath') @@ -295,7 +384,46 @@ class ClientTest extends \Test\TestCase { $this->assertEquals([ 'verify' => '/my/path.crt', - 'proxy' => 'foo', + 'proxy' => [ + 'http' => 'foo', + 'https' => 'foo' + ], + 'headers' => [ + 'User-Agent' => 'Nextcloud Server Crawler' + ], + 'timeout' => 30, + ], self::invokePrivate($this->client, 'buildRequestOptions', [[]])); + } + + public function testSetDefaultOptionsWithProxyAndExclude(): void { + $this->config + ->expects($this->at(0)) + ->method('getSystemValue') + ->with('proxy', null) + ->willReturn('foo'); + $this->config + ->expects($this->at(1)) + ->method('getSystemValue') + ->with('proxyuserpwd', null) + ->willReturn(null); + $this->config + ->expects($this->at(2)) + ->method('getSystemValue') + ->with('proxyexclude', []) + ->willReturn(['bar']); + $this->certificateManager + ->expects($this->once()) + ->method('getAbsoluteBundlePath') + ->with(null) + ->willReturn('/my/path.crt'); + + $this->assertEquals([ + 'verify' => '/my/path.crt', + 'proxy' => [ + 'http' => 'foo', + 'https' => 'foo', + 'no' => ['bar'] + ], 'headers' => [ 'User-Agent' => 'Nextcloud Server Crawler' ],