We're rendering flag for anonymous users and redirecting to login page when it's clicked using JavaScript. Either some users don't have JavaScript enabled, or some crawlers do not respect nofollow, but we see a lot of "cannot modify header information..." noise in our logs. It seems that it's caused by calling drupal_access_denied() which eventually leads to ob_flush() through drupal_deliver_page(), but when flag_page() returns, drupal_deliver_page() is called again. Documentation suggests not calling drupal_access_denied(), but returning MENU_ACCESS_DENIED, which would fix this. So, I'm proposing a very small change:

@@ -970,8 +970,7 @@
     }
     else {
       drupal_set_message($error);
-      drupal_access_denied();
-      return;
+      return MENU_ACCESS_DENIED;
     }
   }
 
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim’s picture

Version: 7.x-2.0 » 7.x-3.x-dev

Seems reasonable, and the docs for http://api.drupal.org/api/drupal/includes!common.inc/function/drupal_acc... confirm this is what should be done.

Could you post this change as a patch though please?

donatasp’s picture

I see that you've changed version to 7.x-3.x so I've made a patch against that. I hope this will get fixed on 2.x too, or will there be no more stable 2.x releases?

joachim’s picture

Status: Active » Needs review

Thanks!

Flag 3.0 is pretty close to getting to an RC.

Flag 2.x is pretty much already in a 'minimal maintenance' state, which means that patches get committed if people backport them. I'll make 2.x branch releases every few months or when major bugs are fixed.

joachim’s picture

Status: Needs review » Fixed

git commit -m "Issue #1921106 by donatasp: Fixed use of drupal_access_denied() causing 'cannot modify header information' errors." --author="donatasp "

Committed to 3.x.

Feel free to backport if you like :)

Status: Fixed » Closed (fixed)

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