Merge pull request #9873 from nextcloud/fix-http-client-given-options-being-overriden-by-default-options

Fix HTTP client given options being overriden by default options

lib/private/Http/Client/Client.php

@@ -158,7 +158,7 @@ class Client implements IClient {
 	 */
 	public function get(string $uri, array $options = []): IResponse {
 		$this->setDefaultOptions();
-		$response = $this->client->request('get', $uri, array_merge($options, $this->getRequestOptions()));
+		$response = $this->client->request('get', $uri, array_merge($this->getRequestOptions(), $options));
 		$isStream = isset($options['stream']) && $options['stream'];
 		return new Response($response, $isStream);
 	}
@@ -189,7 +189,7 @@ class Client implements IClient {
 	 */
 	public function head(string $uri, array $options = []): IResponse {
 		$this->setDefaultOptions();
-		$response = $this->client->request('head', $uri, array_merge($options, $this->getRequestOptions()));
+		$response = $this->client->request('head', $uri, array_merge($this->getRequestOptions(), $options));
 		return new Response($response);
 	}
 
@@ -228,7 +228,7 @@ class Client implements IClient {
 			$options['form_params'] = $options['body'];
 			unset($options['body']);
 		}
-		$response = $this->client->request('post', $uri, array_merge($options, $this->getRequestOptions()));
+		$response = $this->client->request('post', $uri, array_merge($this->getRequestOptions(), $options));
 		return new Response($response);
 	}
 
@@ -263,7 +263,7 @@ class Client implements IClient {
 	 */
 	public function put(string $uri, array $options = []): IResponse {
 		$this->setDefaultOptions();
-		$response = $this->client->request('put', $uri, array_merge($options, $this->getRequestOptions()));
+		$response = $this->client->request('put', $uri, array_merge($this->getRequestOptions(), $options));
 		return new Response($response);
 	}
 
@@ -298,7 +298,7 @@ class Client implements IClient {
 	 */
 	public function delete(string $uri, array $options = []): IResponse {
 		$this->setDefaultOptions();
-		$response = $this->client->request('delete', $uri, array_merge($options, $this->getRequestOptions()));
+		$response = $this->client->request('delete', $uri, array_merge($this->getRequestOptions(), $options));
 		return new Response($response);
 	}
 
@@ -334,7 +334,7 @@ class Client implements IClient {
 	 */
 	public function options(string $uri, array $options = []): IResponse {
 		$this->setDefaultOptions();
-		$response = $this->client->request('options', $uri, array_merge($options, $this->getRequestOptions()));
+		$response = $this->client->request('options', $uri, array_merge($this->getRequestOptions(), $options));
 		return new Response($response);
 	}
 }

tests/lib/Http/Client/ClientTest.php

@@ -27,6 +27,8 @@ class ClientTest extends \Test\TestCase {
 	private $client;
 	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
 	private $config;
+	/** @var array */
+	private $defaultRequestOptions;
 
 	public function setUp() {
 		parent::setUp();
@@ -85,42 +87,167 @@ class ClientTest extends \Test\TestCase {
 		$this->assertSame('username:password@foo', self::invokePrivate($this->client, 'getProxyUri'));
 	}
 
+	private function setUpDefaultRequestOptions() {
+		$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->certificateManager
+			->expects($this->once())
+			->method('getAbsoluteBundlePath')
+			->with(null)
+			->willReturn('/my/path.crt');
+
+		$this->defaultRequestOptions = [
+			'verify' => '/my/path.crt',
+			'proxy' => 'foo'
+		];
+	}
+
 	public function testGet() {
+		$this->setUpDefaultRequestOptions();
+
 		$this->guzzleClient->method('request')
+			->with('get', 'http://localhost/', $this->defaultRequestOptions)
 			->willReturn(new Response(1337));
 		$this->assertEquals(1337, $this->client->get('http://localhost/', [])->getStatusCode());
 	}
 
+	public function testGetWithOptions() {
+		$this->setUpDefaultRequestOptions();
+
+		$options = [
+			'verify' => false,
+			'proxy' => 'bar'
+		];
+
+		$this->guzzleClient->method('request')
+			->with('get', 'http://localhost/', $options)
+			->willReturn(new Response(1337));
+		$this->assertEquals(1337, $this->client->get('http://localhost/', $options)->getStatusCode());
+	}
+
 	public function testPost() {
+		$this->setUpDefaultRequestOptions();
+
 		$this->guzzleClient->method('request')
+			->with('post', 'http://localhost/', $this->defaultRequestOptions)
 			->willReturn(new Response(1337));
 		$this->assertEquals(1337, $this->client->post('http://localhost/', [])->getStatusCode());
 	}
 
+	public function testPostWithOptions() {
+		$this->setUpDefaultRequestOptions();
+
+		$options = [
+			'verify' => false,
+			'proxy' => 'bar'
+		];
+
+		$this->guzzleClient->method('request')
+			->with('post', 'http://localhost/', $options)
+			->willReturn(new Response(1337));
+		$this->assertEquals(1337, $this->client->post('http://localhost/', $options)->getStatusCode());
+	}
+
 	public function testPut() {
+		$this->setUpDefaultRequestOptions();
+
 		$this->guzzleClient->method('request')
+			->with('put', 'http://localhost/', $this->defaultRequestOptions)
 			->willReturn(new Response(1337));
 		$this->assertEquals(1337, $this->client->put('http://localhost/', [])->getStatusCode());
 	}
 
+	public function testPutWithOptions() {
+		$this->setUpDefaultRequestOptions();
+
+		$options = [
+			'verify' => false,
+			'proxy' => 'bar'
+		];
+
+		$this->guzzleClient->method('request')
+			->with('put', 'http://localhost/', $options)
+			->willReturn(new Response(1337));
+		$this->assertEquals(1337, $this->client->put('http://localhost/', $options)->getStatusCode());
+	}
+
 	public function testDelete() {
+		$this->setUpDefaultRequestOptions();
+
 		$this->guzzleClient->method('request')
+			->with('delete', 'http://localhost/', $this->defaultRequestOptions)
 			->willReturn(new Response(1337));
 		$this->assertEquals(1337, $this->client->delete('http://localhost/', [])->getStatusCode());
 	}
 
+	public function testDeleteWithOptions() {
+		$this->setUpDefaultRequestOptions();
+
+		$options = [
+			'verify' => false,
+			'proxy' => 'bar'
+		];
+
+		$this->guzzleClient->method('request')
+			->with('delete', 'http://localhost/', $options)
+			->willReturn(new Response(1337));
+		$this->assertEquals(1337, $this->client->delete('http://localhost/', $options)->getStatusCode());
+	}
+
 	public function testOptions() {
+		$this->setUpDefaultRequestOptions();
+
 		$this->guzzleClient->method('request')
+			->with('options', 'http://localhost/', $this->defaultRequestOptions)
 			->willReturn(new Response(1337));
 		$this->assertEquals(1337, $this->client->options('http://localhost/', [])->getStatusCode());
 	}
 
+	public function testOptionsWithOptions() {
+		$this->setUpDefaultRequestOptions();
+
+		$options = [
+			'verify' => false,
+			'proxy' => 'bar'
+		];
+
+		$this->guzzleClient->method('request')
+			->with('options', 'http://localhost/', $options)
+			->willReturn(new Response(1337));
+		$this->assertEquals(1337, $this->client->options('http://localhost/', $options)->getStatusCode());
+	}
+
 	public function testHead() {
+		$this->setUpDefaultRequestOptions();
+
 		$this->guzzleClient->method('request')
+			->with('head', 'http://localhost/', $this->defaultRequestOptions)
 			->willReturn(new Response(1337));
 		$this->assertEquals(1337, $this->client->head('http://localhost/', [])->getStatusCode());
 	}
 
+	public function testHeadWithOptions() {
+		$this->setUpDefaultRequestOptions();
+
+		$options = [
+			'verify' => false,
+			'proxy' => 'bar'
+		];
+
+		$this->guzzleClient->method('request')
+			->with('head', 'http://localhost/', $options)
+			->willReturn(new Response(1337));
+		$this->assertEquals(1337, $this->client->head('http://localhost/', $options)->getStatusCode());
+	}
+
 	public function testSetDefaultOptionsWithNotInstalled() {
 		$this->config
 			->expects($this->at(0))