Always send HTTP/1.1 in headers

Looks good. Committed to CVS HEAD. Thanks, Chris.

applied

I've reverted the patch from 4.7 based on a bug report:

http://lists.drupal.org/pipermail/development/2006-December/021403.html

Setting to critical to attract attention.

its ok to revert 4.7, but i think we shold default to 1.1. if some requesterors don't want 1.1, they can patch drupal_http_request() or simply issue own request. so, i think this is OK for 5.

Is there any solid benefit we are getting by sending HTTP/1.1? Might we avoid any similar issues in reverting D5 to HTTP/1.0? I'm just worried how quickly we got a bug report after putting this in 4.7.x-dev might be an indication of some other potential problems. Maybe this was just a bit of a coincidence.

Technically we should be sending HTTP/1.1 for all headers to be properly interpreted. I'm OK with rolling back because HTTP/1.0 always worked so far. I'm going to leave this as is for a while -- maybe we're missing something. We're not the first to request pages with a HTTP/1.1 header. ;-)

There are two separate issues here: what we send for Drupal pages and how we request pages.

For sending, I think changing the status code may be okay to leave in.

Requesting is what the problem was reported against and where the code comments apply. Leaving this as 1.0 means that we recognize a superset of HTTP 1.0 status codes. I am not aware of any other differences, that is the most obvious and what the comments indicate. I don't see this as a huge problem because we are being liberal in what we accept.

It would be nice to see some more concrete verification, however I am ready to consider the drupal_http_request() change an API change which could be rolled back.

Chris: it shows up in my issue lists. How do you navigate the issues queues? Through the contributor block? Maybe that block needs to be updated then. I use local bookmarks and had to update them a while ago. :)

I agree that that we *should* be sending HTTP 1.1 headers. The bug report was a *request* which is different and might be a Google issue and not Drupal, without more info hard to say.

But here's something concrete. Our caching headers (which I've alluded to are wrong in another issue and will have more info to back that other issue up later) are 1.1 compliant. They don't work at the 1.0 level.

This part of the 1.1 spec outlines the cache headers for 1.1.

Here is what Drupal sends:

/**
* Set HTTP headers in preparation for a page response.
*
* Authenticated users are always given a 'no-cache' header, and will
* fetch a fresh page on every request. This prevents authenticated
* users seeing locally cached pages that show them as logged out.
*
* @see page_set_cache
*/
function drupal_page_header() {
header("Expires: Sun, 19 Nov 1978 05:00:00 GMT");
header("Last-Modified: " . gmdate("D, d M Y H:i:s") . " GMT");
header("Cache-Control: no-store, no-cache, must-revalidate");
header("Cache-Control: post-check=0, pre-check=0", FALSE);
header("Pragma: no-cache");
}

Note the Pramga: no-cache is 1.0 while Cache-Control is 1.1. Pragma still works in 1.1 because of backwards compatibility.

But regardless, we are sending a 1.0 request with 1.1 headers -- not good :-)

I suggest we leave the 1.1 headers and fix the aboe to remove the Pragma: no-cache, since that is equivalent to Cache-control: no-cache

I'm ok with sticking to HTTP 1.1 and getting rid of the old header. But, can we reproduce the originally reported issue (that Gerhard linked) and confirm that it's fixed? Or is this just one broken configuration that does not have much global relevance?

I'm tempted to stick to the standards for now. HTTP 1.1 is old and has been working for a decade. Let's not get hung up on this.

I committed the pragma-patch but it is not clear whether that will solve the problem. Committing it looks right though. :) I'll set this to 'active (needs more info)' for now ...

+1 on the drupal_http_request rollback -- as per Neil's suggestion.

I submitted a patch to http://drupal.org/node/106506 which improves HTTP/1.1 compatibility by processing chunked transfer encodings.

I'm inclined to lean towards better implementing HTTP/1.1. IIRC this makes it easier to do advanced HTTP (conditional GET, caching, ...).

Yes, the "chunked" handling is a bit of extra code, but it's self-contained and work well. If we are to use HTTP/1.1, let's do it properly.

see:
http://drupal.org/node/106506

I committed Chris' patch to roll back this change. It worked for years so let's stick with what works. We can work on the chunked stuff for D6. Thanks all.