Re: [Zope-dev] Unauthorized handling in Zope2
Hi! Wichert Akkerman wrote: > Unauthorised is doing stupid things here: > > (Pdb) p v > Unauthorized() > (Pdb) p unicode(v) > u'' > (Pdb) p str(v) > *** UnicodeEncodeError: UnicodeEncodeError('ascii', >u' > I added an extra change (see diff below) to fix that, after which things > seemed to work. Looks like my patch didn't work for you because of this: http://bugs.python.org/issue6108 (fixed in Python 2.6.5) I committed a modified Unauthorized patch that should work with all supported Python versions: http://svn.zope.org/?rev=97&view=rev And I hope together with the change in HTTPResponse your issue is resolved now: http://svn.zope.org/?rev=99&view=rev Cheers, Yuppie ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Unauthorized handling in Zope2
Hi! Wichert Akkerman wrote: > On 4/20/10 15:17 , yuppie wrote: >> Wichert Akkerman wrote: >>> I added an extra change (see diff below) to fix that, after which things >>> seemed to work. >> >> Great! > > Can you commit that change along with your other changes? Yes. I'll write some more tests and commit it in time for the 2.12.5 release. Thanks for catching this issue early enough! >> Re-raising the exceptions makes sure the post-processing in >> HTTPResponse.exception is called. That is also expected by >> CookieCrumbler and PAS. > > The authentication dance between the publisher, request, PAS and > CookieCrumbler really is a bit contrived :( An other advantage of the re-raising pattern is the fact that you can easily change the response type by raising a different exception inside the view. I plan to use that for replacing the nasty unauth redirect code in CookieCrumbler. The exception view will raise Redirect or Forbidden if you are already logged in. >> A better fix would be to store the rendered exception value in the >> response object instead of the exception object. That way we could >> re-raise *all* exceptions as it was done in older Zope versions. >> >> But this would have been a bigger refactoring with more risks to break >> something else. > > Perhaps something for 2.13 :) Yes. Perhaps ;) Cheers, Yuppie ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Unauthorized handling in Zope2
On 4/20/10 15:17 , yuppie wrote: > Hi! > > > Wichert Akkerman wrote: >> Unauthorised is doing stupid things here: >> >> (Pdb) p v >> Unauthorized() >> (Pdb) p unicode(v) >> u'' >> (Pdb) p str(v) >> *** UnicodeEncodeError: UnicodeEncodeError('ascii', >> u'> >> I added an extra change (see diff below) to fix that, after which things >> seemed to work. > > Great! Can you commit that change along with your other changes? >> Still, I can not see any good reason to reraise >> Unauthorised exceptions if there is a valid exception view for them. >> This approach feels like we are attacking the symptom instead of fixing >> the problem. > > Zope 2.12.4 was definitely broken: > > 401 Unauthorized responses "MUST include a WWW-Authenticate header > field", see > http://tools.ietf.org/html/rfc2616#section-10.4.2 Hm, fair point. > Re-raising the exceptions makes sure the post-processing in > HTTPResponse.exception is called. That is also expected by > CookieCrumbler and PAS. The authentication dance between the publisher, request, PAS and CookieCrumbler really is a bit contrived :( > A better fix would be to store the rendered exception value in the > response object instead of the exception object. That way we could > re-raise *all* exceptions as it was done in older Zope versions. > > But this would have been a bigger refactoring with more risks to break > something else. Perhaps something for 2.13 :) Wichert. ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Unauthorized handling in Zope2
Hi! Wichert Akkerman wrote: > Unauthorised is doing stupid things here: > > (Pdb) p v > Unauthorized() > (Pdb) p unicode(v) > u'' > (Pdb) p str(v) > *** UnicodeEncodeError: UnicodeEncodeError('ascii', >u' > I added an extra change (see diff below) to fix that, after which things > seemed to work. Great! > Still, I can not see any good reason to reraise > Unauthorised exceptions if there is a valid exception view for them. > This approach feels like we are attacking the symptom instead of fixing > the problem. Zope 2.12.4 was definitely broken: 401 Unauthorized responses "MUST include a WWW-Authenticate header field", see http://tools.ietf.org/html/rfc2616#section-10.4.2 Re-raising the exceptions makes sure the post-processing in HTTPResponse.exception is called. That is also expected by CookieCrumbler and PAS. A better fix would be to store the rendered exception value in the response object instead of the exception object. That way we could re-raise *all* exceptions as it was done in older Zope versions. But this would have been a bigger refactoring with more risks to break something else. Cheers, Yuppie ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Unauthorized handling in Zope2
On 4/20/10 09:51 , yuppie wrote: > Hi! > > > Wichert Akkerman wrote: >> v is the html as generated by my view. Reraising the exception transfers >> control to the bare except in >> ZPublisher.Publish.publish_module_standard, which generates the standard >> site error page and returns that. > > Could it be that your v is unicode? > > Please let me know if the attached patch fixes the issue. Unauthorised is doing stupid things here: (Pdb) p v Unauthorized() (Pdb) p unicode(v) u'' (Pdb) p str(v) *** UnicodeEncodeError: UnicodeEncodeError('ascii', u'https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Unauthorized handling in Zope2
On 4/20/10 09:51 , yuppie wrote: > Hi! > > > Wichert Akkerman wrote: >> v is the html as generated by my view. Reraising the exception transfers >> control to the bare except in >> ZPublisher.Publish.publish_module_standard, which generates the standard >> site error page and returns that. > > Could it be that your v is unicode? Indeed it is: Chameleon returns a unicode response it seems. > Please let me know if the attached patch fixes the issue. I'm afraid it doesn't. The result is this: Site Error An error was encountered while publishing this resource. Unauthorized Sorry, a site error occurred. Traceback (innermost last): Module ZPublisher.Publish, line 238, in publish_module_standard Module Products.PDBDebugMode.runcall, line 83, in pdb_publish Module ZPublisher.Publish, line 165, in publish Module plone.app.linkintegrity.monkey, line 21, in zpublisher_exception_hook_wrapper Module ZPublisher.Publish, line 116, in publish Module ZPublisher.BaseRequest, line 609, in traverse Module ZPublisher.HTTPResponse, line 720, in unauthorized Unauthorized: Wichert. ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Unauthorized handling in Zope2
Hi! Wichert Akkerman wrote: > v is the html as generated by my view. Reraising the exception transfers > control to the bare except in > ZPublisher.Publish.publish_module_standard, which generates the standard > site error page and returns that. Could it be that your v is unicode? Please let me know if the attached patch fixes the issue. Cheers, Yuppie Index: public/src/zExceptions/unauthorized.py === --- public/src/zExceptions/unauthorized.py (revision 62) +++ public/src/zExceptions/unauthorized.py (working copy) @@ -43,7 +43,7 @@ provides are added to needed. """ if name is None and ( -not isinstance(message, StringType) or len(message.split()) <= 1): +not isinstance(message, basestring) or len(message.split()) <= 1): # First arg is a name, not a message name=message message=None Index: public/src/ZPublisher/HTTPResponse.py === --- public/src/ZPublisher/HTTPResponse.py (revision 62) +++ public/src/ZPublisher/HTTPResponse.py (working copy) @@ -800,7 +800,10 @@ b = v if isinstance(b, Exception): try: -b = str(b) +try: +b = str(b) +except UnicodeEncodeError: +b = self._encode_unicode(unicode(b)) except: b = '' % type(b).__name__ ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Unauthorized handling in Zope2
Hi again! yuppie wrote: > Wichert Akkerman wrote: >> v is the html as generated by my view. Reraising the exception transfers >> control to the bare except in >> ZPublisher.Publish.publish_module_standard, which generates the standard >> site error page and returns that. > > I would have expected that HTTPResponse.exception uses v as the body of > the returned page. Without further information I can't tell you why that > doesn't work in your case. Meanwhile I have some ideas what might go wrong. Tomorrow I'll have a closer look at it. Cheers, Yuppie ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Unauthorized handling in Zope2
Hi! Wichert Akkerman wrote: > I can't get an Unauthorized exception view to work with current Zope > 2.12 svn. My approach was: > > - add a dummy PAS challenge plugin which does nothing, effectively > delegating everything to my Unauthorized exception view > - register a browser view for Unauthorized and return a proper error > message there > > unexpectedly the result is a stock Zope2 site error page. What seems to > happen is that everything works correctly, up to this point the > zpublisher exception hook in Zope2.App.startup: > > if issubclass(t, Unauthorized): > # Re-raise Unauthorized to make sure it is handled > # correctly. We can't do that with all exceptions > # because some don't work with the rendered v as > # argument. > raise t, v, traceback > > v is the html as generated by my view. Reraising the exception transfers > control to the bare except in > ZPublisher.Publish.publish_module_standard, which generates the standard > site error page and returns that. I would have expected that HTTPResponse.exception uses v as the body of the returned page. Without further information I can't tell you why that doesn't work in your case. > What is the reason for re-raising Unauthorized there? There is no > special processing for it anywhere up in the call stack, so I can see no > benefit to it. If I remove that code block so we always return the > result of the view everything works as I would expect. I tried to fix the bug discussed in comments #15ff of https://bugs.launchpad.net/zope2/+bug/372632 Cheers, Yuppie ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Unauthorized handling in Zope2
I can't get an Unauthorized exception view to work with current Zope 2.12 svn. My approach was: - add a dummy PAS challenge plugin which does nothing, effectively delegating everything to my Unauthorized exception view - register a browser view for Unauthorized and return a proper error message there unexpectedly the result is a stock Zope2 site error page. What seems to happen is that everything works correctly, up to this point the zpublisher exception hook in Zope2.App.startup: if issubclass(t, Unauthorized): # Re-raise Unauthorized to make sure it is handled # correctly. We can't do that with all exceptions # because some don't work with the rendered v as # argument. raise t, v, traceback v is the html as generated by my view. Reraising the exception transfers control to the bare except in ZPublisher.Publish.publish_module_standard, which generates the standard site error page and returns that. What is the reason for re-raising Unauthorized there? There is no special processing for it anywhere up in the call stack, so I can see no benefit to it. If I remove that code block so we always return the result of the view everything works as I would expect. Wichert. ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )