Make WordPress Core

Opened 13 years ago

Closed 12 years ago

#18273 closed defect (bug) (fixed)

Flaw in WP_Http_Encoding::accept_encoding() or related.

Reported by: jfarthing84's profile jfarthing84 Owned by: dd32's profile 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
13 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
13 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
13 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
13 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
13 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
13 years ago

  • Milestone changed from Awaiting Review to 3.3

#7 follow-up: @dd32
13 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
13 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
13 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 13 years ago by dd32 (previous) (diff)

#10 @dd32
13 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
12 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
12 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.