On 28/08/2012 20:37, Konstantin Kolinko wrote: > 2012/8/28 Mark Thomas <ma...@apache.org>: >> On 28/08/2012 19:36, Mark Thomas wrote: >>> On 28/08/2012 14:24, Konstantin Kolinko wrote: >>>> 2012/8/28 Mark Thomas <ma...@apache.org>: >>>>> On 28/08/2012 03:10, Konstantin Kolinko wrote: >>>>>> 2012/8/13 <ma...@apache.org>: >>>>>>> Author: markt >>>>>>> Date: Mon Aug 13 12:29:51 2012 >>>>>>> New Revision: 1372394 >>>>>>> >>>>>>> URL: http://svn.apache.org/viewvc?rev=1372394&view=rev >>>>>>> Log: >>>>>>> Additional fix for >>>>>>> http://issues.apache.org/bugzilla/show_bug.cgi?id=53584 >>>>>>> Store decoded and original request URI. Restore both. Use decoded for >>>>>>> matching. >>>>>>> >>>>>> >>>>>> The "Restore both" mentioned above was not implemented. >>>>>> The #restoreRequest(..) method was not changed and so it does not >>>>>> restore decodedURI. >>>>> >>>>> Thanks for spotting that. I have done a little digging and the >>>>> decodedURI is only used during the mapping phase (that has already >>>>> happened at this point). I'll amend the commit message. >>>> >>>> See "o.a.c.connector.Request#getDecodedRequestURI()" >>>> >>>> It is used in o.a.c.connector.Response#toAbsolute(). >>> >>> Hmm. I wonder how I managed to miss that. Let me take another look. >>> Either way, the comment is correct, but a further patch may be required. >> >> I think they may be some scope for removing code rather than adding >> code. When restoring the original request the URI is already correct >> since a redirect has been issued to the URI. Hence there is no need to >> restore the original URI or the decoded URI. >> >> I'll clean up trunk but I probably won't back-port it. >> > > Agreed. > You are right "if (matchRequest(request))" always has to succeed > before "restoreRequest(..)" is called. > > BTW, noticed one thing > in FormAuthenticator#authenticate() the following piece of code occurs twice: > > [[[ > // Make the authenticator think the user originally requested > // the landing page > String uri = request.getContextPath() + landingPage; > SavedRequest saved = new SavedRequest(); > saved.setMethod("GET"); > saved.setRequestURI(uri); > ]]] > > In those 2 places saved.setDecodedRequestURI() is never called, so > maybe this feature is currently broken. (Specific to Tomcat 7)
Looks like. I'll fix that now. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
