`http_api_curl` hook no longer available for adding custom cURL options

[ChaseWiseman]

Now that 4.6 has implemented the new Requests library, the http_api_curl hook is no longer used when building cURL requests, and I see that its removal was mentioned and expected in #33055. That said, as far as I can tell there is no longer an easy way to intercept before curl_exec is run. There are a lot of posts and projects that suggest this old hook for setting options like CURLOPT_SSLVERSION and the like, so I think it'd be beneficial to maintain this ability.

The only way around it I've found is to skip wp_remote_request() and roll your own Requests::request() call and pass it the $options['hooks'] after registering your own callback for the curl.before_send hook.

Would it make sense to add a filter to $options before calling Requests::request() so folks can register their own callbacks, similar to this https://core.trac.wordpress.org/browser/trunk/src/wp-includes/class-http.php#L311? Or perhaps there is another way of going about it that I'm missing, and I'm happy to work up a patch if someone more familiar with the library can suggest a good place for this.

Thanks!

[ptasker]

Voting for this one as well. Would be good to be able to filter the curl_setopt() flags again.

[JRGould]

The HTTP API currently feels pretty crippled without the ability to set curl options or utilize Requests' hooks in general. Adding a filter on $options with an additional $url arg just before calling Requests::request() would be incredibly helpful.

[rekmla]

We're also at a loss as to how we'll access curl_setopt without giving up on using wp_remote_request() in certain situations.

[smerriman]

I'm in the same boat here - our server is unable to correctly route IPv6 addresses. This happens regularly, for example, with Facebook graph requests.

We avoid this by using http_api_curl to set the CURL_IPRESOLVE option to CURL_IPRESOLVE_V4. WP4.6 has broken this.

[ocean90]

Another report: ​https://github.com/rmccue/Requests/issues/233.

#33055 had a hook adapter WP_Requests_Hooks but it was removed before the final commit:

rmccue:

[…] removes the hook adapter. Hooks in Requests are scoped to a single request/session, and the internal plugins (Basic auth, proxying, etc) rely on this fact. Attaching the hooks to WP actions/filters breaks this.

See also [37429].

---

37843.patch​ allows to define your own hooks for custom requests. But this still doesn't allow to customize all requests per default. A filter like @JRGould has proposed makes probably more sense here.

@dd32, @rmccue: What are your thoughts on this?

[dd32]

What are your thoughts on this?

It was intentional to drop support for that hook in my mind.
We can restore the action easily enough by passing a custom hook to Requests and then firing our action(s) for BC.

[jaredh123]

This is affecting us pretty badly too. We have a percentage of our users unable to get updates from our servers now that we cannot intercept the curl request and customize it.

I would love to see this make it into 4.6.x instead of 4.7, because this feels like a pretty major bug/regression, not a feature. We'll be hurting for a while if this isn't possible until 4.7.

[SteveNetRivet]

+1
Our users rely on auto-update of our theme software for bug fixes and feature releases.
Until this is resolved - hopefully in the near future, during 4.6 and not waiting for 4.7 - a large group of our users won't be able to receive our patches.

[lalunecreative]

+1 I'm also adding to this one, our site uses auto updates for bug fixes and features for our custom theme which is currently in a broken state until this gets resolved.

[rmccue]

Attach Requests hooks to native WP hooks

[rmccue]

Added a patch that adds a requests-{$hook} dynamic hook for WordPress usage of Requests internal hooks. Anything in Requests will be mapped to this automatically (e.g. curl.before_send will be available as requests-curl.before_send).

Additionally, it also maps the curl.before_send hook to the old http_api_curl hook.

Please note: if you're hooking into this, please consider filtering something else, as your code won't work if the user doesn't have cURL installed. The point of WP_Http is to abstract the need for cURL away, so using this hook is a bad idea.

[jaredh123]

@rmccue I tried to test your patch, but got errors. Looks like your WP_Requests_Hooks class is supposed to be constructed with some parameters, but your code doesn't pass any in.

[rmccue]

Updated instantiation with params

[rmccue]

@jaredh123 Whoops, forgot to include that change in my patch. Updated in 37843.2.diff​.

[jaredh123]

Replying to rmccue:

@jaredh123 Whoops, forgot to include that change in my patch. Updated in 37843.2.diff​.

@rmccue tested again with updated diff on one of our users sites experiencing problems, and it worked well. Thanks for your time and attention to this!

[ocean90]

#38205 was marked as a duplicate.

[reidbusi]

Wouldn't this be a lot simpler?:

WP 4.6.1 wp-includes\class-http.php lines 299-314:

                // Setup arguments
                $headers = $r['headers'];
                $data = $r['body'];
                $type = $r['method'];
                $options = array(
                        'timeout' => $r['timeout'],
                        'useragent' => $r['user-agent'],
                        'blocking' => $r['blocking'],
                        'hooks' => new Requests_Hooks(),
                );

                // Ensure redirects follow browser behaviour.
                $options['hooks']->register( 'requests.before_redirect', array( get_class(), 'browser_redirect_compatibility' ) );
                
                // Allow registration of additional Requests hooks
                apply_filters( 'http_api_requests_lib_hooks', $options['hooks'] );

Simply added one comment and a one line apply_filters call. I just tested this approach and it works fine for me. I don't really see the need for anything more complicated unless I am missing something?

Thus in my plugin I can just do this:

        if ( version_compare( $wp_version, "4.6", ">=" ) ) {
                if ( class_exists( 'Requests' ) ) {
                        add_filter( 'http_api_requests_lib_hooks', 'rpco_http_api_requests_lib_hooks', PHP_INT_MAX, 1  );
                }
        } else {
                add_action( 'http_api_curl', 'rpco_http_api_curl', PHP_INT_MAX, 1 );
        }

// ...

function rpco_http_api_requests_lib_hooks( $hooks ) {
        $hooks->register( 'curl.before_send', 'rpco_http_api_curl', PHP_INT_MAX );
}

Also...:

Please note: if you're hooking into this, please consider filtering something else, as your code won't work if the user doesn't have cURL installed. The point of WP_Http is to abstract the need for cURL away, so using this hook is a bad idea.

Thanks for the advice, but I'd prefer if you leave that to us:

​https://wordpress.org/plugins/reid-plugins-curl-options/ (the whole point of the plugin is to be able to control PHP cURL)

e.g.:

        if ( function_exists( 'curl_version' ) ) {
                // do cURLy stuff
        } else {
                echo '<p>PHP cURL is not installed or enabled on your server, this plugin will not do anything.</p>';
        }

I suppose I can test this patch, but I'm not convinced so complicated/convoluted a solution is required.

Also, like others, I would like to see this fixed before WP 4.7. In my opinion, once the best solution is settled, it is worth an immediate minor version release.

[reidbusi]

It occurs to me that my one-liner filter suggested above only addresses the http(s) protocol case. I will proceed to test rmmcue's patch, which in theory should handle other protocols as well. However, I am concerned by the special case applied to curl.before_send, the most important one (especially to me and my clients).

[rmccue]

Replying to reidbusi:

Wouldn't this be a lot simpler?

This enshrines the hooks object into the WordPress API, and would make it harder if we wanted to migrate away from it later (or if Requests changes the format).

Thanks for the advice, but I'd prefer if you leave that to us:

That's why the action still exists. In 99% of cases, you're best off using the higher level abstractions for compatibility.

[reidbusi]

Thanks for the explanation! Makes sense. So now I can just add a regular WP action inside a WP version conditional like so:

        if ( version_compare( $wp_version, "4.6", ">=" ) ) {
                if ( class_exists( 'Requests' ) ) {
                        add_action( 'requests-curl.before_send', 'rpco_http_api_curl', PHP_INT_MAX, 1 );
                }
        } else {
                add_action( 'http_api_curl', 'rpco_http_api_curl', PHP_INT_MAX, 1 );
        }

That makes it relatively simple. I have tested your posted patch (on CentOS 6.8) and it is behaving as expected and desired.

Current PHP version: 		7.0.9
cURL version:			7.19.7
Host:				x86_64-redhat-linux-gnu
IPv6 support:			yes
Kerberos V4 support:		no
SSL support:			yes
SSL version:			NSS/3.21 Basic ECC
libz HTTP deflate support:	yes
libz version:			1.2.3
Protocols:			tftp, ftp, telnet, dict, ldap, ldaps, http, file, https, ftps, scp, sftp

That's why the action still exists. In 99% of cases, you're best off using the higher level abstractions for compatibility.

Thanks for keeping it, because CentOS is pretty popular with hosts and as of CentOS 7, cURL still uses NSS instead of OpenSSL, and depending on the build environment cURL may or may not be able to auto-negotiate SSL versions and cipher suites, which are pretty critical to eCommerce websites.

Edit: so now with a little more testing, we just need to get this pushed out as soon as possible, like others above, I feel that waiting for WP v4.7 might be too long to let it go like it is.

[reidbusi]

I would like to point out, I believe that some developers have already abandoned the WP HTTP API because of this issue.

2016.08.30 - version 1.0.7
Tweak - Replace all wp_remote_requests with raw cURL.

​https://woocommerce.com/products/square/

Note the date. Business must go on, regardless of what happens to the platform it is running on.

I admit, this is speculation on my part since I do not have access to the development repository, but the timing of such a change sure looks like this is the reason.

Actually, looking back through the versions I have saved, I see a comment from 1.0.6:

                        // if posting image we need to use regular curl because
                        // for some unknown reason wp_remote_request does not work.
                        // @TODO: Need to look deeper to see why wp_remote_request cannot do this.

So I would suspect that given this issue, and the change to the WP core HTTP API, they just went with straight PHP cURL for greater control and reliability regardless of what happens to the WP core functionality.

Note the author of WooCommerce Square:

Author: Automattic

I don't want to seem ungrateful for anyone's contributions to WP, they are all appreciated, but I feel it is important to point these kinds of things out. Things need to be more carefully thought through and more thoroughly tested in my opinion.

[dd32]

@rmccue Patch approach seems good to me, is there anything else you'd like to change here?

For the 4.6 backport (which is needed, at least for the curl.before_send => http_api_curl handling) the class should be suffixed to an existing file to abide by our no-new-files-in-minor-releases policy for autoupdates.

[dd32]

pushing back into the 4.7 milestone as at this point it doesn't look like a 4.6.2 will be released before 4.7, and we can review at the point of commit.

@rmccue I'll ping you directly to chat about this and work out what we're doing.

[dd32]

In 39212:

WP_HTTP: Map internal Requests hooks to WordPress actions.
This change introduces a requests-{$hook} action which can be used to hook into Requests hooks, and restores the http_api_curl action.

Props rmccue.
Fixes #37843.

[lkraav]

r39212 was never backported to 4.6? Why not, because the change looks miniscule enough? Just had a nasty surprise for a client where core version upgrade strategy is a bit on the slow side.

EDIT I manually backported this on 4.6.10 right now. Everything works and I'm not seeing any bad side effects.