Opened 3 years ago

Last modified 12 months ago

#11888 new defect (bug)

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

Reported by: Denis-de-Bernardy Owned by:
Priority: normal Milestone: Future Release
Component: HTTP Version: 2.9.1
Severity: normal Keywords: has-patch needs-testing
Cc: georgemamadashvili@…

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

Download all attachments as: .zip

Change History (62)

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

comment:2   dd323 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.

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.

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.

comment:5   dd323 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).

comment:6   dd323 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.

dd323 years ago

comment:7   dd323 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?

comment:8   dd323 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.

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.

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)

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

http://example.com/?http_test

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)

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

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. :-|

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

test plugin

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.

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

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

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.

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

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

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.

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

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

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.

comment:26 follow-up: ↓ 27   dd323 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?

comment:27 in reply to: ↑ 26   Denis-de-Bernardy3 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.

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.

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

disable streams when curl wrappers are enabled

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.

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.

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.

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)

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

dd323 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)

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

11888-streams-test.diff works for me.

  • Keywords tested added

works on the php5 server with curl wrappers too.

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

  • Keywords commit added

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.

  • 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.

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

  • Milestone changed from 3.0 to 3.1

Bumping to 3.1

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.
  • Milestone changed from Awaiting Triage to Future Release
  • Cc georgemamadashvili@… added
Note: See TracTickets for help on using tickets.