Add support to drupal_http_request() for proxy servers

Jhef.Vicedo - May 19, 2004 - 05:01
Project:Drupal
Version:7.x-dev
Component:base system
Category:feature request
Priority:critical
Assigned:Unassigned
Status:needs work
Issue tags:Favorite-of-Dries
Description

I think the RSS feeds don't work if there is proxy server running. The request/connection is somehow being blocked.

The connection should be made first to the proxy and once it is established, then request can made.

I think the core function drupal_http_request() under /includes/common.inc doesn't support this.

One workaround is change the code from:

<?php
switch ($uri['scheme']) {
  case
'http':
    
//$fp = @fsockopen($uri['host'], ($uri['port'] ? $uri['port'] : 80), $errno, $errstr, 15);
?>

into:

<?php
   
//use proxy settings
   
$fp = @fsockopen('proxy2', '8080');
?>

And then create the request by change from:

<?php
$request
= "$method $path HTTP/1.0\r\n";
?>

Into:

<?php
$request
= "$method ".$uri['scheme']."://".$uri['host'].$path." HTTP/1.1\r\n";
?>

And comment out the lines:

<?php
$request
.= implode("\r\n", $defaults);
?>

AttachmentSizeStatusTest resultOperations
common.inc37.11 KBIgnoredNoneNone

#1

JonBob - August 25, 2004 - 19:13
Category:support request» bug report
Priority:critical» normal

#2

Anonymous - February 1, 2005 - 23:38

Did this ever get taken care of in 4.5.2. I can't seem to get any feed to work from behind a firewall. Getting error....

Failed to parse RSS feed Microsoft Security Info: 10060 A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond..

At home this same feed works fine, so I know it's not the feed.

Thanks,

SlackNet

#3

Jhef.Vicedo - February 4, 2005 - 04:24
Category:bug report» feature request

+1 on having a facility to modify proxy settings...

here's a patch of /includes/common.inc for drupal-4.4.2 version

AttachmentSizeStatusTest resultOperations
common.inc_4.patch1.37 KBIgnoredNoneNone

#4

Jhef.Vicedo - February 4, 2005 - 04:25

+1 on having a facility to modify proxy settings...

here's a patch of /modules/system.module for drupal-4.4.2 version

AttachmentSizeStatusTest resultOperations
system.module_2.patch956 bytesIgnoredNoneNone

#5

Jhef.Vicedo - February 4, 2005 - 04:31

patch for /includes/common.inc 4.5.2 version to facilitate rss feeds behind firewall

AttachmentSizeStatusTest resultOperations
common.inc_5.patch1.35 KBIgnoredNoneNone

#6

Jhef.Vicedo - February 4, 2005 - 04:32

patch of /modules/system.module 4.5.2 version to set proxy settings

AttachmentSizeStatusTest resultOperations
system.module_3.patch1.03 KBIgnoredNoneNone

#7

Bèr Kessels - February 4, 2005 - 08:23

hi, tso small issues. one is described at: http://drupal.org/node/9706
The other is that the code is not "drupal compliant". Drupal developrs are (luckily) very picky about spaces, indentation, etc.

#8

Jhef.Vicedo - February 4, 2005 - 08:51

i forgot to remove my hardcoded proxy name.. sorry

Ber, which is not drupal compliant?

AttachmentSizeStatusTest resultOperations
common.inc_6.patch1.37 KBIgnoredNoneNone

#11

Steven - February 5, 2005 - 07:09

1) New features only get added to CVS/HEAD, not to the 4.5.x branch.

2) 'Blind' variables that cannot be changed through an admin interface are not a good idea.

3) Your proxy codepath ignores the $defaults array completely. At this point in the code, this doesn't just contain default headers but also any extra headers passed to drupal_http_request(). This is not good as they are simply not used anymore.

4) Code style:
//use proxy settings
There should be a space between '//' and text. MInd capitalization and punctuation.

$request = "$method ".$uri['scheme']."://".$uri['host'].$path." HTTP/1.1\r\n";
Rules for string concatenation: never a space between . and a quote, always a space otherwise.

Tab usage: we never use tabs due to editor differences. Your patch contains several.

String quote usage: we prefer single quotes in Drupal to double quotes, except in the case where single quotes would be too hard to read (e.g. backslash escapes should be avoided as well).

#12

Anonymous - February 5, 2005 - 14:05

In a very tight setup, users must authenticate to access the proxy, to prevent unauthorised usage, or to track authorised usage.

Can the proxy support also be modified to allow username/password to be supplied to the proxy?

#13

Jhef.Vicedo - February 7, 2005 - 06:46

Thanks Steven.

I hope this patch works and be accepted... This will allow proxy variables to be modified using the admin interface.

AttachmentSizeStatusTest resultOperations
system.module_4.patch1.01 KBIgnoredNoneNone

#14

Jhef.Vicedo - February 7, 2005 - 06:47

compliant patch for proxy suport, thanks Steven.

AttachmentSizeStatusTest resultOperations
common.inc_7.patch1.38 KBIgnoredNoneNone

#15

Bèr Kessels - February 7, 2005 - 10:01

-1 on this. Please do not introduce more config options. IT makes no sense, since you only set that option once.

Also: you solve a single case of fopen, please look for an overal solution that will solve all drupa fopens, rather than hack into the feed code to fix that specific issue.

* either make the code smarter so it can detect proxy errors (http://drupal.org/node/9706) OR
* add this to conf.php

#16

killes@www.drop.org - March 13, 2005 - 19:44

I'd also like to avoid YAS (yet another setting). Can't we include proxies settings.php?

#18

rp0999 - May 24, 2006 - 22:43
Title:RSS Feeds behind Firewall» Get me past my proxy server
Category:feature request» support request

I tried to apply the patch and from the fsockopen(): unable to connect to error i get the Warning: Unknown: A session is active. You cannot change the session module's ini settings at this time. in Unknown on line 0 error.
I am on the version v4.6.3 on windows with
php
PHP 5.0.5 (cli) (built: Sep 22 2005 10:49:43)
Copyright (c) 1997-2004 The PHP Group
Zend Engine v2.0.5, Copyright (c) 1998-2004 Zend Technologies
with Zend Extension Manager v1.0.8, Copyright (c) 2003-2005, by Zend Technol
ogies
with Zend Optimizer v2.5.10, Copyright (c) 1998-2005, by Zend Technologies
apache 2.0.55 for windows and mysql as backend.
Any help or pointers in the right direction much appreciated.
my commons.inc file version is 1.443.3.10
Regards,
Rajesh

#19

karppa - June 9, 2006 - 11:01
Category:support request» feature request

As dlr noted before (http://drupal.org/node/34340), wouldn't it be cleaner to add the proxy support in the core system? Have I missunderstood, but don't I have to patch (http://drupal.org/node/7881) common.inc every time after an update? If it were possible to define the proxy-configuration as an option, it wouldn't be neccesery to do the patch all over again after each update.

I bet there are lots of other corporate users (in addition to dlr and me) behind firewall that aren't able to get aggregated content without proxy support.

#20

killefiz - June 26, 2006 - 21:16
Version:<none>» 4.7.2

I would also love to see the possibility to configure a proxy server as part of the drupal distribution.

Until that happens I have created a patch with the "won't make it into a proper release" hacks from above that applies cleanly to 4.7.2.

AttachmentSizeStatusTest resultOperations
add_proxy_settings.patch.txt2.5 KBIgnoredNoneNone

#21

magico - August 29, 2006 - 17:55
Version:4.7.2» x.y.z
  1. Features only go to HEAD
  2. We must avoid the cluttering of the "admin" settings page for "system" variables like this one, that are defined only once. (Perhaps we should create a special page with all variables in the table, if someone does not like to play with settings.php)

#22

jlwestsr - September 20, 2006 - 16:31

Proxy server should be a core based setting. For instance, here at Cingular Wireless all the servers are sitting behind a proxy server as well as the users. So anything that is hosted on this network is forced to access external links through a proxy server.

#23

rediguana - September 27, 2006 - 01:45

I need this functionality too. I have just finished dealing with support at my hosting company that have implemented a proxy server for security. This is only going to become a bigger issue as more hosting companies implement similar techniques to increase security of thier servers. Please, can we have a generalised solution - and may it not take 2 years to implement? Unfortunately, I'm not a coder so can't do it myself. I agree that it needs to be generalised not just for RSS feeds, but for any external communication by Drupal.

#24

Elfod - February 7, 2007 - 16:29
Version:x.y.z» 5.1

Made the changes described to a fresh install of Drupal 5.1 and it worked like a charm. Thanks ;-)

Would be better if it was just a config settings tho.

#25

dskennedy - February 20, 2007 - 13:40

@#24:

What did you do to get it working with 5.1? I've tried it with my fresh install here and it doesn't work, it fails with the following error:
The feed from BBC News: Education seems to be broken, due to " ".

I've attached the changes I made to my common.inc file.

AttachmentSizeStatusTest resultOperations
common-differences.txt1.82 KBIgnoredNoneNone

#27

caign - June 19, 2007 - 01:41

To get drupal_http_request() working with a proxy server which requires authentication, you need to add the username and password to the request, like this:

$request = $method . ' ' . $uri['scheme'] . '://' . $uri['host'] . $path . ' ' . "HTTP/1.1\r\n";
$request .= 'Proxy-Authorization: Basic ' . base64_encode("$username:$password") . "\r\n";

where you have set $username and $password appropriately of course.

#31

MWLimburg - February 14, 2008 - 23:36
Version:5.1» 6.0

+1 to this feature.

One of the greatest pushes for Drupal that I've seen has been behind proxies, using Drupal as a knowledge base or as an intranet. Some form of proxy awareness would be a godsend. In my instance, I am currently behind a NTLM proxy (which makes life real interesting on Linux) although I've managed to get NTLMaps set up so apt-get (etc, etc) can talk to the outside world. Some method to allow proxy details to be passed to Drupal (whether it's a pointer to the NTLMaps application, or to the main proxy itself) would allow us to restore a serious slab of functionally back to the installation.

I might also point out, that with Drupal6, I now have a permanent alert on my admin page, saying "One or more problems were detected with your Drupal installation. Check the status report for more information." ... why?

"Update notifications are not enabled. It is highly recommended that you enable the update status module from the module administration page in order to stay up-to-date on new releases. For more information please read the Update status handbook page."

So now, it's a security issue.

I feel that this issue is starting to grow in significance.

#32

catch - February 15, 2008 - 00:11
Version:6.0» 7.x-dev
Category:feature request» bug report

All issues are now fixed in 7.x, then backported if applicable.

#34

zxombie - February 20, 2008 - 01:40

I've attached a patch to common.inc to make drupal_http_request work through a proxy. I've allows the proxy host, port, username and password to be specified.

I don't have a proxy that required authentication so authentication has not been fully tested.

This is against Drupal CVS HEAD.

AttachmentSizeStatusTest resultOperations
drupal_intranet_proxy_7.x.diff2.05 KBIdleFailed: Failed to apply patch.View details | Re-test

#35

mooffie - February 22, 2008 - 14:09

Perhaps, instead of amending drupal_http_request(), we should make it plugable/replaceable?

We already do that with drupal_mail().

Every once in a while somebody discovers another deficiency in drupal_http_request() and suggests to improve it. I myself had to forgo drupal_http_request in a certain project, in favor of CURL, because I discovered it doesn't behave well in strained environments.

#36

thememex - February 22, 2008 - 18:48
Version:7.x-dev» 6.0

+1 on proxy settings.

I'd really like a place to edit in common.inc or through the Drupal interface.

I've been trying to demonstrate Drupal 6.0 inside the corporate network. We have a proxy firewall, so I'm unable to complete the setup with OpenID, Update Status, and RSS. I need to specify (unauthenticated) proxy access to [ip address]:8080.

Check out the post from agentrickard. He provided a link to an fsockopen example

Since the Update Status module is now in the core, and Drupal nags if it isn't on, I do not consider this a feature request. This is a bug. OpenID is now in the core and cannot work for many Intranet sites without an area to specify a proxy. RSS does work under the same environment as well. These individual module developers should not have to create their own proxy module, either.

#38

catch - February 23, 2008 - 23:19
Version:6.0» 7.x-dev

All changes get made to 7.x first.

#39

boydjd - February 28, 2008 - 09:19
Status:active» needs review

Added configuration options to the UI under Site Information. Probably needs to be tweaked some for wording and such.

AttachmentSizeStatusTest resultOperations
proxy.patch3.79 KBIdleFailed: Failed to apply patch.View details | Re-test

#40

boydjd - February 28, 2008 - 09:23
Title:Get me past my proxy server» Add support for intranet sites behind proxy servers

Probably be changing this tomorrow so that it adds a new link called Proxy Settings rather than throwing under Site Information, but it's a start.

#41

zilla - February 28, 2008 - 20:46

okay, i know nada about php, but i found this commentary in a discussion of this specific issue from a web host blocking http requests for security reasons and they offer these ideas:

1) Using the PHP environment variable $_SERVER['DOCUMENT_ROOT'], which returns the absolute path to the web root directory.
2) cURL is another method that could be used.

would option 1 be possible in the current common.inc file as a manual edit? and if so, how?

#42

zxombie - February 28, 2008 - 21:34

I've added changed the patch in 39 to include:
* Validation of the proxy port to be a number and in the range of valid tcp ports
* Use a valid port by default in the settings page
* Fix a bug in my original patch with the setting of $auth_string.

AttachmentSizeStatusTest resultOperations
proxy.patch4.41 KBIdleFailed: Failed to apply patch.View details | Re-test

#43

boydjd - February 28, 2008 - 23:48

@zxombie good catch.

Opinions on creating a new menu item under Site Configuration called "Proxy Settings"? Also, I'm thinking there should be an on/off option.

#44

boydjd - February 28, 2008 - 23:50

@zilla the fsockopen method used in this patch is much better than either of those two alternatives.

#45

boydjd - February 28, 2008 - 23:53

I'm thinking we should also allow the option of a persistent connection (via a checkbox), and use pfsockopen rather than fsockopen. On a dedicated server, it would make sense to use a persistent connection, which is probably the majority of sites that are stuck inside of an intranet.

#51

zxombie - March 3, 2008 - 20:58

@boydjd, I've updated the patch to create a "Proxy Server" page that allows setting the 4 proxy settings. I've added to the description a note that when the proxy host name is empty it will use a direct connection to the internet.

AttachmentSizeStatusTest resultOperations
proxy.patch8.94 KBIdleFailed: Failed to apply patch.View details | Re-test

#52

Slim Pickens - March 5, 2008 - 05:12

I've applied this patch in a 6.1 test site on my school webserver which is behind a firewall requiring proxy authentication. There were no errors reported.

The Proxy Settings form appears in Administer-->Site Configuration-->Proxy Server and accepts input.

But I'm still getting "HTTP request status Fails" from Status Report and the Aggregator module reports "" The server can't issue HTTP requests".

I appreciate the work that has gone into this, but I am also amazed that this is even still an issue. There must be countless educational/corporate intranet/webservers that must authenticate through an upstream proxy service.

As someone observed, this is not a feature request, it is a design flaw.

I hope this can be resolved sometime soon and built into core. 6.2 perhaps?

Cheers

#53

zxombie - March 9, 2008 - 23:04

@Slim Pickens, what method of authentication does the proxy server expect? The patch only implements basic authentication.

#54

MWLimburg - March 11, 2008 - 04:02

If you need this environment to support NTLM (which continues to be the bane of many of our existence), you will need to potentially review a solution that uses http://ntlmaps.sourceforge.net/ ... I use this on my Ubuntu machine at work to handle the NTLM Proxy so apt-get (etc) works.

#55

Slim Pickens - March 11, 2008 - 04:39

#53 zxombie - the proxy server expects a username and password. I thought the common.inc patch was creating that capability?

#56

Dries - March 15, 2008 - 12:25
Status:needs review» needs work

Hey guys -- I haven't tested this yet, but it looks reasonable to me. A couple of suggestions:

1. We don't capitalize each word in a sentence or title. Only the first word gets a capital letter. So things like 'Forward Proxy Settings' should become 'Forward proxy settings'.

2. Proxy settings are quite technical. It would be good if there was a little bit of help on the settings page. If my mom reads it, she should probably be able to determine that this is not a page she should make changes to.

3. What is an 'empty Drupal site'?

4. Add an entry to CHANGELOG.txt please.

Given some additional polish, it should be good to go.

#57

boydjd - March 17, 2008 - 18:17

I've re-rolled the patch so that it applies cleanly against head. Working on implementing Dries' changes.

AttachmentSizeStatusTest resultOperations
proxy_11.patch9.23 KBIdleFailed: Failed to apply patch.View details | Re-test

#58

tanakskool - March 26, 2008 - 16:42
Version:7.x-dev» 6.1
Assigned to:Anonymous» tanakskool
Status:needs work» active

Thanks, it works!

#61

boydjd - April 2, 2008 - 02:21
Version:6.1» 7.x-dev
Assigned to:tanakskool» Anonymous
Status:active» needs work

#70

Dries - April 19, 2008 - 22:23

'Forward Proxy Settings' should be 'Forward proxy settings'.

'the Internet' should be 'the internet'.

'Proxy Server' should be 'Proxy server'.

Etc.

#71

Crell - April 20, 2008 - 06:28

Actually "Internet" is a proper noun, and should therefore be capitalized.

#72

wad - April 27, 2008 - 18:41

I'm attaching a tarball of a module + core patch which enables the use of a http proxy in drupal 5.7. It's based almost entirely on proxy_11.patch at #57 above.

One exception though, TCP port 0 is reserved so I don't allow it.

AttachmentSizeStatusTest resultOperations
system_proxy-drupal-5.7.tar_.gz2.49 KBIgnoredNoneNone

#73

akolahi - May 28, 2008 - 15:33

Thank you for the module/core patch.... does this allow for the option to have only secure https requests to go through a proxy server? That feature would be awesome as it would allow Drupal to run on shared hosting plans (such as GoDaddy). GoDaddy requires https calls to run through s proxy server but not http.

Edit: While my initial post referenced the Drupal 5 patch, I think it would be of great value to separate the http and https calls through proxy options in Drupal 7 for the same reason. Some hosts require only secure https calls to be done through a proxy server.

#74

Lucict - May 31, 2008 - 15:55
Version:7.x-dev» 6.2

I edited my files with the proxy_11.patch above for version 6.2. It is currently working and solved my problems completely...THANK YOU to everyone who worked on this and provided this solution.

I have attached my common.inc, system.admin.inc, and system.module patched files in a zip for those who are unable to patch their files. Hopefully, this will work for you as well.

Thanks again.

AttachmentSizeStatusTest resultOperations
Patch.zip73.47 KBIgnoredNoneNone

#75

craigcarboni - June 1, 2008 - 19:37
Category:bug report» support request
Status:needs work» postponed (maintainer needs more info)

Any system that does not support NTLM/Digest authentication is just not ready for the Enterprise! Proxies have been around for along time. How come Mozilla Firefox gets it right?

#76

craigcarboni - June 3, 2008 - 16:45

Suggestion for Proxies that require Digest/NTLM authentication.
If the proxy you are authenticating against requires than try NTLMAPS from sourceforge.
NTLMAPS works in Linux as well as Windows or any OS for that matter as it require Python.
I have used it in Ubuntu and Windows XP and it works well.

#77

zxombie - June 4, 2008 - 04:51
Version:6.2» 7.x-dev
Category:support request» bug report
Status:postponed (maintainer needs more info)» needs work

This is not a support forum. This needs to go into D7 first.

#80

Rob Loach - June 24, 2008 - 13:34

Subscribing....

#81

kaneod - June 27, 2008 - 05:28
Version:7.x-dev» 6.2
Status:needs work» needs review

Hi all,

I'm new here so sorry if I tread on toes - just wanted to note that the patch in #57 *almost* works behind our basic-authentication proxy at the University of Newcastle for Drupal 6.2. Common.inc is not quite right though and needs to be modified slightly as given in the attached patch. I now have proper update and RSS functionality through the proxy as far as I can tell, though more testing is obviously required.

I also can't confirm that this works for 7.x-dev as I'm not actively involved with HEAD.

AttachmentSizeStatusTest resultOperations
proxy.patch2.31 KBIdleFailed: Failed to apply patch.View details | Re-test

#82

Rob Loach - June 27, 2008 - 18:16
Version:6.2» 7.x-dev

Very cool, I'd love to test it, but I haven't encountered the need of a proxy server. New features have to go into Drupal 7, and then back ported to Drupal 6. It's wonderful to get as many patches and tests out there as possible though, so thanks! If I get the chance, I'll try porting this to HEAD.

#83

brahms - July 8, 2008 - 20:50

@boydjd:

I think I found following problem with the patch from your comment#57: your modification of drupal_http_request would by itself work fine in my site (which is behing an ISA proxy with basic authentification) if there wasn't the call for system_check_http_request at the beginning. Here Drupal checks if it is able to load a local page per HTTP request. The relative url for this check is 'admin/reports/request-test' and gets completed to an absolute url (like 'http://www.example.com/admin/reports/request-test' if 'http://www.example.com' is the url of the Drupal Site). This check is now done over the modified drupal_http_request function and it fails in my site because it tries to make the request over the proxy I configured before in the new proxy settings. And because this check fails, drupal_http_request returns with an error and does not try further requests.

So the self_test (system_check_http_request) has to be a direct request on my site and I think on every site behind a proxy to be successful.

To achieve this behaviour I added the parameter No proxy for to the proxy settings. It works like the same parameter in Firefox. If you define something like 'example.com' as "No proxy for", then every request to a site which has the string 'example.com' in it's url is requestet directly, not over the proxy.

With the additions in the patch (against Drupal 7 head) I provide here I could use update status and the feed aggregator behind the proxy.

AttachmentSizeStatusTest resultOperations
drupal-7881-83.patch10.29 KBIdleFailed: Failed to apply patch.View details | Re-test

#85

brahms - August 17, 2008 - 16:45

Hi asilva,

here is the patch from comment#83 above for D6.3 and D6.4. It works for my site behind an ISA proxy server with authentification. On another drupal site I had still troubles with this patch. There I added some code to switch off Drupal's HTTP self test to make the patch work. Tell me if the patch here works for you. If it doesn't I will provide the other patch with the disabled HTTP self test.

To apply the patch for D6.3 please copy the file drupal-6.3-proxy.patch into your Drupal base directory and apply it with the command: patch -p0 < drupal-6.3-proxy.patch.

The 2nd file drupal-6.4-proxy.patch is the same patch as patch file for D6.4

AttachmentSizeStatusTest resultOperations
drupal-6.3-proxy.patch9.96 KBIdleFailed: Failed to apply patch.View details | Re-test
drupal-6.4-proxy.patch9.96 KBIdleFailed: Failed to apply patch.View details | Re-test

#86

Rob Loach - August 18, 2008 - 13:24
Status:needs review» needs work

Four things in response to the patch at #83:

  1. I'm not quite sure how I feel about a is_in_no_proxy_list() function as it isn't really that descriptive. drupal_in_proxy_list() maybe?
  2. I'm also not sure how I feel about foreaching through the list whenever the drupal_http_request function is called, but can't really think of a cleaner alternative.
  3. $output .= '<li>'. t('basic configuration options for .... The space before and after the period is part of Drupal 7 coding standards ;-) .
  4. Is there anywhere else that the proxy settings could live? They seem pretty small for just one page.

#88

brahms - August 19, 2008 - 20:47

Hi Rob and thanks for your response.

I'm not quite sure how I feel about a is_in_no_proxy_list() function as it isn't really that descriptive. drupal_in_proxy_list() maybe?

I suggest to rename the function is_in_no_proxy_list() to drupal_is_proxy_exception(). But I'm not sure if it's good to use the suffix drupal_ for this internal helper function. Wouldn't it be better to name it _is_proxy_exception() instead?

2. I'm also not sure how I feel about foreaching through the list whenever the drupal_http_request function is called, but can't really think of a cleaner alternative.

I agree with your concern. Maybe it would be a better alternative to use an additional variable proxy_enabled which would be represented by a new checkbox labeled "Use proxy server" in the proxy settings page? Then the foreaching through the list would only be necessary when "Use proxy server" is checked.

3. $output .= '<li>'. t('basic configuration options for .... The space before and after the period is part of Drupal 7 coding standards ;-) .

Shame on me, I used ancient coding convention for string concatenations (Drupal 4 I think...). I'll adjust the syntax in my next patch file.

4. Is there anywhere else that the proxy settings could live? They seem pretty small for just one page.

You are right, but looking at D7's site configuration menu I find some other small settings pages like "Clean URLs", "File system", "Image toolkit" or "Error reporting". On the other hand I don't feel that the proxy settings would match with any of them from a topical point of view. So I suggest to keep the proxy settings on their own page for now. Maybe in the future someone will collect all those small pages into one or more bigger pages?

Please tell me what you think of my suggestions, so I will be able to modify my patch.

#89

brahms - August 19, 2008 - 12:23

Hi rizamp,

here is a patch for D6.4 with the disabled self test. But in the meantime I found a better replacement for the problem I had on one of my sites behind a proxy server. I had to enable the reverse_proxy setting in settings.php:

$conf = array(
   'reverse_proxy' => TRUE,
);

With this setting the http self test workes fine for me. please try to enable the reverse_proxy setting, before you apply the new patch and disable the http self test (by checking the checkbox "Skip HTTP self test" at the end of the proxy settings page).

If the proxy patch still does not work on your site and your proxy server regards authentification: be sure that your proxy server accepts "basic authentification method". If he does not allow basic authentification but only allows other methods like Digest, Radius or NTLM the proxy patch won't work. It works only for basic authentification (or if no proxy authentification is required at all).

AttachmentSizeStatusTest resultOperations
drupal-6.4-proxy-skip_selftest.patch11.25 KBIdleFailed: Failed to apply patch.View details | Re-test

#90

Rob Loach - August 20, 2008 - 02:54

Another thing we have to think about is testing. Is there anyway to create an effective SimpleTest for this for self-testing?

#91

Rob Loach - August 20, 2008 - 04:08
Title:Add support for intranet sites behind proxy servers» Add support for proxy servers
Status:needs work» needs review
  • Removed the is_in_no_proxy_list entirely for a stripos call
  • Fixed some strings that didn't seem that helpful
  • Added hook_help text
  • Fixed some of the minor logic which looked kind of weird (variable scope issues)
  • Replaced != '' with calls to empty()
  • Removed the fieldset in the settings, following what they have for Clean URLs, and other settings with just straight textfields
  • Logic in the validation wasn't optimal

To test:

  1. Apply patch, clear cache
  2. Visit admin/settings/proxy and enter your proxy settings
  3. Make a PHP call using drupal_http_request
AttachmentSizeStatusTest resultOperations
7881.patch10.26 KBIdleFailed: Failed to apply patch.View details | Re-test

#92

Rob Loach - August 20, 2008 - 04:15
Category:bug report» feature request

... And I'd consider this a feature.

#93

markus_petrux - August 20, 2008 - 04:37

Hi,

This is interesting feature, however I believe the word "proxy" used here might be confusing with different kinds of proxies. For instance, default/settings.php use "reverse_proxy" variables for something different.

I would suggest to be a bit more explicit and use "forward_proxy" or something similar as prefix for variables, etc.

#95

brahms - August 20, 2008 - 09:47

Hi Rob,

your fixes look good and make the code cleaner. But after applying your patch from comment#91 I get following error when I access my site:

Fatal error: Can't use function return value in write context in /var/www/dev/d7head/includes/common.inc on line 445

The reason for this is that empty() only checks variables as anything else will result in a parse error. In other words, the following will not work: empty(trim($name)) or - like it is done in common.inc - !empty(variable_get('proxy_server', '')) and !empty(variable_get('proxy_username', '')).

#97

Rob Loach - August 20, 2008 - 23:42
Status:needs review» needs work

Hi subru77, unfortunately this can't be implemented through a module because it requires modifying the drupal_http_request function. We could implement a hook in drupal_http_request to expose how HTTP requests are made. Then we could create add on module that could make different kinds of requests, like HTTP requests, HTTPS requestions, FTP requests, etc. But that would have to be thought about, and still wouldn't work if we required a proxy in from of the request..... Thoughts?

Thanks for taking a look at it, brahms. Is that the only issue you saw with it?

#98

brahms - August 22, 2008 - 12:18

Hi Rob,

I'm not shure if the two places in code with empty(variable_get()) are the only issues. I just tested the code (with fixes for the empty issue) on my site behind an ISA proxy. First it didn't work because the variable drupal_http_request_fails had been set to 1 before. This caused a call to the self test (function system_check_http_request) and the self test always seems to fail on my site. The reason for this may be the special network configuration I have to use to reach my customer's site. I am connecting to the web server over an SSH tunnel and there is a network address translation configured between the client I use and the web server.

If I set the variable system_check_http_request) to 0 or delete the variable everything works fine. Starting the update status check I am able to connect from my site to the drupal.org update server.

So I think the patch itself works fine. But I am not sure if Drupal's http self test is working from behind a proxy server. (This is why I tried a piece of code which turns of the self test as I provided in my comment#89) In the next days I will visit my customer with the affected Drupal site. There I'll try to check the self test in a network environment without the SSH tunnel and without the NAT.

EDIT:
I just returned from a visit at my customer. I had enough time there to test the code and everything worked fine including the http self test. For me - and much more: for Drupal - it would make sense if your code from comment#91 will make it's way into core after the empty issue has been fixed.

#99

Dries - August 23, 2008 - 07:10

Yep, the !empty() is broken. This patch looks close though. :)

#100

yellek - September 10, 2008 - 02:25

A workaround for those not comfortable patching Drupal is to use cntlm (http://cntlm.sourceforge.net/) and then using apache to rewrite the URL to point to the cntlm proxy. You then need to alter the URL of what you are looking for to point to your local box. I have posted an example of configuring update module to authenticate with an IIS proxy at http://drupal.org/node/172708#comment-1003511.

#101

arhak - September 16, 2008 - 05:52

subscribing

#102

arhak - September 16, 2008 - 07:54

same as #91 backported to Drupal 6.4

  • corrects the diff context respect Drupal 6.4
  • corrects the empty calls to variables
  • adds one line to make a callback be valid
AttachmentSizeStatusTest resultOperations
7881-Drupal-6.4.patch10.04 KBIdleFailed: Failed to apply patch.View details | Re-test

#103

dankinon - September 25, 2008 - 00:06

I applied the patch provided in post #102 to a fresh drupal-6.4 installation without issue (thank you for that). After configuring my proxy server I still can't check for updates. I am now getting the HTTP Request Self-Test error. I followed the instructions in post #89 to enable reverse_proxy, that didn't work for me either. I'm going to attempted to patch drupal with the patch from post #89 (drupal-6.4-proxy-skip_selftest.patch) but I have to mess with it first. In the mean time can someone answer these questions:

- Since I'm getting the Self-Test error should my update checks still be failing?

- Does someone have another solution to the Self-Test problem.

- It seems like most people applied the patch (#91 or #102) like I did and were able to get updates. Is this true or did the rest of you have to do something more to get it to work? The reason I ask is I have been hitting my head on this for 2 weeks and can't get it to work.

Thanks

#104

Rob Loach - September 25, 2008 - 00:53
Status:needs work» needs review

I'm having second thoughts on this. Instead of adding this straight into core, would we be able to create some kind of hook that would manage altering the request so that if we wanted a proxy contrib module to handle this, it could?

Anyway, here's a patch with the !empty() fix along with an added user permission (administer proxy configuration) because you don't want to give your administrator user team permission to see your proxy server password......

AttachmentSizeStatusTest resultOperations
7881.patch10.96 KBIdleFailed: Failed to apply patch.View details | Re-test

#105

arhak - September 25, 2008 - 08:10

is this for D5/6/7?

I'm having second thoughts on this. Instead of adding this straight into core, would we be able to create some kind of hook that would manage altering the request so that if we wanted a proxy contrib module to handle this, it could?

+1 of course!!!

#107

mooffie - September 25, 2008 - 08:37

I'm having second thoughts on this. Instead of adding this straight into core, would we be able to create some kind of hook that would manage altering the request so that if we wanted a proxy contrib module to handle this, it could?

+1

I suggested this in comment #35. On the other hand, I see people are feeling very strongly that this proxy thing should be in core.

====

It will be interesting to see how this plug/hook mechanism will be implemented. It will require only few lines of code, but nevertheless we should "standardize" the way we "plug" custom code into Drupal. I'll have to do the same for format_date() (to enhance i18n support). drupal_mail() too has a "plug" mechanism and it could be replaced with the one we finally settle on.

#108

arhak - September 25, 2008 - 20:48

about the mail there is some work already done http://drupal.org/project/smtp, but it doesn't seems to be proxy aware

#110

Dries - September 27, 2008 - 21:00

I'd prefer to have this in core, but I can be convinced (as always).

If someone can confirm that this solution works, I'm ready to commit it to core.

We should probably add a CHANGELOG.txt entry though.

#111

brahms - September 28, 2008 - 19:09

I can confirm that the most recent patch from Rob in comment#104 works in following environments:

  • Drupal 6.4 Site behind a proxy server without authentification
  • Drupal 6.4 Site behind a proxy server which allows basic authentification

(I tested those Drupal 6.4 Sites with a back port of the patch, because I didn't have the chance to install Drupal 7 in those environments. I include this backported patch here as g-zipped file for Drupal 6.4).

Though there are proxy configurations where the solution will not work (if there is another authentification method than basic authentification required) I think having the given solution here in core is an important step for bringing Drupal to enterprise environments. Nevertheless we should continue to work on solutions for the other authentification methods, but this should go into a new issue.

AttachmentSizeStatusTest resultOperations
7881-drupal-6.4.patch.gz3.17 KBIgnoredNoneNone

#112

arhak - September 28, 2008 - 22:23

I tested #91 backported to D6 in #102 on Drupal 6.4 behind a proxy server without authentification

#113

Slim Pickens - September 30, 2008 - 06:20

I vote for inclusion in core. I work in public education. Our State department has more than 1600 schools - all of them are obliged to use a proxy server for internet access. The other 5 Australian States education departments would operate similarly.

Surely a proxy server is the norm for corporate environments? Proxy setup should be a part of the site configuration through core. If you don't need it, don't configure it. We're talking a few lines of code here. No biggie.

Cheers

#114

caign@drupal.org - September 30, 2008 - 06:33

Include in core.

A major wish and focus of mine is to get Drupal better recognised by large corporations and government. This is not going to happen without "thinking corporate" right out of the box.

#115

Rob Loach - September 30, 2008 - 07:32

The patch at #104 still applies.

#116

Dries - October 2, 2008 - 02:15

Did someone test #104?

#117

keith.smith - October 2, 2008 - 02:52
Status:needs review» needs work

On a quick code style and string review, there are several concatenation operators that aren't set off with spaces, plus several code comments not ending in a period. Also, one of the things we need to clean up (in another patch) is the variety of ways we show examples. This patch uses "eg.". Elsewhere in code we have "e.g.," and "Example: xxxx" and "Example: xxx". As long as this patch is internally consistent and hits one of the existing styles it should be fine, especially since there are other issues in the queue to conform all these to a single standard.

What about:

+    '#description' => t('The host name of the proxy server, eg. localhost. If this is empty, it will be assumed that there is already an existing connection to the Internet.')

The host name of the proxy server. Leave empty if your site has an existing connection to the Internet or does not use a proxy server. Example: localhost.

+    '#description' => t('The password used to connect to the proxy server. This is kept as plain text.', '')

The password used to connect to the proxy server, if necessary. Warning: This password is stored as plain text.

+    '#description' => t("A comma seperated list of sites that don't require use of the proxy server.")

"comma seperated" should be "comma-separated".

+    'administer proxy configuration' => t('Configure if connecting to the Internet requires a proxy connection.'),
+    'description' => 'Configure settings when the site is behind a proxy server.',

These could just be "Configure proxy server settings."

I can roll some of these changes into a new patch tomorrow if nobody beats me to it.

#118

lilou - October 7, 2008 - 00:09
Status:needs work» needs review

Patch #104 modified according comment #117.

AttachmentSizeStatusTest resultOperations
issue-7881-118.patch11.28 KBIdleFailed: Failed to apply patch.View details | Re-test

#121

Dries - October 9, 2008 - 20:06
Status:needs review» needs work

Doesn't seem to apply anymore for me ...

#123

jcwatson11 - October 10, 2008 - 02:07
Version:7.x-dev» 6.5
Status:needs work» needs review

The attached patch works on my installation of Drupal 6.5. Please review. Looks like 2 hrs, 33 minutes. A new record! Hurraaahhh!

AttachmentSizeStatusTest resultOperations
issue-7881-123.patch9.91 KBIdleFailed: Failed to apply patch.View details | Re-test

#124

Gos77 - October 10, 2008 - 05:05

patch #123 patch without errors for my 6.5, but in status report HTTP request status fails and check manually Drupal core update status also fails. Proxy settings are correct without auth.

patch #123 removes slowing down from update-status module behind a proxy.

Has someone other infos?

#125

brahms - October 10, 2008 - 09:54

@Gos77:

Just some things you can try to make the proxy settings work on your site:

reverse proxy settings
I had to enable the reverse_proxy setting in settings.php to make HTTP connections work on my site.
Drupals HTTP Self Test and the variable drupal_http_request_fails
When the HTTP self test in Drupal fails, Drupal sets the variable drupal_http_request_fails to 1. This is the intended Drupal behaviour and it was the case on my site before I applied the proxy patch. Then I applied the patch but though the proxy patch itself would work, update status still failed because the self test did not reset the variable to 0. I had to reset the variable by SQL (setting the column value to b:0; in the variable table) to make the proxy connection work. An alternative way is to delete the variable drupal_http_request_fails from the table. (This is no bad hack because Drupal itself writes this variable into the table once the self test fails. If Drupal's self test on your site never had a problem, there is also no row for the variable drupal_http_request_fails in the table.)

#126

mot - October 12, 2008 - 12:45

As in #125 me tends to add the remark to think about stuff like this: if the self-test request fails, drupals internal logic will ZAP any further http request. Adding a proxy in this whole drupal-http logic is not only imlementing it the technical way. The whole dataflow needs to be thought of. I would love to see proxies out of drupal therefore, it makes configuration too complex. A reverse proxy transparently configured somewhere else can do the job as well.

Instead of this, an own connection and transport class for php5 based druapl version(s) might lead to a more and propper way to solve such things. in this case, you could switch to a simple http proxy transport class instead of the default http one.

#127

Gos77 - October 14, 2008 - 06:53

works for my blank 6.5 with patch from #123 and hint #125 .. thx

#128

mot - October 14, 2008 - 07:22

Just reviewd the patch. Looks OK for me so far (6.5 patch). Right now I still have no Idea to solve the problem with a sleaner integration if the http self test fails. I had self test fails with DNS problems but they recovered automatically. Why doesn't this happen here. Any Idea brahms, Gos77?

#131

jcwatson11 - October 16, 2008 - 00:50

subscribe

#132

boydjd - October 19, 2008 - 06:56
Version:6.5» 7.x-dev

This is a feature request, and as such, goes to HEAD, not 6.x.

Looks like the patch doesn't apply to HEAD cleanly anymore, I'll re-roll when I find a spare moment. :)

#133

boydjd - October 19, 2008 - 06:56
Status:needs review» needs work

Changing to CNW, since it doesn't apply anymore.

#135

Crell - October 23, 2008 - 16:19

No, new features go only against the dev version, regardless of how long they've been requested. Sometimes people maintain backport patches of them, but they do not go into core. Of course, this has to get committed to Drupal 7 first.

#136

mooffie - October 23, 2008 - 16:46

Why not use cURL instead of implementing it ourselves? As a bonus, we'll get its 'timeout' feature (it's good for Aggregator).

For Drupal 7, we could define a Drupal::HTTP interface. The default implementation will be the current one. A 'cURL' module, when enabled, will plug in its own implementation.

#137

silid - October 24, 2008 - 09:40

without getting into the politics of whether or not to backport here is my patch for 6.6 - it works for me, others may find it useful as i found there patches extremely helpful.

AttachmentSizeStatusTest resultOperations
proxy6.6.patch7.84 KBIdleFailed: Failed to apply patch.View details | Re-test

#147

silid - January 5, 2009 - 12:19

Now I am back at work I have attached a patch file for Drupal 6.8 - I hope this helps anyone who needs it.

Nitin - have you tried anything else that requires proxy such as checking for module updates - does that work?

AttachmentSizeStatusTest resultOperations
proxy6.8.patch7.78 KBIdleFailed: Failed to apply patch.View details | Re-test

#149

Dries - January 11, 2009 - 09:41

#151

silid - January 16, 2009 - 10:17

There have been some changes in 6.9 that affect the patch. Try this version, it works for me.

AttachmentSizeStatusTest resultOperations
proxy6.9.patch6.83 KBIdleFailed: Failed to apply patch.View details | Re-test

#152

roball - January 22, 2009 - 00:19

I can confirm the 6.9 proxy patch to work fine with D6.9. Thank you silid!

#153

Rob Loach - January 25, 2009 - 21:49

If we got this into Drupal 7, a patch for Drupal 6 would be very easy. As a side note, #64866: Pluggable architecture for drupal_http_request() would be helpful to stick proxy support in contrib.

#154

borzoj - February 17, 2009 - 11:28

Hi everyone!

The patch outlined in the original post didn't work on our proxy. We needed to leave this bit that the author advised to comment out:
$request .= implode("\r\n", $defaults);
With this line commente dproxy complained about missing content-length header. With the line in it works.
Cheers,
Michal

#155

minorOffense - February 17, 2009 - 14:59

Subscribing

#158

silid - February 26, 2009 - 13:23

Here is a patch for 6.10

I haven't tested this so I don't know that it works. Please give feedback

AttachmentSizeStatusTest resultOperations
proxy6.10.patch6.83 KBIdleFailed: Failed to apply patch.View details | Re-test

#159

Gos77 - February 26, 2009 - 15:19

patch from #158 working fine for me for D6.10

only getting (Stripping trailing CRs from patch.) for all three patched files while patching.

#161

Crell - February 26, 2009 - 16:17
Priority:normal» critical

Feature additions do not get committed to stable branches, unless it is necessary to close a security hole. Getting this into Drupal 7, however, is something that really does need to happen. Can we focus on that? (Even if it were to go into D6, it won't until it's landed in D7. Focus your efforts there.)

#162

roball - February 26, 2009 - 20:47

Crell, I know that this is the rule - but there is no rule without exceptions. For example, see http://drupal.org/node/243524 and http://drupal.org/node/276174 that now went into 6.10 without being a security fix.

This issue is in fact a security fix because the lack of proxy support breaks the Update status module on servers behind a proxy, with the consequence that it no longer warns you about essential security updates. It's not an option that some may want to enable because it's a fancy feature - it's essential to work behind a proxy. Without proxy support, all those servers are locked away from using Drupal 6 at all. So I don't see this patch as a new feature, but rather as a critical bug fix for the Update status, Aggregator and other modules to work.

The D6 patch is working excellent - what's so hard in porting it to D7?

#166

silid - March 18, 2009 - 14:18
Status:needs work» needs review

This patch has been made

AttachmentSizeStatusTest resultOperations
proxy_7.x-01.patch6.18 KBIdleFailed: 10503 passes, 32 fails, 0 exceptionsView details | Re-test

#167

System Message - March 18, 2009 - 14:35
Status:needs review» needs work

The last submitted patch failed testing.

#168

Rob Loach - March 18, 2009 - 16:22

Missing the is_in_no_proxy_list() function?

#172

mabland - April 20, 2009 - 17:19

Patch for 6.10 works great for me for http requests. But it appears that https requests are not proxified so any module that uses https, such as OpenID, does not work - request times out.

#173

silid - April 21, 2009 - 07:53

Hmm. That appears to be true. I must say that I have never needed to access https by proxy and so didn't realise, but looking at the patch it doesn't do anything to https requests at all.

So in other words, it isn't broken it just hasn't ever done it. I will have a look at the patch when I get time but feel free to submit your suggestion.

Si

#176

silid - May 1, 2009 - 13:07
AttachmentSizeStatusTest resultOperations
proxy6.11.patch6.83 KBIdleFailed: Failed to apply patch.View details | Re-test

#177

chinko - May 12, 2009 - 12:56

I applied the 6.11 patch in #176 and I have the 'available updates" report working for the very first time on a PC and Drupal servers in our company. Thanks for all the great work.

However our team is debating whether we should start the precedence of patching the core. If we do, when we upgrade to future 6.x releases, we will have to keep patching (either relying on someone releasing a patch or we create the patch ourselves).

Proxy support is essential in a corporate environment. D7 supports proxy but it is still a long way away.

Not being able to check "available updates" is a security risk. Including this patch into the next 6.12 release as a security fix will make the life of many corporate Drupal users easier and is another step towards getting Drupal wider acceptance as an enterprise CMS.

Thanks.

#181

maykelmoya - May 19, 2009 - 22:41

This is 6.11 patch updated for 6.12.

AttachmentSizeStatusTest resultOperations
proxy6.12.patch7.22 KBIdleFailed: Failed to apply patch.View details | Re-test

#183

robdinardo - May 22, 2009 - 19:29

Attached file contains the three files to add proxy support to D 6.12

AttachmentSizeStatusTest resultOperations
drupal-proxy-6.12.zip75.27 KBIgnoredNoneNone

#185

chx - June 3, 2009 - 23:28
Priority:critical» normal

I am closing this issue and deleting every "I tried to patch" comment from it and then reopen.

#186

chx - June 3, 2009 - 23:50

Please focus on getting a working patch for HEAD (http://drupal.org/node/320). Also, please refrain from posting questions regarding 'how to apply patches' and 'why this is not in Drupal N". Those who do will be rewarded with a two week ban.

#187

Damien Tournoud - June 4, 2009 - 00:12

I've done my best to ignore the horrible // PROXY-HACK comments, so here is a review of the patch in #181.

<?php
+      $proxy_not_required = is_in_no_proxy_list($uri['host']);
+      if ((
variable_get('proxy_server', '') != '') && (FALSE == $proxy_not_required)) {
+       
$proxy_server = variable_get('proxy_server', '');
+       
$proxy_port = variable_get('proxy_port', 8080);
+       
$fp = @fsockopen($proxy_server, $proxy_port, $errno, $errstr, 15);
+      }
+      else {
      
$fp = @fsockopen($uri['host'], $port, $errno, $errstr, 15);
+      }
?>

Drupal doesn't use value-as-first-component comparisons. You can simply rewrite that as $proxy_not_required && ($proxy_server = variable_get('proxy_server', ''))

<?php
      
break;
     case
'https':
      
// Note: Only works for PHP 4.3 compiled with OpenSSL.
?>

No support for HTTPS (via the CONNECT proxy command)? It seems like this could be useful in several use cases I can think about.

<?php
+  if ((variable_get('proxy_server', '') != '') && (FALSE == $proxy_not_required)) {
+   
$path = $url;
+  }
+  else {
  
$path = isset($uri['path']) ? $uri['path'] : '/';
   if (isset(
$uri['query'])) {
    
$path .= '?'. $uri['query'];
   }
+  }
?>

This indentation is wrong, and same remark as above for the condition.

<?php
+  if ((variable_get('proxy_username', '') != '') && (FALSE == $proxy_not_required)) {
+   
$username = variable_get('proxy_username', '');
+   
$password = variable_get('proxy_password', '');
+   
$auth_string = base64_encode($username . ($password != '' ? ':'. $password : ''));
+   
$defaults['Proxy-Authorization'] = 'Proxy-Authorization: Basic '. $auth_string ."\r\n";
+  }
?>

Same remark.

<?php
+function is_in_no_proxy_list($host) {
$rv = FALSE;

$proxy_exceptions = variable_get('proxy_exceptions', '');
+  if (
FALSE == empty($proxy_exceptions)) {
+   
$patterns = explode(",",$proxy_exceptions);
+    foreach (
$patterns as $pattern) {
+     
$pattern = trim($pattern, " ");
+      if (
strstr($host,$pattern)) {
+       
$rv = TRUE;
+        break;
+      }
+    }
+  }
+  return
$rv;
+}
?>

The function is poorly named and it is not documented.

The condition (that uses type-agnostic comparison on the result of strstr()) is severely broken: it would allow only partial matches that do not start at the first characters (ocalhost will match localhost, but localhost will not match localhost).

We probably want this match to be either on the hostname or on the IP address. This is not possible right now.

<?php
+/**
+ * Form builder; Configure the site proxy settings.
+ *
+ * @ingroup forms
+ * @see system_settings_form()
+ */
+function system_proxy_settings() {
+
$form['forward_proxy'] = array(
+   
'#type' => 'fieldset',
+   
'#title' => t('Forward Proxy Settings'),
+   
'#description' => t('The proxy server used when Drupal needs to connect to other sites on the Internet.'),
+  );
$form['forward_proxy']['proxy_server'] = array(
+   
'#type' => 'textfield',
+   
'#title' => t('Proxy host name'),
+   
'#default_value' => variable_get('proxy_server', ''),
+   
'#description' => t('The host name of the proxy server, eg. localhost. If this is empty Drupal will connect directly to the internet.')
+  );
$form['forward_proxy']['proxy_port'] = array(
+   
'#type' => 'textfield',
+   
'#title' => t('Proxy port number'),
+   
'#default_value' => variable_get('proxy_port', 8080),
+   
'#description' => t('The port number of the proxy server, eg. 8080'),
+  );
$form['forward_proxy']['proxy_username'] = array(
+   
'#type' => 'textfield',
+   
'#title' => t('Proxy username'),
+   
'#default_value' => variable_get('proxy_username', ''),
+   
'#description' => t('The username used to authenticate with the proxy server.'),
+  );
$form['forward_proxy']['proxy_password'] = array(
+   
'#type' => 'textfield',
+   
'#title' => t('Proxy password'),
+   
'#default_value' => variable_get('proxy_password', ''),
+   
'#description' => t('The password used to connect to the proxy server. This is kept as plain text.', '')
+  );
$form['forward_proxy']['proxy_exceptions'] = array(
+   
'#type' => 'textfield',
+   
'#title' => t('No proxy for'),
+   
'#default_value' => variable_get('proxy_exceptions', 'localhost'),
+   
'#description' => t('Example: .example.com,localhost,192.168.1.2', '')
+  );
$form['forward_proxy']['proxy_skip_selftest'] = array(
+   
'#type' => 'checkbox',
+   
'#title' => t('Skip HTTP self test'),
+   
'#description' => t('Skip HTTP request self test.'),
+   
'#default_value' => variable_get('proxy_skip_selftest', '0'),
+  );
$form['#validate'][] = 'system_proxy_settings_validate';
+
+  return
system_settings_form($form);
+}
?>

This form should use the new autodefault feature of system_settings_form() in D7.

<?php
+/**
+ * Validate the submitted proxy form.
+ */
+function system_proxy_settings_validate($form, &$form_state) {
// Validate the proxy settings
$form_state['values']['proxy_server'] = trim($form_state['values']['proxy_server']);
+  if (
$form_state['values']['proxy_server'] != '') {
+   
// TCP allows the port to be between 0 and 65536 inclusive
+    if (!is_numeric($form_state['values']['proxy_port'])) {
+     
form_set_error('proxy_port', t('The proxy port is invalid. It must be a number between 0 and 65535.'));
+    }
+    elseif (
$form_state['values']['proxy_port'] < 0 || $form_state['values']['proxy_port'] >= 65536) {
+     
form_set_error('proxy_port', t('The proxy port is invalid. It must be between 0 and 65535.'));
+    }
+  }
+}
?>

$form_state['values']['proxy_server'] != '' should be strlen($form_state['values']['proxy_server']) > 0, or "0" will not match. The two error messages should be collapsed in one.

#188

FrankT - June 18, 2009 - 16:46

Could anyone give me a hint where I have to enter the settings of the proxy server (IP, Port, maybe authentication) with proxy6.12.patch? I searched the issue with no idea on that...

#189

roball - June 19, 2009 - 08:29

Administer -> Site configuration -> Proxy Server (admin/settings/proxy)

#190

roball - July 2, 2009 - 17:59

Sorry to ask here, but does the 6.x patch also work with Drupal 6.13?

#191

Gos77 - July 3, 2009 - 09:02

Hi roball .. patch from #181 works fine for my 6.13 installation.

#192

silid - July 3, 2009 - 09:19

As requested here is a patch for 6.13 - I haven't reviewed the suggestions or made any amendments to the 6.11 patch I last uploaded except to apply it to 6.13.

As mentioned previously I haven't written the code included here but have produced the patch from other code submitted in the thread to help other users.

The 'horrible // PROXY-HACK comments' are to make it easier for me to roll the patch from one version to the next.
It doesn't use any D7 feature because it is a patch for D6.

I haven't yet upgraded my site to 6.13 so this is untested but I see no reason why it shouldn't work. When I get some time I may try to take all feedback into consideration but that won't be soon.

AttachmentSizeStatusTest resultOperations
proxy6.13.patch6.76 KBIdleFailed: Failed to apply patch.View details | Re-test

#193

roball - July 5, 2009 - 08:41

Thanks a lot silid, your patch works fine with 6.13 :-)

#194

Damien Tournoud - July 5, 2009 - 08:46
Status:needs work» won't fix

As far as I understand, nobody wants this feature to go into D7. Marking as won't fix accordingly.

If you do want this to go into Drupal 7, please reroll a patch taking into account my comments in #187 and previous remarks by others.

#195

silid - July 6, 2009 - 12:15
Status:won't fix» needs work

It will get the work when there is time. This thread is full of people that want this feature.

#196

Damien Tournoud - July 11, 2009 - 16:48
Status:needs work» won't fix

Until we have a proper patch for this, please let the status at "won't fix".

#197

roball - July 11, 2009 - 20:53
Status:won't fix» needs work

#198

Damien Tournoud - July 11, 2009 - 21:12
Status:needs work» won't fix

Still waiting for a proper patch.

#199

silid - July 11, 2009 - 23:25
Status:won't fix» needs work

The handbook says that 'needs work' is defined as:

The patch needs additional work before it should be reviewed. The author of the patch may indicate that it is incomplete, or the patch has gone through the reviewing process and has received feedback about areas where it needs improvement.

where as 'won't fix' is defined as:

It has been decided that this issue will not be fixed. This includes feature requests that are deemed to be outside the scope of the project, and bug reports that cannot be reproduced or are unsupported.

By your own comments you have described it as a 'needs work', but for some reason you don't want it to say 'needs work' until the work is done. See http://drupal.org/node/156119 for details.

#200

Crell - July 13, 2009 - 02:23

I believe the point is that since no one is trying to make a Drupal 7 version of this patch, that must mean that "it has been decided that this issue will not be fixed". If it was going to be fixed, someone would have taken then 20 min to make a Drupal 7 version of it rather than rerolling a Drupal 6 version forever, which is a dead end.

Take-away: Work on getting better proxy support into Drupal 7 and stop wasting time on anything else. This thread is too long already. Those who keep rolling Drupal 6 versions in this issue, I'm looking at you...

#201

ghoti - July 13, 2009 - 05:50

subscribe

#202

David Strauss - July 13, 2009 - 05:53
Title:Add support for proxy servers» Add support to drupal_http_request() for proxy servers

Who actually needs this feature?

Also, re-titling to reflect that this is for upstream proxies, not downstream (like Varnish in front of Drupal).

#203

roball - July 13, 2009 - 05:59

Are you kidding?

Thousands of people who want to/must run Drupal on a server behind a proxy *needs* it.

#204

Roavei - July 13, 2009 - 23:31

subscribe

#205

glowrocks - July 16, 2009 - 16:48

I'm confused. I run drupal behind a firewall and need to use a proxy. Is this hard to understand?

Many many thanks to those who provide this very essential patch.

I *hate* patching my site; it's a maintenance nightmare and my management isn't happy.

And no relief planned for Drupal 7?

What's wrong with this picture?

Anyway, thanks again to those who provide this critical patch!

#206

glowrocks - July 16, 2009 - 16:51

Another note: it's in no way a waste of time to make my site work as required by my management. Have we been taken over by "kids" w/no real responsibilities and no real life experience?

What's a waste of time is me and many others having to find and apply this patch every time drupal is updated; the right thing to do is *fix the damn problem*, not hide behind "policies."

#207

Damien Tournoud - July 16, 2009 - 16:53
Status:needs work» active

The right thing to do is for *someone* to roll a patch for *Drupal 7*, so that it can be committed there. Apparently nobody is willing here to do that simple thing... how can I not conclude that nobody wants this feature?

Come on, people.

#208

glowrocks - July 16, 2009 - 16:58
Status:active» needs work

I'm probably not the right person to roll that patch, but will give it some consideration.

#209

glowrocks - July 16, 2009 - 16:59

Seriously, the problem may be that the folks who need the proxy capability may be like myself and not really in a position to contribute code. Hence, we're reduced to writing grumpy posts :-)

#210

Crell - July 16, 2009 - 17:03

glowrocks: If your management hates how much time you spend repatching Drupal, tell them that you can instead spend a couple of hours getting it patched in Drupal 7 so that after you upgrade to D7 you'll never have to run a forked version of Drupal again (at least not for this reason). That's how open source works. :-) What's wrong with this picture is that no one who needs this feature is willing to take any time at all to carry it home so that it can be solved. Help yourself by helping Drupal. It won't happen unless someone steps up to work on it.

#211

roball - July 16, 2009 - 17:08

"Apparently nobody is willing here to do that simple thing". Not true - see #166 - silid has already posted a patch for D7, but the validation failed for some reason. If you say that is so simple, or other "Drupal core people" mention it only takes 20 minutes, so why do you waste your time in making this thread more end more endless instead of helping out with that "easy 20 minutes work"? Not everybody here has these skills, but a lot simply need this patch for D6 now!

#212

glowrocks - July 16, 2009 - 17:09

Thanks Crell. I probably read this thread too quickly; I came away with the impression that patches were being rejected as no one saw this as a problem.

I wanted to make the case that this is a problem.

If the meta problem is that no one is working on a patch, then that's what needs to be addressed (as you so clearly stated).

#213

brahms - July 19, 2009 - 00:19

Here is my attempt to provide a patch for Drupal 7. I took the last D7 patch from comment 166 respecting most of the recommendations from comment 187.

One thing I kept is the strstr comparison between the host part of the request and the proxy exception patterns. I am not able to see why this should be broken. strstr('localhost', 'localhost') returns 'localhost' and so localhost does match localhost. Maybe there has been a mix-up of strpos with strstr in the review?

AttachmentSizeStatusTest resultOperations
proxy-7881-213.patch7.37 KBIdleFailed: Failed to apply patch.View details | Re-test

#214

wad - July 19, 2009 - 03:30

@brahms: sorry to tell you this but that patch is far from ready. You have included code that is commented and haven't resolved several of the issues noted in #187. Also, the accepted patch should provide unit tests so that it can be tested properly using the ATS. In fact, unit tests should be written first.

@Damien Tournaud:
@Crell:
@chx:
You don't seem to realise that most of the people in this thread are from enterprises running a Drupal RELEASE, not CVS. Setting up and maintaining a CVS site behind a proxy is not a trivial task, especially when the expected result is that some things won't work and other things work differently to the version you are actually trying to run (D5/6). Many enterprises are not prepared to pay someone to test their less than perfect code against the production proxy server and most workers leave their proxy server behind when they go home at night. This is a significant issue with significant work required, not a 20 minute re-roll.

@everyone: if you have nothing to say that will progress this to resolution, please don't say anything. Posting a 'subscribe' comment or correcting someone's grammar is pointless. D.o. really needs #34496: Add Flag integration - a way for people to subscribe without cluttering the issue with 'subscribe' comments.

What this issue needs is an expert who will see it through to the end. None of us seem to be experts but I am prepared to do what is necessary to get it into D7, with unit tests, all passing the ATS with full support of HTTP and HTTPS (using CONNECT). My only problem is time - I won't be able to start until mid August. If somebody wants it sooner, maybe they should start a Chip-In bounty for it that might attract a D7 expert.

#215

Slim Pickens - July 21, 2009 - 08:04

I've probably said before that this should be part of a standard D7 setup. Unfortunately I do not have the skills to create an approved standard patch for consideration.

I work for a school system with at least 1600 schools behind a firewall/authenticated proxy server - and this is just in one state of Australia.

For promoting the use of Drupal around the world, this feature is a no-brainer - it must be done. How hard can it be? Seems like the work has been done - and the argument is over dotting 'i's and crossing 't's.

Come on Dries - throw some resources at it to get it into D7, please!

#216

c960657 - September 9, 2009 - 22:46
Status:needs work» needs review

New patch for D7 based on the latest patch by brahms.

The patch adds proxy support to the new browser in includes/browser.inc instead (this is supposed to replace the implementation in drupal_http_request(), see #553280: Integrate browser.inc and drupal_http_request()).

I haven't tested the proxy username/password stuff (because my local proxy does not require this).

AttachmentSizeStatusTest resultOperations
proxy-1.patch12 KBIdleFailed: Failed to apply patch.View details | Re-test

#217

Damien Tournoud - September 9, 2009 - 23:31

@c960657: thanks for working on this.

+++ includes/browser.inc 9 Sep 2009 22:37:08 -0000
@@ -631,37 +631,80 @@ class Browser {
+        elseif (substr($exception, 0, 1) == '.' && substr_compare($hostname, $exception, -strlen($exception)) == 0) {
+          return FALSE;
+        }

The first part is just $exception[0] == '.', and I find the substr_compare() here a little bit cryptic. substr($hostname, -strlen($exception)) == $exception seems easier to understand.

+++ includes/browser.inc 9 Sep 2009 22:37:08 -0000
@@ -631,37 +631,80 @@ class Browser {
+        elseif (filter_var($exception, FILTER_VALIDATE_IP) && $exception == gethostbyname($hostname)) {
+          return FALSE;
+        }

This should probably be FILTER_FLAG_IPV4, because gethostbyname() doesn't support IPV6 yet.

+++ includes/browser.inc 9 Sep 2009 22:37:08 -0000
@@ -685,18 +728,26 @@ class Browser {
+    if ($this->shouldUseProxy($options[CURLOPT_URL])) {
+      $options[CURLOPT_PROXY] = variable_get('proxy_hostname');
+      $options[CURLOPT_PROXYPORT] = variable_get('proxy_port', 8080);
+      if (variable_get('proxy_username')) {
+        $options[CURLOPT_PROXYUSERPWD] = variable_get('proxy_username') . ':' . variable_get('proxy_password');
+      }
+    }

cURL options are persistent. Don't we need a else clause here?

+++ modules/simpletest/tests/browser.test 9 Sep 2009 22:37:09 -0000
@@ -35,18 +35,40 @@ class BrowserTestCase extends DrupalWebT
+    variable_set('proxy_exceptions', implode(',', gethostbynamel('drupal.org')));
+    $this->assertTrue($browser->shouldUseProxy('http://example.com/'), t('Proxy is used for host not in exception list.'));
+    $this->assertFalse($browser->shouldUseProxy('http://drupal.org/'), t('Proxy is not used for host that resolves to an IP address in exception list.'));

Please, try to avoid network lookups during the tests (the secret plan is to allow tests to run in full network isolation one day). You can probably safely assume that localhost resolves to 127.0.0.1.

+++ modules/system/system.admin.inc 9 Sep 2009 22:37:10 -0000
@@ -1461,18 +1461,88 @@ function system_performance_settings() {
+  // Check that an IP address or a valid hostname is specified.
+  if (strlen($form_state['values']['proxy_hostname']) > 0
+      && !filter_var($form_state['values']['proxy_hostname'], FILTER_VALIDATE_IP)
+      && gethostbyname($form_state['values']['proxy_hostname']) == $form_state['values']['proxy_hostname']) {
+    form_set_error('proxy_hostname', t('The proxy host name cannot be found.'));
+  }
+  if (strlen($form_state['values']['proxy_port']) > 0) {
+    $proxy_port = $form_state['values']['proxy_port'];
+    // TCP allows the port to be between 0 and 65535 inclusive
+    if (!is_numeric($proxy_port) || ($proxy_port < 0) || ($proxy_port > 65535)) {
+      form_set_error('proxy_port', t('The proxy port is invalid. It must be a number between 0 and 65535.'));
+    }
+  }

Those should be #element_validate functions of their own element.

+++ modules/system/system.module 9 Sep 2009 22:37:11 -0000
@@ -878,18 +878,26 @@ function system_menu() {
+   $items['admin/config/system/proxy'] = array(
+     'title' => 'Proxy server',
+     'description' => 'Configure how Drupal connects to other websites.',
+     'page callback' => 'drupal_get_form',
+     'page arguments' => array('system_proxy_settings'),
+     'access arguments' => array('administer site configuration'),
+     'file' => 'system.admin.inc',
+   );

There is a small indentation issue here.

This review is powered by Dreditor.

#218

c960657 - September 10, 2009 - 19:26

Great comments. All points are addressed in this patch.

AttachmentSizeStatusTest resultOperations
proxy-2.patch12.43 KBIdlePassed: 13576 passes, 0 fails, 0 exceptionsView details | Re-test

#219

Damien Tournoud - September 10, 2009 - 19:45
Status:needs review» needs work

This patch is getting even more awesome by the day ;)

+++ includes/browser.inc 10 Sep 2009 19:23:28 -0000
@@ -631,37 +631,80 @@ class Browser {
+        elseif (filter_var($exception, FILTER_VALIDATE_IP, FILTER_VALIDATE_IP) && $exception == gethostbyname($hostname)) {
+          return FALSE;
+        }

The second parameter should probably be FILTER_FLAG_IPV4.

+++ includes/browser.inc 10 Sep 2009 19:23:28 -0000
@@ -685,18 +728,29 @@ class Browser {
+      $options[CURLOPT_PROXY] = FALSE;

cURL can be a strange beast sometimes. Have you verified that this works?

+++ modules/system/system.admin.inc 10 Sep 2009 19:23:28 -0000
@@ -1461,18 +1461,93 @@ function system_performance_settings() {
+/**
+ * Element validate function for proxy port number.
+ */
+function system_proxy_port_validate($element, &$form_state) {
+  if (strlen($element['#value']) > 0) {
+    // TCP allows the port to be between 0 and 65535 inclusive
+    if (!is_numeric($element['#value']) || ($element['#value'] < 0) || ($element['#value'] > 65535)) {
+      form_set_error('proxy_port', t('The proxy port is invalid. It must be a number between 0 and 65535.'));
+    }
+  }
+}

form_set_error() should be form_error($element) here.

Why not abstracting those element validation a little bit? This is simply a system_port_validate() ;)

I'm on crack. Are you, too?

#220

c960657 - September 10, 2009 - 20:08
Status:needs work» needs review

cURL can be a strange beast sometimes. Have you verified that this works?

Yes, setting it to FALSE seems sufficient (and not resetting it like in proxy-1.patch does indeed cause trouble).

The other points are fixed.

(BTW I look forward to the day when FAPI will support something like this: '#type' => 'number', '#min' => 0, '#max' => 65535 - and transform it into a nice HTML5 input field <input type="number" min="0" max="65535" /> and of course also do server-side validation :-)

AttachmentSizeStatusTest resultOperations
proxy-3.patch12.4 KBIdlePassed: 13609 passes, 0 fails, 0 exceptionsView details | Re-test

#221

Damien Tournoud - September 10, 2009 - 21:05
Status:needs review» reviewed & tested by the community

Good, let's get this in ;)

Next stop: make drupal_http_request() use the browser ;)

#222

webchick - September 10, 2009 - 23:18
Status:reviewed & tested by the community» needs work

This will need a sign-off from Dries, IMO, since this patch has changed significantly since the incarnation where he was on the cusp of committing it. Too bad there was so much jiggering with the version selector, or this likely would've been resolved (and possibly backported) a year ago. Sigh...

I'm not sure of this new approach. It doesn't look like the latest patch actually fixes the problem; just fixes it in the brower.inc which hinges on drupal_http_request() moving over to it. There is quite literally nothing at #553280: Integrate browser.inc and drupal_http_request(), which doesn't give me confidence that we'll end up doing this in time for the end of code slush. And even if we do get a patch over there, we're going to need to talk over the implications of basically forcing core to require cURL (unless I am mistaken about the state of the final browser.inc that made it in).

Couple of minor things:

From a usability perspective, it'd be nice to have a line or two description in hook_help() for this form that would indicate to me as an end-user whether I need to worry about this or whether I can ignore it. It seems very odd to me to include such a 'scary' settings form in core when only 1/10000000 sites will use it, but this passed Dries's sniff test, so that's fine.

+  $form['forward_proxy']['proxy_password'] = array(
+    '#type' => 'textfield',
+    '#title' => t('Password'),
+    '#default_value' => '',
+    '#description' => t('The password used to connect to the proxy server. This is stored as plain text.'),
+  );

Why display it as plain-text as well? That's odd. Shouldn't this be a password field?

+function system_port_validate($element, &$form_state) {
+  if (strlen($element['#value']) > 0) {
+    // TCP allows the port to be between 0 and 65535 inclusive
+    if (!is_numeric($element['#value']) || ($element['#value'] < 0) || ($element['#value'] > 65535)) {
+      form_error($element, t('The port number is invalid. It must be a number between 0 and 65535.'));
+    }
+  }
+}

I'm pretty sure 0 is not a valid proxy port. So probably make this > 1. Also, there should be a period at the end of this comment.

Other than those small things, this looks fine from my perspective.

#223

c960657 - September 14, 2009 - 17:57
Status:needs work» needs review

Why display it as plain-text as well? That's odd. Shouldn't this be a password field?

Changed to '#type' => 'password'. I'm not sure if this is the best way to populate the field: '#attributes' => array('value' => variable_get('proxy_password')) The point is that we want to indicate to the user whether a password is specified or not.

I'm pretty sure 0 is not a valid proxy port.

According to http://en.wikipedia.org/wiki/TCP_and_UDP_port#Technical_details, 0 is a valid port (but probably not much used in practice).

Added a line to hook_help().

There is quite literally nothing at #553280: Integrate browser.inc and drupal_http_request(), which doesn't give me confidence that we'll end up doing this in time for the end of code slush.

I share your concern, though it was initially the goal to get that fixed in time for D7 (see #340283: Abstract SimpleTest browser in to its own object comment #99). I suggest we postpone this patch until #553280: Integrate browser.inc and drupal_http_request() has landed.

AttachmentSizeStatusTest resultOperations
proxy-4.patch15.46 KBIdleFailed: Invalid PHP syntax in modules/simpletest/tests/browser.test.View details | Re-test

#224

roball - September 16, 2009 - 12:34
Priority:normal» critical

Changing the priority to critical, since the code freeze for D7 already began.

#225

System Message - October 2, 2009 - 10:50
Status:needs review» needs work

The last submitted patch failed testing.

#226

ganglion - October 12, 2009 - 14:22

subscribe.

Waiting for a 6.14 patch.

#227

roball - October 12, 2009 - 22:17

ganglion, "proxy6.13.patch" from #192 works also with D6.14.

#228

ganglion - October 14, 2009 - 15:23

I have tried, and I get this:

% patch -b -p0 < proxy6.13.patch
patching file `includes/common.inc'
Hunk #1 FAILED at 416.
Hunk #2 FAILED at 437.
Hunk #3 FAILED at 479.
Hunk #4 FAILED at 518.
Hunk #5 FAILED at 603.
5 out of 5 hunks FAILED -- saving rejects to includes/common.inc.rej
patching file `modules/system/system.admin.inc'
Hunk #1 FAILED at 1362.
1 out of 1 hunk FAILED -- saving rejects to modules/system/system.admin.inc.rej
patching file `modules/system/system.module'
Hunk #1 FAILED at 317.
1 out of 1 hunk FAILED -- saving rejects to modules/system/system.module.rej

I am in the top-level directory:
% pwd
/home/www-depts/fass/drupal/expt
% ls
CHANGELOG.txt      MAINTAINERS.txt    misc               sites
COPYRIGHT.txt      UPGRADE.txt        modules            themes
INSTALL.mysql.txt  cron.php           profiles           update.php
INSTALL.pgsql.txt  includes           proxy6.13.patch    xmlrpc.php
INSTALL.txt        index.php          robots.txt
LICENSE.txt        install.php        scripts

Am I doing something wrong? The patch looks OK when I compare it visually to the code.

#229

ganglion - October 14, 2009 - 16:34

I just tried the 6.13 patch again, against a freshly downloaded drupal-6.14, and it fails with the same errors.

#230

silid - October 14, 2009 - 21:32

I haven't tried it against 6.14 yet. Have you tried increasing your fuzz factor?

Si.

#231

ganglion - October 15, 2009 - 08:12

I can't do this - the version of patch on my server doesn't have a -F option.

Are you likely to have a 6.14 patch ready in the next few days, or should I try to make my own from the 6.13 patch?

#232

roball - October 15, 2009 - 10:16

It *did* work for me:

[root@enxofre ~]# cd /var/www/drupal-6.14
[root@enxofre drupal-6.14]# wget http://drupal.org/files/issues/proxy6.13.patch
[root@enxofre drupal-6.14]# patch -p0 < proxy6.13.patch
(Stripping trailing CRs from patch.)
patching file includes/common.inc
Hunk #1 succeeded at 435 with fuzz 1 (offset 19 lines).
Hunk #2 succeeded at 439 (offset 2 lines).
Hunk #3 succeeded at 499 (offset 20 lines).
Hunk #4 succeeded at 528 (offset 10 lines).
Hunk #5 succeeded at 623 (offset 20 lines).
(Stripping trailing CRs from patch.)
patching file modules/system/system.admin.inc
Hunk #1 succeeded at 1369 (offset 7 lines).
(Stripping trailing CRs from patch.)
patching file modules/system/system.module
Hunk #1 succeeded at 318 (offset 1 line).

#233

silid - October 15, 2009 - 10:46

I haven't tested this. I applied the patch for 6.13 to 6.14 and rolled a new patch.

AttachmentSizeStatusTest resultOperations
proxy6.14-D6.patch6.66 KBIgnoredNoneNone

#234

ganglion - October 17, 2009 - 07:04

This also fails on the live server (a solaris machine):

% patch -p0 -b < proxy6.14-D6.patch
patching file `includes/common.inc'
Hunk #1 FAILED at 435.
Hunk #2 FAILED at 458.
Hunk #3 FAILED at 501.
Hunk #4 FAILED at 548.
Hunk #5 FAILED at 633.
5 out of 5 hunks FAILED -- saving rejects to includes/common.inc.rej
patching file `modules/system/system.admin.inc'
Hunk #1 FAILED at 1369.
1 out of 1 hunk FAILED -- saving rejects to modules/system/system.admin.inc.rej
patching file `modules/system/system.module'
Hunk #1 FAILED at 318.
1 out of 1 hunk FAILED -- saving rejects to modules/system/system.module.rej
% patch --version
patch 2.5
Copyright 1988 Larry Wall
Copyright 1997 Free Software Foundation, Inc.

This program comes with NO WARRANTY, to the extent permitted by law.
You may redistribute copies of this program
under the terms of the GNU General Public License.
For more information about these matters, see the file named COPYING.

written by Larry Wall with lots o' patches by Paul Eggert

But not on my home debian laptop:
andy@monkey:~/Desktop/drupal-6.14$ patch -p0 -b < proxy6.14-D6.patch
(Stripping trailing CRs from patch.)
patching file includes/common.inc
(Stripping trailing CRs from patch.)
patching file modules/system/system.admin.inc
(Stripping trailing CRs from patch.)
patching file modules/system/system.module
andy@monkey:~/Desktop/drupal-6.14$ less includes/common.inc
andy@monkey:~/Desktop/drupal-6.14$ patch --version
patch 2.5.9
Copyright (C) 1988 Larry Wall
Copyright (C) 2003 Free Software Foundation, Inc.

This program comes with NO WARRANTY, to the extent permitted by law.
You may redistribute copies of this program
under the terms of the GNU General Public License.
For more information about these matters, see the file named COPYING.

written by Larry Wall and Paul Eggert

Which is strange, but it looks like it's my problem to sort out somehow.

#235

EmanueleQuinto - October 22, 2009 - 15:31

Patch #223 was with DOS carriage return settings...

Same patch after dos2unix attached below.

AttachmentSizeStatusTest resultOperations
proxy6.14-D6.dos2unix.patch6.47 KBIgnoredNoneNone
 
 

Drupal is a registered trademark of Dries Buytaert.