On Mon, Jul 26, 2010 at 11:48 PM, Christophe Cordenier <christophe.corden...@gmail.com> wrote: > 2010/7/26 Kalle Korhonen <kalle.o.korho...@gmail.com> >> 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? >> > Right it matters : you will not be able to pre-allocate form control > names... I have to look deeper to see where storeActivePageName() should be > called to work properly with every type of request...
Very good, that's what I thought. Makes sense now why calling it in PageResponseRender may not necessarily be the right place but there doesn't seem to be a lot of wiggle room for this case. Well I trust you to figure out something better than I could :) Thanks to Ville for the symbol version tip! Kalle >> 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 >> >> > > > -- > 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
