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

Improve handling of POST requests and FORM authentication, particularly when the user agent responds to the 302 response by repeating the POST request including a request body. Any request body provided at this point is now swallowed.
Clean up the FormAuthenticator test case and extend the coverage to include bug 49779 and the remaining combinations of request methods.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@987955 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/catalina/authenticator/FormAuthenticator.java b/java/org/apache/catalina/authenticator/FormAuthenticator.java
index 609adcb..e199ca5 100644 (file)
--- a/java/org/apache/catalina/authenticator/FormAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/FormAuthenticator.java
@@ -401,7 +401,8 @@ public class FormAuthenticator
      * @param request The request to be restored
      * @param session The session containing the saved information
      */
-    protected boolean restoreRequest(Request request, Session session) {
+    protected boolean restoreRequest(Request request, Session session)
+            throws IOException {
 
         // Retrieve and remove the SavedRequest object from our session
         SavedRequest saved = (SavedRequest)
@@ -447,6 +448,13 @@ public class FormAuthenticator
         request.getCoyoteRequest().getParameters().setQueryStringEncoding(
                 request.getConnector().getURIEncoding());
 
+        // Swallow any request body since we will be replacing it
+        byte[] buffer = new byte[4096];
+        InputStream is = request.getInputStream();
+        while (is.read(buffer) >= 0) {
+            // Ignore request body
+        }
+        
         if ("POST".equalsIgnoreCase(saved.getMethod())) {
             ByteChunk body = saved.getBody();
             

diff --git a/test/org/apache/catalina/authenticator/TestFormAuthenticator.java b/test/org/apache/catalina/authenticator/TestFormAuthenticator.java
index 9691b72..eca77a8 100644 (file)
--- a/test/org/apache/catalina/authenticator/TestFormAuthenticator.java
+++ b/test/org/apache/catalina/authenticator/TestFormAuthenticator.java
@@ -26,116 +26,155 @@ import org.apache.catalina.startup.TomcatBaseTest;
 
 public class TestFormAuthenticator extends TomcatBaseTest {
 
-    public void testExpect100Continue() throws Exception {
-        Tomcat tomcat = getTomcatInstance();
-        File appDir = new File(getBuildDirectory(), "webapps/examples");
-        Context ctx =
-            tomcat.addWebapp(null, "/examples", appDir.getAbsolutePath());
-        
-        MapRealm realm = new MapRealm();
-        realm.addUser("tomcat", "tomcat");
-        realm.addUserRole("tomcat", "tomcat");
-        ctx.setRealm(realm);
+    public void testGet() throws Exception {
+        doTest("GET", "GET", false);
+    }
+
+    public void testPostNoContinue() throws Exception {
+        doTest("POST", "GET", false);
+    }
 
-        tomcat.start();
+    public void testPostWithContinue() throws Exception {
+        doTest("POST", "GET", true);
+    }
+
+    // Bug 49779
+    public void testPostNoContinuePostRedirect() throws Exception {
+        doTest("POST", "POST", false);
+    }
+
+    // Bug 49779
+    public void testPostWithContinuePostRedirect() throws Exception {
+        doTest("POST", "POST", true);
+    }
+
+
+    private void doTest(String resourceMethod, String redirectMethod,
+            boolean useContinue) throws Exception {
+        FormAuthClient client = new FormAuthClient();
 
-        Expect100ContinueClient client = new Expect100ContinueClient();
-        client.setPort(getPort());
-        
         // First request for authenticated resource 
-        Exception e = client.doRequest(null);
-        assertNull(e);
+        client.setUseContinue(useContinue);
+        client.doResourceRequest(resourceMethod);
         assertTrue(client.isResponse200());
         assertTrue(client.isResponseBodyOK());
-        
-        String sessionID = client.getSessionId();
+        client.reset();
         
         // Second request for the login page
-        client.reset();
-        e = client.doRequest(sessionID);
-        assertNull(e);
+        client.setUseContinue(useContinue);
+        client.doLoginRequest();
         assertTrue(client.isResponse302());
         assertTrue(client.isResponseBodyOK());
+        client.reset();
 
         // Third request - follow the redirect
-        client.reset();
-        e = client.doRequest(sessionID);
-        assertNull(e);
+        client.doResourceRequest(redirectMethod);
+        if ("POST".equals(redirectMethod)) {
+            client.setUseContinue(useContinue);
+        }
         assertTrue(client.isResponse200());
         assertTrue(client.isResponseBodyOK());
-
-        // Session ID changes after successful authentication
-        sessionID = client.getSessionId();
+        client.reset();
 
         // Subsequent requests - direct to the resource
         for (int i = 0; i < 5; i++) {
-            client.reset();
-            e = client.doRequest(sessionID);
-            assertNull(e);
+            client.setUseContinue(useContinue);
+            client.doResourceRequest(resourceMethod);
             assertTrue(client.isResponse200());
             assertTrue(client.isResponseBodyOK());
+            client.reset();
         }
     }
-    
-    private final class Expect100ContinueClient extends SimpleHttpClient {
 
+
+    private final class FormAuthClient extends SimpleHttpClient {
+
+        private static final String LOGIN_PAGE = "j_security_check";
+
+        private String protectedPage = "index.jsp";
+        private String protectedLocation = "/examples/jsp/security/protected/";
         private int requestCount = 0;
+        private String sessionId = null;
 
-        private Exception doRequest(String sessionId) {
-            try {
-                String request[] = new String[2];
-                if (requestCount == 1) {
-                    request[0] =
-                            "POST /examples/jsp/security/protected/j_security_check HTTP/1.1" + CRLF;
-                } else if (requestCount == 2) {
-                    request[0] =
-                            "GET /examples/jsp/security/protected/index.jsp HTTP/1.1" + CRLF;
-                } else {
-                    request[0] =
-                            "POST /examples/jsp/security/protected/index.jsp HTTP/1.1" + CRLF;
-                }
+        private FormAuthClient() throws Exception {
+            Tomcat tomcat = getTomcatInstance();
+            File appDir = new File(getBuildDirectory(), "webapps/examples");
+            Context ctx =
+                tomcat.addWebapp(null, "/examples", appDir.getAbsolutePath());
+            
+            MapRealm realm = new MapRealm();
+            realm.addUser("tomcat", "tomcat");
+            realm.addUserRole("tomcat", "tomcat");
+            ctx.setRealm(realm);
+
+            setPort(getPort());
 
+            tomcat.start();
+        }
+        
+        private void doResourceRequest(String method) throws Exception {
+            String request[] = new String[2];
+            request [0] = method + " " + protectedLocation + protectedPage + 
+                    " HTTP/1.1" + CRLF;
+            request[0] = request[0] + 
+                    "Host: localhost" + CRLF +
+                    "Connection: close" + CRLF;
+            if (getUseContinue()) {
                 request[0] = request[0] + 
-                        "Host: localhost" + CRLF +
-                        "Expect: 100-continue" + CRLF +
-                        "Connection: close" + CRLF;
-                
-                if (sessionId != null) {
-                    request[0] = request[0] +
-                            "Cookie: JSESSIONID=" + sessionId + CRLF;
-                }
-                
-                if (requestCount == 1) {
-                    request[0] = request[0] +
-                            "Content-Type: application/x-www-form-urlencoded" + CRLF +
-                            "Content-length: 35" + CRLF +
-                            CRLF;
-                    request[1] = "j_username=tomcat&j_password=tomcat";
-                } else if (requestCount ==2) {
-                    request[1] = CRLF;
-                } else {
-                    request[0] = request[0] +
-                            "Content-Type: application/x-www-form-urlencoded" + CRLF +
-                            "Content-length: 7" + CRLF +
-                            CRLF;
-                    request[1] = "foo=bar";
-                }
-
-                setRequest(request);
-                if (requestCount != 2) {
-                    setUseContinue(true);
-                }
-                
-                connect();
-                processRequest();
-                disconnect();
-                
-                requestCount++;
-            } catch (Exception e) {
-                e.printStackTrace();
-                return e;
+                        "Expect: 100-continue" + CRLF;
+            }
+            if (sessionId != null) {
+                request[0] = request[0] +
+                        "Cookie: JSESSIONID=" + sessionId + CRLF;
             }
-            return null;
+            if ("POST".equals(method)) {
+                request[0] = request[0] +
+                        "Content-Type: application/x-www-form-urlencoded" + CRLF +
+                        "Content-length: 7" + CRLF +
+                        CRLF;
+                request[1] = "foo=bar";
+            } else {
+                request[1] = CRLF;
+            }
+            doRequest(request);
+        }
+
+        private void doLoginRequest() throws Exception {
+            String request[] = new String[2];
+            request [0] = "POST " + protectedLocation + LOGIN_PAGE + 
+                    " HTTP/1.1" + CRLF;
+            request[0] = request[0] + 
+                    "Host: localhost" + CRLF +
+                    "Connection: close" + CRLF;
+            if (getUseContinue()) {
+                request[0] = request[0] + 
+                        "Expect: 100-continue" + CRLF;
+            }
+            if (sessionId != null) {
+                request[0] = request[0] +
+                        "Cookie: JSESSIONID=" + sessionId + CRLF;
+            }
+            request[0] = request[0] +
+                    "Content-Type: application/x-www-form-urlencoded" + CRLF +
+                    "Content-length: 35" + CRLF +
+                    CRLF;
+            request[1] = "j_username=tomcat&j_password=tomcat";
+            
+            doRequest(request);
+        }
+
+        private void doRequest(String request[]) throws Exception {
+            setRequest(request);
+            
+            connect();
+            processRequest();
+            String newSessionId = getSessionId();
+            if (newSessionId != null) {
+                sessionId = newSessionId;
+            }
+            disconnect();
+            
+            requestCount++;
         }
 
         @Override

diff --git a/test/org/apache/catalina/startup/SimpleHttpClient.java b/test/org/apache/catalina/startup/SimpleHttpClient.java
index a3d4050..8085a65 100644 (file)
--- a/test/org/apache/catalina/startup/SimpleHttpClient.java
+++ b/test/org/apache/catalina/startup/SimpleHttpClient.java
@@ -78,6 +78,10 @@ public abstract class SimpleHttpClient {
         useContinue = theUseContinueFlag;
     }
 
+    public boolean getUseContinue() {
+        return useContinue;
+    }
+
     public void setRequestPause(int theRequestPause) {
         requestPause = theRequestPause;
     }
@@ -183,6 +187,8 @@ public abstract class SimpleHttpClient {
         request = null;
         requestPause = 1000;
         
+        useContinue = false;
+
         responseLine = null;
         responseHeaders = new ArrayList<String>();
         responseBody = null;

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a5c604a..c174620 100644 (file)
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -54,6 +54,12 @@
         Provide 100 Continue responses at appropriate points during FORM
         authentication if client indicates that they are expected. (markt)
       </fix>
+      <fix>
+        <bug>49779</bug>: Improve handling of POST requests and FORM
+        authentication, particularly when the user agent responds to the 302
+        response by repeating the POST request including a request body. Any
+        request body provided at this point is now swallowed. (markt)  
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">