[wp-trac] [WordPress Trac] #44532: Extreme memory leak related to wp_is_stream in wp-includes/functions.php in WordPress 4.9.7

[WordPress Trac]

#44532: Extreme memory leak related to wp_is_stream in wp-includes/functions.php in
WordPress 4.9.7
-----------------------------------------+---------------------
 Reporter:  timbowesohft                 |       Owner:  (none)
     Type:  defect (bug)                 |      Status:  new
 Priority:  high                         |   Milestone:  4.9.8
Component:  Media                        |     Version:  4.9.7
 Severity:  major                        |  Resolution:
 Keywords:  has-patch reporter-feedback  |     Focuses:
-----------------------------------------+---------------------

Comment (by pbiron):

 Replying to [comment:36 ddenev]:
 > I can confirm that the provided patch DOES indeed solve the crashes.

 Thanx for the confirmation.

 It's important to note that the patch does not __solve__ the problem,
 which is actually a bug in PHP's [[http://php.net/manual/en/function
 .stream-get-wrappers.php|stream_get_wrappers()]]).  Rather, it is simply
 intended to reduce the likelihood that that PHP bug will be surfaced in
 WP.  See below for an explantation of **how** the patch intends to achieve
 that.

 We still need confirmation from others who are experiencing this problem
 that the patch reduces the occurrences of the problem for you as well.

 > What would be the scenarios in which
 > {{{#!php
 > if ( false === strpos( $path, '://' ) )
 > }}}
 > would return true and then we will have the crashes again?

 Let me see if I can explain what the patch is intened to do.

 Many (most?) of PHP's file-related functions (e.g.,
 [[http://php.net/manual/en/function.fopen.php|fopen()]],
 [[http://php.net/manual/en/function.file-get-
 contents.php|file_get_contents()]], etc) not only operate on files, but
 are able to seemlessly operate on URLs (referred to, in this context, as
 `streams`) as well:

 * `file_get_contents( '/path/to/file.txt' )`
 * `file_get_contents( 'https://example.com' )`
 * etc.

 In some cases, WP needs to do extra or special processing when asked to
 operate on a stream; hence, the existence of
 [[https://developer.wordpress.org/reference/functions/wp_is_stream/|wp_is_stream()]].

 In the vast majority (99% ?) of cases, the `$path` passed to
 `wp_is_stream()` is a file system path and not a URL so there is no need
 to call PHP's `stream_get_wrappers()`, which returns an array of protocols
 or URL "schemes" that PHP supports, to test whether the URL is a stream
 that PHP can handle.

 To the best of everyone's understanding, the simple act of calling
 `stream_get_wrappers()` does **not** cause the PHP bug to surface, but
 repeated calling of it can.

 Since **ALL** URL's will contain the substring `://` and **NO** file
 system paths will, the

 {{{#!php
 if ( false === strpos( $path, '://' ) ) {
         return false;
 }
 }}}

 test in the patch avoids calling PHP's `stream_get_wrappers()` in the vast
 majority of cases.

 Changes in WP 4.9.7 drasticly increased the number of times
 `wp_is_stream()` is called in a typical WP request; hence, drastically
 increasing the number of times PHP's `stream_get_wrappers()` is called.
 This appears to be why there have been so many more reports of the problem
 since the release of 4.9.7.

 So the hope is that by reducing the number of times
 `stream_get_wrappers()` is called, the patch will return the number of
 times the PHP bug surfaces in WP to pre-4.9.7 levels (and maybe even
 lower).

 Does that explanation help?

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/44532#comment:37>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform