WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#18273 closed defect (bug) (fixed)

Flaw in WP_Http_Encoding::accept_encoding() or related.

Reported by: jfarthing84 Owned by: dd32
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.2.1
Component: HTTP API Keywords:
Focuses: Cc:

Description

SO, I'm trying to interface with the MailChimp API via WP_Http() and no matter what I put in the settings, I get back encoded data in the body?. If I enter the API URL into the browser, it comes back fine. If I edit /wp-includes/class-http.php and change

$r['headers']['Accept-Encoding'] = WP_Http_Encoding::accept_encoding();

to

$r['headers']['Accept-Encoding'] = '';

it also comes back fine.

Change History (12)

#1 @jfarthing84
6 years ago

Interstingly enough, after reading this post, I tested with a call that returns a larger amount of data, and it works as expected. If it helps to pinpoint, the class being used in my situation is WP_Http_Streams().

#2 @dd32
6 years ago

Can you capture/dump out the full headers being sent, and the raw data being recieved before WP_HTTP processes it?

You should be able to get that just by var_dump()'ing at the right spots before the request is made, and directly after it in class-http.php, If you're developing on localhost, You can could also catch that with a packet sniffer such as Wireshark.

Alternativly, if theres a url thats public that exhibits the problem, I might be able to test directly against that as well..

#3 @jfarthing84
6 years ago

Headers just before $wp_http->_dispatch_request():

array(3) {
  ["Accept-Encoding"]=>
  string(29) "deflate;q=1.0, compress;q=0.5"
  ["Content-Type"]=>
  string(48) "application/x-www-form-urlencoded; charset=UTF-8"
  ["Content-Length"]=>
  int(54)
}

Not sure exactly what you wanted here but this is the result of stream_get_meta_data() within WP_Http_Streams::request().

array(10) {
  ["wrapper_data"]=>
  array(10) {
    [0]=>
    string(15) "HTTP/1.1 200 OK"
    [1]=>
    string(35) "Date: Thu, 28 Jul 2011 06:14:57 GMT"
    [2]=>
    string(29) "Server: Apache/2.2.3 (CentOS)"
    [3]=>
    string(24) "X-Powered-By: PHP/5.2.10"
    [4]=>
    string(44) "Set-Cookie: _AVESTA_ENVIRONMENT=prod; path=/"
    [5]=>
    string(25) "Content-Encoding: deflate"
    [6]=>
    string(21) "Vary: Accept-Encoding"
    [7]=>
    string(18) "Content-Length: 41"
    [8]=>
    string(17) "Connection: close"
    [9]=>
    string(38) "Content-Type: text/html; charset=UTF-8"
  }
  ["wrapper_type"]=>
  string(4) "http"
  ["stream_type"]=>
  string(14) "tcp_socket/ssl"
  ["mode"]=>
  string(1) "r"
  ["unread_bytes"]=>
  int(0)
  ["seekable"]=>
  bool(false)
  ["uri"]=>
  string(45) "http://us2.api.mailchimp.com/1.3/?method=ping"
  ["timed_out"]=>
  bool(false)
  ["blocked"]=>
  bool(true)
  ["eof"]=>
  bool(true)
}

#4 @dd32
6 years ago

And the content you're seeing is compressed data correct?

Also, What's the server environment? (PHP Version, and PHP zlib version at a minimum)

#5 @jfarthing84
6 years ago

Yes, it is compressed data. If I do the exact same API call via direct URL in the browser, I get the expected result.

PHP: 5.3.2

zlib Compiled: 1.2.1.1

zlib Linked: 1.2.3.3

#6 @dd32
6 years ago

  • Milestone changed from Awaiting Review to 3.3

#7 follow-up: @dd32
6 years ago

  • Owner set to dd32
  • Resolution set to fixed
  • Status changed from new to closed

In [18718]:

Add inflation support for java.util.zip.Deflater in WP_Http_Encoding::compatible_gzinflate(). Fixes #18273

#8 in reply to: ↑ 7 @westi
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to dd32:

In [18718]:

Add inflation support for java.util.zip.Deflater in WP_Http_Encoding::compatible_gzinflate(). Fixes #18273

What are the errors we are suppressing here? Do we really need to suppress them?

Can we have some more detail on how the magix number here were worked out - I think we should at least document this code better so it is easier to test in future.

#9 @dd32
6 years ago

Can we have some more detail

I was in the process of writing that comment :)

After getting my hands on a Mailchimp API key, I did some brute force decoding on the data.

$i = 0;
while ( $i++ <= strlen($gzData) ) {
  $decompressed = @gzinflate( substr($gzData, $i) );
  if ( false !== $decompressed ) {
    echo "You need $i!";
    return $decompressed;
  }
}

turns out, 2 is the magical number, as some others have also found and Stack Exchange post on Mailchimp API + wp_remote_post() not returning properly

What are the errors we are suppressing here? Do we really need to suppress them?

Unfortunately yes, As done elsewhere in that class, the PHP decompression functions issue warnings when it can't decompress the data, and we can't know if it can decompress the data without trying.

The first case there which I added the @ to, hasn't (to my knowledge) errored out before, as if the data stream starts with that header, it's reasonably reliable to expect it to decode it - but if there's a data error(ie. incomplete transmission), it'll issue warnings. Same for the added case for 2, it'll issue a warning if it's not compressed with that specific compressor.

I'd also like to note, that I don't like it returning compressed data when it can't decompress it..

Last edited 6 years ago by dd32 (previous) (diff)

#10 @dd32
6 years ago

Example of warnings raised:

( ! ) Warning: gzinflate() [function.gzinflate]: data error in C:\www\wordpress-clean\wp-includes\class-http.php on line 1656

#11 @dd32
6 years ago

The PHP documentation here has 4 different implementations, offset 2, offset 10, offset 11, and one which determines the offset based on the contents of the compressed data. The Transfer Encoding code shouldn't apply here, as it's handled by the API elsewhere.
The "complex" check appears to cover the 10 and 11 offset checks - although I haven't found a server to test that against (mod_gzip data is being handled by PHP's functions directly in my test installs it appears).

From what I can see, unless someone posts another server which is having trouble, these 2 cases here cover all the "known" cases.

#12 @dd32
6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [18806]:

Document the Magic numbering in WP_Http_Encoding::compatible_gzinflate(). Fixes #18273

Note: See TracTickets for help on using tickets.