On Mon, Jul 26, 2010 at 3:17 AM, Christophe Cordenier <christophe.corden...@gmail.com> wrote: > 2010/7/26 Kalle Korhonen <kalle.o.korho...@gmail.com> >> On Sun, Jul 25, 2010 at 11:45 PM, Christophe Cordenier >> <christophe.corden...@gmail.com> wrote: > Actually i was not thinking about catching the exception at the filter level > since Tapestry does it well and as you said, decoration should do the trick, > but simply let Tapestry initialize its context (ActivatePage) In fact i was > wondering, does your security filter implement a ComponentRequestFilter or a > RequestFilter ?
Ah yes, forgive me for being a little slow here. Yes, the SecurityFilter is a ComponentRequestFilter. If I contribute it right after "InitializeActivePageName", the fairly comprehensive integration tests for the module pass so it seems that would do it. Should have found that out and tried it before myself. But wouldn't the wrong page be stored as the active page in this case or does it not matter? This is not directly related to the issue, but assuming the above solution is the right way to go about it and given that InitializeActivePageName didn't exist in T5.1, what's the best way for checking the version of the core library at runtime (so I can contribute it differently depending on the version)? Kalle >> > Actually, i am not against applying your patch, but as >> PageResponseRenderer >> > is internal, as a new committer i am definitely cautious in what i am >> doing >> > and committing ;) >> >> No worries, I fully understand where you are coming from. The code >> worked as is for T5.1 and I already experimentally modified it to call >> storeActivePageName() from outside just to verify that'd be enough for >> T5.2. So it's not a matter of making it work, but making it work the >> right way. Although we both may end up spending more time on it this >> way, I much prefer spending my resources on validating the concept >> than just hacking it in. >> >> So in the above case, we don't really want to replace the >> RequestExceptionHandler as handling the non-security related >> exceptions are delegated back to the original, but if somebody >> replaces the DefaultRequestExceptionHandler with one that contains a >> form on it, it'd throw an exception as well (just putting together >> what you said and reading the code), right? Hopefully that proves the >> need for the fix. >> >> Kalle >> >> >> > 2010/7/26 Kalle Korhonen <kalle.o.korho...@gmail.com> >> > >> >> On Sun, Jul 25, 2010 at 5:35 AM, Christophe Cordenier >> >> <christophe.corden...@gmail.com> wrote: >> >> > I am currently working on this JIRA, i have seen that the >> >> getActivatePage() >> >> > is only called to pre allocate names in Form component so it should >> have. >> >> So >> >> > i guess the page you are returning contains a Form. >> >> >> >> Right and thanks so much Christophe for taking a look at the issue. >> >> >> >> > Despite it sound right that PageResponseRenderer should set the active >> >> page, >> >> > don't you think that in case of a security exception it would be >> easier >> >> to >> >> > send an HTTP error and let the application developer use standard >> web.xml >> >> > request dispatching to configure the page to display ? >> >> >> >> Specifically in the case of security exception, no, I don't think >> >> that'd be easier. While it's an option, I don't see what the advantage >> >> would be. The exception handler in tapestry-security catches the >> >> exception, sets the error code and renders the configured page. >> >> Configuring the page somewhere else (in standard web.xml) seems to me >> >> like an unnecessary complication compared to contributing >> >> configuration. >> >> >> >> Security exception aside, in the general case http error codes alone >> >> wouldn't be enough to differentiate all the different error cases. I'm >> >> not strongly against exploring alternatives, but it makes sense to me >> >> that PageResponseRenderer would set the active page and I don't see >> >> any disadvantages in doing so, do you? >> >> >> >> Kalle >> >> >> >> >> >> > 2010/7/20 Kalle Korhonen <kalle.o.korho...@gmail.com> >> >> >> Pretty please, any committer? It's a one-liner to fix >> >> >> https://issues.apache.org/jira/browse/TAP5-1201 and the patch is >> >> >> attached. >> >> >> >> >> >> Kalle >> >> >> >> >> >> >> >> >> On Mon, Jul 19, 2010 at 2:23 PM, Pierce Wetter <pie...@paceap.com> >> >> wrote: >> >> >> > >> >> >> > On Jul 9, 2010, at 1:40 PM, Kalle Korhonen wrote: >> >> >> > >> >> >> >> Sorry to be a pest, but how could I find a champion to apply the >> >> patch >> >> >> >> for TAP5-1201? I understand Howard's busy and his time is probably >> >> >> >> better spent on bigger issues than this, but I hope that some >> other >> >> >> >> committer would be able to pick this up and apply the one-line >> change >> >> >> >> that would restore functionality that existed in T5.1. It's my >> >> >> >> understanding its perfectly safe to do as suggested so we can't go >> >> >> >> that far off even if somebody wants to refactor it later. >> >> >> > >> >> >> > Ditto, since I'm using tapestry-security with 5.2-SNAPSHOT. >> >> >> > >> >> >> > >> >> >> > >> --------------------------------------------------------------------- >> >> >> > To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org >> >> >> > For additional commands, e-mail: users-h...@tapestry.apache.org >> >> >> > >> >> >> > >> >> >> >> >> >> --------------------------------------------------------------------- >> >> >> To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org >> >> >> For additional commands, e-mail: users-h...@tapestry.apache.org >> >> >> >> >> >> >> >> > >> >> > >> >> > -- >> >> > Regards, >> >> > Christophe Cordenier. >> >> > >> >> > Committer on Apache Tapestry 5 >> >> > Co-creator of wooki @wookicentral.com >> >> > >> >> >> >> --------------------------------------------------------------------- >> >> To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org >> >> For additional commands, e-mail: users-h...@tapestry.apache.org >> >> >> >> >> > >> > >> > -- >> > Regards, >> > Christophe Cordenier. >> > >> > Committer on Apache Tapestry 5 >> > Co-creator of wooki @wookicentral.com >> > >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org >> For additional commands, e-mail: users-h...@tapestry.apache.org >> >> > > > -- > Regards, > Christophe Cordenier. > > Committer on Apache Tapestry 5 > Co-creator of wooki @wookicentral.com > --------------------------------------------------------------------- To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org For additional commands, e-mail: users-h...@tapestry.apache.org
