google-ads-php icon indicating copy to clipboard operation
google-ads-php copied to clipboard

Proxy settings overrides curl option globally with http transport

Open mrpavlikov opened this issue 4 years ago • 8 comments

Your client library and Google Ads API versions:

  • Client library version: v10.1.0
  • Google Ads API version: V8

Your environment:

  • PHP Version => 7.3.29-1~deb10u1
  • System => Linux 18c018ba8d92 5.11.0-37-generic #41-Ubuntu SMP Mon Sep 20 16:39:20 UTC 2021 x86_64
  • The PHP Extension grpc is not installed
  • The PHP Extension protobuf is not installed
  • REST transport

Description of the bug:

Using proxy settings in .ini file or having it set manually while building client with (new GoogleAdsClientBuilder())->withProxy('protocol://user:pass@host:port') sets environment variable http_proxy and after that everything that uses curl automatically starts using this proxy too, which is not desired behavior and kinda unexpected side effect.

Steps to reproduce:

$ini = 'path/to/file.ini';
$oAuth2Credential = (new OAuth2TokenBuilder())->fromFile($ini)->build();
$googleAdsClient = (new GoogleAdsClientBuilder())
    ->fromFile($ini)
    ->withOAuth2Credential($oAuth2Credential)
    ->withProxy('protocol://user:pass@host:port')
    ->build();
$client = $googleAdsClient->getGoogleAdsServiceClient();

// now this will use proxy too
$ch = curl_init();
curl_setopt($ch, CURLOPT_URL, "google.com");
curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
$output = curl_exec($ch);
curl_close($ch);

Expected behavior:

Curl calls must stay unaffected.

mrpavlikov avatar Oct 18 '21 15:10 mrpavlikov

Hi @mrpavlikov - Thank you for reporting.

Workaround: You should be able to override existing environment variables that set the proxy to use by setting the curl option proxy to an empty string. See the curl man page and curl_setopt documentation.

Potential fix: The library could set the proxy from the gRPC channel options instead of setting environment variables. See grpc.http_proxy and Python's implementation.

PierrickVoulet avatar Oct 22 '21 13:10 PierrickVoulet

So far I'm doing putenv('http_proxy'); after library call to erase global proxy setting, but thanks!

mrpavlikov avatar Oct 22 '21 14:10 mrpavlikov

Is it "won't fix" or something? Like affecting global curl setting is fine?

mrpavlikov avatar Nov 09 '21 08:11 mrpavlikov

Sorry, it was a mistake. Let's keep this open for now.

fiboknacky avatar Nov 09 '21 13:11 fiboknacky

This problem still exists. The current method of setting environment variables in the source code has no effect, and the value of the proxy parameter is not transparently transmitted to the underlying guzzle and curl. Client library version: v12 Composer version:v17.1

bobollm avatar Feb 06 '23 01:02 bobollm

For grpc transport it's need to use grpc.http_proxy='proxy_address'

$options['transportConfig']['grpc']['stubOpts']['grpc.http_proxy'] = $this->getProxy();

But since the GoogleAdsClient class is final, it is not possible to override the getGoogleAdsClientOptions method in the proper way

And withProxy doesn't work for rest transport at all

ictdevel avatar Apr 26 '23 13:04 ictdevel

For grpc transport it's need to use grpc.http_proxy='proxy_address'

$options['transportConfig']['grpc']['stubOpts']['grpc.http_proxy'] = $this->getProxy();

But since the GoogleAdsClient class is final, it is not possible to override the getGoogleAdsClientOptions method in the proper way

And withProxy doesn't work for rest transport at all

yes

bobollm avatar May 29 '23 01:05 bobollm

Since version 14, GoogleAdsClient is no longer a final class and therefore it is easy to extend from it and override the method

    public function getGoogleAdsClientOptions(): array
    {
        $options = parent::getGoogleAdsClientOptions();
        $options['transportConfig']['grpc']['stubOpts']['grpc.http_proxy'] = $this->getProxy();

        return $options;
    }

ictdevel avatar Dec 15 '23 10:12 ictdevel