[UNDERTOW-2269] Handle query string properly after UNDERTOW-2206

core/src/main/java/io/undertow/UndertowMessages.java

@@ -658,4 +658,7 @@ public interface UndertowMessages {
     @Message(id = 211, value = "Exchange '%s' already has body or is blocking.")
     IOException exchangeBlockingOrBlocking(HttpServerExchange e);
 
+    @Message(id = 212, value = "Failed to encode query string '%s' with '%s' encoding.")
+    IllegalArgumentException failedToEncodeQueryString(String q, String e);
+
 }

core/src/main/java/io/undertow/server/HttpServerExchange.java

@@ -208,7 +208,7 @@ public final class HttpServerExchange extends AbstractAttachable {
     private String resolvedPath = "";
 
     /**
-     * the query string
+     * the query string - percent encoded
      */
     private String queryString = "";
     /**

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
@@ -141,13 +152,14 @@ private static String decodeParam(String newQueryString, int startPos, int equal
         return key;
     }
 
-    @Deprecated
+    @Deprecated (forRemoval = true)
     public static Map<String, Deque<String>> mergeQueryParametersWithNewQueryString(final Map<String, Deque<String>> queryParameters, final String newQueryString) {
         return mergeQueryParametersWithNewQueryString(queryParameters, newQueryString, StandardCharsets.UTF_8.name());
     }
 
+    @Deprecated (forRemoval = true)
     public static Map<String, Deque<String>> mergeQueryParametersWithNewQueryString(final Map<String, Deque<String>> queryParameters, final String newQueryString, final String encoding) {
-
+        //DEPRECATED 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();

servlet/src/main/java/io/undertow/servlet/util/DispatchUtils.java

@@ -17,19 +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.BadRequestException;
-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;
@@ -57,6 +44,21 @@
 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.BadRequestException;
+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
@@ -221,24 +223,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, BadRequestException {
         final StringBuilder sb = new StringBuilder();
@@ -264,7 +248,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;
     }