tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1727899 - in /tomcat/trunk: java/org/apache/catalina/core/ApplicationHttpRequest.java test/org/apache/catalina/core/TestApplicationHttpRequest.java webapps/docs/changelog.xml
Date Mon, 01 Feb 2016 09:20:09 GMT
Author: markt
Date: Mon Feb  1 09:20:09 2016
New Revision: 1727899

URL: http://svn.apache.org/viewvc?rev=1727899&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=58946
Ensure that the request parameter map remains immutable when processing via a RequestDispatcher.

Modified:
    tomcat/trunk/java/org/apache/catalina/core/ApplicationHttpRequest.java
    tomcat/trunk/test/org/apache/catalina/core/TestApplicationHttpRequest.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/core/ApplicationHttpRequest.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/ApplicationHttpRequest.java?rev=1727899&r1=1727898&r2=1727899&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/ApplicationHttpRequest.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/ApplicationHttpRequest.java Mon Feb  1 09:20:09
2016
@@ -24,7 +24,6 @@ import java.io.UnsupportedEncodingExcept
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Enumeration;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.NoSuchElementException;
 
@@ -40,6 +39,7 @@ import org.apache.catalina.Context;
 import org.apache.catalina.Globals;
 import org.apache.catalina.Manager;
 import org.apache.catalina.Session;
+import org.apache.catalina.util.ParameterMap;
 import org.apache.tomcat.util.buf.B2CConverter;
 import org.apache.tomcat.util.buf.MessageBytes;
 import org.apache.tomcat.util.http.Parameters;
@@ -134,7 +134,7 @@ class ApplicationHttpRequest extends Htt
 
     /**
      * The request parameters for this request.  This is initialized from the
-     * wrapped request, but updates are allowed.
+     * wrapped request.
      */
     protected Map<String, String[]> parameters = null;
 
@@ -634,27 +634,6 @@ class ApplicationHttpRequest extends Htt
 
 
     /**
-     * Perform a shallow copy of the specified Map, and return the result.
-     *
-     * @param orig Origin Map to be copied
-     */
-    Map<String, String[]> copyMap(Map<String, String[]> orig) {
-
-        if (orig == null) {
-            return (new HashMap<>());
-        }
-        HashMap<String, String[]> dest = new HashMap<>();
-
-        for (Map.Entry<String, String[]> entry : orig.entrySet()) {
-            dest.put(entry.getKey(), entry.getValue());
-        }
-
-        return (dest);
-
-    }
-
-
-    /**
      * Set the context path for this request.
      *
      * @param contextPath The new context path
@@ -750,9 +729,10 @@ class ApplicationHttpRequest extends Htt
             return;
         }
 
-        parameters = new HashMap<>();
-        parameters = copyMap(getRequest().getParameterMap());
+        parameters = new ParameterMap<>();
+        parameters.putAll(getRequest().getParameterMap());
         mergeParameters();
+        ((ParameterMap<String,String[]>) parameters).setLocked(true);
         parsedParams = true;
     }
 
@@ -879,8 +859,6 @@ class ApplicationHttpRequest extends Htt
         if ((queryParamString == null) || (queryParamString.length() < 1))
             return;
 
-        HashMap<String, String[]> queryParameters = new HashMap<>();
-
         // Parse the query string from the dispatch target
         Parameters paramParser = new Parameters();
         MessageBytes queryMB = MessageBytes.newInstance();
@@ -901,23 +879,18 @@ class ApplicationHttpRequest extends Htt
         paramParser.setQueryStringEncoding(encoding);
         paramParser.handleQueryParameters();
 
-        // Copy the original parameters
-        queryParameters.putAll(parameters);
-
         // Insert the additional parameters from the dispatch target
         Enumeration<String> dispParamNames = paramParser.getParameterNames();
         while (dispParamNames.hasMoreElements()) {
             String dispParamName = dispParamNames.nextElement();
             String[] dispParamValues = paramParser.getParameterValues(dispParamName);
-            String[] originalValues = queryParameters.get(dispParamName);
+            String[] originalValues = parameters.get(dispParamName);
             if (originalValues == null) {
-                queryParameters.put(dispParamName, dispParamValues);
+                parameters.put(dispParamName, dispParamValues);
                 continue;
             }
-            queryParameters.put(dispParamName, mergeValues(dispParamValues, originalValues));
+            parameters.put(dispParamName, mergeValues(dispParamValues, originalValues));
         }
-
-        parameters = queryParameters;
     }
 
 

Modified: tomcat/trunk/test/org/apache/catalina/core/TestApplicationHttpRequest.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestApplicationHttpRequest.java?rev=1727899&r1=1727898&r2=1727899&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/core/TestApplicationHttpRequest.java (original)
+++ tomcat/trunk/test/org/apache/catalina/core/TestApplicationHttpRequest.java Mon Feb  1
09:20:09 2016
@@ -33,6 +33,7 @@ import org.junit.Test;
 import org.apache.catalina.Context;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.buf.ByteChunk;
 
 public class TestApplicationHttpRequest extends TomcatBaseTest {
@@ -210,6 +211,32 @@ public class TestApplicationHttpRequest
     }
 
 
+    @Test
+    public void testParameterImmutability() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        Tomcat.addServlet(ctx, "forward", new ForwardServlet("/modify"));
+        ctx.addServletMapping("/forward", "forward");
+
+        Tomcat.addServlet(ctx, "modify", new ModifyParameterServlet());
+        ctx.addServletMapping("/modify", "modify");
+
+        tomcat.start();
+
+        ByteChunk response = new ByteChunk();
+        StringBuilder target = new StringBuilder("http://localhost:");
+        target.append(getPort());
+        target.append("/forward");
+        int rc = getUrl(target.toString(), response, null);
+
+        Assert.assertEquals(200, rc);
+        Assert.assertEquals("OK", response.toString());
+    }
+
+
     private static class ForwardServlet extends HttpServlet {
 
         private static final long serialVersionUID = 1L;
@@ -294,4 +321,37 @@ public class TestApplicationHttpRequest
             }
         }
     }
+
+
+    private static class ModifyParameterServlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        // Suppress warnings generated because the code is trying to put the
+        // wrong type of values into the Map
+        @SuppressWarnings({"rawtypes", "unchecked"})
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+            Map map = req.getParameterMap();
+
+            boolean insertWorks;
+            try {
+                map.put("test", new Integer[] { Integer.valueOf(0) });
+                insertWorks = true;
+            } catch (Throwable t) {
+                ExceptionUtils.handleThrowable(t);
+                insertWorks = false;
+            }
+
+            resp.setContentType("text/plain");
+            resp.setCharacterEncoding("UTF-8");
+            PrintWriter pw = resp.getWriter();
+            if (insertWorks) {
+                pw.print("FAIL");
+            } else {
+                pw.print("OK");
+            }
+        }
+    }
 }

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1727899&r1=1727898&r2=1727899&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Feb  1 09:20:09 2016
@@ -72,6 +72,10 @@
         <bug>58768</bug>: Log a warning if a redirect fails because of an
         invalid location. (markt)
       </fix>
+      <fix>
+        <bug>58946</bug>: Ensure that the request parameter map remains
+        immutable when processing via a RequestDispatcher. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">



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


Mime
View raw message