Problem/Motivation
Some installations require proxy authentication for external communications (as with enterprise scenarios). This leads to failure of RSS feeds, OpenID, Update Status, etc.
Proposed resolution
Implement optional configuration in default.settings.php to define proxy authentication. Configuration settings are used rather than an administration page because proxy authentication is defined only once per system and is often not required.
If enabled, establishes a proxied connection for requests to hosts not explicitly excluded by the configuration (which defaults to 'localhost' and '127.0.0.1'). Only basic authentication is supported.
First rolled (by pwolanin): http://drupal.org/node/7881#comment-3458064
Reviewed (by jpsoto): http://drupal.org/node/7881#comment-3465836
Discussed/reviewed with jhodgdon in IRC: http://drupal.org/node/7881#comment-3785774
Applied to D8, reviewed and passed testing (by pwolanin): http://drupal.org/node/7881#comment-4406026
Tested/verified (by jrbeeman): http://drupal.org/node/7881#comment-4470430
Remaining tasks
Alternative authentications: Suggested hook for drupal_http_request to allow module-based alternative authentications such as Digest/Radius/NTLM/etc.; users requiring these in the meantime might look at NTLMAPS.
HTTPS by proxy: Future implementation may allow this through HTTP 1.1 and cURL.
Persistent connections: Support for configuring pfsockopen instead of fsockopen (as for dedicated servers) is not enabled by the patch.
#1664784: drupal_http_request() has problems, so allow it to be overridden
User interface changes
API changes
Additional (optional) configuration parameters in default.settings.php.
API addition: Adds helper function _drupal_http_use_proxy($host) to determine whether a specified host is in the exclusion list.
Original report by Jhef.Vicedo
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:
switch ($uri['scheme']) {
case 'http':
//$fp = @fsockopen($uri['host'], ($uri['port'] ? $uri['port'] : 80), $errno, $errstr, 15);
into:
//use proxy settings
$fp = @fsockopen('proxy2', '8080');
And then create the request by change from:
$request = "$method $path HTTP/1.0\r\n";
Into:
$request = "$method ".$uri['scheme']."://".$uri['host'].$path." HTTP/1.1\r\n";
And comment out the lines:
$request .= implode("\r\n", $defaults);
Comments
Comment #1
JonBob CreditAttribution: JonBob commentedComment #2
(not verified) CreditAttribution: commentedDid 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
Comment #3
Jhef.Vicedo CreditAttribution: Jhef.Vicedo commented+1 on having a facility to modify proxy settings...
here's a patch of /includes/common.inc for drupal-4.4.2 version
Comment #4
Jhef.Vicedo CreditAttribution: Jhef.Vicedo commented+1 on having a facility to modify proxy settings...
here's a patch of /modules/system.module for drupal-4.4.2 version
Comment #5
Jhef.Vicedo CreditAttribution: Jhef.Vicedo commentedpatch for /includes/common.inc 4.5.2 version to facilitate rss feeds behind firewall
Comment #6
Jhef.Vicedo CreditAttribution: Jhef.Vicedo commentedpatch of /modules/system.module 4.5.2 version to set proxy settings
Comment #7
Bèr Kessels CreditAttribution: Bèr Kessels commentedhi, 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.
Comment #8
Jhef.Vicedo CreditAttribution: Jhef.Vicedo commentedi forgot to remove my hardcoded proxy name.. sorry
Ber, which is not drupal compliant?
Comment #11
Steven CreditAttribution: Steven commented1) 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).
Comment #12
(not verified) CreditAttribution: commentedIn 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?
Comment #13
Jhef.Vicedo CreditAttribution: Jhef.Vicedo commentedThanks Steven.
I hope this patch works and be accepted... This will allow proxy variables to be modified using the admin interface.
Comment #14
Jhef.Vicedo CreditAttribution: Jhef.Vicedo commentedcompliant patch for proxy suport, thanks Steven.
Comment #15
Bèr Kessels CreditAttribution: Bèr Kessels commented-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
Comment #16
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedI'd also like to avoid YAS (yet another setting). Can't we include proxies settings.php?
Comment #18
rp0999 CreditAttribution: rp0999 commentedI 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
Comment #19
karppa CreditAttribution: karppa commentedAs 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.
Comment #20
killefiz CreditAttribution: killefiz commentedI 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.
Comment #21
magico CreditAttribution: magico commentedComment #22
jlwestsr CreditAttribution: jlwestsr commentedProxy 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.
Comment #23
rediguana CreditAttribution: rediguana commentedI 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.
Comment #24
Elfod CreditAttribution: Elfod commentedMade 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.
Comment #25
dskennedy CreditAttribution: dskennedy commented@#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.
Comment #27
Mark Matuschka CreditAttribution: Mark Matuschka commentedTo 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:
where you have set $username and $password appropriately of course.
Comment #31
MWLimburg CreditAttribution: MWLimburg commented+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.
Comment #32
catchAll issues are now fixed in 7.x, then backported if applicable.
Comment #34
zxombie CreditAttribution: zxombie commentedI'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.
Comment #35
mooffie CreditAttribution: mooffie commentedPerhaps, 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.
Comment #36
thememex CreditAttribution: thememex commented+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.
Comment #38
catchAll changes get made to 7.x first.
Comment #39
Anonymous (not verified) CreditAttribution: Anonymous commentedAdded configuration options to the UI under Site Information. Probably needs to be tweaked some for wording and such.
Comment #40
Anonymous (not verified) CreditAttribution: Anonymous commentedProbably 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.
Comment #41
zilla CreditAttribution: zilla commentedokay, 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?
Comment #42
zxombie CreditAttribution: zxombie commentedI'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.
Comment #43
Anonymous (not verified) CreditAttribution: Anonymous commented@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.
Comment #44
Anonymous (not verified) CreditAttribution: Anonymous commented@zilla the fsockopen method used in this patch is much better than either of those two alternatives.
Comment #45
Anonymous (not verified) CreditAttribution: Anonymous commentedI'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.
Comment #51
zxombie CreditAttribution: zxombie commented@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.
Comment #52
Slim Pickens CreditAttribution: Slim Pickens commentedI'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
Comment #53
zxombie CreditAttribution: zxombie commented@Slim Pickens, what method of authentication does the proxy server expect? The patch only implements basic authentication.
Comment #54
MWLimburg CreditAttribution: MWLimburg commentedIf 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.
Comment #55
Slim Pickens CreditAttribution: Slim Pickens commented#53 zxombie - the proxy server expects a username and password. I thought the common.inc patch was creating that capability?
Comment #56
Dries CreditAttribution: Dries commentedHey 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.
Comment #57
Anonymous (not verified) CreditAttribution: Anonymous commentedI've re-rolled the patch so that it applies cleanly against head. Working on implementing Dries' changes.
Comment #58
tanakskool CreditAttribution: tanakskool commentedThanks, it works!
Comment #61
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #70
Dries CreditAttribution: Dries commented'Forward Proxy Settings' should be 'Forward proxy settings'.
'the Internet' should be 'the internet'.
'Proxy Server' should be 'Proxy server'.
Etc.
Comment #71
Crell CreditAttribution: Crell commentedActually "Internet" is a proper noun, and should therefore be capitalized.
Comment #72
waddles CreditAttribution: waddles commentedI'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.
Comment #73
akolahi CreditAttribution: akolahi commentedThank 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.
Comment #74
Lucict CreditAttribution: Lucict commentedI 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.
Comment #75
craigcarboni CreditAttribution: craigcarboni commentedAny 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?
Comment #76
craigcarboni CreditAttribution: craigcarboni commentedSuggestion 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.
Comment #77
zxombie CreditAttribution: zxombie commentedThis is not a support forum. This needs to go into D7 first.
Comment #80
RobLoachSubscribing....
Comment #81
kaneod CreditAttribution: kaneod commentedHi 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.
Comment #82
RobLoachVery 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.
Comment #83
brahms CreditAttribution: brahms commented@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.
Comment #85
brahms CreditAttribution: brahms commentedHi 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
Comment #86
RobLoachFour things in response to the patch at #83:
is_in_no_proxy_list()
function as it isn't really that descriptive.drupal_in_proxy_list()
maybe?drupal_http_request
function is called, but can't really think of a cleaner alternative.$output .= '<li>'. t('basic configuration options for
.... The space before and after the period is part of Drupal 7 coding standards ;-) .Comment #88
brahms CreditAttribution: brahms commentedHi Rob and thanks for your response.
I suggest to rename the function
is_in_no_proxy_list()
todrupal_is_proxy_exception()
. But I'm not sure if it's good to use the suffixdrupal_
for this internal helper function. Wouldn't it be better to name it_is_proxy_exception()
instead?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.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.
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.
Comment #89
brahms CreditAttribution: brahms commentedHi 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: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).
Comment #90
RobLoachAnother thing we have to think about is testing. Is there anyway to create an effective SimpleTest for this for self-testing?
Comment #91
RobLoachTo test:
Comment #92
RobLoach... And I'd consider this a feature.
Comment #93
markus_petrux CreditAttribution: markus_petrux commentedHi,
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.
Comment #95
brahms CreditAttribution: brahms commentedHi 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:
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', ''))
.Comment #97
RobLoachHi 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?
Comment #98
brahms CreditAttribution: brahms commentedHi 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 variabledrupal_http_request_fails
had been set to 1 before. This caused a call to the self test (functionsystem_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.Comment #99
Dries CreditAttribution: Dries commentedYep, the !empty() is broken. This patch looks close though. :)
Comment #100
yellek CreditAttribution: yellek commentedA 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.
Comment #101
arhak CreditAttribution: arhak commentedsubscribing
Comment #102
arhak CreditAttribution: arhak commentedsame as #91 backported to Drupal 6.4
Comment #103
dankinon CreditAttribution: dankinon commentedI 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
Comment #104
RobLoachI'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......
Comment #105
arhak CreditAttribution: arhak commentedis this for D5/6/7?
+1 of course!!!
Comment #107
mooffie CreditAttribution: mooffie commented+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.
Comment #108
arhak CreditAttribution: arhak commentedabout the mail there is some work already done http://drupal.org/project/smtp, but it doesn't seems to be proxy aware
Comment #110
Dries CreditAttribution: Dries commentedI'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.
Comment #111
brahms CreditAttribution: brahms commentedI can confirm that the most recent patch from Rob in comment#104 works in following environments:
(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.
Comment #112
arhak CreditAttribution: arhak commentedI tested #91 backported to D6 in #102 on Drupal 6.4 behind a proxy server without authentification
Comment #113
Slim Pickens CreditAttribution: Slim Pickens commentedI 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
Comment #114
caign@drupal.org CreditAttribution: caign@drupal.org commentedInclude 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.
Comment #115
RobLoachThe patch at #104 still applies.
Comment #116
Dries CreditAttribution: Dries commentedDid someone test #104?
Comment #117
keith.smith CreditAttribution: keith.smith commentedOn 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:
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.
The password used to connect to the proxy server, if necessary. Warning: This password is stored as plain text.
"comma seperated" should be "comma-separated".
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.
Comment #118
lilou CreditAttribution: lilou commentedPatch #104 modified according comment #117.
Comment #121
Dries CreditAttribution: Dries commentedDoesn't seem to apply anymore for me ...
Comment #123
jcwatson11 CreditAttribution: jcwatson11 commentedThe attached patch works on my installation of Drupal 6.5. Please review. Looks like 2 hrs, 33 minutes. A new record! Hurraaahhh!
Comment #124
IT-Crupatch #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?
Comment #125
brahms CreditAttribution: brahms commented@Gos77:
Just some things you can try to make the proxy settings work on your site:
Comment #126
mot CreditAttribution: mot commentedAs 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.
Comment #127
IT-Cruworks for my blank 6.5 with patch from #123 and hint #125 .. thx
Comment #128
mot CreditAttribution: mot commentedJust 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?
Comment #131
jcwatson11 CreditAttribution: jcwatson11 commentedsubscribe
Comment #132
Anonymous (not verified) CreditAttribution: Anonymous commentedThis 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. :)
Comment #133
Anonymous (not verified) CreditAttribution: Anonymous commentedChanging to CNW, since it doesn't apply anymore.
Comment #135
Crell CreditAttribution: Crell commentedNo, 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.
Comment #136
mooffie CreditAttribution: mooffie commentedWhy 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.
Comment #137
silid CreditAttribution: silid commentedwithout 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.
Comment #147
silid CreditAttribution: silid commentedNow 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?
Comment #149
Dries CreditAttribution: Dries commentedComment #151
silid CreditAttribution: silid commentedThere have been some changes in 6.9 that affect the patch. Try this version, it works for me.
Comment #152
roball CreditAttribution: roball commentedI can confirm the 6.9 proxy patch to work fine with D6.9. Thank you silid!
Comment #153
RobLoachIf 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.
Comment #154
borzoj CreditAttribution: borzoj commentedHi 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
Comment #155
minorOffense CreditAttribution: minorOffense commentedSubscribing
Comment #158
silid CreditAttribution: silid commentedHere is a patch for 6.10
I haven't tested this so I don't know that it works. Please give feedback
Comment #159
IT-Crupatch from #158 working fine for me for D6.10
only getting (Stripping trailing CRs from patch.) for all three patched files while patching.
Comment #161
Crell CreditAttribution: Crell commentedFeature 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.)
Comment #162
roball CreditAttribution: roball commentedCrell, 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?
Comment #166
silid CreditAttribution: silid commentedThis patch has been made
Comment #168
RobLoachMissing the
is_in_no_proxy_list()
function?Comment #172
mabland CreditAttribution: mabland commentedPatch 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.
Comment #173
silid CreditAttribution: silid commentedHmm. 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
Comment #176
silid CreditAttribution: silid commentedComment #177
chinko CreditAttribution: chinko commentedI 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.
Comment #181
maykelmoya CreditAttribution: maykelmoya commentedThis is 6.11 patch updated for 6.12.
Comment #183
robdinardo CreditAttribution: robdinardo commentedAttached file contains the three files to add proxy support to D 6.12
Comment #185
chx CreditAttribution: chx commentedI am closing this issue and deleting every "I tried to patch" comment from it and then reopen.
Comment #186
chx CreditAttribution: chx commentedPlease 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.
Comment #187
Damien Tournoud CreditAttribution: Damien Tournoud commentedI've done my best to ignore the horrible
// PROXY-HACK
comments, so here is a review of the patch in #181.Drupal doesn't use value-as-first-component comparisons. You can simply rewrite that as
$proxy_not_required && ($proxy_server = variable_get('proxy_server', ''))
No support for HTTPS (via the CONNECT proxy command)? It seems like this could be useful in several use cases I can think about.
This indentation is wrong, and same remark as above for the condition.
Same remark.
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.
This form should use the new autodefault feature of system_settings_form() in D7.
$form_state['values']['proxy_server'] != ''
should bestrlen($form_state['values']['proxy_server']) > 0
, or "0" will not match. The two error messages should be collapsed in one.Comment #188
FrankT CreditAttribution: FrankT commentedCould 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...
Comment #189
roball CreditAttribution: roball commentedAdminister -> Site configuration -> Proxy Server (admin/settings/proxy)
Comment #190
roball CreditAttribution: roball commentedSorry to ask here, but does the 6.x patch also work with Drupal 6.13?
Comment #191
IT-CruHi roball .. patch from #181 works fine for my 6.13 installation.
Comment #192
silid CreditAttribution: silid commentedAs 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.
Comment #193
roball CreditAttribution: roball commentedThanks a lot silid, your patch works fine with 6.13 :-)
Comment #194
Damien Tournoud CreditAttribution: Damien Tournoud commentedAs 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.
Comment #195
silid CreditAttribution: silid commentedIt will get the work when there is time. This thread is full of people that want this feature.
Comment #196
Damien Tournoud CreditAttribution: Damien Tournoud commentedUntil we have a proper patch for this, please let the status at "won't fix".
Comment #197
roball CreditAttribution: roball commentedComment #198
Damien Tournoud CreditAttribution: Damien Tournoud commentedStill waiting for a proper patch.
Comment #199
silid CreditAttribution: silid commentedThe handbook says that 'needs work' is defined as:
where as 'won't fix' is defined as:
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.
Comment #200
Crell CreditAttribution: Crell commentedI 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...
Comment #201
ghoti CreditAttribution: ghoti commentedsubscribe
Comment #202
David StraussWho actually needs this feature?
Also, re-titling to reflect that this is for upstream proxies, not downstream (like Varnish in front of Drupal).
Comment #203
roball CreditAttribution: roball commentedAre you kidding?
Thousands of people who want to/must run Drupal on a server behind a proxy *needs* it.
Comment #204
Roavei CreditAttribution: Roavei commentedsubscribe
Comment #205
glowrocks CreditAttribution: glowrocks commentedI'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!
Comment #206
glowrocks CreditAttribution: glowrocks commentedAnother 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."
Comment #207
Damien Tournoud CreditAttribution: Damien Tournoud commentedThe 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.
Comment #208
glowrocks CreditAttribution: glowrocks commentedI'm probably not the right person to roll that patch, but will give it some consideration.
Comment #209
glowrocks CreditAttribution: glowrocks commentedSeriously, 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 :-)
Comment #210
Crell CreditAttribution: Crell commentedglowrocks: 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.
Comment #211
roball CreditAttribution: roball commented"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!
Comment #212
glowrocks CreditAttribution: glowrocks commentedThanks 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).
Comment #213
brahms CreditAttribution: brahms commentedHere 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?Comment #214
waddles CreditAttribution: waddles commented@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: [meta] Add Flag module to allow users to subscribe/unsubscribe without posting a comment - 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.
Comment #215
Slim Pickens CreditAttribution: Slim Pickens commentedI'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!
Comment #216
c960657 CreditAttribution: c960657 commentedNew 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).
Comment #217
Damien Tournoud CreditAttribution: Damien Tournoud commented@c960657: thanks for working on this.
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.This should probably be FILTER_FLAG_IPV4, because gethostbyname() doesn't support IPV6 yet.
cURL options are persistent. Don't we need a else clause here?
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.
Those should be #element_validate functions of their own element.
There is a small indentation issue here.
This review is powered by Dreditor.
Comment #218
c960657 CreditAttribution: c960657 commentedGreat comments. All points are addressed in this patch.
Comment #219
Damien Tournoud CreditAttribution: Damien Tournoud commentedThis patch is getting even more awesome by the day ;)
The second parameter should probably be FILTER_FLAG_IPV4.
cURL can be a strange beast sometimes. Have you verified that this works?
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?
Comment #220
c960657 CreditAttribution: c960657 commentedYes, 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 :-)Comment #221
Damien Tournoud CreditAttribution: Damien Tournoud commentedGood, let's get this in ;)
Next stop: make drupal_http_request() use the browser ;)
Comment #222
webchickThis 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.
Why display it as plain-text as well? That's odd. Shouldn't this be a password field?
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.
Comment #223
c960657 CreditAttribution: c960657 commentedChanged 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.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().
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.
Comment #224
roball CreditAttribution: roball commentedChanging the priority to critical, since the code freeze for D7 already began.
Comment #226
highfellow CreditAttribution: highfellow commentedsubscribe.
Waiting for a 6.14 patch.
Comment #227
roball CreditAttribution: roball commentedganglion, "proxy6.13.patch" from #192 works also with D6.14.
Comment #228
highfellow CreditAttribution: highfellow commentedI have tried, and I get this:
I am in the top-level directory:
Am I doing something wrong? The patch looks OK when I compare it visually to the code.
Comment #229
highfellow CreditAttribution: highfellow commentedI just tried the 6.13 patch again, against a freshly downloaded drupal-6.14, and it fails with the same errors.
Comment #230
silid CreditAttribution: silid commentedI haven't tried it against 6.14 yet. Have you tried increasing your fuzz factor?
Si.
Comment #231
highfellow CreditAttribution: highfellow commentedI 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?
Comment #232
roball CreditAttribution: roball commentedIt *did* work for me:
Comment #233
silid CreditAttribution: silid commentedI haven't tested this. I applied the patch for 6.13 to 6.14 and rolled a new patch.
Comment #234
highfellow CreditAttribution: highfellow commentedThis also fails on the live server (a solaris machine):
But not on my home debian laptop:
Which is strange, but it looks like it's my problem to sort out somehow.
Comment #235
EmanueleQuinto CreditAttribution: EmanueleQuinto commentedPatch #223 was with DOS carriage return settings...
Same patch after dos2unix attached below.
Comment #236
b.one CreditAttribution: b.one commentedWhat about a proxy patch for Drupal 6.15 ?
I've test the 6.14 but I can't make a Cron now...
Comment #237
roball CreditAttribution: roball commentedThe patch for D6.14 from #233 works fine with D15:
Comment #238
catchThe more times this issue gets moved back to 6.x without being resolved in 7.x, the less change it has of ever being fixed.
Comment #239
roball CreditAttribution: roball commented@238: I'm pretty sure most users changing a tag while they add a comment may not be aware that this would apply to the whole thread, rather than their actual post. Do you think it is better to open a new ticket for D6 proxy support and keep this one for D7 only?
Comment #240
EmanueleQuinto CreditAttribution: EmanueleQuinto commentedProxy patch for 6.15 attached.
Comment #241
catchLooks like the latest D7 patch here relies on stream wrappers, so yeah a separate D6 issue seems appropriate since there'll be no straight backport possible of the D7 solution.
Comment #242
c960657 CreditAttribution: c960657 commentedThe D7 patch in #223 is no longer relevant. It is based on browser.inc that was backed out (#600032: Back out browser.inc). So I think the approach suggested for D6 is still relevant for HEAD.
Comment #243
graphicmist CreditAttribution: graphicmist commentedThanks a lot... you dont know.. u saved my day.. i love you man
Comment #244
RobLoachThis is a feature for Drupal 8. It's unfortunate people didn't review the numerous Drupal 7 patches that went up.
Comment #245
catchThere's no code base change between D7 and D8 yet ;)
Comment #246
RobLoachOh, I was talking the Drupal 6 patches they're upload :-P .
Comment #247
stefanhapper CreditAttribution: stefanhapper commentedPatch provided in #240 works great for Drupal 6.15
To help those who - like me - apply patches manually I am attaching the changed files. Just replace the core files with the ones in the ZIP, clear your cache and set your proxy in site configuration > proxy settings
Comment #248
ryan.nauman CreditAttribution: ryan.nauman commentedAfter applying the patch it was necessary to run the drupal database update script before the Proxy area was added to the Site Configuration area.
Comment #249
Damien Tournoud CreditAttribution: Damien Tournoud commented@Rob Loach: all the D7 patches that got posted got reviewed. The issue here is not the lack of review, but the lack of patches to begin with. Posting a Drupal 6.15 patch here helps no one.
Comment #250
Kisama CreditAttribution: Kisama commentedOk, look. That 6.15 patch worked. Now we've had a version increase and it's time to manually patch this into 6.16. What is going to take to get this code merged into the core code so we can move on. It's been almost 6 years now since someone asked for this functionality. What's the hold up?
Comment #251
Crell CreditAttribution: Crell commented@Kisama: As stated repeatedly in this thread, never. This functionality will never be in any release of Drupal 6, ever. Drupal 6 is feature frozen and has been since the 6.0 release was made. New functionality goes into the development version only. Easily half the posts in this thread are people asking for or demanding Drupal 6 versions, despite it being repeatedly explained that it will never happen. Those posts are the reason this issue never got resolved in Drupal 7, and now we have to wait for Drupal 8.
Repeating: Because people kept demanding a Drupal 6 version of this patch, we will have to wait for Drupal 8 for it. Yes, there is a causal relationship there. Maybe if one of those enterprise users who need it so much spend the time working on the patch properly rather than just demanding free work in ways that violate Drupal's development process this would be resolved by now.
The hold up is someone working on this patch *on the development version of Drupal*. Until/unless that happens, it will never get in. If you want this functionality, that's all you've got to do. Until then, make do with patching core.
Comment #252
roball CreditAttribution: roball commentedI have created a new thread dedicated to proxy server support for Drupal 6 here: #735420: Drupal 6 proxy server support, so this thread might finally turn into productivity again and leads to a working patch for D7.
Please NO MORE MENTION OF D6 HERE - otherwise some core developers might get angry...
Comment #253
arhak CreditAttribution: arhak commented@#252 I would preferred to start a clean D7 issue instead, this one is reaching the 300 posts
Comment #254
vvanaparthy CreditAttribution: vvanaparthy commentedI am working on a patch to use CURL instead of primitive fsockopen with proxy support. Please be tuned!
Comment #255
glowrocks CreditAttribution: glowrocks commented"Repeating: Because people kept demanding a Drupal 6 version of this patch, we will have to wait for Drupal 8 for it."
In the real world, many of us will have to continue to violate one of Drupal's biggest caveats: don't modify core.
Well, if required functionality isn't in core, we'll have to keep patching.
I hate to see Drupal devolve this way, where standing on "principle" is more important than providing a feature that has been requested for what appears to be 6 years.
And yeah, I'm not a coder, so I can't provide the necessary functionality myself.
What's frustrating is that the fix appears to be somewhat well known, but for obscure reasons it's not making it into core.
Comment #256
braindrift CreditAttribution: braindrift commentedworks for me on 6.16
subscribe
Comment #257
cooperaj CreditAttribution: cooperaj commentedReading this issue is like some sort of never ending bad dream. Can we please get some sort of organisation in so that backport requests, patches and subscriptions can be moved off to another issue number. Would it be worth closing this one as some bad history and creating a new issue for D7/8?
So I'm not commenting uselessly and contributing to this bad dream please find attached a D7 Alpha 4 patch that I'm using sourced mostly from #231 above. I'll investigate a D8 patch shortly (though I assume it's mostly identical to D7 at this point).
I've removed the UI elements from that patch too as IMHO this is a feature needed by a minority only and should not clutter the UI further. Setting the necessary variables can now be done using settings.php and as such I've added the placeholders to default.settings.php.
Comment #258
arhak CreditAttribution: arhak commentedtag
Comment #259
arhak CreditAttribution: arhak commentedtag
Comment #260
Damien Tournoud CreditAttribution: Damien Tournoud commentedNothing happens by magic.
#257 looks like a decent base.
Comment #262
arhak CreditAttribution: arhak commented@#260
no one said it has to happen magically
also it is pretty obvious why it has been delayed
but you removed what it is NOT a useless tag, which grouped (no longer since you destroyed it) issues from 2007 still opened
(now incidentally bumped with the tagging)
certainly you did the favor of getting ride of a couple of them that should be marked as "won't fix" or "duplicate" a long time ago, but still they are others that do make sense to keep tagged as opened a long time ago and still pending
is there any shame on that?
won't you call "ancient" issues three years old?
if the tag doesn't makes sense to you, why not letting others use it to follow up their interest?
isn't this a community any more?
when did it turned into a dictatorship?
Comment #263
Crell CreditAttribution: Crell commented"Issues that are old" don't need a tag. You just go back a few pages in any queue and boom, you've got old stuff. The tag adds nothing other than "wah, this is old, why won't someone work on it". That's especially true of "let it end".
Your community vs. dictatorship comment at the end is also out of line. Damien has just as much right to say that the tag you're adding is unnecessary noise as you have to claim that it isn't. In this case, I happen to agree with him.
Comment #264
arhak CreditAttribution: arhak commented@#263 I just didn't mean to boom anything, just tagging to group them under a common radar, not useful for you, ok, but certainly useful for others, ergo your argument doesn't hold, that's what free tagging use to be for, "free"
lets stop it there, I'll say ok, since now we are really off-topic
Comment #265
Damien Tournoud CreditAttribution: Damien Tournoud commented@arhak: you are just unnecessary adding noise. Tagging an issue as old just because it is old doesn't make a lot of sense: you can just order the issues by date and come to exactly the same conclusion.
The amount of time you spent tagging those issues could have been better spent actually *reviewing* them. Over the five or six issues you tagged as "Ancient", I marked two as won't fixed, marked one as a duplicate and reviewed one and marked it as RTBC.
What about doing the same the next time?
Easy task here for example: what about rerolling the patch that failed the test bot, instead of discussing about my "attitude"?
Comment #266
arhak CreditAttribution: arhak commentedI am quite off-topic, I sent you a mail
Comment #267
cooperaj CreditAttribution: cooperaj commentedMade changes to the patch format. Git diff seems to do something that the patch god doesn't like.
Edit..
Ah, hidden in a comment on the patch docs page is the chestnut --no-prefix for git diffing. Good advice that :)
Comment #268
arhak CreditAttribution: arhak commentedhitting the bot
Comment #269
cooperaj CreditAttribution: cooperaj commentedSorry to do this.
After having read though the patch I found that the bit where it adds Proxy authorisation was a load of nonsense I'd not corrected (I don't use a proxy that needs it). I've changed it now so it should work but it's still untested.
Comment #270
nc.anwar CreditAttribution: nc.anwar commented#8: common.inc_6.patch queued for re-testing.
Comment #271
alekas CreditAttribution: alekas commented#269: 7881-d7a4.patch queued for re-testing.
Comment #272
dk1844 CreditAttribution: dk1844 commented#34: drupal_intranet_proxy_7.x.diff queued for re-testing.
Comment #273
Rob Knight CreditAttribution: Rob Knight commentedFor those of you with proxy issues, I've created a module which provides a workaround without needing to patch core modules - the update status proxy module. It's beta code, so I'd appreciate some feedback. This shouldn't distract from the need for a proper fix in core, but it does make it possible to get important module update notifications without patches.
Comment #274
c960657 CreditAttribution: c960657 commentedSimply !empty() should be enough.
You should use single quotes here.
Doing a substring search seems like an odd thing. If anything, it should do suffix matching. How about allowing the admin to specify a regexp instead? At least it is flexible and allows both suffix and complete string matching. And though the syntax may be bit complex, at least it is wellknown format (compared to anything. we invent ourself).
Just return early.
Comment #275
miouge CreditAttribution: miouge commented#220: proxy-3.patch queued for re-testing.
Comment #276
pwolanin CreditAttribution: pwolanin commentedcode style issues not corrected - likely stale after other changes to this function.
Comment #277
pwolanin CreditAttribution: pwolanin commentedHere's a substantially simplified patch that avoids needless code.
Perhaps someone with a proxy setup would care to test it?
Comment #278
pwolanin CreditAttribution: pwolanin commentedoops - that was missing one critical line. Also improved the code comments in default.settings.php.
Comment #279
pwolanin CreditAttribution: pwolanin commentedI think the handling is wrong if the original URL has a username/pass. This should fix it.
Also - do we need to urlencode the original URL when it's used as a path?
Comment #280
pwolanin CreditAttribution: pwolanin commentedTesting with Charles proxy.
While this looks nice and simple, the wrong Host header gets sent.
Comment #281
pwolanin CreditAttribution: pwolanin commentedThis seems to work better.
works with both Charles and Paros as local proxies.
Comment #282
groovehunter CreditAttribution: groovehunter commentedokay last one #281 worked for me on current 7.x-dev
thx!
Comment #283
roball CreditAttribution: roball commentedwow, so is there a chance to get in into 7.x?
Comment #284
pinventado CreditAttribution: pinventado commentedI'm behind a proxy without authentication so applied the patch on Drupal 7.0-alpha6. When I install a module from a URL, it gives of the following error:
Notice: Undefined variable: fp in drupal_http_request() (line 840 of /var/www/adric/includes/common.inc).
Notice: Undefined variable: errno in drupal_http_request() (line 843 of /var/www/adric/includes/common.inc).
Notice: Undefined variable: errstr in drupal_http_request() (line 844 of /var/www/adric/includes/common.inc).
HTTP error 0 occurred when trying to fetch http://ftp.drupal.org/files/projects/wysiwyg-7.x-2.x-dev.tar.gz.
Unable to retrieve Drupal project from http://ftp.drupal.org/files/projects/wysiwyg-7.x-2.x-dev.tar.gz.
The error refers to the codes that were patched. Any ideas what's wrong? Is there a way to get more specific errors for this?
Thanks!
Comment #285
pwolanin CreditAttribution: pwolanin commented@pinventado - why did you change the issue tags?
Comment #286
jpsoto CreditAttribution: jpsoto commentedI found out this patch yesterday and I have just seen the mentioned errors (patch applied to D7 Alpha6).
Using Xdebug, it is clear the issue: no socket is opened (@fscokopen) when the uri['scheme'] is 'proxy'.
I will try to find out a workaround.
However, it would be easier for who knows how worked this patch on previous D7 releases.
Indeed, present code should be not merged into the core yet.
¿May be, the patch should be applied to the HEAD?
Comment #287
jpsoto CreditAttribution: jpsoto commentedOK, I see ... clearly this patch cannot be applied to the D7 Alpha6, only to the HEAD
Comment #288
pwolanin CreditAttribution: pwolanin commented@jpsoto - indeed - we almost never patch against a tag, only against HEAD.
Comment #289
jpsoto CreditAttribution: jpsoto commentedDear friends ...
since some proxies refuse to forward requests with any user-agents, I would like to submit for your consideration a light variation of the last patch, being able to configure the Header User-Agent.
Comment #290
pwolanin CreditAttribution: pwolanin commentedThe patch doesn't seem to satisfy the use case you lay out. Also , ideally as much as possible of the proxy stuff should be within the case 'proxy':
Comment #291
pwolanin CreditAttribution: pwolanin commentedHow about this? By default it leaves the Drupal user agent header, but gives you the option to unset or change it.
Comment #292
jpsoto CreditAttribution: jpsoto commented@pwolanin - Thanks for appreciating the suggestion related to the Header User-Agent.
Yes ... , your approach is better.
Comment #293
jpsoto CreditAttribution: jpsoto commentedDear friends, ...
I am noting failures when forwarding https urls through a proxy. Result "Bad-Request" is returned.
I will try to make more specific this comment.
Meanwhile, has anybody checked https urls using the scheme 'proxy'?, what is your experience?
Regards
Comment #294
pwolanin CreditAttribution: pwolanin commented@jpsoto - I think the code assumes you are not proxing ssl, though it might work
Comment #295
jpsoto CreditAttribution: jpsoto commentedAfter deeping into the issue, it seems clear.
Code implements RFC-2616 and RFC-2617 (i.e. proxy support for non-SSL).
Unfortunately, there is no support for RFC-2817 yet. Such a support could provoke a moderately heavy rewrite of the patch.
A reference implementation could be this Python recipe
http://code.activestate.com/recipes/456195-urrlib2-opener-for-ssl-proxy-...
See you soon ...
Comment #296
pwolanin CreditAttribution: pwolanin commented@jpsoto - this issue has been open since Drupal 4.5 - let's get what we can.
In fact PHP fsockopen() (or at least our use of it) only handles HTTP 1.0 connections, so we need to conform to RFC-1945 and cannot use HTTP 1.1 tricks.
note the code:
Likely HTTP 1.1 and https proxy support will require use of the curl library and will not be in core until Drupal 8.
Comment #297
jpsoto CreditAttribution: jpsoto commentedOK ... I understand.
May be, at present we could try to cover this gap using the function stream_socket_enable_crypto().
Attached you can find a patch with a tentative approach. A moderately heavy writting of the drupal_http_request() has been needed.
I have just carried out a few preliminary tests to verify requests without proxy are still working.
Unfortunately, after this weekend I cannot verify requests by a (corporative) proxy.
Therefore, please, consider this patch as a tentative proof-of-concept patch and ... please check it up within your environment (if possible).
See you next days ...
UPDATE 22/sep. Patch is not working properly. Please, do not use; it will be improved.
Comment #298
pwolanin CreditAttribution: pwolanin commented@jpsoto - the patch in #297 doesn't make any sense to me. It might work in limited special applications, but would not be seamless. Callers are not passing in the now parameter and you we DO NOT support HTTP 1.1 responses, which may include chunked encoding. There is no point in the patch since a special application would likely use the curl functions or something else instead of just getting the connection passed back.
Basically, if we get any support for proxies, we are not going to support https proxying in Drupal 7.
Can someone please review #291?
Comment #299
jpsoto CreditAttribution: jpsoto commented@pwolanin - After reading this long issue, I think without support for https proxying the patch #291 could be not nominated to be applied into the core. Https proxying is not an exotic requirement . Without https support, usefulness would be heavily limited; a selected example (among many) could be Google OpenID which requires https access:
https://www.google.com/accounts/o8/id
Of course, ... the main goal has to be getting #291 into the core. Extremely agree. I will support it.
Without detriment to the main goal, the tentative patch #297 demonstrates an approach to achieve https proxying. Yes, ... an approach, untested by now, but an approach for D7 (may be never D8 becomes real). Because, there is a clear fact: D7 uses the PHP stream functions, not curl functions
http://www.php.net/manual/en/ref.stream.php
I will try to improve the tentative patch #297.
Answering your final question, ... yes, I have tested the patch #291. I think patch #291 is working.
Comment #300
chx CreditAttribution: chx commentedHm, i see the question but not the answer. So remind me, why are we reimplementing cURL?
Comment #301
pwolanin CreditAttribution: pwolanin commented@chx - many PHP installs are not built with cURL, so we'd need to add it to requirements. Right now we only require it for Testing module.
I'm tempted to say it's too late for 7 to change this.
Comment #302
jpsoto CreditAttribution: jpsoto commentedI understand the prefer solution for SSL proxy support is based on the curl library, and likely delayed to Drupal 8.
However, I would like to demonstrate SSL proxy support is feasible without the curl library in D7. I have just submitted a patch, opening a new issue (#924498: Proxy https support for drupal_http_request()) to not interfere in this issue.
Comment #303
pwolanin CreditAttribution: pwolanin commentedBased on #299 (jpsoto verified it works, in addition to my verification) and discussion and review in IRC with jhodgdon I think the patch in #291 (NOT the last patch in #297) is ready to mark RTBC.
Comment #304
jpsoto CreditAttribution: jpsoto commentedI agree, patch at #291 works ...
I back to commit it into Drupal core. Proxy support is a real need to allow Drupal sites in intranets, behind corporative proxies. Indeed, a big ecosystem.
Patch #297 was a tentative approach to add SSL proxy support. To not interfere in this issue I decide to create a separate issue to demonstrate SSL proxy support without curl library: #924498: Proxy https support for drupal_http_request().
Comment #305
pwolanin CreditAttribution: pwolanin commented#291: 7881-proxy-please-291.patch queued for re-testing.
Comment #306
Dries CreditAttribution: Dries commentedI'd prefer to see a version based on CURL. I think it could still go in into Drupal 7 -- it is a transparent addition.
Comment #307
pwolanin CreditAttribution: pwolanin commented@Dries - how would using curl be transparent? It would be a rewrite of drupal_http_request() to use curl instead of sockets and adding curl as a new requirement?
Comment #308
Dave ReidThat is just *not* possible in D7.
Comment #309
pwolanin CreditAttribution: pwolanin commented@Dave Reid - *what* is not possible? Adding the use of cURL?
Comment #310
jpsoto CreditAttribution: jpsoto commentedRegarding of the proxy support, I am not able to understand what real need entails to introduce cURL: a net dependence for many PHP distributions and then, a messy dependence for Drupal.
This patch has proved to work using low-level functions. Even more, the fork #924498: Proxy https support for drupal_http_request() proves https proxy support using low-level functions. No cURL is needed, at all.
If everybody agree proxy support is a clear Drupal's gap, then, ... what is the doubt?
In all modesty, I back to commit this patch into Drupal core, and later to upgrade it for https proxy support.
Comment #311
GAMe CreditAttribution: GAMe commentedHi all,
please forgive my ignorance but could this not be made as a seperate module? sadly im not a PHP dev so I cant comment on the complexity of this beast but obviously this is a long standing issue for many people.
but great work on what has been done so far, could anyone else comment on if this patch works correctly for D6.20.
Thanks all!
Comment #312
pwolanin CreditAttribution: pwolanin commented@GAMe - no this could not be made a module - it requires changes to the core internals.
Comment #313
GAMe CreditAttribution: GAMe commentedI figured this was likely to be the case as this has not been done already but thanks for the responce back :-)
Comment #314
jrbeemanI agree that it'd be great to see this done in cURL. Maybe a solution could be to use cURL if it's there, if not fall back to the way we've currently got it...?
Comment #315
pwolanin CreditAttribution: pwolanin commented@jrbeeman - I think that has to be Drupal 8 material - basically we'd need to implement a more OO-based approach where we can have a strategy pattern, such as Drupal 7 has for the database driver.
Comment #316
jpsoto CreditAttribution: jpsoto commentedObviously, this old issue and its recent fork #924498: Proxy https support for drupal_http_request() are ssssstuck -->|||||
I am thinking about an alternative way.
Maybe, a suitable approach is to implement a new alter-type hook: a hook_http_request_alter() which would allow to override the default implementation of drupal_http_request(). A good reference could be the hook_custom_theme().
Advantages??
On the one hand, a solution for this old issue (a real advantage). On the other hand, such an approach would circumvent difficulties and hard decisions on the Drupal core (i.e. cURL and so on)
Please, I would appreciate your comments ...
Comment #317
Dries CreditAttribution: Dries commented+++ includes/common.inc 18 Sep 2010 11:21:55 -0000
@@ -767,7 +767,7 @@ function drupal_access_denied() {
+function drupal_http_request($url, array $options = array(), $fp_in = NULL) {
- What does $fp_in mean? Not very descriptive, and not documented. It doesn't look like we set/pass $fp_in anywhere in the code, either. Feels like a fishy parameter so I'd like to better understand that. Thanks!
- Do we have any data on how many PHP installations might, or might not have CURL installed? (Don't spend much time looking. I'm just curious.)
Comment #318
jpsoto CreditAttribution: jpsoto commented@Dries,
likely you have got this code excerpt from a patch initially submitted by me in this issue. Later, I opened a different (but related) issue to not interfere, because I was convinced SSL support is feasible without cURL support and without waiting for D8.
Please, check the patch in the issue #924498: Proxy https support for drupal_http_request(), there is a short description of the mentioned argument.
Following I reproduce it together with code excerpts to explain it
Please, note the proposed SSL support for drupal_http_request() requires an embedded call for the own drupal_http_request() to carried out the CONNECT method, with a notable remark: using the same socket.
This because the https requests have two different stages, first a CONNECT method and then, the usual methods after enabling crypto. See RFC-2817.
Please, note every call for drupal_http_request() is oriented to manage only one HTTP method, this could explain why I have to introduce such an argument (fp_in) to manage the embedded CONNECT using the same socket.
Regarding of how many PHP distributions might have (or not) cURL installed, I have only contributed my knowledge on the PHP packaged within Debian and Ubuntu distributions. Such distributions have not installed.
Comment #319
c960657 CreditAttribution: c960657 commentedFWIW, this has been proposed in #786074: Allow hooks for drupal_http_request() request and responses.
Comment #320
pwolanin CreditAttribution: pwolanin commentedThis is an unaltered re-roll (except for offset) of the patch in #291. That's the patch that's marked RTBC.
Comment #321
jpsoto CreditAttribution: jpsoto commented@c960657
I beg your pardon for overlooking the mentioned feature request, and your proposed patch.
I am strongly interested on testing it.
The presentation of #786074: Allow hooks for drupal_http_request() request and responses describes clearly many benefits to introduce such hooks.
Maybe, the more sense solution include two actions:
Comment #322
StephenRobinson CreditAttribution: StephenRobinson commentedThese patches caused me all sorts of problems, they assume port 80 and tend to break post requests, if you have a proxy and wish to use apache-solr, the correct syntax is:
Comment #323
pwolanin CreditAttribution: pwolanin commented@SangersDrupalDude - I don't understand the problem you are having. Also, we can't use HTTP/1.1 in the request, since drupal only handle HTTP/1.0 responses.
Can you please provide more details as to how to reproduce your problem or post an actual patch?
Comment #324
Raf CreditAttribution: Raf commentedGot noooooooooooooo idea how to make Drupal-complient patches, but after going through this thread, finding the 6.14 one, noticing it didn't work on my 6.20 site and applying the changes manually, I created these three patches (one for each file that's changed). They're probably not according to Drupal guidelines, but they do add in proxy support for Drupal 6.20.
It's tested on a Pressflow site (6.20), but the code in the 6.14 patch and the one in the Pressflow site were the same, so it should work under regular 6.20 as well.
Comment #326
tstoecklerFor information on creating patches see http://drupal.org/patch/create.
Comment #327
pwolanin CreditAttribution: pwolanin commented@Raf - please don't derail this issue. We are trying to resolve it for Drupal 7 only at this point.
re-roll of #320, no changes. If anyone has functional issues with this patch, please post your steps to reproduce.
Comment #328
Raf CreditAttribution: Raf commentedThis issue's been going on since 2004 (!). Proxy settings are mostly useful for enterprises. These same enterprises prefer to use Drupal 6 over Drupal 7 for now, since it's been thoroughly tested and stable, while Drupal 7's just recently been released. Because of that -- and seeing as this issue's been going on from even before Drupal 6, I don't see why we should ignore all other releases and fix this for a version the target audience won't use just yet, leaving them with no real solution until Drupal 7's mature enough to adapt.
So in other words: I'll fix it for 6.20, whether or not you want it to be D7 only. I see way more reasons to make it 6.20-compatible first than to make it 7-compatible first. If I'm not allowed to try to give a hand in making this thing work for everybody, then jeez louis, sorry for trying to help!
Comment #329
pwolanin CreditAttribution: pwolanin commented@Raf - I agree that we should have a good Drupal 6 patch, and I agree that it's enterprises that need it, but I'd like to make sure we have this first. It's possible we could convince David Strauss to put such a change into Pressflow even if it's not officially backported.
Comment #330
Dave Reid#327: 7881-proxy-please-327.patch queued for re-testing.
Comment #331
silid CreditAttribution: silid commented@raf - if you want a version for Drupal 6 as many people do, see the seperate issue #735420: Drupal 6 proxy server support.
Comment #332
Raf CreditAttribution: Raf commented@Silid: Thanks. Didn't find it when I searched for a patch (only found this thread). Bit odd that a thread that existed before Drupal 5 doesn't allow a Drupal 6 patch, but I'll go to that thread to help out on it.
Comment #333
StephenRobinson CreditAttribution: StephenRobinson commentedI believe the issue was caused by using jquery above 1.4, I am on 1.6 and the $request .= "\r\n\r\n"; breaks the json requests as it is read as the data by solr, not the data after this?
Comment #334
davidwhthomas CreditAttribution: davidwhthomas commentedThe patch works OK from here, but no HTTPS support?
e.g for core openid support ( yahoo )
it's connecting to the proxy and saying 'POST https://open.login.yahooapis.com/openid/op/auth HTTP/1.0'
which you can't do for https:// ... you need to do 'CONNECT open.login.yahooapis.com HTTP/1.0'
Maybe the patch can be updated to support HTTPS?
DT
EDIT: I actually ended up using this patch http://drupal.org/node/924498#comment-4204694 which works with HTTPS
Comment #335
wojtha CreditAttribution: wojtha commentedSubscribing.
Comment #336
rfayD8 first. Let's get these fixed on D8 and into D7.
Comment #337
pwolanin CreditAttribution: pwolanin commented@rfay - why back to cnr?
Comment #338
rfay@pwolanin, I was just assuming that when an issue was changed to another Drupal major version, somebody should at least try it out there (and re-upload the patch to test it on the testbot). Seems reasonable?
Comment #339
wojtha CreditAttribution: wojtha commented#327: 7881-proxy-please-327.patch queued for re-testing.
Comment #340
anavarreSubscribing
Comment #341
pwolanin CreditAttribution: pwolanin commentedI applied this patch to Drupal 8 and re-verified using Paros proxy that I could do update status requests and aggregate a feed via the proxy.
setting back to RTBC since the code is unchanged. I think this is still appropriate for D7 too.
Comment #342
jrbeemanApplied, tested and verified @pwolanin's patch in comment #341 to a Drupal 7 site behind a proxy. The site is now able to make proxied HTTP requests! A note on the configuration -- it does not utilize authentication, so I'm unable to verify the username/password-based proxy functionality.
Comment #343
szadok CreditAttribution: szadok commentedAny chances this will get into D7?
Comment #344
thomasmurphy CreditAttribution: thomasmurphy commentedThanks, very helpful
Comment #345
chateau_dur CreditAttribution: chateau_dur commentedHi everyone,
Just tried the #341 patch but I still can't aggregate my twitter feeds.
I can't find where I can set my proxy.pac path?
When I patched common.inc, the lines to define the server where rejected to another file, is it normal?
Thanks for the help and the effort.
Comment #346
thomasmurphy CreditAttribution: thomasmurphy commentedsubscribing
Comment #347
nvd_ai61 CreditAttribution: nvd_ai61 commented#341: 7881-proxy-please-341.patch queued for re-testing.
I applied the patch in #341 to Drupal 7.2 with no success. I am wondering if I am making some mistake in configuration. I am running nginx on port 80 and drupal on port 8080 both on my localhost. Here is my settings.php:
$conf['reverse_proxy'] = TRUE;
# $conf['reverse_proxy_header'] = 'HTTP_X_CLUSTER_CLIENT_IP';
$conf['reverse_proxy_addresses'] = array('127.0.0.1','localhost', );
# $conf['omit_vary_cookie'] = TRUE;
/**
* External access proxy settings:
*
* If your site must access the internet via a web proxy then you can enter
* the proxy settings here. Currently only basic authentication is supported
* by using the username and password variables. The proxy_exceptions variable
* is an array of host names to be accessed directly, not via proxy.
*/
$conf['proxy_server'] = 'localhost';
#$conf['proxy_port'] = 80;
# $conf['proxy_username'] = '';
# $conf['proxy_password'] = '';
# $conf['proxy_exceptions'] = array('127.0.0.1', 'localhost');
does anyone know if this settings is correct based on the information above?
Comment #348
chateau_dur CreditAttribution: chateau_dur commentedHave you modified anything else in the file?
If not, could you send me your common.inc file, I could test it on my website after changing the proxy settings.
I'd like to test on my own but I can't seem to be able to patch my file... no idea why and it's getting on my nerves.
Thanks, here's a temporary email address you can use if you like : bi1311252078erv@mail-temporaire.fr
Comment #349
pwolanin CreditAttribution: pwolanin commented@nvd_ai61 - those settings don't make much sense - do you have nginx set up to proxy to external sites?
Normally nginx is configured as a load-balancer and reverse proxy, not a forward proxy.
Comment #350
chateau_dur CreditAttribution: chateau_dur commentedDoes this patch work with 7.4? I get an error right from the beginning.
Comment #351
pwolanin CreditAttribution: pwolanin commentedThe patch works fine for me against 7.x HEAD. Use patch -p1
Comment #352
pwolanin CreditAttribution: pwolanin commented#327: 7881-proxy-please-327.patch queued for re-testing.
Comment #353
chateau_dur CreditAttribution: chateau_dur commentedHi again,
Ok I finally managed to install the patch, thanks.
Problem is, I have a new error in the aggregator module.
I get a "0 php_network_getaddresses: getaddrinfo failed: Name or service not known". Used to get an "Error 400" before.
I've set up my proxy in the settings.php file as follows:
$conf['proxy_server'] = 'http://wwwcache.myproxy.com';
$conf['proxy_port'] = 8080;
$conf['proxy_username'] = '';
$conf['proxy_password'] = '';
$conf['proxy_exceptions'] = array('127.0.0.1', 'localhost');
Am I missing something?
Some help would be welcome again if you don't mind :) I don't know much about proxies but I know mine doesn't require authentication.
Thanks again!
Comment #354
xjmTagging issues not yet using summary template.
Comment #355
sanpi CreditAttribution: sanpi commentedSubscribing
Comment #356
Juan C CreditAttribution: Juan C commentedSubscribe
Comment #357
pveltsos CreditAttribution: pveltsos commentedSubscribing....
Patched worked fine on 7.7
Reporting that the patch is no longer working on 7.8
Patch applied successfully to setttings.php but *not* to includes/common.inc
Comment #358
pveltsos CreditAttribution: pveltsos commentedDid you verify the port for your proxy (8080)?
Did you try removing the "http://" from your poxy_server
What about leaving the # sign in front of proxy_username, proxy_password and proxy_exceptions if you don't need them?
Comment #359
chateau_dur CreditAttribution: chateau_dur commentedI tried commenting what I didn't use, i tried removing the http://, using single quotes and not on the port, adding the port to the URL, nothing's working.
I'm really desperate :( I just don't know what to do.
Am I missing something?
1- run the patch
2- fill in the proxy details in default.settings.php and settings.php
3- clear the site cache
4- check for updates/run the aggregator and see if it works
Is it correct?
Comment #360
Raf CreditAttribution: Raf commentedThe patch I used (which, admittedly, is a bit old. Used the patch for D6.15 and adapted it for D6.19 or 6.20, some time ago), required me to install the patch, flush caches (as it adds a new menu item), then go to the proxy settings menu (something like system -> proxy), then filling in the proxy address + port + possible other settings there. I don't think I've touched settings.php (unless the patch did, but then that's part of applying the patch)
Comment #361
Caligan CreditAttribution: Caligan commentedIssue summary posted.
Comment #362
pwolanin CreditAttribution: pwolanin commented@pveltsos I just checked and the patch still applies for me to 7 and 8 with minor offset.
Comment #363
Cameron Tod CreditAttribution: Cameron Tod commentedsubscribing
Comment #364
Mac_Weber CreditAttribution: Mac_Weber commentedsubscribing
Comment #365
lukevanblerk CreditAttribution: lukevanblerk commentedsubscribing
Comment #366
sunLet's drop everything after the hyphen here, please, it only confuses, because you make an assumption about what a "normal" site might be, or not.
Would actually be nice to have a short inline comment explaining what that second part of the condition and helper function is supposed to check and do -- right now, I've to follow the call stack to figure it out.
1) s/url/URL/
2) I do not understand this comment, and why the query parameters are dropped here.
Why do we set the scheme to "proxy" if we use "tcp" as scheme afterwards?
If I get this right, then we're always unsetting the user-agent by default -- unless proxy_user_agent has been defined in settings.php. Thus, I think this code can be heavily simplified.
We can set 'Host' to $uri['host'], and move the condition after that, skip the $port variable, and just simply conditionally append the 'port' if necessary.
Shouldn't the proxy exceptions also contain $GLOBALS['base_url'] or something?
s/internet/Internet/ if I'm not mistaken.
6 days to next Drupal core point release.
Comment #367
halcyonCorsair CreditAttribution: halcyonCorsair commentedsubscribing
Comment #368
oscaralsubscribing
Comment #369
gwynnebaer CreditAttribution: gwynnebaer commentedsubscribing
Comment #370
gwynnebaer CreditAttribution: gwynnebaer commented@sun:
I made the comment changes (cleanup, additional comments) that I could reasonably make.
Most cache servers don't cache any URI with a query string attached, but I agree that it looks like required information would be lost here, so I think this is a bug.
The scheme 'proxy' here really means "use tcp:// with additional parameters." This could be rewritten to make all methods use the same logic, but I find this implementation easier to read.
Re: User-Agent logic, I could only improve the logic by one line, so I left it as-is.
Re: 'Host', I followed your lead and simplified the logic per other examples in common.inc.
Re: proxy_exceptions, I think what should be excluded needs to be $_SERVER['SERVER_NAME'] or similar. $GLOBALS['site_url'] contains the scheme, etc, but proxy_exceptions need to be bare host names/IPs.
Comment #371
chx CreditAttribution: chx commentedAdd a pluggalbe browser to core, have a basic implementation from drupal_http_request, add curl from simpletest (there's an issue for that already), expose curl option additions into a hook, provide a contrib to do this. It's not core's responsibility to reimplement curl in userspace nor it is the role of core to have interface for all the eight bazillion CURLOPT.
Comment #372
Aeternum CreditAttribution: Aeternum commented#327 confirmed to apply cleanly and work on 7.8
Comment #373
kenorb CreditAttribution: kenorb commentedPatch #320 works great for me, thank you.
Comment #374
StephenRobinson CreditAttribution: StephenRobinson commentedthe 1.1 was wrong, should be 1.0, didn't cause an issue with squid2 proxy, but needed fixing with squid3 proxy
Comment #375
Patrizio CreditAttribution: Patrizio commentedHere it is my backport patch for d6
Apply on commin.inc
Comment #376
z-buffer CreditAttribution: z-buffer commentedAfter applying this patch "/**" is missing in common.inc, the result is:
and must be:
Comment #377
pwolanin CreditAttribution: pwolanin commented@chx - wow I never noticed this got closed, I thought it was committed.
Given that core (especially 7.x) uses drupal_http_request for everything, asserting the curl is the answer is not reasonable.
Comment #378
denix CreditAttribution: denix commentedThere is a "Following" link at the top of issues so you can subscribe to the post without flood the issue.
If you missed it because it is unclear what a green "follow" button means, you can comment here #1376380: "Following" button purpose and use may be unclear
Comment #379
pwolanin CreditAttribution: pwolanin commentedassigning to Dries, since that's what webchick suggests.
sun's last review is basically asking for some improved code comments - I don't think there is any problem with the code itself.
Comment #380
PolPeople looking for a patch for Drupal 6.25: http://drupal.org/node/735420#comment-5724654
Comment #381
Dries CreditAttribution: Dries commentedI'm still ok with this going in.
Just wondering if there is a Symfony2 component that we could or should leverage instead of building our own.
Any thoughts on that?
Comment #382
pwolanin CreditAttribution: pwolanin commented@Dries - the main goal for this issue was a simple implementation that could be used at least in D7 also (well, or originally, 6, 5, 4.7. 4.6, or 4.5!)
Certainly, D8 may evolve beyond this in another issue, but the for you the question was whether this should go in based on this patch which is back-portable to D7.
Comment #383
devonwarren CreditAttribution: devonwarren commented#341: 7881-proxy-please-341.patch queued for re-testing.
Comment #384
mikeytown2 CreditAttribution: mikeytown2 commentedre-roll of #370. I have no good way to test the functionally of this; I'm mainly interested in getting this working for #1320172: Bring in proxy support.
Comment #385
denix CreditAttribution: denix commented#384: drupal-7881-384-add-proxy-support-for-http-request.patch queued for re-testing.
Comment #386
RobLoachBuzy and Buzz handle external requests via Symfony components. Not entirely what this issue is attempting to solve, but worth mentioning here.
Comment #387
pwolanin CreditAttribution: pwolanin commented@Rob - see above. Try to get it RTBC now so it can be backported to 7.
Comment #388
mikeytown2 CreditAttribution: mikeytown2 commentedAbove patch (#384) forgot to have the default headers move higher up. Added in more comments and better explained why
unset($uri['query'])
is used.Comment #389
chx CreditAttribution: chx commentedI am still not OK with this going in! Can we close this and do #64866: Pluggable architecture for drupal_http_request() and or re-commit #340283: Abstract SimpleTest browser in to its own object ? Why proxies? Why are we reimplementing cURL piecemeal in userspace? This is not a good idea. I am not buying the "php is not built with curl" option -- if ypu can do a proxy you can do your own PHP.
Comment #390
pwolanin CreditAttribution: pwolanin commented@chx - this is a pretty trivial mount of code for a feature that's long been missing. Yes for Drupal 8 let's then proceed to rip it out if something better gets into core.
Comment #391
chx CreditAttribution: chx commentedI am but one voice. I have been overridden by core committers more than once.
Comment #392
sylus CreditAttribution: sylus commentedI really think we need to commit a working solution to Drupal 7 as soon as possible. Many government sites are starting to jump on the Drupal bandwagon and use such distributions as Open Public etc.
Without this patch I am assuming such Distributions (Apps) are unusable which would mean a lot less government adoption. At the very least questions as to why we can't support a proxy setup.
Comment #393
wroxbox CreditAttribution: wroxbox commentedOur client has at the moment a new server infrastructure were the patch in #388 is the only solution to get drupal working correct inside the firewalls and proxies. Patch is tested and working as supposed. +1 to get it into core asap!
Comment #394
sylus CreditAttribution: sylus commentedI had to reroll #388 as it would not work with drush make. Though could have been a problem on my part.
Below are 2 git patches one for common.inc and the other settings.php. (Drupal 7.14)
Fails testing on Drupal 8 as patch is meant for Drupal 7.14 (Verified and tested with Drush Make)
Comment #396
Vincenzo CreditAttribution: Vincenzo commentedThat's exactly what I've been needing these days. I haven't tested it yet, so I have a question: if the original URI I have to request is over HTTPS, is it going to be able to handle that via the proxy?
Comment #397
chx CreditAttribution: chx commentedMeh. It's easier to commit #388 than clean up this mess of an issue. I could easily delete half of the comments here. People think core is contrib, meh, go with it,like I care. Before you commit consider how bloated common.inc already is. This might or might not matter. But , no matter what it is just bloat. It's utterly wrong to reimplement cURL in userspace but as this issue testifies people so do not care so I do not care either.
Comment #398
effulgentsia CreditAttribution: effulgentsia commented#388 doesn't apply any more. This is a reroll. Additionally, I removed the "Proxy setup" code comment since it seemed redundant with the code comment right below it, and I added 'proxy_user_agent' to default.settings.php.
Trivial changes, so leaving at RTBC.
Comment #399
effulgentsia CreditAttribution: effulgentsia commentedI won't object to #398 being committed, but I think #1664784: drupal_http_request() has problems, so allow it to be overridden is a better approach. There's no need for us to keep plugging holes in our outdated drupal_http_request() implementation when more robust libraries exist.
Comment #400
pwolanin CreditAttribution: pwolanin commented@effulgensia - as before, getting this into D8 as-is would be useful as a basis for backport to D7. Obviously we will want to improve the http request offerings in D8, especially as the plugin system somes in to place.
Comment #401
Dries CreditAttribution: Dries commentedI committed this to 8.x.
Next steps:
1) Backport to Drupal 7
2) Improve the implementation in Drupal 8
Let's start with 1, and than go back to 2.
Comment #402
pwolanin CreditAttribution: pwolanin commenteda note for the D8 changelog?
Comment #403
Dries CreditAttribution: Dries commentedCommitted the CHANGELOG update to 8.x. Thanks!
Comment #404
gwynnebaer CreditAttribution: gwynnebaer commentedExcellent news, Dries. The 20th oldest open issue with Drupal is moving forward again. Much appreciated. We are heavy users of this feature and can provide regression testing for 7.x if needed.
Comment #405
mikeytown2 CreditAttribution: mikeytown2 commentedRolling #394 in one as this is for D7 and setting to needs review for testbot.
Comment #406
mikeytown2 CreditAttribution: mikeytown2 commentedThis patch is the one above with the trivial changes made in #398 merged in.
Comment #407
andypost8.x changes was reverted in #1619446-40: All autocomplete and drupal_json_output() responses are broken (returning Ajax instead of JSON)
Comment #408
effulgentsia CreditAttribution: effulgentsia commented#407 looks to have gotten resolved. Doing a diff of #406 vs. #398 verifies it to be a straight port. All that's missing from #406 is a 7.x CHANGELOG entry ported from #402, and this can be RTBC.
Comment #409
mikeytown2 CreditAttribution: mikeytown2 commentedPatch is #406 & #402 merged together. Marking as RTBC.
Comment #410
nemo_Anhoa CreditAttribution: nemo_Anhoa commentedHi there,
The network engineers at my place just put everything through the proxy, and the RSS Aggregator seems cannot be updated anymore.
Our intranet website use Drupal CMS 7.12
It keeps showing the Error 111 Connection Refused. However, the current version of Drupal 7 we use is 7.12. I wonder did the patch is included in the latest version which is 7.14.
Do I have to apply the patch seperately or simply just update to Drupal 7.14 will do ?
Many thanks in advance.
Ann
Comment #411
effulgentsia CreditAttribution: effulgentsia commentedThe fix is only in Drupal 8 so far, so for Drupal 7, you need to apply #409 yourself. It's marked "reviewed & tested by the community", so will likely be committed soon, and be included in 7.15 or 7.16.
Comment #412
Dries CreditAttribution: Dries commentedMoving to David instead.
Comment #413
chx CreditAttribution: chx commentedNot like we couldnt backport pluggability into D7. Sad, sad thing to see this go in.
Comment #414
sysadmin.logibel@retail-sc.com CreditAttribution: sysadmin.logibel@retail-sc.com commented#406: drupal-7881-406-add-proxy-support-for-http-request.patch queued for re-testing.
Comment #415
David_Rothstein CreditAttribution: David_Rothstein commentedSorry, I didn't have much time to work on Drupal the last few weeks, and I was focusing most of the time I did have on Drupal 7.15 release blockers (which I was only getting help from a couple other people on) and other bugs. So, I won't be committing this in time for Drupal 7.15 because it's too late to be reviewing a big new feature.
But, skimming the last 30 comments or so, I get the impression that this isn't really "Drupal 7 RTBC"? Meaning, it's OK to commit a patch to Drupal 8 if the idea is along the lines of "let's get something in even if it's not quite right because we can always change it 6 months from now". But not Drupal 7; once this goes into Drupal 7 it's staying in, so we need to be sure this is the fix we want to go with.
I don't have a strong opinion between this issue and #1664784: drupal_http_request() has problems, so allow it to be overridden - between those I'll commit to Drupal 7 whichever one people tell me to :) My main interest is in making sure we only have to fix this once...
So, moving back to "needs review" for that to get settled.
Comment #416
effulgentsia CreditAttribution: effulgentsia commentedI don't see any comments suggesting that #409 or what it's backported from introduces any problems or fails to solve the issue. chx dislikes that we're adding features to drupal_http_request() instead of making it overridable so that proxy support can be done as a contrib feature, but I think Dries overrode him on that in #401. #1664784: drupal_http_request() has problems, so allow it to be overridden is therefore postponed until there's either more progress on the D8 version in #1447736: Adopt Guzzle library to replace drupal_http_request() or a separate D7 need that would justify it.
Comment #417
pwolanin CreditAttribution: pwolanin commentedThe patch here is not that large, and at one point prior to D7 release, was RTBC for D7. The patch has not really changed since then - and the main goal of getting it in 8 was to make it eligible for commit to 7 too, since I expect the larger refactoring of http requests in D8 are unlikely to be suitable for backport.
Comment #418
denix CreditAttribution: denix commentedHi all, can the patch be easily applied to D7.15 without risks?
Comment #419
effulgentsia CreditAttribution: effulgentsia commented#409: drupal-7881-409-add-proxy-support-for-http-request.patch queued for re-testing.
Comment #421
effulgentsia CreditAttribution: effulgentsia commented"Without risks" is a subjective decision for you to make. I queued a retest of #409: if it comes back green, then at least you'll know it passes all of our automated tests. The issue is marked "reviewed and tested by the community", so as far as we know, it's good to go and just waiting for David to commit to the 7.x branch. If you apply it to your production environment before it's included in a stable 7.x release, you'll be among the first people to test it in the wild. If you choose to do that, please comment here with how it works out, because that's very helpful feedback to get.
Comment #422
effulgentsia CreditAttribution: effulgentsia commentedRerolled. Just moved the CHANGELOG entry, so leaving at RTBC.
Comment #423
denix CreditAttribution: denix commentedThank you Alex,
until now I was using the patch in 7.14 in a development environment without any problem. I am not a skilled Drupal developer, but I checked the logs for a while and haven't see any negative impact on the whole, complex, installation (multilingual, multi-install, multi-domain). As soon as I move it to production I will come back with a more detailed report.
Comment #424
Taz CreditAttribution: Taz commentedI'd love to see this in Core ASAP, the patch works well - but only for HTTP traffic. HTTPS fails
Comment #425
pwolanin CreditAttribution: pwolanin commentedIt is not expected to work for https. It adds support for simple http proxies
There is another patch here: #924498: Proxy https support for drupal_http_request() you might want to look at.
Comment #426
edsonsalesjr CreditAttribution: edsonsalesjr commented#218: proxy-2.patch queued for re-testing.
Comment #427
edsonsalesjr CreditAttribution: edsonsalesjr commented#240: proxy.6.15.patch queued for re-testing.
Comment #428
David_Rothstein CreditAttribution: David_Rothstein commentedSo, I was mainly looking at comments like @chx's in #413 which pointed out that from the perspective of backportability alone, there is no reason to prefer one of these patches over the other (and I think that is certainly correct)....
@chx, do you have any further thoughts on this issue? Is it OK to commit this to Drupal 7?
Comment #429
David_Rothstein CreditAttribution: David_Rothstein commentedBy the way, the above patch already no longer applies correctly.... Here's one that does (with the CHANGELOG.txt entry removed).
I've been adding changelog entries on commit anyway, so there's no reason to have it in the patch because it will just make it necessary to keep rerolling this.
Comment #430
chx CreditAttribution: chx commentedwebchick asked me to give my opinion for the last time, so here it is: it was wrong to commit to d8, it is wrong to commit to d7 but that's what the community wants so let's do it.
Comment #431
webchickOk, I'm counting that as a "yes." There are 140 people following this issue, over 400 comments, so given that chx's blessing seemed to be David's only concern, I am going to be bold and commit this.
Committed and pushed to 7.x. Yay!
Tagging with 7.16 release announcement, which I think (?) is the proper course of action. I also added a note to CHANGELOG.txt.
Comment #432
stacysimpson CreditAttribution: stacysimpson commentedFirst of all, I cannot tell you how happy we are to _finally_ have HTTP proxy support in core!
One issue that we ran across...
The patch above results in the following PHP warning:
Adding an explicit cast to array in _drupal_http_use_proxy() seems to resolve the warning.
If it matters, we are using PHP Version 5.2.10-2ubuntu6.4
Comment #433
lotyrin CreditAttribution: lotyrin commentedre #432:
Older versions of this patch used a comma seperated list (string).
It's possible you have such a string stored in the variables table (or even in your settings.php).
You will need to replace it with an array (or remove it in the case that it matches the default).
The cast here will suppress the error but is not likely to behave correctly.
Comment #434
stacysimpson CreditAttribution: stacysimpson commented@lotyrin, yep, that was it. Thanks!
Comment #436
David_Rothstein CreditAttribution: David_Rothstein commentedDrupal 7.16 was a security release only, so this issue is now scheduled for Drupal 7.17 instead.
Fixing tags accordingly.
Also, I invented the "release announcement" tag for issues that require special mention even for non-technical users (i.e., in the release announcement itself, rather than just in the technical release notes).... So the idea is that's mainly for very rare issues that might have social/community implications (such as #1036780: Drupal.org should collect stats on enabled sub-modules and core modules). Of course, I never said that anywhere except in a comment buried in that issue, so no one but me understood what that tag is intended for :) Given that, though, I'm moving this one over to the more normal "release notes" tag as well.
Comment #437
David_Rothstein CreditAttribution: David_Rothstein commentedHm, it looks like those tags didn't get switched correctly. Trying again. Sorry for the noise...
Comment #438
aaron.r.carlton CreditAttribution: aaron.r.carlton commentedI've applied this patch to a D7 site I'm working on and I'm having an issue with requesting HTTPS url's via the configured proxy. In my particular example, I'm using the fboauth module to implement OAuth by connecting to https://graph.facebook.com. My client has Drupal configured to use a Bluecoat proxy to make all outgoing server-server requests. Debugging via TCP dump indicates that Drupal's connection to the proxy issues a GET request.
According to RFC2817 5.2, it appears that a CONNECT type is required when making an SSL request through a proxy in order to establish a tunnel connection.
I've since found this issue: http://drupal.org/node/924498 but it appears that vector of the issue wasn't accounted for in the above patch.
I'm not sure if it's appropriate to indicate that this issue "needs more work", especially since it's been committed already. I also recognize that the best place for this discussion is probably in the other ticket, but I thought it was worth bringing up here since it feels like the inability to request SSL resources when using a proxy makes the feature 'incomplete' at best.
I am really appreciative that proxy support was added to D7, but I think it should either accommodate HTTPS requests and work entirely, or be decided that living without SSL is acceptable and well documented. I anticipate someone reviewing my feedback and would be happy to help on a followup patch if it had community support and a chance of getting into D7.
Comment #439
laurentchardin CreditAttribution: laurentchardin commentedaaron,
You might want to follow this issues instead : http://drupal.org/node/924498
which is the current thread about SSL support. The patch obviously need a reroll with this one, which is about to be shipped with the next release.
I recommend closing this thread again since the patch has been validated in its current state.
And continue the work on the follow-up.
Comment #440
aaron.r.carlton CreditAttribution: aaron.r.carlton commentedComment #441
drupalycious CreditAttribution: drupalycious commentedthis patch doesn't seem to apply to Drupal 7.17
Comment #442
drupalycious CreditAttribution: drupalycious commented#429: drupal-7881-429-add-proxy-support-for-http-request.patch queued for re-testing.
Comment #444
laken CreditAttribution: laken commentedThis patch has been committed and released in Drupal 7.17, see http://drupal.org/drupal-7.17-release-notes – no need to apply it to 7.17 'cause it's already in there!
Comment #445
David_Rothstein CreditAttribution: David_Rothstein commentedIt's already in Drupal 7.17. (edit: that was a crosspost :)
Comment #446
Tor Arne Thune CreditAttribution: Tor Arne Thune commented@sp-drupy: The patch in #429 is already a part of Drupal 7.17.
Comment #447
drupalycious CreditAttribution: drupalycious commentedsorry,
I am still receiving the error
Notice: Undefined offset: 2 in drupal_http_request() (line 986 of /Drupal/includes/common.inc).
that's why I thought it was not there.
Thank you
Comment #448
sanette CreditAttribution: sanette commented(I'm coming here from http://drupal.org/node/848522)
I have installed drupal 7.17 on my laptop, and setup one RSS feed via the aggregator module
* from home, I can download the feed by clicking "update items" in "admin/config/services/aggregator"
* from work, I still get "The feed from (...) seems to be broken, because of error "-110 Connection timed out".
In Firefox, the feed is always downloadable. And there is no proxy configured...
any hint ?
Comment #449
Hant-1 CreditAttribution: Hant-1 commentedI've been here because of an issue of the module Feeds. In fact, I have a proxy in my workplace.
I've got the Error: 'HRCurlException: cURL error (7) couldn't connect to host for ....'.
So here is what I do
- Verify you have 'http://' before your feed URL (in the configuration of Content - Feed).
- If you have a proxy, you could add the line below just between the 171-172 lines of 'http_request.inc' file in directory 'feeds\libraries'.
curl_setopt($download, CURLOPT_PROXY,"YOUR_PROXY:PROXY_PORT");
For example, just like what I have,
Hope that helps.
Best regards.
Comment #449.0
Hant-1 CreditAttribution: Hant-1 commentedUpdated issue summary.
Comment #449.1
effulgentsia CreditAttribution: effulgentsia commentedUpdated issue summary.
Comment #450
dgtlmoon CreditAttribution: dgtlmoon commentedUpdated the title to reflect what is really going on here (from what I understand) #924498: Proxy https support for drupal_http_request() is the fallout
Comment #451
apaderno