undefined index/header already sent on 403/404 error [#311270]

because drupal_access_denied, returned from print_controller in case of 404/403 up to now, does not actually return anything and thereafter is not checked but just processed, a lot of errors silently drop into the watchdog table. This patch sets 404/403 page and after that stops all processing..

Comment	File	Size	Author
	print-pages-inc-fix-403-404-error-handling.patch	604 bytes	eMPee584

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

jcnventura CreditAttribution: jcnventura commented 22 September 2008 at 13:31

Status:

Needs review

» Needs work

Hi,

I accept that your code looks good but I don't like the sudden exit() calls there. What is the harm in doing empty returns instead of those exits?

João

Comment #2

jcnventura CreditAttribution: jcnventura commented 22 September 2008 at 21:45

Status:

Needs work

» Postponed (maintainer needs more info)

Hi,

I have committed this code (and another one like this when generating books) using return instead of exit.

I see no errors in the log table, but I suspect that your Drupal is configured to issue a lot more warnings than mine..

João

Comment #3

jcnventura CreditAttribution: jcnventura commented 22 September 2008 at 21:47

And by the way, the reason your footer goes up and down is that you're using <p> around your footer text. TCPDF is terrible with p's.

Joao

Comment #4

eMPee584 CreditAttribution: eMPee584 commented 23 September 2008 at 14:35

the point of calling exit() after drupal_access_denied/not_found becomes after clear after lookin at those functions. There's simply nothing to do after that. These calls render the full error page.
If you just return undefined, the value checks thereafter trigger warnings. These warnings are actual non-fatal *ERRORs* btw..!
gonna check out the code later and report back.

Comment #5

jcnventura CreditAttribution: jcnventura commented 23 September 2008 at 15:52

From
http://api.drupal.org/api/function/drupal_not_found/6/references
http://api.drupal.org/api/function/drupal_access_denied/6/references

no function that calls drupal_not_found() does an exit() after.. Some simply fall out of the function or specifically call return (and some do return drupal_not_found()). The same holds for drupal_access_denied().

An exit simply kills the ability of Drupal core to continue any work that it could have programmed after calling the module's hook_menu for generating the print* pages. There are very few instances of an exit() call in Drupal and I really don't want to add any. If there are any warnings left after after the return, we can simply fix them. At the moment, I am not seeing any.

One place where it might be useful to place an exit() call is at the end of print_pdf_controller(), to maybe avoid that the TCPDF and dompdf warnings get logged by Drupal. I would have to check, as maybe they get logged while they are happening.

João