Make WordPress Core

Opened 13 years ago

Closed 10 years ago

#11888 closed defect (bug) (wontfix)

The streams & fopen HTTP transport arn't sending headers that are passed to it

Reported by: denis-de-bernardy's profile Denis-de-Bernardy Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.9.1
Component: HTTP API Keywords:
Focuses: Cc:

Description

In particular, cookie headers. Related php bugs:

http://bugs.php.net/bug.php?id=41051

http://bugs.php.net/bug.php?id=45092

Based on the second PHP ticket, this would affect systems up to PHP 5.2.9.

I tried to replace $strHeaders with an $arrHeaders array based on the discussions in the second ticket, but it didn't make the slightest difference.

I'm running out of ideas, too...

Attachments (14)

11888.diff (957 bytes) - added by dd32 13 years ago.
macosx-php53.log (3.6 KB) - added by Denis-de-Bernardy 13 years ago.
fopen-with-patch-applied.log (982 bytes) - added by Denis-de-Bernardy 13 years ago.
freebsd-php5.2.11.log (4.0 KB) - added by Denis-de-Bernardy 13 years ago.
linux-php5.2.12-cgi.log (2.5 KB) - added by Denis-de-Bernardy 13 years ago.
http-test.php (1.8 KB) - added by Denis-de-Bernardy 13 years ago.
test plugin
localhost-tests.log (3.4 KB) - added by Denis-de-Bernardy 13 years ago.
test-array.diff (1.4 KB) - added by Denis-de-Bernardy 13 years ago.
localhost-tests-with-arrays.log (3.1 KB) - added by Denis-de-Bernardy 13 years ago.
aarons-server-with-curl-wrappers.log (3.3 KB) - added by Denis-de-Bernardy 13 years ago.
xavisys.com-with-streams.log (3.2 KB) - added by aaroncampbell 13 years ago.
xavisys.com-phpinfo.html (57.0 KB) - added by aaroncampbell 13 years ago.
11888.2.diff (510 bytes) - added by Denis-de-Bernardy 13 years ago.
disable streams when curl wrappers are enabled
11888-streams-test.diff (2.9 KB) - added by dd32 13 years ago.

Download all attachments as: .zip

Change History (63)

#1 @Denis-de-Bernardy
13 years ago

fopen() doesn't seem to be sending them either, on the same server. (curl and the last transport both work.)

#2 @dd32
13 years ago

If it comes to end that it doesnt pass the headers on, we could return false on ::test() for that particular request (If it has access to the custom headers)

That'd rely on my other ticket for a better key for the transport cache however, and would require extra info there too.

#3 @Denis-de-Bernardy
13 years ago

Re the WP_Http_Fopen class, I'm not understanding how it's even sending the headers. It's like, it calls fopen(), and then just fread()'s the mess without the slightest bit of sending headers.

#4 @Denis-de-Bernardy
13 years ago

Just to confirm DD32's hunch, the ticket with the $args thingy for a better transport cache key is needed to work around this one.

#5 @dd32
13 years ago

Re the WP_Http_Fopen class, I'm not understanding how it's even sending the headers.

That would be, because its not. Its also not sending custom user agents.

IIRC, its possible to use ini_set() and put all the headers into the user agent field (As its not sanitized by PHP, \r\n just gets outputed directly).

#6 @dd32
13 years ago

  • Summary changed from The streams HTTP transport isn't sending headers that are passed to it to The streams & fopen HTTP transport arnt't sending headers that are passed to it

Ref:

Custom headers may be sent with an HTTP request prior to version 5 by taking advantage of a side-effect in the handling of the user_agent INI setting. Set user_agent to any valid string (such as the default PHP/version setting) followed by a carriage-return/line-feed pair and any additional headers. This method works in PHP 4 and all later versions. 


Example #1 Sending custom headers with an HTTP request
<?php
ini_set('user_agent', "PHP\r\nX-MyCustomHeader: Foo");

$fp = fopen('http://www.example.com/index.php', 'r');
?>

http://php.net/manual/en/wrappers.http.php

It seems to suggest theres a PHP5+ method thats better suited, I'm going to assume this is what the Streams function should be using.

@dd32
13 years ago

#7 @dd32
13 years ago

  • Keywords has-patch needs-testing added; 2nd-opinion removed

attachment 11888.diff added

  • WP_HTTP_fopen
    • Adds support for sending user agents - At present, it only sends the default PHP one.
    • Adds support for custom headers

Tested on PHP 5.3.1 on windows

Can someone else on a different PHP version (PHP4 possibly?) test if it works correctly for them?

#8 @dd32
13 years ago

  • Summary changed from The streams & fopen HTTP transport arnt't sending headers that are passed to it to The streams & fopen HTTP transport arn't sending headers that are passed to it

With regards to the Streams transport.

I cant seem to fault it under PHP 5.2.6 on windows.

Eg:

wp_remote_get('http://localhost/test/headers.php', array( 'headers' => array( 'Custom' => 'Headers', 'Authorization' => 'Basic 1234') ) )

results in

array(5) {
  ["headers"]=>
  array(6) {
    ["server"]=>   string(31) "Apache/2.2.14 (Win32) PHP/5.2.6"
  }
  ["body"]=>
  string(298) "


array(5) {
  ["Host"]=>
  string(9) "localhost"
  ["User-Agent"]=>
  string(53) "WordPress/3.0-alpha; http://localhost/wordpress-trunk"
  ["Custom"]=>
  string(7) "Headers"
  ["Authorization"]=>
  string(10) "Basic 1234"
  ["Accept-Encoding"]=>
  string(29) "deflate;q=1.0, compress;q=0.5"
}
"
  ["response"]=> 200 OK....
  ["debug"]=>
  array(1) {
    ["transport"]=>
    string(15) "WP_Http_Streams"
  }
}

Any suggestions of a config change which could expose it denis? This is being tested on windows and just using <?php var_dump(apache_request_headers()); in the requested page.

#9 @Denis-de-Bernardy
13 years ago

I got the issue with cookies on Mac / PHP 5.3.1. The response's meta reports that it's using a curl transport. I'll try to find some time to do some more testing today.

#10 @dd32
13 years ago

Can you post some generic code that exhibits it consistantly on your setup when you've tested it? (And any output which shows underlying connections in use.. etc)

#11 @Denis-de-Bernardy
13 years ago

I'm attaching a test plugin to run tests. Activate and visit the site's front page as:

http://example.com/?http_test

#12 @Denis-de-Bernardy
13 years ago

attaching two logs. first is all transports, without any patches applied.

second log confirms that patch works on the same platform (macosx / php 5.3)

#13 @Denis-de-Bernardy
13 years ago

log for freebsd. confirming that the fopen patch works on that one too.

#14 @Denis-de-Bernardy
13 years ago

log for a linux server using php 5.2.12 as cgi. the customer had his host upgrade his VPS since yesterday. it was running php 5.2.6 or 5.2.9 (can't remember) when I used the server to diagnose the problem. :-|

#15 @Denis-de-Bernardy
13 years ago

Cross-site testing still fails, however. And it looks like I've an issue on my own server that I need to look into.

@Denis-de-Bernardy
13 years ago

test plugin

#17 @Denis-de-Bernardy
13 years ago

When the test plugin is run from my server using the streams transport, headers are correctly passed, too.

The Health Check plugin is complaining about mod_security on Pat's server, but I'm suspicious that it makes any difference.

#18 @Denis-de-Bernardy
13 years ago

additional finding. adding a var_dump over at:

$meta = stream_get_meta_data($handle);
var_dump($meta);

reveals the following for my laptop and my dev site:

  ["wrapper_type"]=>
  string(4) "http"
  ["stream_type"]=>
  string(14) "tcp_socket/ssl"

but on Pat's dev site, however:

  ["wrapper_type"]=>
  string(4) "cURL"
  ["stream_type"]=>
  string(4) "cURL"

so maybe this is what's playing tricks on us:

http://www.php.net/manual/en/function.curl-setopt.php#70043

#19 @Denis-de-Bernardy
13 years ago

additional information from phpinfo, in case it's useful:

laptop:

Registered PHP Streams	compress.zlib, compress.bzip2, https, ftps, php, file, glob, data, http, ftp, phar
Registered Stream Socket Transports	tcp, udp, unix, udg, ssl, sslv3, sslv2, tls
Registered Stream Filters	zlib.*, bzip2.*, string.rot13, string.toupper, string.tolower, string.strip_tags, convert.*, consumed, dechunk, convert.iconv.*

my dev site:

Registered PHP Streams	zip, compress.bzip2, php, file, data, http, ftp, compress.zlib, https, ftps
Registered Stream Socket Transports	tcp, udp, unix, udg, ssl, sslv3, sslv2, tls
Registered Stream Filters	bzip2.*, string.rot13, string.toupper, string.tolower, string.strip_tags, convert.*, consumed, zlib.*

pat's dev site:

Registered PHP Streams	compress.zlib, tftp, ftp, telnet, dict, http, https, ftps, php, file, data, zip
Registered Stream Socket Transports	tcp, udp, unix, udg, ssl, sslv3, sslv2, tls
Registered Stream Filters	zlib.*, convert.iconv.*, string.rot13, string.toupper, string.tolower, string.strip_tags, convert.*, consumed

#20 @Denis-de-Bernardy
13 years ago

their respective libcurl:

  • 7.19.7 (laptop)
  • libcurl/7.19.6 OpenSSL/0.9.8e zlib/1.2.3 (my dev)
  • libcurl/7.19.5 OpenSSL/0.9.8i zlib/1.2.3 libidn/0.6.5 (pat's dev)

php on pat's site is configured --with-curlwrappers, the other two do not.

#21 @Denis-de-Bernardy
13 years ago

and googling the --with-curlwrappers leads us to the php bug that initially mentioned:

http://bugs.php.net/bug.php?id=45092

#22 @Denis-de-Bernardy
13 years ago

test-array.diff implements what is suggested in the related php bugs, but it ends up no longer passing any cookie at all (see log with array), so no good.

#23 @Denis-de-Bernardy
13 years ago

another log from Aaron's server, which has --with-curl-wrappers as well, but seems to be sending cookies properly.

#24 @Denis-de-Bernardy
13 years ago

aaron's server has:

  • libcurl/7.19.5 OpenSSL/0.9.8b zlib/1.2.3 libidn/0.6.5, --with-curl=/opt/curlssl/ --with-curlwrappers (just like pat's site)

and:

Registered PHP Streams	compress.zlib, compress.bzip2, tftp, ftp, telnet, dict, http, https, ftps, php, file, data, zip
Registered Stream Socket Transports	tcp, udp, unix, udg, ssl, sslv3, sslv2, tls
Registered Stream Filters	zlib.*, bzip2.*, convert.iconv.*, string.rot13, string.toupper, string.tolower, string.strip_tags, convert.*, consumed

#25 @aaroncampbell
13 years ago

I have a site compiled with --with-curlwrappers ...the links won't work for long because the site is supposed to be migrated from Layered Tech to RackSpace in the next hour or so. However, I'll attach the log and a saved copy of the phpinfo.

If you want to take a look yourself, just force xavisys.com to 72.233.81.243 in your hosts file. Once the site is moved, we can do whatever we need to test it out (as long as we don't crash the whole server, it still has one site on it until Saturday). The server is a dedicated managed server at Layered Tech, but I only have it until the 31st.

#26 follow-up: @dd32
13 years ago

OK, Seems my local install is setup with:

  ["wrapper_type"]=>  string(4) "http"
  ["stream_type"]=>  string(10) "tcp_socket"
  ["mode"]=>  string(2) "r+"

So i cant test that on my local system.. I'll see if i can find a linux box thats affected.

Just to confirm since theres so much raw data there to sift through:

  • Using streams, with cURL wrappers, NO custom headers are sent.
    • Because of that, Cookies are not sent, And thus, are irrelivant to the testing.
  • Affects those using --with-curlwrappers with PHP < 5.3

Did you get a chance to test if WP_HTTP_fopen works with the above patch?

#27 in reply to: ↑ 26 @Denis-de-Bernardy
13 years ago

Replying to dd32:

Just to confirm since theres so much raw data there to sift through:

  • Using streams, with cURL wrappers, NO custom headers are sent.
    • Because of that, Cookies are not sent, And thus, are irrelivant to the testing.
  • Affects those using --with-curlwrappers with PHP < 5.3

I think it actually affects server that has with php --with-curlwrappers

Did you get a chance to test if WP_HTTP_fopen works with the above patch?

It works on all three servers I've tested it on. We'd ideally need someone to test this with PHP4 as well.

#28 @Denis-de-Bernardy
13 years ago

based on:

http://svn.php.net/viewvc/php/php-src/branches/PHP_5_2/NEWS?view=markup&pathrev=284747

the --with-curlwrappers issues should be fixed in PHP 5.2.11 and PHP 5.3.1.

#29 @Denis-de-Bernardy
13 years ago

I've tried to google for a way to disable the curl wrappers, but to no avail. Considering the amount of bugs in it:

http://www.google.com/search?hl=en&q=site:bugs.php.net+curlwrappers

... I'd like to suggest the following fix for the curl wrappers problem:

  1. we use phpinfo(), once, to detect if php got configured --with-curlwrappers
  2. if so, we disable streams

@Denis-de-Bernardy
13 years ago

disable streams when curl wrappers are enabled

#30 @Denis-de-Bernardy
13 years ago

Patched attached. It disables streams when curl wrappers are enabled.

11888.diff and 11888.2.diff are tested on all three servers. PHP4 tests would be welcome for 11888.diff.

#32 @dd32
13 years ago

I'd be tempted to disable Streams entirely when custom headers are included, due to the fact that running phpinfo() all the time isnt the best way to check.

throw a site_transient cache around it too, If ( !empty(headers) && (get_transient..
WP_HTTP_Streams::has_curl_wrappers() ) ) return false

But from what i can see, Reports are thjat the bug should only occur when passing a string, an array should be fine. Thats the reason i'm wanting to test it out for myself.

#33 @Denis-de-Bernardy
13 years ago

based on my own tests, it's not working with arrays:

http://core.trac.wordpress.org/attachment/ticket/11888/test-array.diff

http://core.trac.wordpress.org/attachment/ticket/11888/localhost-tests-with-arrays.log

that's without curl wrappers, btw.

my understanding of the transport caching is that phpinfo() would only ever get used once, too. but I see the logic in wanting it disabled later on.

#34 @dd32
13 years ago

my understanding of the transport caching is that phpinfo() would only ever get used once, too.

Per page load, Or if my other patch for a better cache key does in, per-different-request.

The other thing to note however, Is that The user agent can only be set via a custom header, so its virtually always going to have extra headers. Without it, No user agent is sent at all from the look of it (Will have to see if ini_set('user_agent'... can be used instead)

#35 @dd32
13 years ago

(In [12748]) Allow WP_HTTP_Fopen to send extra headers and custom user-agents. See #11888

#36 @dd32
13 years ago

attachment 11888-streams-test.diff added

Can someone with an affected setup please test that on a Streams affected install?

It appears to work as expected on my PHP4 and PHP5 installs.

In short: If you cant trust the context options, just use the fopen user_agent "hack", As we're using fopen() it'll still work.

(Also, I tested the previous fopen patch on a PHP4 install, and it was working as described on the PHP manual)

#37 @dd32
13 years ago

  • Keywords reporter-feedback added; has-patch needs-testing removed

#38 @Denis-de-Bernardy
13 years ago

  • Keywords has-patch added; reporter-feedback removed

11888-streams-test.diff works for me.

#39 @Denis-de-Bernardy
13 years ago

  • Keywords tested added

works on the php5 server with curl wrappers too.

#40 @aaroncampbell
13 years ago

11888-streams-test.diff works on my server too.

#41 @Denis-de-Bernardy
13 years ago

  • Keywords commit added

#42 @dd32
13 years ago

I've noticed that the streams changes affects plugin update checks, So i've been running it locally with a few modifications. I'll update this ticket tonight with what i've found so far.

#43 @dd32
13 years ago

  • Keywords needs-testing added; tested commit removed

I noticed errors such as this poping up:

Notice: fopen() [function.fopen]: Content-type not specified assuming application/x-www-form-urlencoded

The test patch seems to work for GET requests with custom headers, But i cant manage a POST request to suceed.

A selection of the errors included a plain timeout, 413 Request entity too large and 400 Bad request, I couldnt get a set of headers/method of injecting the headers that worked reliably for me

I somewhat feel we'll be back to square one with Streams.

#44 @Denis-de-Bernardy
13 years ago

maybe we should disable it entirely for servers with curl wrappers, then...

#45 @dd32
13 years ago

  • Milestone changed from 3.0 to 3.1

Bumping to 3.1

#46 @jacobsantos
13 years ago

Clarifications:

  • WP_Http_Fopen is for PHP4 only, PHP5 must be using WP_Http_Streams instead.
  • On PHP4 fopen does not support sending any data besides the default PHP ones. It is good for reading requests, but not good for writing requests. It is currently the only Transport that isn't part of the POST transports. This is for a reason. With support for PHP4 required and therefore fopen with it (for when cURL and HTTP extension aren't install and fsockopen is diabled).
  • I wonder if switching to sockets instead of streams would solve this problem if so, then the #9072 be the solution for Streams.

#47 @nacin
13 years ago

  • Milestone changed from Awaiting Triage to Future Release

#48 @Mamaduka
11 years ago

  • Cc georgemamadashvili@… added

#49 @dd32
10 years ago

  • Keywords has-patch needs-testing removed
  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Due to the age of this ticket, lack of traction, minimal versions of PHP affected, and lack of available fixes, I'm afraid there isn't much we can do here.

Future changes with the transports will hopefully result in this particular use-case being fixed as a side effect #25007 #16606

Note: See TracTickets for help on using tickets.