Fixes #3628 - Make sure RequestDispatcher.include honors parameter precendence

core/impl/src/main/java/cloud/piranha/core/impl/DefaultServletRequestDispatcher.java

@@ -263,7 +263,7 @@ public void include(ServletRequest servletRequest, ServletResponse servletRespon
  * Unwrap the passed ServletRequest to the underlying WebApplicationRequest.
  */
  WebApplicationRequest request = unwrap(servletRequest, WebApplicationRequest.class);
- 
+
  /**
  * Set the include attributes.
  */
@@ -272,36 +272,67 @@ public void include(ServletRequest servletRequest, ServletResponse servletRespon
  request.setAttribute(INCLUDE_QUERY_STRING, request.getQueryString());
  request.setAttribute(INCLUDE_REQUEST_URI, request.getRequestURI());
  request.setAttribute(INCLUDE_SERVLET_PATH, request.getServletPath());
- 
+
  /**
  * Set the new dispatcher type.
  */
  request.setDispatcherType(INCLUDE);
  request.setServletPath(path == null ? "/" + servletEnvironment.getServletName() : getServletPath(path));
  request.setPathInfo(null);
+
+ if (path != null) {
+ request.setQueryString(getQueryString(path));
+ }
 
  invocationFinder.addFilters(INCLUDE, servletInvocation, request.getServletPath(), "");
 
  if (servletInvocation.getServletEnvironment() != null) {
  request.setAsyncSupported(request.isAsyncSupported() && isAsyncSupportedInChain());
  }
 
+ /*
+ * Aggregate the request parameters with giving the new parameter values
+ * precedence over the old ones.
+ */
+ if (request.getQueryString() != null) {
+ String queryString = request.getQueryString();
+ Map<String, String[]> parameters = request.getModifiableParameterMap();
+ for (String param : queryString.split("&")) {
+ String[] pair = param.split("=");
+ String key = URLDecoder.decode(pair[0], StandardCharsets.UTF_8);
+ String value = "";
+ if (pair.length > 1) {
+ value = URLDecoder.decode(pair[1], StandardCharsets.UTF_8);
+ }
+ String[] values = parameters.get(key);
+ if (values == null) {
+ values = new String[]{value};
+ parameters.put(key, values);
+ } else {
+ String[] newValues = new String[values.length + 1];
+ System.arraycopy(values, 0, newValues, 1, values.length);
+ newValues[0] = value;
+ parameters.put(key, newValues);
+ }
+ }
+ }
+
  /*
  * Execute the filter chain.
  */
  try {
  servletEnvironment.getWebApplication().linkRequestAndResponse(servletRequest, servletResponse);
  servletInvocation.getFilterChain().doFilter(servletRequest, servletResponse);
  servletEnvironment.getWebApplication().unlinkRequestAndResponse(servletRequest, servletResponse);
- } catch(Exception e) {
+ } catch (Exception e) {
  rethrow(e);
  } finally {
  /*
  * Set servlet path and path info back to original values.
  */
  request.setServletPath((String) request.getAttribute(INCLUDE_SERVLET_PATH));
  request.setPathInfo((String) request.getAttribute(INCLUDE_PATH_INFO));
- 
+
  /*
  * Remove include attributes.
  */
@@ -699,7 +730,7 @@ private void dispatchSyncForward(
 
  /*
  * If response is already committed we need to throw an IllegalStateException.
- */
+  */
  if (servletResponse.isCommitted()) {
  throw new IllegalStateException("Response already committed");
  }
@@ -723,7 +754,7 @@ private void dispatchSyncForward(
  * Unwrap the passed ServletRequest to the underlying WebApplicationRequest.
  */
  WebApplicationRequest request = unwrap(servletRequest, WebApplicationRequest.class);
- 
+
  /*
  * Set the forward attributes.
  */
@@ -732,7 +763,7 @@ private void dispatchSyncForward(
  request.setAttribute(FORWARD_QUERY_STRING, request.getQueryString());
  request.setAttribute(FORWARD_REQUEST_URI, request.getRequestURI());
  request.setAttribute(FORWARD_SERVLET_PATH, request.getServletPath());
- 
+
  /*
  * Set the new dispatcher type.
  */
@@ -748,7 +779,7 @@ private void dispatchSyncForward(
  request.setServletPath("/" + servletEnvironment.getServletName());
  request.setQueryString(request.getQueryString());
  }
- 
+
  /*
  * Aggregate the request parameters with giving the new parameter values
  * precedence over the old ones.
@@ -775,7 +806,7 @@ private void dispatchSyncForward(
  }
  }
  }
- 
+
  invocationFinder.addFilters(FORWARD, servletInvocation, request.getServletPath(), "");
 
  if (servletInvocation.getServletEnvironment() != null) {

core/impl/src/test/java/cloud/piranha/core/impl/RequestDispatcherTest.java

@@ -141,7 +141,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t
  .build();
  webApplication.initialize();
  webApplication.start();
- 
+
  WebApplicationRequest request = new DefaultWebApplicationRequestBuilder()
  .webApplication(webApplication)
  .build();
@@ -739,6 +739,61 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t
  assertEquals("123456", new String(byteOutput.toByteArray()));
  }
 
+ /**
+ * Test an include with a query string.
+ *
+ * @throws Exception when an error occurs.
+ */
+ @Test
+ void testIncludeWithQueryString() throws Exception {
+ WebApplication webApplication = new DefaultWebApplicationBuilder()
+ .servlet("Include", new HttpServlet() {
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+ /*
+ * Force the original parameter to be parsed out of the query string.
+ */
+ req.getParameter("p");
+ /*
+ * Dispatch with a new parameter value.
+ */
+ getServletContext()
+ .getRequestDispatcher("/include2?p=New")
+ .include(req, resp);
+ }
+ })
+ .servletMapping("Include", "/include")
+ .servlet("Include2", new HttpServlet() {
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+ /*
+ * We should be getting the new parameter value here as it takes precendence.
+ */
+ resp.getWriter().print(req.getParameterMap().get("p")[0]);
+ }
+ })
+ .servletMapping("Include2", "/include2")
+ .build();
+ webApplication.initialize();
+ webApplication.start();
+
+ WebApplicationRequest request = new DefaultWebApplicationRequestBuilder()
+ .webApplication(webApplication)
+ .servletPath("/include")
+ .queryString("p=Original")
+ .build();
+
+ WebApplicationResponse response = new DefaultWebApplicationResponseBuilder()
+ .webApplication(webApplication)
+ .bodyOnly(true)
+ .build();
+ ByteArrayOutputStream byteOutput = new ByteArrayOutputStream();
+ response.getWebApplicationOutputStream().setOutputStream(byteOutput);
+
+ webApplication.service(request, response);
+ assertEquals("New", byteOutput.toString());
+ }
+
  @Test
  void testErrorDispatcher() throws Exception {
  DefaultWebApplication webApp = new DefaultWebApplication();