drupal_http_request() adds 'Content-Length' header even if there's no content in the request
skiminki - July 9, 2008 - 07:15
| Project: | Drupal |
| Version: | 5.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | patch (to be ported) |
| Issue tags: | needs backport to D6 |
Description
The function drupal_http_request() adds 'Content-Length' header even if there is no content in the request. Even if this should be ok, some web servers seem to have problems with it. For example, you cannot use aggregator to fetch feeds from http://www.yle.fi/uutiset/rss/ymparisto.xml .
The following patch makes 'Content-Length' header go away when there's no content in the request.
The patch command to run is: (run it in your drupal base dir)
patch -Np1 <drupal_http_request_content_length_0.patch| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| drupal_http_request_content_length_0.patch | 897 bytes | Idle | Failed: Failed to apply patch. | View details | Re-test |
#1
#2
The patch looks good, but bugs have to be fixed in the current development version (7.x) first, than backported. Can you reroll a patch for 7.x?
#3
Here is an updated patch which should work for 7.x-dev.
#4
#5
Some servers (e.g. Squid that is often used in a reverse proxy scenario) seems to require the Content-Length header for POST and PUT requests, even if the message body is empty. You can verify this by sending the following request to drupal.org:80 using a telnet client - the server responds with 411 Length Required:
POST /foo HTTP/1.0Host: drupal.org
I suggest that Content-Length is also added if the request method is POST or PUT, even if $options['data'] is empty. I assume that the problem described in this issue only occurs with GET requests.
BTW, the HTTP spec isn't very clear about whether sending a Content-Length for GET requests is allowed:
http://lists.w3.org/Archives/Public/ietf-http-wg/2006AprJun/0103.html
#6
In my case (http://johannes.hansen.name/) it is at least HEAD and GET requests. POST and PUT works fine for me with zero-length content.
#7
The last submitted patch failed testing.
#8
Patch contained "incorrect" file path (i believe)
#9
I tested this, and it works.
One teeny-weeny nit: Most short strings in Drupal use single quotes (unless they contain escaped characters like \n etc.), so for consistency I suggest you use that.
#10
The last submitted patch failed testing.
#11
Reroll.
#12
The last submitted patch failed testing.
#13
#14
Two possible micro optimizations:
1. It is not clear whether strlen() is a fast operation (e.g. the length is cached) or a slow operation (e.g. we need to count all characters in the string), but we are calling it twice now on the same string.
2. It might be faster to check the request type before doing a strlen().
#15
I couldn't find a good reference, but I think that
if (!$foo)may be slightly faster than using strlen().#16
+ // Only add 'Content-Length' if we actually have any content or if it is a+ // POST or PUT request. Some non-standard servers get confused by content length
+ // in at least HEAD/GET requests, and Squid always requires content length in
+ // POST/PUT requests.
Please use Content-Length consistently here.
+ if ($options['data'] ||+ $options['method'] == 'POST' ||
+ $options['method'] == 'PUT') {
I don't see why that needs to be multi-line. Additionally, !empty() for $options['data'] wouldn't hurt for clarity.
#17
Thanks for the review. This patch addresses your comments.
#18
Unfortunately, this patch no longer applies. It needs a quick re-roll. I'll commit after reroll.
#19
Reroll.
#20
Committed to CVS HEAD. Thanks!
#21
It's easy to port first to 6.x
#22
+ $defaults['headers']['Content-Length'] = strlen($data);+ $defaults['Content-Length'] = strlen($data);#23
Here reroll with #22 and against current DRUPAL-6
#24
Seems my advice in #22 was bad. Now the Content-Length header isn't added at all.
#25
@c960657 This was my fault, in drupal 6 array of headers holds values same as keys
<?php// Create HTTP request.
$defaults = array(
// RFC 2616: "non-standard ports MUST, default ports MAY be included".
// We don't add the port to prevent from breaking rewrite rules checking the
// host that do not take into account the port number.
'Host' => "Host: $host",
'User-Agent' => 'User-Agent: Drupal (+http://drupal.org/)',
- 'Content-Length' => 'Content-Length: '. strlen($data)
);
?>
Here examples
<?phpdrupal_http_request('http://www.drupal.org/');
?>
GET / HTTP/1.0Host: www.drupal.org
User-Agent: Drupal (+http://drupal.org/)
<?phpdrupal_http_request('http://www.drupal.org/', array(), 'POST');
?>
POST / HTTP/1.0Host: www.drupal.org
User-Agent: Drupal (+http://drupal.org/)
Content-Length: 0
#26
Looks good now.
#27
there is a possible flaw here - I think I can't GET/DELETE/etc the character '0' as the entire entity-body.
However, that's probably irrelevant to normal operation.
if we care about that (maybe not) we want something like this change instead:
+ // Only add Content-Length if we actually have any content or if it is a POST+ // or PUT request. Some non-standard servers get confused by Content-Length in
+ // at least HEAD/GET requests, and Squid always requires Content-Length in
+ // POST/PUT requests.
+ $content_length = strlen($data);
+ if ($content_length > 0 || $method == 'POST' || $method == 'PUT') {
+ $defaults['Content-Length'] = 'Content-Length: '. $content_length;
+ }
+
#28
+ $defaults['Content-Length'] = 'Content-Length: '. $content_length;btw, also using wrong string concatenation (the patch, not pwolanin who just copied).
#29
@sun - no this is the D6 backport, why is that wrong?
#30
oopsie. sorry, I'm slow.
#31
POST with "0" is never happen because it's contents always name: value
http://en.wikipedia.org/wiki/HTTP_POST
#32
The POST body in the general case is free-form, so we should allow posting '0'.
#33
Fixed as @pwolanin proposed
#34
Committed to CVS HEAD. Thanks!
#36
needs to be ported for D6
#37
Look at #33 thare's a D6 patch
#38
Reroll
#39
Looks good.
#40
Committed to Drupal 6, thanks!
#41
backport for Drupal 5? http://api.drupal.org/api/function/drupal_http_request/5