Retrieve HTTP response headers

This should be discussed and committed to core before being backported to SimpleTest 6.x-2.x.

I really miss this feature.

It would be useful if headers were returned in a name => value array, e.g. array('Expires' => 'Sun, 19 Nov 1978 05:00:00 GMT', 'Last-Modified' => 'Fri, 07 Nov 2008 16:59:08 GMT', …), like $result->headers from drupal_http_request(), and/or if there was function to get a header with a specific name, e.g. drupalGetHeader($name).

According to RFC 2616, section 4.2 (last paragraph), two headers with the same name may be combined in one by concatenating the values separated by comma. The following two header blocks are equivalent:

I would only keep the headers from the last request like drupalGetContent() and assertResponse() do. If a test wishes to inspect the "intermediate" request, I suggest it call curlExec() with an option to do only one request (i.e. CURLOPT_FOLLOWLOCATION = FALSE etc). Not sure whether this is possible with the current version of drupal_web_test_case, though.

See: #335122: Test clean HEAD after every commit and http://pastebin.ca/1258476

... the patch:

This patch adds access to the response headers from intermediate requests (see the unit test for an example). Will that work?

This feature would be useful in several other issues: #327269: drupal_page_cache_header() should compare timestamp, #201122: Drupal should support disabling anonymous sessions, #43462: cache_set and cache_get base_url brokenosity. I hope you will reconsider your decision.

Is the increased memory usage more than 1-2 KB total? We don't save the headers from old requests when drupalGet() is called again, and the old headers are garbage collected, so the 1-2 KB don't add up.

Here is what pwolanin is doing in #280934: Use httponly cookie support when available:

Would that kill you?

Simpletest is already slow and already requires a lot of memory. No need to add clutter to the clutter.

One possible way to address the performance concerns, is to do the heavy lifting in drupalGetHeaders(). Is there a reason the header parsing could not be postponed until drupalGetHeaders() is called?

With this patch, the headers are parsed on demand in drupalGetHeaders().

To sum up: The patches in #25 and #26 both implement drupalGetHeaders() in largely the same way. In addition, the one in #25 includes a test, adds drupalGetHeader($name) for getting only one header, updates the terminology for header names and values in common.inc to be consistent with than in drupal_web_test_case, and adds the ability to add request headers. Some of this is outside the scope of this bug but was included due to discussion in #327269: drupal_page_cache_header() should compare timestamp where earlier versions of the patch was posted.

Simpler version of c960657's patch, limited to the current issue.

If a page is redirected, it would not be sufficient to examine only the last set of headers received. The same applies when using HTTP authentication. $all_requests may be a misleading name for this variable.

Instead of Status-Line I suggest using a key that doesn't give the impression that this is an actual header, e.g. :status. This is not a valid HTTP header name (see the token production in RFC 2616, section 2.2).

+          list($name, $value) = array('Status-Line' => $header);
This is an unusual way of assigning two variables. It took me a few moments to figure out what was going on.

+          $headers[$request][$name] .= ','. trim($value);
Small coding style glitch (misses a space before the period).

Now that the headers are accessible through drupalGetHeaders(), I don't think that drupalHead() should put them in drupalGetContent() too.

The last submitted patch failed testing.

Implemented more suggestions. This patch will cause the current cache test to fail.

The test that failed was the page cache test, as expected. The test should succeed after this patch is committed.

Fixed.

Suggestions implemented. There is no test for drupalGetContent(), so I don't think we need one for drupalGetHeader() or drupalGetHeaders() either.

Apart from the conflict with the #338403: Make standards for inherited methods consistent for all classes (including tests) that prevents the patch for applying, it looks like this wont work, because the cookie is set in an intermediate request:

There is no test for drupalGetContent(), so I don't think we need one for drupalGetHeader() or drupalGetHeaders() either.

drupalGetHeaders() is considerably more complex than drupalGetContent(). Also, the construction of a complete test suite for Drupal is still work in progress, so some functions may not have tests even though they should have (specifically drupalGetContent() is used by almost every other unit test, so testing it directly as well probably isn't that relevant).

simpletest-headers-3.patch contains an example that covers the case where drupalGetHeaders(TRUE) returns headers for several requests. You could also test whether the two Cache-Control headers in drupal_page_header() are properly concatenated using comma. Then all code paths of the new functions would be covered.

The tests may not only prevent future bugs in Drupal itself but also help discover if cURL behaves differently on different platforms (it doesn't seem unlikely that e.g. the header callback has subtle differences in different versions of cURL and/or PHP). The test suite has helped me report several bugs that occur with PHP 5.2.0 but not with newer versions (e.g #310904: Use early fetch and document why).

Left an unused variable in the last patch.

this is PHP5, if you are using, there is hardly any need for protected $_headers; the underscore where the underscore is the pseudo-visibility marker that Drupal uses. Just protected $headers will do.

I reviewed this patch and it looks ready. I committed it to CVS HEAD. Thanks all.

Got all passes.

Committed.