Re: [Zope-dev] SVN: Zope/branches/2.12/ Correctly handle unauthorized exceptions in the ZPublisherExceptionHook.

10 Jul 2009

Hi.

I'm not sure how exactly to approach this.

The code in the ZPublisherExceptionHook reads:

def __call__(self, published, REQUEST, t, v, traceback):
    try:
        if isinstance(t, StringType):
            if t.lower() in ('unauthorized', 'redirect'):
                raise
        else:
            if (t is SystemExit or
                issubclass(t, Redirect) or issubclass(t, Unauthorized)):
                raise
[...]

I added the "issubclass(t, Unauthorized)" to the second condition,
which now seems to cause test failures.

Looking at the first if-statement, it looked like in the times of
string exceptions, both 'unauthorized' and 'redirect' would be checked
upon first and always raised. I tried to follow the same logic for
modern class based exceptions here for the second if-statement.
Especially since the check for "issubclass(t, Redirect)" was already
in place, this looked like an obvious omission.

Can someone shed some light on this and tell me what is supposed to happen here?

Thanks,
Hanno

On Sat, Jul 4, 2009 at 8:03 PM, Tres Seaver<tseaver@palladion.com> wrote:
...
Hanno Schlichting wrote:
...
Log message for revision 101181:
  Correctly handle unauthorized exceptions in the ZPublisherExceptionHook.
Modified: Zope/branches/2.12/src/Zope2/App/startup.py
===================================================================

--- Zope/branches/2.12/src/Zope2/App/startup.py       2009-06-20 19:53:08 UTC (rev 101180)
+++ Zope/branches/2.12/src/Zope2/App/startup.py       2009-06-21 00:00:53 UTC (rev 101181)
@@ -24,6 +24,7 @@
 from time import asctime
 from types import StringType, ListType
 from zExceptions import Redirect
+from zExceptions import Unauthorized
 from ZODB.POSException import ConflictError
 import transaction
 import AccessControl.User
@@ -170,7 +171,7 @@
                 if t.lower() in ('unauthorized', 'redirect'):
                     raise
             else:
-                if t is SystemExit or t is Redirect:
+                if t is SystemExit or t is Redirect or t is Unauthorized:
                     raise
                 if issubclass(t, ConflictError):
What is the motivation here?  Zope2 applications have hooked
Unauthorized exceptions *forever*:
 - You can see them in the error_log (if you take them out of the ignore
  list).
This change (the fixed version) breaks unit tests which assert that the
exception can be hooked:

    