From a513ee70c8335c2da07225e4af3b42ecf6e0873d Mon Sep 17 00:00:00 2001 From: HazA Date: Fri, 22 Feb 2019 13:43:42 +0100 Subject: [PATCH 01/12] feat: Add http proxy option for curl client --- UPGRADE-2.0.md | 2 -- src/ClientBuilder.php | 14 ++++++++++++++ src/Options.php | 24 ++++++++++++++++++++++++ tests/OptionsTest.php | 1 + 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index dc0a663eb..bc7d39319 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -2,8 +2,6 @@ ### Client options -- The `http_proxy` option has been removed. - - The `exclude` option has been removed. - The `excluded_app_path` option has been renamed to `in_app_exclude` diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php index 28a61293c..f019665af 100644 --- a/src/ClientBuilder.php +++ b/src/ClientBuilder.php @@ -314,6 +314,20 @@ private function createTransportInstance(): TransportInterface $this->messageFactory = $this->messageFactory ?? MessageFactoryDiscovery::find(); $this->uriFactory = $this->uriFactory ?? UriFactoryDiscovery::find(); + + if ($this->options->getHttpProxy()) { + try { + $curlClientClass = 'Http\Client\Curl\Client'; + if (class_exists($curlClientClass)) { + $this->httpClient = new $curlClientClass(null, null, [ + CURLOPT_PROXY => $this->options->getHttpProxy(), + ]); + } + } catch (\Exception $e) { + throw new \RuntimeException('The http_proxy option requires curl client to be installed.'); + } + } + $this->httpClient = $this->httpClient ?? HttpAsyncClientDiscovery::find(); if (null === $this->messageFactory) { diff --git a/src/Options.php b/src/Options.php index 720685566..793f6f100 100644 --- a/src/Options.php +++ b/src/Options.php @@ -619,6 +619,28 @@ public function setMaxValueLength(int $maxValueLength): void $this->options = $this->resolver->resolve($options); } + /** + * Gets the http proxy setting. + * + * @return string + */ + public function getHttpProxy(): ?string + { + return $this->options['http_proxy']; + } + + /** + * Sets the http proxy. Be aware this option only works when curl client is used. + * + * @param string $httpProxy The http proxy + */ + public function setHttpProxy(?string $httpProxy): void + { + $options = array_merge($this->options, ['http_proxy' => $httpProxy]); + + $this->options = $this->resolver->resolve($options); + } + /** * Configures the options of the client. * @@ -657,6 +679,7 @@ private function configureOptions(OptionsResolver $resolver): void 'in_app_exclude' => [], 'send_default_pii' => false, 'max_value_length' => 1024, + 'http_proxy' => null, ]); $resolver->setAllowedTypes('send_attempts', 'int'); @@ -682,6 +705,7 @@ private function configureOptions(OptionsResolver $resolver): void $resolver->setAllowedTypes('send_default_pii', 'bool'); $resolver->setAllowedTypes('default_integrations', 'bool'); $resolver->setAllowedTypes('max_value_length', 'int'); + $resolver->setAllowedTypes('http_proxy', ['null', 'string']); $resolver->setAllowedValues('dsn', \Closure::fromCallable([$this, 'validateDsnOption'])); $resolver->setAllowedValues('integrations', \Closure::fromCallable([$this, 'validateIntegrationsOption'])); diff --git a/tests/OptionsTest.php b/tests/OptionsTest.php index 2832584d9..688ff969c 100644 --- a/tests/OptionsTest.php +++ b/tests/OptionsTest.php @@ -58,6 +58,7 @@ public function optionsDataProvider(): array ['send_default_pii', true, 'shouldSendDefaultPii', 'setSendDefaultPii'], ['default_integrations', false, 'hasDefaultIntegrations', 'setDefaultIntegrations'], ['max_value_length', 50, 'getMaxValueLength', 'setMaxValueLength'], + ['http_proxy', '127.0.0.1', 'getHttpProxy', 'setHttpProxy'], ]; } From d850cb9c147ae3aaf98889893d4dfdd645bd937e Mon Sep 17 00:00:00 2001 From: HazA Date: Fri, 22 Feb 2019 14:09:27 +0100 Subject: [PATCH 02/12] fix: Add check for existing httpClient --- src/ClientBuilder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php index f019665af..c08474124 100644 --- a/src/ClientBuilder.php +++ b/src/ClientBuilder.php @@ -315,7 +315,7 @@ private function createTransportInstance(): TransportInterface $this->messageFactory = $this->messageFactory ?? MessageFactoryDiscovery::find(); $this->uriFactory = $this->uriFactory ?? UriFactoryDiscovery::find(); - if ($this->options->getHttpProxy()) { + if (null === $this->httpClient && null !== $this->options->getHttpProxy()) { try { $curlClientClass = 'Http\Client\Curl\Client'; if (class_exists($curlClientClass)) { From 9ad0b26a3fb71bd4f0dcd84c42ee2a363d5820ee Mon Sep 17 00:00:00 2001 From: HazA Date: Fri, 22 Feb 2019 14:15:29 +0100 Subject: [PATCH 03/12] fix: Class discovery --- src/ClientBuilder.php | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php index c08474124..c498a7338 100644 --- a/src/ClientBuilder.php +++ b/src/ClientBuilder.php @@ -13,6 +13,7 @@ use Http\Client\Common\Plugin\RetryPlugin; use Http\Client\Common\PluginClient; use Http\Client\HttpAsyncClient; +use Http\Discovery\ClassDiscovery; use Http\Discovery\HttpAsyncClientDiscovery; use Http\Discovery\MessageFactoryDiscovery; use Http\Discovery\UriFactoryDiscovery; @@ -316,15 +317,13 @@ private function createTransportInstance(): TransportInterface $this->uriFactory = $this->uriFactory ?? UriFactoryDiscovery::find(); if (null === $this->httpClient && null !== $this->options->getHttpProxy()) { - try { - $curlClientClass = 'Http\Client\Curl\Client'; - if (class_exists($curlClientClass)) { - $this->httpClient = new $curlClientClass(null, null, [ - CURLOPT_PROXY => $this->options->getHttpProxy(), - ]); - } - } catch (\Exception $e) { - throw new \RuntimeException('The http_proxy option requires curl client to be installed.'); + $curlClientClass = 'Http\Client\Curl\Client'; + if (ClassDiscovery::safeClassExists($curlClientClass)) { + $this->httpClient = new $curlClientClass(null, null, [ + CURLOPT_PROXY => $this->options->getHttpProxy(), + ]); + } else { + throw new \RuntimeException('The `http_proxy` option requires the `php-http/curl-client` package to be installed.'); } } From d19b2ba63479638053017ad9ba1da3cf5f692ef9 Mon Sep 17 00:00:00 2001 From: HazA Date: Fri, 22 Feb 2019 14:18:14 +0100 Subject: [PATCH 04/12] ref: CodeReview --- src/ClientBuilder.php | 1 + src/Options.php | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php index c498a7338..7bcde0624 100644 --- a/src/ClientBuilder.php +++ b/src/ClientBuilder.php @@ -318,6 +318,7 @@ private function createTransportInstance(): TransportInterface if (null === $this->httpClient && null !== $this->options->getHttpProxy()) { $curlClientClass = 'Http\Client\Curl\Client'; + if (ClassDiscovery::safeClassExists($curlClientClass)) { $this->httpClient = new $curlClientClass(null, null, [ CURLOPT_PROXY => $this->options->getHttpProxy(), diff --git a/src/Options.php b/src/Options.php index 793f6f100..4db10a91c 100644 --- a/src/Options.php +++ b/src/Options.php @@ -622,7 +622,7 @@ public function setMaxValueLength(int $maxValueLength): void /** * Gets the http proxy setting. * - * @return string + * @return string|null */ public function getHttpProxy(): ?string { @@ -632,7 +632,7 @@ public function getHttpProxy(): ?string /** * Sets the http proxy. Be aware this option only works when curl client is used. * - * @param string $httpProxy The http proxy + * @param string|null $httpProxy The http proxy */ public function setHttpProxy(?string $httpProxy): void { From 9430e4a39464a8e7242013130adcf3e07de535e3 Mon Sep 17 00:00:00 2001 From: HazA Date: Fri, 22 Feb 2019 14:23:54 +0100 Subject: [PATCH 05/12] ref: Use use instead of class name --- src/ClientBuilder.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php index 7bcde0624..30a040b55 100644 --- a/src/ClientBuilder.php +++ b/src/ClientBuilder.php @@ -13,6 +13,7 @@ use Http\Client\Common\Plugin\RetryPlugin; use Http\Client\Common\PluginClient; use Http\Client\HttpAsyncClient; +use Http\Client\Curl\Client as CurlClient; use Http\Discovery\ClassDiscovery; use Http\Discovery\HttpAsyncClientDiscovery; use Http\Discovery\MessageFactoryDiscovery; @@ -317,10 +318,8 @@ private function createTransportInstance(): TransportInterface $this->uriFactory = $this->uriFactory ?? UriFactoryDiscovery::find(); if (null === $this->httpClient && null !== $this->options->getHttpProxy()) { - $curlClientClass = 'Http\Client\Curl\Client'; - - if (ClassDiscovery::safeClassExists($curlClientClass)) { - $this->httpClient = new $curlClientClass(null, null, [ + if (ClassDiscovery::safeClassExists(CurlClient::class)) { + $this->httpClient = new CurlClient(null, null, [ CURLOPT_PROXY => $this->options->getHttpProxy(), ]); } else { From 35e86a6c113124f768cfa4a4c6ffa0e6e2b94916 Mon Sep 17 00:00:00 2001 From: HazA Date: Fri, 22 Feb 2019 14:25:18 +0100 Subject: [PATCH 06/12] ref: Rename CurlClient to HttpCurlClient --- src/ClientBuilder.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php index 30a040b55..5d719e899 100644 --- a/src/ClientBuilder.php +++ b/src/ClientBuilder.php @@ -13,7 +13,7 @@ use Http\Client\Common\Plugin\RetryPlugin; use Http\Client\Common\PluginClient; use Http\Client\HttpAsyncClient; -use Http\Client\Curl\Client as CurlClient; +use Http\Client\Curl\Client as HttpCurlClient; use Http\Discovery\ClassDiscovery; use Http\Discovery\HttpAsyncClientDiscovery; use Http\Discovery\MessageFactoryDiscovery; @@ -318,8 +318,8 @@ private function createTransportInstance(): TransportInterface $this->uriFactory = $this->uriFactory ?? UriFactoryDiscovery::find(); if (null === $this->httpClient && null !== $this->options->getHttpProxy()) { - if (ClassDiscovery::safeClassExists(CurlClient::class)) { - $this->httpClient = new CurlClient(null, null, [ + if (ClassDiscovery::safeClassExists(HttpCurlClient::class)) { + $this->httpClient = new HttpCurlClient(null, null, [ CURLOPT_PROXY => $this->options->getHttpProxy(), ]); } else { From 8fe4a4352955e8c21189c66dce26e33c6ad21289 Mon Sep 17 00:00:00 2001 From: HazA Date: Fri, 22 Feb 2019 14:26:14 +0100 Subject: [PATCH 07/12] ref: Remove unused class --- src/ClientBuilder.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php index 5d719e899..ade210d0c 100644 --- a/src/ClientBuilder.php +++ b/src/ClientBuilder.php @@ -14,7 +14,6 @@ use Http\Client\Common\PluginClient; use Http\Client\HttpAsyncClient; use Http\Client\Curl\Client as HttpCurlClient; -use Http\Discovery\ClassDiscovery; use Http\Discovery\HttpAsyncClientDiscovery; use Http\Discovery\MessageFactoryDiscovery; use Http\Discovery\UriFactoryDiscovery; @@ -318,7 +317,7 @@ private function createTransportInstance(): TransportInterface $this->uriFactory = $this->uriFactory ?? UriFactoryDiscovery::find(); if (null === $this->httpClient && null !== $this->options->getHttpProxy()) { - if (ClassDiscovery::safeClassExists(HttpCurlClient::class)) { + if (HttpAsyncClientDiscovery::safeClassExists(HttpCurlClient::class)) { $this->httpClient = new HttpCurlClient(null, null, [ CURLOPT_PROXY => $this->options->getHttpProxy(), ]); From 66301c8a5cbf02504173b9e453dc337d504191f0 Mon Sep 17 00:00:00 2001 From: HazA Date: Fri, 22 Feb 2019 19:43:26 +0100 Subject: [PATCH 08/12] ref: Fail if custom client is configured with the http_proxy option --- src/ClientBuilder.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php index ade210d0c..9541a38ac 100644 --- a/src/ClientBuilder.php +++ b/src/ClientBuilder.php @@ -316,7 +316,11 @@ private function createTransportInstance(): TransportInterface $this->messageFactory = $this->messageFactory ?? MessageFactoryDiscovery::find(); $this->uriFactory = $this->uriFactory ?? UriFactoryDiscovery::find(); - if (null === $this->httpClient && null !== $this->options->getHttpProxy()) { + if (null !== $this->options->getHttpProxy()) { + if (null !== $this->httpClient) { + throw new \RuntimeException('The `http_proxy` option does not work together with a custom client.'); + } + if (HttpAsyncClientDiscovery::safeClassExists(HttpCurlClient::class)) { $this->httpClient = new HttpCurlClient(null, null, [ CURLOPT_PROXY => $this->options->getHttpProxy(), From 129c3e16000b2e1199134125aa7f06eaa9713c5a Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 22 Feb 2019 22:11:01 +0100 Subject: [PATCH 09/12] Fix CS --- src/ClientBuilder.php | 2 +- src/Context/Context.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php index 9541a38ac..33ab7c922 100644 --- a/src/ClientBuilder.php +++ b/src/ClientBuilder.php @@ -12,8 +12,8 @@ use Http\Client\Common\Plugin\HeaderSetPlugin; use Http\Client\Common\Plugin\RetryPlugin; use Http\Client\Common\PluginClient; -use Http\Client\HttpAsyncClient; use Http\Client\Curl\Client as HttpCurlClient; +use Http\Client\HttpAsyncClient; use Http\Discovery\HttpAsyncClientDiscovery; use Http\Discovery\MessageFactoryDiscovery; use Http\Discovery\UriFactoryDiscovery; diff --git a/src/Context/Context.php b/src/Context/Context.php index 1a784b017..b395fc0d2 100644 --- a/src/Context/Context.php +++ b/src/Context/Context.php @@ -119,7 +119,7 @@ public function toArray(): array */ public function offsetExists($offset): bool { - return isset($this->data[$offset]) || \array_key_exists($offset, $this->data); + return isset($this->data[$offset]) || array_key_exists($offset, $this->data); } /** From 864e28615c1eb0e442fec9e57a8c8279a3346a0b Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 22 Feb 2019 22:51:35 +0100 Subject: [PATCH 10/12] Add test --- src/Context/Context.php | 2 +- tests/ClientBuilderTest.php | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/Context/Context.php b/src/Context/Context.php index b395fc0d2..1a784b017 100644 --- a/src/Context/Context.php +++ b/src/Context/Context.php @@ -119,7 +119,7 @@ public function toArray(): array */ public function offsetExists($offset): bool { - return isset($this->data[$offset]) || array_key_exists($offset, $this->data); + return isset($this->data[$offset]) || \array_key_exists($offset, $this->data); } /** diff --git a/tests/ClientBuilderTest.php b/tests/ClientBuilderTest.php index b9e237d2d..3a45e1523 100644 --- a/tests/ClientBuilderTest.php +++ b/tests/ClientBuilderTest.php @@ -290,6 +290,22 @@ public function testCreateWithNoOptionsIsTheSameAsDefaultOptions(): void ClientBuilder::create([]) ); } + + public function testCreateWithHttpProxyAndCustomTransportThrowsException(): void + { + $options = new Options([ + 'dsn' => 'http://public:secret@example.com/sentry/1', + 'http_proxy' => 'some-proxy', + ]); + + $clientBuilder = new ClientBuilder($options); + $clientBuilder->setHttpClient($this->createMock(HttpAsyncClient::class)); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('http_proxy'); + + $clientBuilder->getClient(); + } } final class StubIntegration implements IntegrationInterface From 1c3d61c06ebafe200330e1eb7d3c1dfe435a7652 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Sat, 23 Feb 2019 00:44:42 +0100 Subject: [PATCH 11/12] Address review --- tests/ClientBuilderTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ClientBuilderTest.php b/tests/ClientBuilderTest.php index 3a45e1523..9dad066f5 100644 --- a/tests/ClientBuilderTest.php +++ b/tests/ClientBuilderTest.php @@ -302,7 +302,7 @@ public function testCreateWithHttpProxyAndCustomTransportThrowsException(): void $clientBuilder->setHttpClient($this->createMock(HttpAsyncClient::class)); $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('http_proxy'); + $this->expectExceptionMessage('The `http_proxy` option does not work together with a custom client.'); $clientBuilder->getClient(); } From b79898c1038229b04a0af04fc4009c223b48f02d Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Sat, 23 Feb 2019 00:54:44 +0100 Subject: [PATCH 12/12] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d43bb46e1..4aca7d071 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -- ... +- Reintroduce `http_proxy` option (#775) ## 2.0.0-beta2 (2019-02-11) - Rename `SentryAuth` class to `SentryAuthentication` (#742)