add global proxy support in options

[tmountjr]

I use the 2.2-bleeding SVN build of WP on a local install to test upcoming changes to the system before putting them on my live blog. In order to test accurately, I installed FeedWordPress to pull my live RSS feed. I couldn't figure out why it wouldn't pull my feeds (even after adding all the appropriate snoopy proxy variables in the plugin) unless I set the proxy in wp-includes/class-snoopy.php. ​Ticket 3082 details the same problem. Perhaps a set of define statements could be added to wp-config.php:

define('WP_PROXYHOST', 'proxy.server.com');
define('WP_PROXYPORT', '8080');
define('WP_USEPROXY', 'TRUE');

In wp-includes/class-snoopy.php, for instance, the current code could be changed to respect the proxy settings if entered:

var $proxy_host = WP_PROXYHOST;
var $proxy_port = WP_PROXYPORT;
var $_isproxy = (WP_USEPROXY == 'TRUE') : true ? false;

These settings could also be carried over to Akismet and any other class that uses snoopy or fsockopen; I successfully tested this in class-snoopy.php. Besides not remembering my ternary operators for the _isproxy variable, the change works and applies to all instances where WP is using snoopy to get something from the web.

I'd be happy to work up something official if anyone thinks it's worth it...

[tmountjr]

sample wp-config file with added support for proxy variables

[tmountjr]

updated Snoopy class with support for proxy variables

[tmountjr]

updated IXR-RPC class file with proxy support for fsockopen

[tmountjr]

I've created and attached a few patches:

/wp-config-sample.php - adds support for proxy variables in the sample config file.

/wp-includes/functions.php - adds proxy support for fsockopen

/wp-includes/class-snoopy.php - adds proxy support for snoopy

/wp-includes/class-IXR.php - adds proxy support for fsockopen in the IXR-RPC class

These have been tested on mirror installations - one live (no proxy, so no options in wp-config.php set) and one local (with proxy, options set in wp-config.php). Both appear to work well. I think if no proxy is set the change would be transparent. What needs further testing is how the changes respond in a proxy environment. At the moment Snoopy works well in my environment, but I'm a little less clear on fsockopen and how that will respond, particularly in an environment where HTTP authentication is used.

[tmountjr]

updated to include support in get_headers and curl libraries

[tmountjr]

I just modified functions.php.diff so that both instances of fsockopen were proxied. I also added the support for curl in the same file.

[Johnny_one_eye]

I think this patch is great, as I could really use it. I've been trying it out, and I only have 1 problem with it. My proxy doesn't require authentication (user/pass). I think you should probably edit it so that it would check if I need a user/pass and adjust accordingly...

[dubelclique]

the patch worked for me on my pre-2.3 installs, but i can't get it to work on my 2.3 install; i'm positive i'm patching the right code, but i think there's something else in need of revision in 2.3.

any ideas? is the 2.4 milestone definite? i'm pretty surprised this isn't more of an issue for folks.

[westi]

Replying to dubelclique:

the patch worked for me on my pre-2.3 installs, but i can't get it to work on my 2.3 install; i'm positive i'm patching the right code, but i think there's something else in need of revision in 2.3.

any ideas? is the 2.4 milestone definite? i'm pretty surprised this isn't more of an issue for folks.

The 2.4 milestone is suggested not definite - if this issue is not resolved when it comes to releasing 2.4 then it will be bumped to 2.5

I think a better fix to this issue is to move all direct uses of fsockopen to a wrapped version which provides the proxy support (and possibly uses curl is fsockopen is disabled)

Then all places where we have duplicated fsockopen code can use this one route.

[futtta]

just bumping up, support for proxies is very important if we want to enable intranet-deployments of wordpress. servers on an intranet rarely have a direct connection to the internet (proxy or even no connection at all). currently this means wordpress will stop working after some time, with timeouts caused by attempts to connect to the internet to check for updates for core/ plugins.

not only should one be able to configure a proxy (+ proxy authentication info), you should also be able to specify there is no connection to the internet at all?

i tried to add 'return false' to a number of update functions, but ended up disabling fsockopen in php.ini all together. a pain, really.

[DD32]

In the event of #4779 being implemented, It should allow a global proxy setting to be added with ease without too much trouble.

[DD32]

Should be possible now that everythings being moved to the HTTP API, Hopefully all the transports can support proxies though (I have a feeling there may be 1 which will not handle it too well)

[DD32]

See Also: #8927 (Allow the disabling of external HTTP Access)

If no-one else gets around to it, I'll implement this in 2.9

[jacobsantos]

Incomplete patch for testing proxy support.

[jacobsantos]

HTTP API cleanup with proxy support for HTTP, CURL and Streams transports.

[DD32]

Problem with cURL:

Doesnt look like you can rely upon the headers length returned from it for proxy requests.. Eg, Full document returned:

HTTP/1.0 200 Connection established
Proxy-agent: CCProxy 6.64

HTTP/1.1 200 OK
Date: Sun, 22 Feb 2009 09:30:52 GMT
Server: Apache/2.2.8 (CentOS)
X-Powered-By: PHP/5.2.6
Content-Length: 4
Connection: close
Content-Type: text/html; charset=UTF-8

1563

$headerLength gets set to 190, which chops the headers off at the end of Content-Length, resulting in:

Headers:

HTTP/1.0 200 Connection established
Proxy-agent: CCProxy 6.64

HTTP/1.1 200 OK
Date: Sun, 22 Feb 2009 09:30:52 GMT
Server: Apache/2.2.8 (CentOS)
X-Powered-By: PHP/5.2.6
Content-Length

Body:

: 4
Connection: close
Content-Type: text/html; charset=UTF-8

1563

[DD32]

I couldnt get fsockopen() working either, The proxy server seems to be returning ..nothing:

array(4) { ["headers"]=> array(0) { } ["body"]=> NULL ["response"]=> array(2) { ["code"]=> int(0) ["message"]=> string(0) "" } ["cookies"]=> array(0) { } }

It might be the proxy server i'm testing with.. but it works with browsers.

[DD32]

Doesnt look like you can rely upon the headers length returned from it for proxy requests..

Hm.. I think its a cURL bug there, Those new lines are throwing it off. I'll test with a different proxy server and see what its output is. I'm not too sure on how standards-complient this rarely used proxy server is.

[DD32]

Alright, I've done some messing around, And i've got it to work 100% with Apache as a proxy server.

The proxy server i was testing with before (ccProxy) now works with HTTP, CURL, and Fopen() - But fails with streams and fsockopen().

I've not tested Squid, which is really probably the main server that needs testing.. I'll test that if i can find a test server, Or i can manage to configure it myself.

I've made a fair few modifications to the original patch( 4011.3.diff ), So What i'm thinking, Is Get that commited first (props jacob), once thats commited, I can make a patch based off that of my changes,

I'm aiming to support HTTP Proxies with this patch, Its near impossible to support every different type of proxy it seems, so on some hosts, disabling some of the HTTP transports -may- be required for clean operation, but only IF they're using a proxy too, So for the majority of users it'll not be a problem. For the majority of proxy users, it'll work fine (provided squid acts nicely), for a small set of the proxy users, they may have to disable them.

Once my patch is applied, The following items are affected:
Apache (As a proxy server, No Username/password): All transports work
CCProxy (With, and without, username/password): Http, Curl, and fopen() work. Streams and fsockopen() do not.

I've not tested apache with a username/password yet.

[westi]

I agree with DD32 about applying one patch and then another to give jacob the due props.

Currently the patch doesn't apply and some of the comments could do with a tidy up ;-)

jacob could you update the patch for current trunk?

[jacobsantos]

Yes. Will do in a few hours.

[DD32]

I've cleaned up some of the comments myself, and some formatting as well (Just FYI)

[jacobsantos]

Update patch for current trunk

[jacobsantos]

I actually like the comments.

[westi]

(In [10692]) First pass of HTTP Proxy support. See #4011 props jacobsantos.

[DD32]

attachment 4011.4.diff added

• Lots of formatting changes..
• Fixes a fair few Proxy items
• Removed the filter 'http_request_port' from the fsockopen transport, Seems to not be used anywhere else, Seemed rather pointless there.. The port should be set in the URL to start with.
• Some proxies require the HTTP header to be in the form of "GET ​http://blah/blah/ HTTP/1.0" instead of just utilising "GET /blah HTTP/1.0" with a "Host: blah"
• HTTP Tunneling can cause issues, Many proxies do not allow tunneling either (Its used to fetch non-http data, so shouldnt be needed anyway)
• Better ::test() functions, ::test() can be called from plugins or other code now without requiring the 'local'/'ssl' $args to be set(Well.. Why not, Else whats the point of the isset()'s?). Should make the ::test() functions much more robust now.
• SSL Verification fixes, use the local ssl verification filter for local requests.. Make sure all the transports abide by it
• Streams transport supports PHP 5.0+, however, Streams + SSL + Proxy requires PHP 5.1.0+
• s/str_replace/preg_replace in certain locations to only replace the scheme part, NOT to replace the scheme in other places (ie. scheme://somewhere/scheme.php)
• Passes $args into the filter functions for ::test()
• A huge patch that needs testing.

[jacobsantos]

There are a bit more problems and that is part of the reasons I didn't want to undertake this project. There inconsistencies are in the various proxy implementations and has to be handled by the Library.

Doing so would really be dirty, so I think the most popular ones should be supported and all of the rest should be ignored.

[DD32]

Patch is aparantly now stale.

[DD32]

Patch Refreshed.

[bforchhammer]

Wow, I love the work you're doing here... We are running one of those corporate wordpress installs behind a firewall and this is helping us very much already. Thanks!

I had a look at http.php HEAD & patch 4011.5.diff and here's a few things I stumbled on:

line 121 and 169: if has_filter() condition then do_action() call.

> Shouldn't that be either has_action() and do_action() or has_filter() and apply_filter() ?

lines 549 and 1498, for example is_array( WP_ACCESSABLE_HOSTS ).

> I am not sure that php allows to set constants to "arrays". Afaik only scalar values (integer, float, string, boolean) are accepted. Solution might be to serialise & unserialize the array or use a string instead (e.g. pipe | separated?).

lines 536 and 1513: get_bloginfo('site_url')

> 'site_url' is not a valid parameter; according to the codex doc on ​get_bloginfo it should be 'url' since 2.7 (or 'siteurl' should work but is deprecated apparently).

[bforchhammer]

And another one:

line 539: if ( $uri == $home['host'] )

> Comparing the full $uri with only the hostname of the current site will always(?) result in false. Should probably be: if ( $check['host'] == $home['host'] ).

[DD32]

line 121 and 169: if has_filter() condition then do_action() call.
Shouldn't that be either has_action() and do_action() or has_filter() and apply_filter() ?

Action, Filter, Same thing to WordPress. The difference is that when you call filters, it passes the value around, When you call action, it doesnt bother.

lines 549 and 1498, for example is_array( WP_ACCESSABLE_HOSTS ).

You are indeed correct, Thats why people should test the damn patches before submitting them!

I'm thinking a Comma seperated list would be better: define(.., 'localhost, api.wordpress.org, etc.someplace');

get_bloginfo('site_url') > 'site_url' is not a valid parameter

*Sigh* I'll get that fixed too.

line 539: if ( $uri == $homehost? )

Hm, Looking at the code, and the fact it checks specifically for 'localhost' I think its supposed to only pick up requests straight to localhost.. But you're right, It would be better based on parsing request.

I'll get a new patch based off this (and any other comments in the next 12~48hrs)

[DD32]

attachment 4011.6.diff added

Refreshed patch with additions of comments from bforchhammer. (Yes, I tested that the Proxy allow/bypass code works too)

[ryan]

(In [10864]) Proxy support. Props DD32. see #4011

[ionfish]

Would it be possible to rename the constant to WP_ACCESSIBLE_HOSTS? 'Accessable' is a spelling mistake.

[jbsil]

Changes to $accessible_hosts and WP_ACCESSIBLE_HOSTS in http.php

[jbsil]

Patch above changes all occurrences of "accessable" to "accessible" in http.php. My search of other files in /wordpress +R did not find the word, so I think this covers it.

[DD32]

Patch above changes all occurrences of "accessable" to "accessible" in http.php

Cheers for that, I was wondering about the spelling of that whilst changing, but forgot to look it up..

[jbsil]

Should it be changed anywhere else that I missed? If not, should this be flagged as ready to commit again?

[ryan]

(In [10870]) Correct spelling. Props jbsil. see #4011

[dd32]

(In [13484]) Support non-BASIC Authentication schemes in the HTTP API if server supports them. Props ssandison, See #4011, Fixes #12200