[line 33]

header($r->type);

should be changed to:

[line 33]

drupal_set_header($r->type);

Comments

HorsePunchKid’s picture

HorsePunchKid commented
Status: Active » Closed (fixed)

Applied, tested, and committed; thanks!

HorsePunchKid’s picture

HorsePunchKid commented
Status: Closed (fixed) » Fixed

Sorry, wrong status.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

as far as I kno drupal_set_header prior to Drupal6 sends the 302 redirect always in spite of whatever code you send it. I think that's the reason this was using header() directly. Can one of you test/confirm this?

I don't have any 4.7 sites left to test. Was there some bug that this was supposed to fix or just for "generally it is good to use the drupal api".

hass’s picture

hass commented

Using PHP's "header" directly in modules could cause negative side effects with other modules. The API way is the clean and way supported by Drupal... you can set everything with drupal_set_header(). I think you are talking about drupal_goto http://api.drupal.org/api/function/drupal_goto/4.7 isn't it? That function does not allow to set the anything else then a 302 before D5, but this feature has been added in D5 ($http_response_code).

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Yes, that makes sense and my concern was misguided. I'm always hesitant about making changes just because it's the "normal way" to do things and when there is no known bug caused by the code, but this seems reasonable.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.