drupal_http_request() adds 'Content-Length' header even if there's no content in the request

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?

Here is an updated patch which should work for 7.x-dev.

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.0
Host: 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

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.

Patch contained "incorrect" file path (i believe)

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.

Reroll.

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().

I couldn't find a good reference, but I think that if (!$foo) may be slightly faster than using strlen().

+  // 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.

Thanks for the review. This patch addresses your comments.

Unfortunately, this patch no longer applies. It needs a quick re-roll. I'll commit after reroll.

Reroll.

Committed to CVS HEAD. Thanks!

It's easy to port first to 6.x

+    $defaults['headers']['Content-Length'] = strlen($data);

should be

+    $defaults['Content-Length'] = strlen($data);

Here reroll with #22 and against current DRUPAL-6

Seems my advice in #22 was bad. Now the Content-Length header isn't added at all.

@c960657 This was my fault, in drupal 6 array of headers holds values same as keys

  // 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
drupal_http_request('http://www.drupal.org/');

GET / HTTP/1.0
Host: www.drupal.org
User-Agent: Drupal (+http://drupal.org/)

drupal_http_request('http://www.drupal.org/', array(), 'POST');

POST / HTTP/1.0
Host: www.drupal.org
User-Agent: Drupal (+http://drupal.org/)
Content-Length: 0

Looks good now.

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;
+  }
+

+    $defaults['Content-Length'] = 'Content-Length: '. $content_length;

btw, also using wrong string concatenation (the patch, not pwolanin who just copied).

@sun - no this is the D6 backport, why is that wrong?

oopsie. sorry, I'm slow.

POST with "0" is never happen because it's contents always name: value
http://en.wikipedia.org/wiki/HTTP_POST

The POST body in the general case is free-form, so we should allow posting '0'.

Fixed as @pwolanin proposed

Committed to CVS HEAD. Thanks!

needs to be ported for D6

Look at #33 thare's a D6 patch

Reroll

Looks good.

Committed to Drupal 6, thanks!

backport for Drupal 5? http://api.drupal.org/api/function/drupal_http_request/5

Marking as won't fix as d5 is end of life.