From 822dd2d47f279eb85f9139dcefc76dac35b4261f Mon Sep 17 00:00:00 2001 From: Vladimir Pavlikov Date: Thu, 6 Feb 2020 15:33:05 +0300 Subject: [PATCH] #640 handlers stack of provided Guzzle HTTP client should not be changed by library --- .../Common/AdsGuzzleHttpClientFactory.php | 8 +++---- .../Common/AdsGuzzleHttpClientFactoryTest.php | 22 ++++++++++++++----- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/Google/AdsApi/Common/AdsGuzzleHttpClientFactory.php b/src/Google/AdsApi/Common/AdsGuzzleHttpClientFactory.php index f5cdfc9f4..db6e777c6 100755 --- a/src/Google/AdsApi/Common/AdsGuzzleHttpClientFactory.php +++ b/src/Google/AdsApi/Common/AdsGuzzleHttpClientFactory.php @@ -64,10 +64,10 @@ public function __construct( public function generateHttpClient() { $config = $this->config; - if (!array_key_exists('handler', $config) - || $config['handler'] === null) { - $config['handler'] = HandlerStack::create(); - } + + $config['handler'] = isset($config['handler']) + ? clone $config['handler'] + : HandlerStack::create(); // Add a logging middleware required by this library. $config['handler']->before( diff --git a/tests/Google/AdsApi/Common/AdsGuzzleHttpClientFactoryTest.php b/tests/Google/AdsApi/Common/AdsGuzzleHttpClientFactoryTest.php index eb7f2c05b..199b65f76 100755 --- a/tests/Google/AdsApi/Common/AdsGuzzleHttpClientFactoryTest.php +++ b/tests/Google/AdsApi/Common/AdsGuzzleHttpClientFactoryTest.php @@ -70,6 +70,7 @@ public function testGenerateHttpClient_userProvidedStack() ); $stack->before('http_errors', Middleware::tap()); $client = new Client(['handler' => $stack]); + $originalStack = clone $stack; $httpClientFactory = new AdsGuzzleHttpClientFactory( $logger, @@ -80,7 +81,12 @@ public function testGenerateHttpClient_userProvidedStack() $this->assertNotNull($httpClient); $this->assertInstanceOf(Client::class, $httpClient); - $this->assertEquals($stack, $httpClient->getConfig()['handler']); + $this->assertEquals($originalStack, $stack, 'Stack of original HTTP client should stay unchanged'); + $this->assertNotEquals( + $stack, + $httpClient->getConfig()['handler'], + 'Stack of factory created HTTP client should have logging middleware, thus differ from original' + ); } /** @@ -97,10 +103,11 @@ public function testGenerateHttpClient_userProvidedConfigs() GuzzleLogMessageHandler::log($logger, $guzzleLogMessageFormatter) ); $client = new Client([ - 'handler' => $stack, - 'verify' => true, - 'cookies' => false + 'handler' => $stack, + 'verify' => true, + 'cookies' => false ]); + $originalStack = clone $stack; $httpClientFactory = new AdsGuzzleHttpClientFactory( $logger, @@ -111,7 +118,12 @@ public function testGenerateHttpClient_userProvidedConfigs() $this->assertNotNull($httpClient); $this->assertInstanceOf(Client::class, $httpClient); - $this->assertEquals($stack, $httpClient->getConfig()['handler']); + $this->assertEquals($originalStack, $stack, 'Stack of original HTTP client should stay unchanged'); + $this->assertNotEquals( + $stack, + $httpClient->getConfig()['handler'], + 'Stack of factory created HTTP Client should have logging middleware, thus differ from original' + ); $this->assertTrue($httpClient->getConfig()['verify']); $this->assertFalse($httpClient->getConfig()['cookies']); }