Posted by pepiqueta on May 22, 2006 at 8:52am
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | base system |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Dave Reid |
| Status: | needs work |
Issue Summary
In case your hosting provider has disabled fsockopen() for some reasen, here's a patch that can bypass that restriction using curl.
I know the code is ugly, but it was a quick hack for a site I administer. Please add suggestions or improvements.
Code below replaces ping_ping() in ping.module.
function ping_ping($name = '', $url = '') {
function curl_xmlrpc_ping ($ping_server, $local_name, $local_url) {
$xml_request = '<?xml version="1.0"?>'; // encoding?
$xml_request .= '<methodCall><methodName>weblogUpdates.ping</methodName>';
$xml_request .= '<params><param><value>'.$local_name.'</value></param>';
$xml_request .= '<param><value>'.$local_url.'</value></param></params></methodCall>';
$curl_handle = curl_init();
$post_header = array( 'Content-type: text/xml' ); // required for xml posting
curl_setopt ($curl_handle, CURLOPT_URL, $ping_server); // hostname with 'http://'
curl_setopt ($curl_handle, CURLOPT_POST, 1);
curl_setopt ($curl_handle, CURLOPT_HTTPHEADER, $post_header);
curl_setopt ($curl_handle, CURLOPT_POSTFIELDS, $xml_request);
curl_setopt ($curl_handle, CURLOPT_RETURNTRANSFER, 1);
$curl_buffer = curl_exec($curl_handle);
curl_close($curl_handle);
$response = trim(strip_tags($curl_buffer));
if (preg_match ('/flerror{0,1}/i',$response)) {
$return_msg = 'Server response: ';
if (preg_match ('/flerror0/i',$response)) {
$return_msg .= 'OK, ';
$return_code = 0;
} else {
$return_msg .= 'ERROR, ';
$return_code = 1;
}
$return_msg .= preg_replace ('/message/i','',stristr ($response, 'message'));
} else {
$return_msg = 'ERROR, NOT a weblogUpdates.ping response: ';
$return_msg .= $response;
$return_code = 2;
}
$return_array[code] = $return_code;
$return_array[msg] = $return_msg;
return $return_array;
}
$pingers = explode("\n", variable_get('ping_pinger_list', 'http://rpc.pingomatic.com'));
foreach ($pingers as $pinger) {
$pinger = trim($pinger);
if (empty($pinger)) {
continue;
}
$result = curl_xmlrpc_ping ($pinger, $name, $url);
if ($result[code] = 0) {
$watchdog_severity = 'WATCHDOG_NOTICE'; //log successful pings too
}
else {
$watchdog_severity = 'WATCHDOG_WARNING';
}
watchdog('directory ping',t('Server: '.$pinger.'. Response: '.$result[msg]), $watchdog_severity);
}
}
Comments
#1
no new features for 4.7
#2
Oh, sorry.
#3
What are the odds that fsockopen is disabled, and curl is enabled? I think it's not very likely.
#4
Core should provide a hook in drupal_http_request through which a curl_http_request can take over. That's a minimal patch. Then curlhttprequest module can be put to contrib (not into core, though).
#5
Let's make ab object out of drupal_http_request ;p
But seriously, chx is right. I think we should make an implementation akin to user_mail.
#6
I don't know how often this happens. In fact I know of only one case: mine! I set up a site and when I tried to ping some servers it failed because fsockopen was disabled (it seems that some users on that hosting provider abused it for spamming or something). So I did this for bypassing that restriction. I don't know if this will be helpful for someone, but I just wanted to share this.
I just wanted that feature for ping.module. I agree that it should appear somewhere else in the source tree, but I'm noob to Drupal (well, PHP newbie too...) so that's too much for my skills. Maybe someone could do this.
BTW, I aplied for CVS access a couple of days ago but got no response from admins.
#7
Any work done in the patch code?
Moving to cvs.
#8
I'm going to pick up this issue. I'd like to be able to over-ride drupal_http_request with curl.
#9
Here's my first attempt at this patch. Introduces a new hook: hook_http_request. The main fetching code done in drupal_http_request has been moved to system_http_request. drupal_http_request now acts as the abstract HTTP request function:
<?php// Perform request.
$module = variable_get('http_request', 'system');
$result = module_invoke($module, 'http_request', $url, $options);
?>
Patch also makes use of the new hook_modules_enabled and hook_modules_disabled hooks to override and un-override the http_request variable:
<?php
/**
* Implementation of hook_modules_enabled().
*/
function system_modules_enabled($modules) {
// Check if modules define hook_http_request().
// Set override variable if it requests successfully.
// TODO: Registry has not been rebuilt yet and module_hook FAILS if module does have hook_http_request().
foreach ($modules as $module) {
if (module_hook($module, 'http_request') && module_invoke($module, 'check_http_request')) {
variable_set('http_request', $module);
break;
}
}
}
/**
* Implementation of hook_modules_disabled().
*/
function system_modules_disabled($modules) {
// Check if modules defined hook_http_request().
// Reset override variableif no longer available.
foreach ($modules as $module) {
if (module_hook($module, 'http_request')) {
variable_set('http_request', 'system');
module_invoke('system', 'check_http_request');
break;
}
}
}
?>
Attached is the patch to core and the first-version curl module to test overriding http requests. Hopefully this can start getting a little momentum and review towards this patch.
#10
I should note that this patch also changes drupal_http_request to use an $options parameter instead of the $headers, $method, $data, and $retry parameters.
#11
Mollom is affected by this too so I'd like to see us fix this. For Mollom, we would like to open a secure 'https' connection to mollom.com. That seems impossible to do with the current drupal_http_request() code but would be possible with CURL.
Looking at your patch, I wonder if it needs to be that configurable. It feels a bit over-engineered to me. Why don't we just use CURL, when CURL is available, and fall-back to the current implementation when CURL is not available?
#12
Someone might want to use the http extension in PECL mayhaps in php core soon? Anyways those system hooks hurt my eyes. Why not jus let moudles set their variables in their install hook instead of burdening system with it? Also, why is this in .module instead of a handy system.http.inc or something? Excellent opportunity to remove rarely run code out of every page request!
#13
The last submitted patch failed testing.
#14
I should probably model this patch off the work done in #303930: Pluggable architecture for aggregator.module, and give the site administrator the option to select which module to use for drupal_http_request instead of having the modules set variables on install/uninstall. I'm also going to split off the parameter array-itizing into #337783: DX: Array-itize drupal_http_request()'s parameters and hopefully get that accepted first before this issue.
#15
Postponing until #337783: DX: Array-itize drupal_http_request()'s parameters gets in HEAD.
#16
Related: #340283: Abstract SimpleTest browser in to its own object
#17
subscribing. i'd love to see this feature added.
#18
#337783: DX: Array-itize drupal_http_request()'s parameters has landed, so I'm switching this back to 'code needs work'.
#19
When CURL is supported, core should use it. For one, it makes SSL support a lot easier, and it is more robust than our own implementation is. So, OK for making it a pluggable solution (if needed), but let's avoid introducing a barrier here -- we don't want people to install or configure anything when CURL is available, IMO. I think CURL should be the default, with a silent and graceful fallback to our own implementation when CURL is not available.
#20
Considering SimpleTest already has a CURL browser I would recommend we focus on it. I already have a working pluggable backend patch at: #340283: Abstract SimpleTest browser in to its own object.
It still needs a bit of work, but getting close.
#21
I still think that the system_http_request should use the HTTP(S) wrapper with file_get_contents. We can use stream_context_create to set options and stream_get_meta_data to read the response headers.
#22
#23
Might assist with #7881: Add support to drupal_http_request() for proxy servers. Subscribing. Also, for those who don't know, #363787: Plugins: Swappable subsystems for core.
#24
subscribing. Looking forward to cURL with Proxy handling
#25
can we focus/look at #20
I think it is rather far along, just been to busy to finish it.
#26
essentially blocked on Crell's mythical handlers in core?
#27
The way we make things pluggable can now be seen in system.queue.inc and soon cache.inc. (and field_sql_storage.module)
#28
I will be working on #20 and I would like some reviews (hope to have documented patch later today). It will provide the same tools that drupal_http_request() does (we could even keep that as a wrapper), but also has a number of advanced browser tools and a pluggable architecture.
#29
any progress on this?
#30
#31
subscribing
#32
Subscribe now that I've created the HTTP Parallel Request Library module.
#33
Working on a version of this - aiming to incorporate suggestions of prior comments. Results won't be immediate but I may post partially usable patches as I go for ideas ...
#34
So, I have a version of this working to the point where the simpletests all pass for the native HTTP client, but I get several fails for the curl client. Part of the problem here is that the CURL client hides numerous steps from the user, for example during multiple redirects, and the tests depend on knowing the details of the interim steps.
Now, I can either change the test so that it also detects CURL and does slightly different tests in those cases, or I can throw away some of the optimizations that curl uses and handle every request individually (so, instead of allowing curl to follow multiple redirects, make each redirect a single curl call and keep track of history). My feeling is that I need to keep curl's behaviour, otherwise we're just neutralising its advantages, but I don't know entirely if we have a policy on conditionally testing different things based upon available capabilities.
#35
discussed with Gabor in chat:
hi Gabor - if you have a moment, do you have an opinion on this: I've been working on a pluggable system for drupal_http_request that wil use curl if available. Now, the simpletests expect redirects to be handled as many individual requests, with detailed info about the interim requests. The curl implementation, if I allow it to use the best of curly goodness, does not do that, so tests fail. The question is: is it better to emulate the native drupal_http_request behaviour to please tests, discarding curl's optimized flow, or fork the tests so that they also detect whether curl is being used and test differently? (don't really like that either - having the tests "unstable") both have downsides to me … I guess there might be a third option: change the tests so that they cover both adequately, but without forking (requires less stringent testing of the native drupal_http_request) Alan Evans @ 1:41:18 PM
yeah, I guess since drupal_http_request() is used at various places in tests, it would be best to modify the original tests instead of forking, no? Gábor Hojtsy @ 1:42:39 PM
I'd lean toward that third option, yeah … forking sounds like a recipe for disaster (you don't absolutely know which fork of tests will run) and emulating is bad too (removes some of curl's advantages) Alan Evans @ 1:45:05 PM
thanks
#36
Attaching a patch warts-and-all in case anyone wants to offer opinions on the general direction this is taking.
Notes: