[UNDERTOW-2269] Encode query string on forward/include and improve me…

…rge of parameters
undertow-io · Apr 5, 2024 · fc9083d · fc9083d
1 parent ddb4aee
commit fc9083d
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 50 deletions.
diff --git a/core/src/main/java/io/undertow/UndertowMessages.java b/core/src/main/java/io/undertow/UndertowMessages.java
@@ -647,4 +647,7 @@ public interface UndertowMessages {
     @Message(id = 208, value = "Failed to allocate resource")
     IOException failedToAllocateResource();
 
+    @Message(id = 209, value = "Failed to encode query string '%s' with '%s' encoding.")
+    IllegalArgumentException failedToEncodeQueryString(final String q, final String e);
+
 }
diff --git a/core/src/main/java/io/undertow/server/HttpServerExchange.java b/core/src/main/java/io/undertow/server/HttpServerExchange.java
@@ -205,7 +205,7 @@ public final class HttpServerExchange extends AbstractAttachable {
     private String resolvedPath = "";
 
     /**
-     * the query string
+     * the query string - percent encoded
      */
     private String queryString = "";
 

diff --git a/core/src/main/java/io/undertow/util/QueryParameterUtils.java b/core/src/main/java/io/undertow/util/QueryParameterUtils.java
@@ -20,13 +20,15 @@
 
 import java.io.UnsupportedEncodingException;
 import java.net.URLDecoder;
+import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayDeque;
 import java.util.Deque;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import org.xnio.OptionMap;
 
+import io.undertow.UndertowMessages;
 import io.undertow.UndertowOptions;
 import io.undertow.server.HttpServerExchange;
 
@@ -42,33 +44,42 @@ private QueryParameterUtils() {
     }
 
     public static String buildQueryString(final Map<String, Deque<String>> params) {
-        StringBuilder sb = new StringBuilder();
-        boolean first = true;
-        for (Map.Entry<String, Deque<String>> entry : params.entrySet()) {
-            if (entry.getValue().isEmpty()) {
-                if (first) {
-                    first = false;
-                } else {
-                    sb.append('&');
-                }
-                sb.append(entry.getKey());
-                sb.append('=');
-            } else {
-                for (String val : entry.getValue()) {
+        return QueryParameterUtils.buildQueryString(params, null);
+    }
+
+    public static String buildQueryString(final Map<String, Deque<String>> params, final String encoding) {
+        try {
+            StringBuilder sb = new StringBuilder();
+            boolean first = true;
+            for (Map.Entry<String, Deque<String>> entry : params.entrySet()) {
+                final String key = encoding != null ? URLEncoder.encode(entry.getKey(), encoding) : entry.getKey();
+                if (entry.getValue().isEmpty()) {
                     if (first) {
                         first = false;
                     } else {
                         sb.append('&');
                     }
-                    sb.append(entry.getKey());
+                    sb.append(key);
                     sb.append('=');
-                    sb.append(val);
+                } else {
+                    for (String val : entry.getValue()) {
+                        if (first) {
+                            first = false;
+                        } else {
+                            sb.append('&');
+                        }
+                        final String _val = encoding != null ? URLEncoder.encode(val, encoding) : val;
+                        sb.append(key);
+                        sb.append('=');
+                        sb.append(_val);
+                    }
                 }
             }
+            return sb.toString();
+        } catch (UnsupportedEncodingException e) {
+            throw UndertowMessages.MESSAGES.failedToEncodeQueryString(buildQueryString(params), encoding);
         }
-        return sb.toString();
     }
-
     /**
      * Parses a query string into a map
      * @param newQueryString The query string
@@ -146,8 +157,9 @@ public static Map<String, Deque<String>> mergeQueryParametersWithNewQueryString(
         return mergeQueryParametersWithNewQueryString(queryParameters, newQueryString, StandardCharsets.UTF_8.name());
     }
 
+    @Deprecated
     public static Map<String, Deque<String>> mergeQueryParametersWithNewQueryString(final Map<String, Deque<String>> queryParameters, final String newQueryString, final String encoding) {
-
+        //DEPRACETED this will create duplicates
         Map<String, Deque<String>> newQueryParameters = parseQueryString(newQueryString, encoding);
         //according to the spec the new query parameters have to 'take precedence'
         for (Map.Entry<String, Deque<String>> entry : queryParameters.entrySet()) {
@@ -160,6 +172,20 @@ public static Map<String, Deque<String>> mergeQueryParametersWithNewQueryString(
         return newQueryParameters;
     }
 
+    public static Map<String, Deque<String>> mergeQueryParameters(final Map<String, Deque<String>> newParams, final Map<String, Deque<String>> oldParams) {
+        //according to the spec the new query parameters have to 'take precedence'
+          for (Map.Entry<String, Deque<String>> entry : oldParams.entrySet()) {
+              if (!newParams.containsKey(entry.getKey())) {
+                  newParams.put(entry.getKey(), new ArrayDeque<>(entry.getValue()));
+              } else {
+                  final Deque<String> newValues = newParams.get(entry.getKey());
+                  final Deque<String> oldValues = entry.getValue();
+                  oldValues.stream().filter(v -> !newValues.contains(v)).forEach(v-> newValues.add(v));
+              }
+          }
+          return newParams;
+      }
+
     public static String getQueryParamEncoding(HttpServerExchange exchange) {
         String encoding = null;
         OptionMap undertowOptions = exchange.getConnection().getUndertowOptions();

diff --git a/servlet/src/main/java/io/undertow/servlet/util/DispatchUtils.java b/servlet/src/main/java/io/undertow/servlet/util/DispatchUtils.java
@@ -17,18 +17,6 @@
  */
 package io.undertow.servlet.util;
 
-import io.undertow.server.Connectors;
-import io.undertow.server.HttpServerExchange;
-import io.undertow.servlet.handlers.ServletPathMatch;
-import io.undertow.servlet.handlers.ServletRequestContext;
-import io.undertow.servlet.spec.HttpServletRequestImpl;
-import io.undertow.servlet.spec.HttpServletResponseImpl;
-import io.undertow.servlet.spec.ServletContextImpl;
-import io.undertow.util.ParameterLimitException;
-import jakarta.servlet.ServletException;
-import java.util.Deque;
-import java.util.Map;
-
 import static jakarta.servlet.AsyncContext.ASYNC_CONTEXT_PATH;
 import static jakarta.servlet.AsyncContext.ASYNC_MAPPING;
 import static jakarta.servlet.AsyncContext.ASYNC_PATH_INFO;
@@ -54,6 +42,20 @@
 import static jakarta.servlet.RequestDispatcher.INCLUDE_REQUEST_URI;
 import static jakarta.servlet.RequestDispatcher.INCLUDE_SERVLET_PATH;
 
+import java.util.Deque;
+import java.util.Map;
+
+import io.undertow.server.Connectors;
+import io.undertow.server.HttpServerExchange;
+import io.undertow.servlet.handlers.ServletPathMatch;
+import io.undertow.servlet.handlers.ServletRequestContext;
+import io.undertow.servlet.spec.HttpServletRequestImpl;
+import io.undertow.servlet.spec.HttpServletResponseImpl;
+import io.undertow.servlet.spec.ServletContextImpl;
+import io.undertow.util.ParameterLimitException;
+import io.undertow.util.QueryParameterUtils;
+import jakarta.servlet.ServletException;
+
 /**
  * <p>Utility class to manage the dispatching parsing of the path. The methods
  * fill the exchange, request and response with the needed data for the
@@ -210,24 +212,6 @@ public static ServletPathMatch dispatchAsync(final String path,
         return pathMatch;
     }
 
-    private static Map<String, Deque<String>> mergeQueryParameters(final Map<String, Deque<String>> newParams, final Map<String, Deque<String>> oldParams) {
-        for (Map.Entry<String, Deque<String>> entry : oldParams.entrySet()) {
-            Deque<String> values = newParams.get(entry.getKey());
-            if (values == null) {
-                // add all the values as new params do not contain this key
-                newParams.put(entry.getKey(), entry.getValue());
-            } else {
-                // merge values new params first
-                for (String v : entry.getValue()) {
-                    if (!values.contains(v)) {
-                        values.add(v);
-                    }
-                }
-            }
-        }
-        return newParams;
-    }
-
     private static String assignRequestPath(final String path, final HttpServletRequestImpl requestImpl,
             final ServletContextImpl servletContext, final boolean include) throws ParameterLimitException {
         final StringBuilder sb = new StringBuilder();
@@ -253,7 +237,11 @@ private static String assignRequestPath(final String path, final HttpServletRequ
         }
         // both forward and include merge parameters by spec
         if (!fake.getQueryString().isEmpty()) {
-            requestImpl.setQueryParameters(mergeQueryParameters(fake.getQueryParameters(), requestImpl.getQueryParameters()));
+            final Map<String, Deque<String>> merged = QueryParameterUtils.mergeQueryParameters(fake.getQueryParameters(), exchange.getQueryParameters());
+            requestImpl.setQueryParameters(null);
+            exchange.getQueryParameters().clear();
+            exchange.getQueryParameters().putAll(merged);
+            requestImpl.getQueryParameters();
         }
         return newRequestPath;
     }