You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2012/08/13 14:29:51 UTC
svn commit: r1372394 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/catalina/authenticator/FormAuthenticator.java
java/org/apache/catalina/authenticator/SavedRequest.java
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.
Modified:
tomcat/tc7.0.x/trunk/ (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/FormAuthenticator.java
tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/SavedRequest.java
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
Merged /tomcat/trunk:r1372390
Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/FormAuthenticator.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/FormAuthenticator.java?rev=1372394&r1=1372393&r2=1372394&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/FormAuthenticator.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/FormAuthenticator.java Mon Aug 13 12:29:51 2012
@@ -498,12 +498,11 @@ public class FormAuthenticator
}
// Does the request URI match?
- String requestURI = request.getDecodedRequestURI();
- if (requestURI == null) {
+ String decodedRequestURI = request.getDecodedRequestURI();
+ if (decodedRequestURI == null) {
return (false);
}
- return (requestURI.equals(sreq.getRequestURI()));
-
+ return (decodedRequestURI.equals(sreq.getDecodedRequestURI()));
}
@@ -658,11 +657,11 @@ public class FormAuthenticator
saved.setMethod(request.getMethod());
saved.setQueryString(request.getQueryString());
- saved.setRequestURI(request.getDecodedRequestURI());
+ saved.setRequestURI(request.getRequestURI());
+ saved.setDecodedRequestURI(request.getDecodedRequestURI());
// Stash the SavedRequest in our session for later use
session.setNote(Constants.FORM_REQUEST_NOTE, saved);
-
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/SavedRequest.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/SavedRequest.java?rev=1372394&r1=1372393&r2=1372394&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/SavedRequest.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/SavedRequest.java Mon Aug 13 12:29:51 2012
@@ -147,6 +147,21 @@ public final class SavedRequest {
/**
+ * The decode request URI associated with this Request. Path parameters are
+ * also excluded
+ */
+ private String decodedRequestURI = null;
+
+ public String getDecodedRequestURI() {
+ return (this.decodedRequestURI);
+ }
+
+ public void setDecodedRequestURI(String decodedRequestURI) {
+ this.decodedRequestURI = decodedRequestURI;
+ }
+
+
+ /**
* The body of this request.
*/
private ByteChunk body = null;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1372394 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/authenticator/FormAuthenticator.java
java/org/apache/catalina/authenticator/SavedRequest.java
Posted by Mark Thomas <ma...@apache.org>.
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1372394 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/catalina/authenticator/FormAuthenticator.java java/org/apache/catalina/authenticator/SavedRequest.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1372394 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/authenticator/FormAuthenticator.java
java/org/apache/catalina/authenticator/SavedRequest.java
Posted by 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.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1372394 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/authenticator/FormAuthenticator.java
java/org/apache/catalina/authenticator/SavedRequest.java
Posted by Mark Thomas <ma...@apache.org>.
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.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1372394 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/catalina/authenticator/FormAuthenticator.java java/org/apache/catalina/authenticator/SavedRequest.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
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().
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1372394 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/authenticator/FormAuthenticator.java
java/org/apache/catalina/authenticator/SavedRequest.java
Posted by 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.
Cheers,
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1372394 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/catalina/authenticator/FormAuthenticator.java java/org/apache/catalina/authenticator/SavedRequest.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
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.
http://tomcat.markmail.org/thread/q2nudipddpwooisn
> Modified:
> tomcat/tc7.0.x/trunk/ (props changed)
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/FormAuthenticator.java
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/SavedRequest.java
>
> Propchange: tomcat/tc7.0.x/trunk/
> ------------------------------------------------------------------------------
> Merged /tomcat/trunk:r1372390
>
> Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/FormAuthenticator.java
> URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/FormAuthenticator.java?rev=1372394&r1=1372393&r2=1372394&view=diff
> ==============================================================================
> --- tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/FormAuthenticator.java (original)
> +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/FormAuthenticator.java Mon Aug 13 12:29:51 2012
> @@ -498,12 +498,11 @@ public class FormAuthenticator
> }
>
> // Does the request URI match?
> - String requestURI = request.getDecodedRequestURI();
> - if (requestURI == null) {
> + String decodedRequestURI = request.getDecodedRequestURI();
> + if (decodedRequestURI == null) {
> return (false);
> }
> - return (requestURI.equals(sreq.getRequestURI()));
> -
> + return (decodedRequestURI.equals(sreq.getDecodedRequestURI()));
> }
>
>
> @@ -658,11 +657,11 @@ public class FormAuthenticator
>
> saved.setMethod(request.getMethod());
> saved.setQueryString(request.getQueryString());
> - saved.setRequestURI(request.getDecodedRequestURI());
> + saved.setRequestURI(request.getRequestURI());
> + saved.setDecodedRequestURI(request.getDecodedRequestURI());
>
> // Stash the SavedRequest in our session for later use
> session.setNote(Constants.FORM_REQUEST_NOTE, saved);
> -
> }
>
>
>
> Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/SavedRequest.java
> URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/SavedRequest.java?rev=1372394&r1=1372393&r2=1372394&view=diff
> ==============================================================================
> --- tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/SavedRequest.java (original)
> +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/SavedRequest.java Mon Aug 13 12:29:51 2012
> @@ -147,6 +147,21 @@ public final class SavedRequest {
>
>
> /**
> + * The decode request URI associated with this Request. Path parameters are
> + * also excluded
> + */
> + private String decodedRequestURI = null;
> +
> + public String getDecodedRequestURI() {
> + return (this.decodedRequestURI);
> + }
> +
> + public void setDecodedRequestURI(String decodedRequestURI) {
> + this.decodedRequestURI = decodedRequestURI;
> + }
> +
> +
> + /**
> * The body of this request.
> */
> private ByteChunk body = null;
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org