Timeout in WP_Http_Streams::request causes Fatal error abort

[wet]

WP_Http_Streams::request() tries to ​open a remote stream with fopen() using the default timeout. The user-supplied value per $r is not applied.

If this connection times out, the resulting $handle ist invalid. Nevertheless $handle is subsequently used as an input to ​stream_set_timeout().

Attached patch applies a timeout to the fopen() context and catches an invalid $handle before use.

[jacobsantos]

Looks good. Need to probably duplicate the error processing to the other transport.

[wet]

You might also cast an eye on way these kind of errors are handled upstream. In my informal test case, it throws a bunch of notices on both the frontend and the backend stemming from wp-cron.php et al. and thus forcefully hinders the user to enter the backend.

[ryan]

[8528]

[wet]

NB: Timeouts in fopen() now decorate the public facing website with a notice:

Warning: fopen(http://example.com/wp-cron.php?check=deadbeef) [function.fopen]: failed to open stream

Additionally, it effectively establishes a denial-of-service barrier to the backend as the PHP notices are the only output from a visit to ​http://example.com/wp-admin/ . The second notice is:

Warning: Cannot modify header information - headers already sent by (output started at /www/wp-includes/http.php:659) in /www/wp-includes/pluggable.php on line 770

Short-circuiting WP_Http_Streams::request() was the only way to regain admin access, see attached diff.

[jacobsantos]

What revision number are you at? I will advise updating to latest trunk r8539 and try again.

You can also write a plugin, which short-circuits the transport.

[wet]

I'm at r8544, which is latest AFAIK.

I don't think that writing a plugin fixes a symptom which disables backend access, which in turn would be required to activate said plugin. Anything I'm missing?

[jacobsantos]

Yeah, you are right, the fix then is to suppress errors from fopen.

[jacobsantos]

Suppresses errors when WP_DEBUG is not enabled and returns WP_Error when malformed url. Actually, parse_url() shouldn't be used to validate urls, but oh well.

[jacobsantos]

Can you try the current patch and see if it works for you? I'm guessing that you don't have HTTP or cURL extensions, since those would be used first and you wouldn't be having problems with CRON. However, hopefully, need to write a plugin, which checks the last time the cron was run or something to test the cron is running correctly.

[ryan]

(In [8545]) Suppress fopen errors if WP_DEBUG is not enabled. Props jacobsantos. fixes #7456 see #4779

[wet]

Doesn't work for me.

Observed behaviour:

# Takes approx. 60 seconds from browsing to ​http://example.com/wp-admin/ to returning a blank screen.

# Adding define('WP_DEBUG', true), I see

Warning: fopen(http://example.com/wp-cron.php?check=deadbeef) [function.fopen]: failed to open stream: [...] timeout [...]. in /www/wp-includes/http.php on line 672

Fatal error: Maximum execution time of 60 seconds exceeded in /www/wp-includes/http.php on line 672

at ​http://example.com/wp-admin/. This is an expected (but imho undesired) behaviour, as the recent patch won't use $context as an input to fopen() and thus ​the previously established timeout from $r['timeout] wouldn't come into effect.

As you suspected, my Apache setup didn't include cURL. I subsequently loaded the cURL extension which remedied the deficiencies. So from my POV, WP_Http_Streams currently exhibits insufficient error handling for setups where a fallback from the higher preferred transports is required.

[jacobsantos]

Indeed. This must be fixed or it is going to trip up the majority of users.

[jacobsantos]

Note to self: Don't create patches when dead tired.

[jacobsantos]

Okay, user error mistake in last patch.

[jacobsantos]

Replying to jacobsantos:

Okay, user error mistake in last patch.

7456.r8546.diff should fix that.

[wet]

Replying to jacobsantos:

7456.r8546.diff should fix that.

This patch works for me, even with cURL disabled.

[ryan]

(In [8548]) Streams transport fixes from jacobsantos. fixes #7456 see #4779