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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
