WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 2 months ago

#22952 new defect (bug)

WP_HTTP can cause PHP Warnings during attempted decompression

Reported by: dd32 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.3
Component: HTTP API Keywords: has-patch needs-testing
Focuses: Cc:

Description (last modified by SergeyBiryukov)

WARNING: wp-includes/class-http.php:1656 - gzinflate(): data error

WP_Http_Encoding can cause PHP Warnings when it attempts to decompress data using gzinflate() which has been encoded in any way.
We currently work around this this in a few ways, but we still take a "try it and see" method instead of detecting the compressed contents signature and handling it appropriately.

Attached is a first-run patch at detecting Huffman coding, which is what we currently use @gzinflate( substr( $gzData, 2 ) ) for (and hey, who doesn't like making magic numbers clearer?)

I have been running a similar patch on WordPress.com and gathering data on how the myriad of different Web Servers out there respond, and so far this causes it to correctly identify the vast majority of responses.

It appears that we may also be attempting to decompress compressed files retrieved through WP_HTTP on some poorly configured servers, but this is something I haven't yet traced properly.

Attachments (3)

22952.diff (2.2 KB) - added by dd32 3 years ago.
22952.2.diff (2.2 KB) - added by dd32 3 years ago.
22952.3.diff (4.6 KB) - added by dd32 7 months ago.

Download all attachments as: .zip

Change History (25)

@dd323 years ago

comment:1 @SergeyBiryukov3 years ago

  • Description modified (diff)

comment:2 @dd323 years ago

I have also considered that we're using gzinflate() completely wrong, for example:

  • gzencode() == gzip
  • gzcompress() == zlib (aka. HTTP deflate)
  • gzdeflate() == *raw* deflate encoding

Currently we use gzinflate() (the raw DEFLATE standard) to decompress the data created by compressors which add their compression header/footer wrappers.
gzuncompress() for example handles Huffman encoding internally, as it's designed for uncompressing HTTP "deflated" content.
gzdecode() on the other hand is designed for gzip encoded files (which uses DEFLATE internally as the compression method) which has it's own headers (as it's designed for multiple files stored within the archive) - I believe this is the appropriate function to decompress data for the block above the changes in WP_Http_Encoding::compatible_gzinflate() which strips the full zlib headers

Ultimately, we've been bitten by poor implementations from other web servers and software many times, which has lead me to disregarding the above and continuing forward and detecting it ourselves and striping the Huffman headers which gzinflate() can't handle. The changes here are 100% backwards compatible so far.

This still has the possibility of generating warnings (which are silenced by all those @'s) but significantly reduces the number of configurations which might cause one.

@dd323 years ago

comment:3 @DrewAPicture3 years ago

  • Cc xoodrew@… added

I asked about this on wp-hackers at the end of November, but from what @dd32 said, I understood it to be something on my end.

However, I never managed to overcome it so +1 for fixing it.

comment:4 @tollmanz2 years ago

  • Cc tollmanz@… added

comment:5 @ocean902 years ago

  • Cc ocean90 added

comment:6 @dd322 years ago

  • Milestone changed from 3.6 to Future Release

comment:7 @WraithKenny2 years ago

  • Cc Ken@… added

comment:8 @m_uysl23 months ago

  • Cc m_uysl@… added

comment:9 follow-up: @nico2322 months ago

+1

I see this error regularly since I have debug bar ( and the extender) installed. Running Ubuntu 13.04 Apache localhost server. I liked to see it go away, did apply the 2nd patch from above and it seems to be working fine, error is gone.

comment:10 in reply to: ↑ 9 @jorgeorpinel17 months ago

I see this error regularly since I have debug bar ( and the extender) installed. Running Ubuntu 13.04 Apache localhost server. I liked to see it go away.

-nico23

Same here. I'm running wp 3.7.1 and its still there.

comment:11 @jmh17 months ago

Same for 3.8 - I see this error on the debug bar only when trying to work with the Twitter API.

comment:12 @thongta14 months ago

  • Type changed from enhancement to defect (bug)
  • Version set to 3.9

I see this issue in 3.9, it appears randomly on the debug bar on both frontend & backend.

comment:13 follow-up: @master41216013 months ago

It get this notice in debug bar too, is this issue patches in next version?

Also get this sometimes:
NOTICE: wp-includes/post-template.php:29 - Trying to get property of non-object

comment:14 @SergeyBiryukov13 months ago

  • Version changed from 3.9 to 3.3

Introduced in [18718].

comment:15 in reply to: ↑ 13 ; follow-up: @SergeyBiryukov13 months ago

Replying to master412160:

Also get this sometimes:
NOTICE: wp-includes/post-template.php:29 - Trying to get property of non-object

See #17034.

comment:16 in reply to: ↑ 15 ; follow-up: @master41216013 months ago

Replying to SergeyBiryukov:

Replying to master412160:

Also get this sometimes:
NOTICE: wp-includes/post-template.php:29 - Trying to get property of non-object

See #17034.

I see so this is a plugin calling a non-object?

comment:17 in reply to: ↑ 16 @SergeyBiryukov13 months ago

Replying to master412160:

I see so this is a plugin calling a non-object?

A plugin or theme calling get_the_ID() outside of the loop.

comment:18 @naomicbush13 months ago

I see this issue in 3.9 when receiving a JSON response from the MailChimp API v2.0

comment:19 @GregLone10 months ago

I'm still having the issue with WordPress 4.0, but this one is new for me:

WARNING: wp-includes/class-http.php:2015 - gzinflate() [function.gzinflate]: data error
require('wp-blog-header.php'), require_once('wp-includes/template-loader.php'), include('/absolute-path/to/wp-content/themes/mash/index.php'), get_template_part, locate_template, load_template, require('/themes/golf/content/content-page.php'), the_content, apply_filters('the_content'), call_user_func_array, WP_Embed->autoembed, preg_replace_callback, WP_Embed->autoembed_callback, WP_Embed->shortcode, wp_oembed_get, WP_oEmbed->get_html, WP_oEmbed->fetch, WP_oEmbed->_fetch_with_format, wp_safe_remote_get, WP_Http->get, WP_Http->request, WP_Http->_dispatch_request, WP_Http_Curl->request, WP_Http_Encoding::decompress, gzinflate

Which leads to WP_Http_Encoding::decompress().

This whole thing is triggered if I have an embedded tweet in my content, and if I cleanup the embeds cache ($GLOBALS['wp_embed']->delete_oembed_caches( $post_ID );). Embedded videos are fine.

comment:20 @swissspidy7 months ago

Just ran into this issue while retrieving images from https://maps.googleapis.com/maps/api/staticmap using download_url().

I'll try to update the patch. Does this need unit tests?

@dd327 months ago

comment:21 @dd327 months ago

22952.3.diff is an updated patch of what's running on WordPress.com after I attempted to fix most of the remote fetching failures we saw (Pingbacks, etc). The code is pretty crazy, but I haven't heard of any false failures.

The basic rule is that there's so many servers out there compressing data in so many different ways, using so many different standards, and all calling it either deflate or gzip that it's not funny..

I never pushed these changes back to core primarily as I never 100% finished tracking down examples of failures (I'd love to unit test it, but need a bunch of obscurely compressed pages that we can use), but secondly because the code looks so crazy that I question if we should be doing so much work here in PHP.. :)

comment:22 @lkraav2 months ago

Since this lights up Debug Bar red on the regular, it'd be nice to somehow improve the situation. A comprehensive core fix here seems to be far and away, perhaps Debug Bar can somehow be made to ignore this particular error?

Note: See TracTickets for help on using tickets.