You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2011/10/03 18:15:05 UTC

DO NOT REPLY [Bug 51940] New: Form Authentication Valve should restore request body on PUT method

https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

             Bug #: 51940
           Summary: Form Authentication Valve should restore request body
                    on PUT method
           Product: Tomcat 6
           Version: 6.0.33
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Catalina
        AssignedTo: dev@tomcat.apache.org
        ReportedBy: nsushkin@openfinance.com
    Classification: Unclassified


In Tomcat 6 (and 7), Form Authentication valve restores the original request
after a POST with successful authentication and redirect is followed by the
client's GET. In case of the POST, the valve also restores the original
request's body. However, it doesn't do that for a PUT. To be consistent, Tomcat
should restore the body on PUT as well.

The patch would be in FormAuthenticator.restoreRequest(Request, Session) [1],
to change from

if ("POST".equalsIgnoreCase(saved.getMethod())) {

to

if ("POST".equalsIgnoreCase(saved.getMethod()) ||
"PUT".equalsIgnoreCase(saved.getMethod())
) {

[1]
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/authenticator/FormAuthenticator.java?view=markup#l450


Maybe related to Bug #48692.


This issue was discussed on users mailing list archived at

http://markmail.org/thread/klafrhln32v3zcau

and

http://mail-archives.apache.org/mod_mbox/tomcat-users/201109.mbox/%3C3052451.ZX31eH6Cz8@strela%3E


Regarding "Re: Should Form Authentication Valve restore request body on a
PUT?", 
on Thursday, September 29, 2011 17:04:27,
Christopher Schultz wrote to Tomcat Users List <us...@tomcat.apache.org>

> ...
> The servlet spec (v3.0, SRV 13.6.3.1) has this to say:
> "
> If the form based login is invoked because of an HTTP request, the
> original request parameters must be preserved by the container for use
> if, on successful authentication, it redirects the call to the
> requested resource.
> "
> 
> It doesn't say what kinds of HTTP verbs should or should not be
> supported, but GET and PUT seem entirely obvious. It doesn't say that
> the request body needs to be maintained, only the "request
> parameters". Since the servlet specification doesn't have any
> provisions for fetching request parameters from PUT operations, I
> suppose the spec therefore doesn't directly recommend that PUT bodies
> be stored for later use like when POST is used.
> ...
> On the face of it, that seems reasonable. I haven't read-through the
> code that then replays the saved-request so I'm not sure if there's
> more to be done.


Regarding "Re: Should Form Authentication Valve restore request body on a
PUT?", 
on Friday, September 30, 2011 13:10:55,
Mark Thomas wrote to Tomcat Users List <us...@tomcat.apache.org>

> I'd have no objection so the proposed change.
> 
> Mark

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


DO NOT REPLY [Bug 51940] Form Authentication Valve should restore request body on PUT method

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

--- Comment #5 from Mark Thomas <ma...@apache.org> 2011-10-10 15:48:59 UTC ---
Thanks for the patch. I used it as the basis for the fox that has been
committed.

The fix has been applied to trunk and 7.0.x and will be included in 7.0.23
onwards.

The fix has also been proposed for 6.0.x.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


DO NOT REPLY [Bug 51940] Form Authentication Valve should restore request body on PUT method

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #13 from Mark Thomas <ma...@apache.org> 2011-10-31 13:06:53 UTC ---
Fixed in 6.0.x and will be included in 6.0.34 onwards.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


DO NOT REPLY [Bug 51940] Form Authentication Valve should restore request body on PUT method

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

--- Comment #11 from Konstantin Kolinko <kn...@gmail.com> 2011-10-20 10:28:18 UTC ---
(In reply to comment #10)
> (In reply to comment #9)
> > 1. I tried to test this in trunk, and replaying a POST request fails for me.
> 
> That is a side-effect of r987955 which is not directly related to this bug.

Confirming that r1186379 / r1186383 fixed it.

Tomcat 6 is not affected by this issue.


> 
> > 2. request.getCoyoteRequest().method().setString("GET");
> > is seen as GET in access log.  Not much of an issue though, as anyway we return
> > not what was originally requested by client.
> > 
> > I think that to fix this one can restore the original method in
> > FormAuthenticator#forwardToLoginPage(..) when disp.forward() call returns.
> 
> That would work. I'm on the fence. We can record what was requested or what we
> process.

Fixed this in r1186712 / r1186719

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


DO NOT REPLY [Bug 51940] Form Authentication Valve should restore request body on PUT method

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

--- Comment #10 from Mark Thomas <ma...@apache.org> 2011-10-19 18:02:15 UTC ---
(In reply to comment #9)
> 1. I tried to test this in trunk, and replaying a POST request fails for me.

That is a side-effect of r987955 which is not directly related to this bug.

> 2. request.getCoyoteRequest().method().setString("GET");
> is seen as GET in access log.  Not much of an issue though, as anyway we return
> not what was originally requested by client.
> 
> I think that to fix this one can restore the original method in
> FormAuthenticator#forwardToLoginPage(..) when disp.forward() call returns.

That would work. I'm on the fence. We can record what was requested or what we
process.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


DO NOT REPLY [Bug 51940] Form Authentication Valve should restore request body on PUT method

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

--- Comment #8 from Mark Thomas <ma...@apache.org> 2011-10-10 18:58:14 UTC ---
Thanks for the review. Those issues have been fixed.

Any proposal for back-port needs a minimum of 3 +1 votes from committers. Once
it has the votes, a committer (usually the proposer but if doesn't have to be)
will commit it and it is included in the next release.

The RM role is 'just' one of creating the release. It has no special role to
play in the back-port process.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


DO NOT REPLY [Bug 51940] Form Authentication Valve should restore request body on PUT method

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

--- Comment #9 from Konstantin Kolinko <kn...@gmail.com> 2011-10-11 13:15:18 UTC ---
1. I tried to test this in trunk, and replaying a POST request fails for me.

Using the following standalone HTML page:
[[[
<FORM action="http://localhost:8080/examples/jsp/security/protected/index.jsp"
method="POST">
<input name="role">
<input type="submit">
</FORM>
]]]

- Configure a sample user with a role of "tomcat" and start Tomcat
- Start a new web browser and load the above page
- Type "aaa" and press submit.
- Respond to FORM authentication.
- The protected page is displayed.
Expected: "You have not been granted role aaaa"
Actual: no such message.

I tried to debug this, but I do not see anything wrong in FormAuthenticator.
The body and method were saved and restored. Request#parametersParsed was
false.

Then, setting a breakpoint in Request#parseParameters() I see that
Request#usingInputStream is true and thus parseParameters() exits early.

This is reproducible in 7.0.22.
This does not happen in 6.0.33 - it replays POSTs correctly.


2. request.getCoyoteRequest().method().setString("GET");
is seen as GET in access log.  Not much of an issue though, as anyway we return
not what was originally requested by client.

I think that to fix this one can restore the original method in
FormAuthenticator#forwardToLoginPage(..) when disp.forward() call returns.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


DO NOT REPLY [Bug 51940] Form Authentication Valve should restore request body on PUT method

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

--- Comment #4 from Mark Thomas <ma...@apache.org> 2011-10-10 15:17:37 UTC ---
Try again, this time with the full message.

RFC 2616 explicitly discusses OPTIONS and a request body. However, any HTTP
request method may use a request body [1] although in many cases it will be
ignored.

It appears that the right thing to do here is to save any request body we find
and restore it after authentication regardless of request method.

[1] http://tech.groups.yahoo.com/group/rest-discuss/message/9962

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


DO NOT REPLY [Bug 51940] Form Authentication Valve should restore request body on PUT method

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

--- Comment #7 from Nicholas Sushkin <ns...@openfinance.com> 2011-10-10 17:29:14 UTC ---
BTW, in your change to 7.0, there seems to be a slight inconsistency in the log
message. In other log messages, when you say "context [{n}]", you pass
context.getName(). In this message, you passed
context.getServletContext().getContextPath() leftover from my patch.

http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/FormAuthenticator.java?view=markup#l375

I wasn't going to say anything, but if you're revisiting this file, might as
well fix the typo in "somethign" :)

http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/authenticator/FormAuthenticator.java?view=markup#l626

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


DO NOT REPLY [Bug 51940] Form Authentication Valve should restore request body on PUT method

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

--- Comment #6 from Nicholas Sushkin <ns...@openfinance.com> 2011-10-10 17:08:25 UTC ---
Perfect. I am glad Roy Fielding clarified the usage of request body in HTTP and
REST. It makes sense and the code is now simpler and more generic.

How does that request for the bug fix to be included into 6.0.x work? Is that
Release Manager for 6.0.x Jean-Frederic who needs to approve it? I see that
both Bug Fixes and Enhancements are still accepted for 6.0.x (
http://wiki.apache.org/tomcat/TomcatVersions ). It would be most useful to me
if the fix was included in Tomcat 6. Thank you.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


DO NOT REPLY [Bug 51940] Form Authentication Valve should restore request body on PUT method

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

--- Comment #3 from Mark Thomas <ma...@apache.org> 2011-10-10 15:13:20 UTC ---
POST and PUT are not the only methods that may have a request body. Request
bodies are described in RFC2616 for OPTION re

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


DO NOT REPLY [Bug 51940] Form Authentication Valve should restore request body on PUT method

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

--- Comment #12 from Konstantin Kolinko <kn...@gmail.com> 2011-10-20 10:39:24 UTC ---
Created attachment 27825
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=27825
2011-10-20_tc6_51940_v3.patch

TC 6 version of the patch
In trunk these are r1181028, r1181136, r1186378, r1186712

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


DO NOT REPLY [Bug 51940] Form Authentication Valve should restore request body on PUT method

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

--- Comment #1 from Nicholas Sushkin <ns...@openfinance.com> 2011-10-07 17:06:46 UTC ---
Regarding "Re: Should Form Authentication Valve restore request body on a
PUT?", 
on Friday, October 07, 2011 10:13:00,
Christopher Schultz wrote to Tomcat Users List <us...@tomcat.apache.org>

> Nicholas,
> 
> On 10/6/2011 10:08 PM, Nicholas Sushkin wrote:
> > I now reconfigured DefaultServlet in conf/web.xml with
> > readonly=false. Now, an unauthenticated PUT (with or without a
> > body) returns 204 No Content instead of the login form. Seems like
> > a bug. Should I add this behavior to Bug #51940 or a new bug?
> 
> I'll bet what is happening is that your PUT request is being forwarded
> without modification to the login page, and your login page is some
> static content. Is that right?
> 
> If that's what's happening, the DefaultServlet is handling the
> request, seeing that it is a PUT, and then complaining that it's
> read-only. When you make the DefaultServlet read-write you tell the
> DefaultServlet to accept uploads, and you'll probably end up
> overwriting your login form with the request entity (oops).
> 
> It looks like the authenticator code needs to transform the PUT
> request into a GET (or POST?) so that the DefaultServlet doesn't try
> to do an upload.
> 
> I think you'd have similar problems if trying to use a JSP for your
> login-page, because JSPs can't accept PUT requests unless specifically
> configured to do so.
> 
> Since you're just hacking, try setting the request method to "GET"
> when you detect a PUT request that requires authentication.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


DO NOT REPLY [Bug 51940] Form Authentication Valve should restore request body on PUT method

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51940

--- Comment #2 from Nicholas Sushkin <ns...@openfinance.com> 2011-10-07 17:40:01 UTC ---
Created attachment 27729
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=27729
Patch fixing PUT handling with form authentication

Implemented Charles's suggestion. Before forwarding to the login page, but
after saving the request, in the request, set method (type) to GET.

Also, in save/restore of a request, when the method is PUT, allowed
saving/restoring of the method body.

Tested this patch on unauthenticated GET, PUT, POST, and DELETE.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org