search

CiviMRF / CMRF_Abstract_Core

Abstract implementation for the the CMRF Core. Should be included into the concrete module implementation
GNU Affero General Public License v3.0
1 stars 4 forks source link

Cannot set HTTP proxy through CURLOPT_PROXY and CURLOPT_PROXYPORT #12

Open grischard opened 4 years ago

grischard commented 4 years ago

Curl options are hard-coded in CMRF/Connection/Curl.php, making it impossible to use an http proxy, which are required in some environments.

https://github.com/CiviMRF/CMRF_Abstract_Core/blob/a89b4dbc893822dc1137888e05aa2cdca1f356ee/CMRF/Connection/Curl.php#L44-L51

Ideally, it should be possible to set or override all of these curl options in a config file - I can for example imagine wanting to override CURLOPT_SSL_VERIFYPEER, CURLOPT_SSL_VERIFYHOST or CURLOPT_SSLVERSION in this context.

As a workaround, it is possible to edit CMRF/Connection/Curl.php locally, but that's obviously a hack.

kwisatz commented 4 years ago

Well, ideally, the module wouldn't be using curl here at all, but use the HTTP API made available by drupal, which takes care of setting proxy settings globally if required.

jensschuppe commented 4 years ago

The CMRF_Abstract_Core framework is supposed to be used without Drupal as well. If at all, this would be an option for the cmrf_core Drupal module.

kwisatz commented 4 years ago

The CMRF_Abstract_Core framework is supposed to be used without Drupal as well. If at all, this would be an option for the cmrf_core Drupal module.

OK, I see. Yeah, maybe it can swap out the HTTP Api/Implementation based on where it's being used. In any case, to add to @grischard 's example above, we found that proxies also don't like requests that don't present a user agent, so CURLOPT_USERAGENT should also be set to some string identified the CMRF Core.

jaapjansma commented 4 years ago

It uses curl and it make sense to add configuration for proxy settings and other parameters (in the case reproted by @grischard we also had to set the User Agent header)

jensschuppe commented 4 years ago

I'd vote for passing configuration through code instead of using a config file, since the latter wouldn't allow for an easy configuration UI e.g. in Drupal. An implementation of hook_cmrf_core_connectors_alter() could then be used to add the config params to override defaults.

jaapjansma commented 4 years ago

Agree and we can easily add our own connection classes in custom modules.