We have many issues in the queue pointing out various limitations of drupal_http_request(). A particularly old one, #7881: Add support to drupal_http_request() for proxy servers (http not https), is basically requiring some government departments and other organizations to run production websites on a hacked core. #64866: Pluggable architecture for drupal_http_request() is working on a robust solution for D8, but that will take a while to settle out. Meanwhile, here's an interim step that can be applied to D8 and D7 that will allow these sites to revert their core hacks, and plug in a contrib solution like httprl, cURL, or some other feature-rich library.

CommentFileSizeAuthor
#25 drupal-1664784-13-override-core-D6.patch762 byteshass
#13 drupal-1664784-13-override-core-D6-do-not-test.patch762 bytesmikeytown2
#13 drupal-1664784-13-override-core-D7.patch726 bytesmikeytown2
#1 drupal_http_request_override.patch894 byteseffulgentsia
drupal_http_request_override.patch896 byteseffulgentsia

Comments

effulgentsia’s picture

effulgentsia commented
StatusFileSize
new drupal_http_request_override.patch894 bytes 
+++ b/core/includes/common.inc
@@ -729,6 +729,17 @@ function drupal_goto($path = '', array $options = array(), $http_response_code =
+  $alternate_http_system = &drupal_static('__FUNCTION__');

This one removes the erroneous quotes.

freblasty’s picture

freblasty commented

You could use the Webclient module which supports overwriting the drupal_http_request function (see Webclient extension) with a version that supports request altering, proxy, ... as a temporary solution until this issue gets fixed.

Note, this requires the PHP extension runkit or APD to be installed.

Crell’s picture

Crell commented
Status: Needs review » Reviewed & tested by the community

Nothing that requires runkit should ever be used in production. runkit is dangerous by design.

This patch is a total hack, but that's about all we could do for Drupal 7. For Drupal 8, I'm really hoping this entire routine goes away in favor of Guzzle or similar. So, eh, I can live with it for now, for the sake of D7 folks. But #64866: Pluggable architecture for drupal_http_request() and company are still mandatory for WSCCI, with or without this.

robloach’s picture

robloach
he/him
commented

Ew.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Postponed

Let's wait for #64866: Pluggable architecture for drupal_http_request().

Crell’s picture

Crell commented

I think we already concluded to not make it pluggable but to just use a good library: #1447736-19: Adopt Guzzle library to replace drupal_http_request(). We know what we want to do for Drupal 8, but it's not backportable. Really, this is a D7 issue that's flagged for D8 only for process reasons.

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
commented
Issue tags: +Needs backport to D7

Fixing tags per http://drupal.org/node/1517250

mikeytown2’s picture

mikeytown2 commented
Version: 8.x-dev » 7.x-dev

Moving to D7. Guzzle is in D8 now.

effulgentsia’s picture

effulgentsia commented
Assigned: Unassigned » dries
Status: Postponed » Needs review
Issue tags: -Needs backport to D7

Back to needs review, since it's been a while since #3. Assigning to Dries for comment, given #5.

Although #7881: Add support to drupal_http_request() for proxy servers (http not https) has been fixed, #924498: Proxy https support for drupal_http_request() and many other D7 issues are still open.

mikeytown2’s picture

mikeytown2 commented

Should we re-open #64866: Pluggable architecture for drupal_http_request() under D7?

ParisLiakos’s picture

ParisLiakos commented

coming from #924498: Proxy https support for drupal_http_request()
I think #64866: Pluggable architecture for drupal_http_request() is too much for d7 and feels that at least this one, will get in faster, so i guess if tests still pass it is RTBC?

#1: drupal_http_request_override.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, drupal_http_request_override.patch, failed testing.

mikeytown2’s picture

mikeytown2 commented
Status: Needs work » Needs review
StatusFileSize
new drupal-1664784-13-override-core-D7.patch726 bytes
new drupal-1664784-13-override-core-D6-do-not-test.patch762 bytes

Here's my take on this. It's simple too.

ParisLiakos’s picture

ParisLiakos commented

i am more fan of function as well, but maybe variable should be http_request_function instead of drupal_http_request_function

dries’s picture

dries commented
Status: Needs review » Reviewed & tested by the community

I'm ok with this patch. I can't think of any unwanted side-effects. Leaving the final call to David though.

effulgentsia’s picture

effulgentsia commented
Assigned: dries » David_Rothstein
David_Rothstein’s picture

David_Rothstein commented
Assigned: David_Rothstein » Unassigned

Looks fine to me but I really don't know what's up with the plans for comitting feature requests these days (thresholds etc). It'll presumably get committed at some point though...

David_Rothstein’s picture

David_Rothstein commented
Category: feature » task
Status: Reviewed & tested by the community » Fixed
Issue tags: +7.22 release notes

Whatever, let's just call it an API limitation and therefore a task and move on. Plenty of other patches have gotten in that way...

Committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/025ad19

ParisLiakos’s picture

ParisLiakos commented

yay!
thanks a lot David

mikeytown2’s picture

mikeytown2 commented

What are the odds of this getting in 6.x? #13 has a patch for D6.

hass’s picture

hass commented
Version: 7.x-dev » 6.x-dev
Status: Fixed » Needs review

Patch in #13 is for D6

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
commented
Status: Needs review » Reviewed & tested by the community

It's identical to what's already been committed to 7.x, so RTBC. Looks great.

chaps2’s picture

chaps2 commented

+1 for getting this in to 6.x please.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 13: drupal-1664784-13-override-core-D7.patch, failed testing.

hass’s picture

hass commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new drupal-1664784-13-override-core-D6.patch762 bytes

Reattaching D6 patch from #13.

hass’s picture

hass commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Can we close this issue now or do we have to wait till the official EoL?

Status: Reviewed & tested by the community » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.