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)
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
