Page Cache doesn't work with HEAD requests

Most browsers will make a conditional GET request (If-Modified-Since), not a HEAD request. But tailing my logs, I do see HEAD requests, some from browsers and many from various bots and scripts.

So this seems like a good idea to me. It may also be a good idea to set the cache on a HEAD request (in common.inc); since we're going to all the effort to bootstrap and generate a page we may as well get something useful out of it.

Patch (for head) to support HEAD requests.

Yes, there is no reason why these HEAD requests should be excluded.

It would be great if we could write a test for this.

Should the tests just check that page cache works with HEAD requests or anything more?

@mfb: we don't have a page cache at the moment, so this will have to be implemented. Moreover, there is no standardized way to call HEAD requests for now, so you will have to suggest an API extension to drupal_web_test_case.

OK this adds drupalHead() and a very basic page cache test.

drupalHead() should be quite useful for future HTTP header tests.

I've made some minor changes to this patch and committed it to CVS HEAD. Thanks.

Looks like something got screwed up, drupalHead() function is repeated twice in drupal_web_test_case.php

By the way, I just did some testing and actually, it was pointless to add the if $_SERVER['REQUEST_METHOD'] == 'HEAD' logic to page_set_cache(), because on a HEAD request, PHP dies as soon as you call ob_end_flush(), just before cache_set() would be called.

Won't cause any trouble to leave it in but it doesn't do anything useful after all.

Marking it critical for the fact that drupalHead() function is repeated twice in drupal_web_test_case.php

Fixed in CVS HEAD.

As a side note, Dries: #168909: Drupal messages (status/error) are cached along with nodes never made it to 7.x.

@eli: I was talking specifically about my patch to page_set_cache() in common.inc, not your patch to page_get_cache() in bootstrap.inc.

I think it's ok to leave the HEAD logic in common.inc so there is some uniformity to how HEAD requests are handled. If you skip this loop for HEAD requests then the HEAD request moves on to the exit hook, whereas HEAD requests normally (with caching disabled or a cache already set) die before reaching the exit hook.

This fix could get backported if desired.

This change broke SimpleTest all over the place in #308186: cURL 7.18.2 and SimpleTest module bitterly despise one another. After removing the CURLOPT_NOBODY => FALSE from curlExec() in drupal_web_test_case.php, things are working okay.

What larger consequence does that have, and can we get some tests in place to make ensure that whatever was changed here is doing what it needs to do?

We ran nto http://curl.haxx.se/mail/tracker-2008-07/0006.html . If you read the manpage though http://pwet.fr/man/linux/fonctions_bibliotheques/curl_easy_setopt When setting CURLOPT_POST to a non-zero value, it will automatically set CURLOPT_NOBODY to 0 (since 7.14.1). As 7.14.1 was released on 09-06-2005 I believe we can conclude this fixed. You can http://ca.php.net/manual/en/function.curl-version.php use this to complain hard when simpletest gets enabled if you so want.

I compiled a libcurl 7.19 to confirm that's the culprit and indeed it is. Behold this:

root@Tumbler:/usr/local/experimental# php -i |grep libcurl
cURL Information => libcurl/7.18.0 OpenSSL/0.9.8g zlib/1.2.3.3 libidn/1.1
root@Tumbler:/usr/local/experimental# bin/php -i |grep libcurl
cURL Information => libcurl/7.19.0
root@Tumbler:/usr/local/experimental# bin/php x.php
array(0) {
}
root@Tumbler:/usr/local/experimental# php x.php
array(1) {
  ["username"]=>
  string(7) "johndoe"
}

$ch = curl_init();
curl_setopt($ch, CURLOPT_URL,'http://localhost/d.php');
curl_setopt($ch, CURLOPT_POST, 1);
curl_setopt($ch, CURLOPT_POSTFIELDS,"username=johndoe");
curl_setopt($ch, CURLOPT_NOBODY, FALSE);
curl_exec($ch);
curl_close ($ch);

where d.php just dump $_POST

Ok, sorry for the freakout. I couldn't quite parse that referenced changelog entry, but chx summed it up as:

"if you set FOO 1 then BAR gets set to 0 but if you set BAR to 0 then FOO gets set to 0 as well.
WTF?"

So this is basically a workaround we need to do in our curlExec function to accommodate cURL's wacky, wacky ways.

And since SimpleTest is not part of D6, this can go back where it belongs.

Reroll due to offset warnings.

Is this still an issue using current Drupal 6?

Yes, re-rolled

Thanks!