Closed (fixed)
Project:
Drupal core
Version:
7.x-dev
Component:
base system
Priority:
Normal
Category:
Feature request
Assigned:
Reporter:
Created:
9 May 2008 at 09:22 UTC
Updated:
8 May 2009 at 17:30 UTC
Jump to comment: Most recent file
Comments
Comment #1
fgmThe separation based on space is safe, thanks to the definition of the "field-name" production, which in turn is a "token", the production of which does prohibit white space (RFC2616, §2.2) (see the "separators" production).
However, if $replace is false, the $stored_headers nonetheless contains a single row per field-name, although multiple headers are allowed to define the same field-name, as in:
which this implementation does not cover: a drupal_get_header() after this would only return the "NTLM" header, in spite of the "false" passed to drupal_set_header.
Comment #2
killes@www.drop.org commentedHow about this one?
Comment #3
fgmI'm not sure there is a guarantee from PHP regarding the order in which the headers are sent when mixing numeric and non-numeric keys as you are doing. But HTTP/1.1 §4.2 specifies that the order in which the headers for the same key are send IS significant, so the code MUST make sure they are output in the order the caller intended, and I think it would be better to make the keys clearer.
Also, if a given key happens to be numeric, like drupal_set_header("0: 42"); I think it will be stored as $stored_headers["1"] and you will probably get the expected result when a repeated header happens, since the real numeric keys do not have the same type, but that really seems relying on an edge behaviour.
What would you think or storing things along the lines of:
This would allow headers under the same key to be automatically ordered. The order of headers of different keys is NOT significant anyway, as per the same 4.2 section.
Comment #4
c960657 commentedI suggest using comma as separator between multiple headers with the same field-name, i.e. "WWW-Authenticate: Negotiate,NTLM". This combined form is equivalent to sending the two headers separately (section 4.2 again).
Note that field-names are case-insensitive, so in the following example, the latter should overwrite the former, and the string "123" should not appear in the output from drupal_get_headers().
I believe you should explode on colon rather than space, because the space is optional - i.e. to support calls like this:
drupal_set_header('Foo:Bar');Comment #5
fgm@c960657: good points about explode.
Comma syntax is fine, and actually a bit more bandwidth-efficient, although not by much. But to make things clear, it should only be used when actually outputting the headers, not be a requirement of drupal_set_header, because forcing the app developer to merge his calls is an undue limitation. And since the headers are currently not buffered, but sent with each call, it does not seem feasible unless that "synchronous output" choice is removed.
Comment #6
c960657 commentedJust to be clear, I wasn't suggesting a change in the way headers are sent, only how they are stored internally and returned by drupal_get_headers().
Comment #7
c960657 commentedWith the changes in #147310: Implement better cache headers for reverse proxies, drupal_set_header() now supports both replacing, unsetting and appending to headers.