Merge pull request #17511 from nextcloud/backport/16721/stable16

[stable16] Correctly handle emtpy string in proxyuserpwd config

2 files changed, 30 insertions, 23 deletions

diff --git a/lib/private/Http/Client/Client.php b/lib/private/Http/Client/Client.php
index 03b09bf54b7..0dde56b28f9 100644
--- a/lib/private/Http/Client/Client.php
+++ b/lib/private/Http/Client/Client.php
@@ -112,19 +112,20 @@ class Client implements IClient {
 	 *
 	 * @return string
 	 */
-	private function getProxyUri(): string {
-		$proxyHost = $this->config->getSystemValue('proxy', null);
-		$proxyUserPwd = $this->config->getSystemValue('proxyuserpwd', null);
-		$proxyUri = '';
+	private function getProxyUri(): ?string {
+		$proxyHost = $this->config->getSystemValue('proxy', '');
 
-		if ($proxyUserPwd !== null) {
-			$proxyUri .= $proxyUserPwd . '@';
+		if ($proxyHost === '' || $proxyHost === null) {
+			return null;
 		}
-		if ($proxyHost !== null) {
-			$proxyUri .= $proxyHost;
+
+		$proxyUserPwd = $this->config->getSystemValue('proxyuserpwd', '');
+
+		if ($proxyUserPwd === '' || $proxyUserPwd === null) {
+			return $proxyHost;
 		}
 
-		return $proxyUri;
+		return $proxyUserPwd . '@' . $proxyHost;
 	}
 
 	/**
diff --git a/tests/lib/Http/Client/ClientTest.php b/tests/lib/Http/Client/ClientTest.php
index 7f12a824d17..ea825266198 100644
--- a/tests/lib/Http/Client/ClientTest.php
+++ b/tests/lib/Http/Client/ClientTest.php
@@ -49,27 +49,22 @@ class ClientTest extends \Test\TestCase {
 		$this->config
 			->expects($this->at(0))
 			->method('getSystemValue')
-			->with('proxy', null)
-			->willReturn(null);
-		$this->config
-			->expects($this->at(1))
-			->method('getSystemValue')
-			->with('proxyuserpwd', null)
-			->willReturn(null);
-		$this->assertSame('', self::invokePrivate($this->client, 'getProxyUri'));
+			->with('proxy', '')
+			->willReturn('');
+		$this->assertNull(self::invokePrivate($this->client, 'getProxyUri'));
 	}
 
 	public function testGetProxyUriProxyHostEmptyPassword() {
 		$this->config
 			->expects($this->at(0))
 			->method('getSystemValue')
-			->with('proxy', null)
+			->with('proxy', '')
 			->willReturn('foo');
 		$this->config
 			->expects($this->at(1))
 			->method('getSystemValue')
-			->with('proxyuserpwd', null)
-			->willReturn(null);
+			->with('proxyuserpwd', '')
+			->willReturn('');
 		$this->assertSame('foo', self::invokePrivate($this->client, 'getProxyUri'));
 	}
 
@@ -77,12 +72,22 @@ class ClientTest extends \Test\TestCase {
 		$this->config
 			->expects($this->at(0))
 			->method('getSystemValue')
-			->with('proxy', null)
+			->with(
+				$this->equalTo('proxy'),
+				$this->callback(function ($input) {
+					return $input === '';
+				})
+			)
 			->willReturn('foo');
 		$this->config
 			->expects($this->at(1))
 			->method('getSystemValue')
-			->with('proxyuserpwd', null)
+			->with(
+				$this->equalTo('proxyuserpwd'),
+				$this->callback(function ($input) {
+					return $input === '';
+				})
+			)
 			->willReturn('username:password');
 		$this->assertSame('username:password@foo', self::invokePrivate($this->client, 'getProxyUri'));
 	}
@@ -260,7 +265,8 @@ class ClientTest extends \Test\TestCase {
 			->willReturn([]);
 
 		$this->assertEquals([
-			'verify' => \OC::$SERVERROOT . '/resources/config/ca-bundle.crt'
+			'verify' => \OC::$SERVERROOT . '/resources/config/ca-bundle.crt',
+			'proxy' => null,
 		], self::invokePrivate($this->client, 'getRequestOptions'));
 	}