Problem/Motivation
Headers set with drupal_add_http_header()
will be ignored if they have a falsy value such as 0
, "0"
, or " "
. This makes it impossible to send (valid) headers like X-XSS-Protection: 0
.
The problem is in drupal_send_headers()
, which tries to skip headers that have been unset using drupal_add_http_header()
but uses an over-aggressive test for that condition:
foreach ($headers as $name_lower => $value) {
if ($name_lower == 'status') {
header($_SERVER['SERVER_PROTOCOL'] . ' ' . $value);
}
// Skip headers that have been unset.
elseif ($value) {
header($header_names[$name_lower] . ': ' . $value);
}
}
Proposed resolution
The documentation on drupal_add_http_header()
says, "if [the $value
parameter is] equal to FALSE, the specified header is unset". So let's only skip values of FALSE
itself.
Remaining tasks
- Test the latest patch.
- Write a test.
- Backport to d7.
- Create a change notice?
User interface changes
None.
API changes
This changes the programming interface, if ever so slightly. Core itself isn't relying on the behavior anywhere (it isn't unsetting headers at all), but contrib modules could be. That's probably not a problem for d8, but this should probably be backported to d7, where contrib modules may be relying on it.
Related issues
This issue may collide with #1400800: drupal_send_headers doesn't unset headers.
Original report by EdgarPE
I'm unsure if it's a functional bug or by design, but you as of 7.14 you can't add a HTTP header with drupal_add_http_header() for anonymous users in hook_boot or hook_init, even if page cache is disabled.
Test code:
function test_module_boot() {
global $user;
drupal_add_http_header('UserLoggedIn', $user->uid == 0 ? 0 : 1);
}
The UserLoggedIn http response header is not present for not logged in users.
API page: http://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/drupa...
Enter a descriptive title (above) relating to drupal_add_http_header, then describe the problem you have found:
Comment | File | Size | Author |
---|---|---|---|
#9 | drupal-falsy-http-header-values-1605040-9-D7.patch | 487 bytes | mitron |
#6 | drupal-falsy-http-header-values-1605040-6.patch | 507 bytes | TravisCarden |
#4 | drupal-falsy-http-header-values-1605040-4-d7-no-test.patch | 487 bytes | TravisCarden |
#4 | drupal-falsy-http-header-values-1605040-4.patch | 507 bytes | TravisCarden |
Comments
Comment #1
jhodgdonSounds like this is not a documentation issue?
Comment #2
EdgarPE CreditAttribution: EdgarPE commentedIt's not about anonymous users. It's inpossible to add a http header containing only the character "0". I don't know if this is by design or bug.
Comment #3
salah1Could it be that the problem is 0 or 1 are not in single quote?
look at comment#2 on below
http://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/drupa...
Comment #3.0
salah1type
Comment #4
TravisCarden CreditAttribution: TravisCarden commentedNope, this is a bug. See updated issue summary. Here's a patch. (And because I happen to need a quick fix for d7, I'm including a no-test patch for that, too.)
Comment #5
TravisCarden CreditAttribution: TravisCarden commentedComment #6
TravisCarden CreditAttribution: TravisCarden commentedAw, nuts. I must not've named the no-test patch properly.* Here's the real patch again so as not to get a false failure from the test bot.
* Edit: To make the testbot skip a patch, end the filename with "do-not-test".
Comment #7
vmunoz CreditAttribution: vmunoz commentedI have succesfully tested the patch.
Comment #8
Dries CreditAttribution: Dries commentedGood catch. Committed to 8.x.
Moving to 7.x.
Also tagging this issue as 'needs tests'. Given that this is pretty small, we can add these later. Would be great if you could write some, Travis.
Thanks!
Comment #9
mitron CreditAttribution: mitron commentedFor 7.x
Comment #10
mitron CreditAttribution: mitron commentedComment #11
TravisCarden CreditAttribution: TravisCarden commentedLooks good. Once this is committed I'll set the issue back to D8 and work on a test.
Comment #12
David_Rothstein CreditAttribution: David_Rothstein commentedAlthough a slight behavior change, this does seem to make sense and match what the documentation already said was happening.
Committed to 7.x (and added to CHANGELOG.txt) - thanks! http://drupalcode.org/project/drupal.git/commit/20a45b1
Back to Drupal 8 for the tests.
Comment #13
David_Rothstein CreditAttribution: David_Rothstein commentedTests can be backported too.
Comment #13.0
David_Rothstein CreditAttribution: David_Rothstein commentedUpdated issue summary.
Comment #14
tibbsa CreditAttribution: tibbsa commentedThis issue is dead/irrelevant in D8 and has been removed in favour of using Symfony\Component\HttpFoundation\Response.
If this is to be worked on at all, it's a 7.x issue.
Comment #15
tibbsa CreditAttribution: tibbsa commentedComment #16
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commentedThis is fixed in 7.x as well.