Skip to content

Commit 9a67076

Browse files
baranowbbaranowb
committed
[UNDERTOW-2269] Encode query string on forward/include and improve merge of parameters
1 parent 4577d88 commit 9a67076

4 files changed

Lines changed: 68 additions & 51 deletions

File tree

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,4 +655,7 @@ public interface UndertowMessages {
655655
@Message(id = 210, value = "Buffer content underflow for exchange '%s', buffer '%s'")
656656
IOException bufferUnderflow(HttpServerExchange exchange, ByteBuffer buf);
657657

658+
@Message(id = 211, value = "Failed to encode query string '%s' with '%s' encoding.")
659+
IllegalArgumentException failedToEncodeQueryString(String q, String e);
660+
658661
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ public final class HttpServerExchange extends AbstractAttachable {
208208
private String resolvedPath = "";
209209

210210
/**
211-
* the query string
211+
* the query string - percent encoded
212212
*/
213213
private String queryString = "";
214214
/**

‎core/src/main/java/io/undertow/util/QueryParameterUtils.java‎

Lines changed: 44 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,15 @@
2020

2121
import java.io.UnsupportedEncodingException;
2222
import java.net.URLDecoder;
23+
import java.net.URLEncoder;
2324
import java.nio.charset.StandardCharsets;
2425
import java.util.ArrayDeque;
2526
import java.util.Deque;
2627
import java.util.LinkedHashMap;
2728
import java.util.Map;
2829
import org.xnio.OptionMap;
2930

31+
import io.undertow.UndertowMessages;
3032
import io.undertow.UndertowOptions;
3133
import io.undertow.server.HttpServerExchange;
3234

@@ -42,33 +44,42 @@ private QueryParameterUtils() {
4244
}
4345

4446
public static String buildQueryString(final Map<String, Deque<String>> params) {
45-
StringBuilder sb = new StringBuilder();
46-
boolean first = true;
47-
for (Map.Entry<String, Deque<String>> entry : params.entrySet()) {
48-
if (entry.getValue().isEmpty()) {
49-
if (first) {
50-
first = false;
51-
} else {
52-
sb.append('&');
53-
}
54-
sb.append(entry.getKey());
55-
sb.append('=');
56-
} else {
57-
for (String val : entry.getValue()) {
47+
return QueryParameterUtils.buildQueryString(params, null);
48+
}
49+
50+
public static String buildQueryString(final Map<String, Deque<String>> params, final String encoding) {
51+
try {
52+
StringBuilder sb = new StringBuilder();
53+
boolean first = true;
54+
for (Map.Entry<String, Deque<String>> entry : params.entrySet()) {
55+
final String key = encoding != null ? URLEncoder.encode(entry.getKey(), encoding) : entry.getKey();
56+
if (entry.getValue().isEmpty()) {
5857
if (first) {
5958
first = false;
6059
} else {
6160
sb.append('&');
6261
}
63-
sb.append(entry.getKey());
62+
sb.append(key);
6463
sb.append('=');
65-
sb.append(val);
64+
} else {
65+
for (String val : entry.getValue()) {
66+
if (first) {
67+
first = false;
68+
} else {
69+
sb.append('&');
70+
}
71+
final String _val = encoding != null ? URLEncoder.encode(val, encoding) : val;
72+
sb.append(key);
73+
sb.append('=');
74+
sb.append(_val);
75+
}
6676
}
6777
}
78+
return sb.toString();
79+
} catch (UnsupportedEncodingException e) {
80+
throw UndertowMessages.MESSAGES.failedToEncodeQueryString(buildQueryString(params), encoding);
6881
}
69-
return sb.toString();
7082
}
71-
7283
/**
7384
* Parses a query string into a map
7485
* @param newQueryString The query string
@@ -146,8 +157,9 @@ public static Map<String, Deque<String>> mergeQueryParametersWithNewQueryString(
146157
return mergeQueryParametersWithNewQueryString(queryParameters, newQueryString, StandardCharsets.UTF_8.name());
147158
}
148159

160+
@Deprecated
149161
public static Map<String, Deque<String>> mergeQueryParametersWithNewQueryString(final Map<String, Deque<String>> queryParameters, final String newQueryString, final String encoding) {
150-
162+
//DEPRACETED this will create duplicates
151163
Map<String, Deque<String>> newQueryParameters = parseQueryString(newQueryString, encoding);
152164
//according to the spec the new query parameters have to 'take precedence'
153165
for (Map.Entry<String, Deque<String>> entry : queryParameters.entrySet()) {
@@ -160,6 +172,20 @@ public static Map<String, Deque<String>> mergeQueryParametersWithNewQueryString(
160172
return newQueryParameters;
161173
}
162174

175+
public static Map<String, Deque<String>> mergeQueryParameters(final Map<String, Deque<String>> newParams, final Map<String, Deque<String>> oldParams) {
176+
//according to the spec the new query parameters have to 'take precedence'
177+
for (Map.Entry<String, Deque<String>> entry : oldParams.entrySet()) {
178+
if (!newParams.containsKey(entry.getKey())) {
179+
newParams.put(entry.getKey(), new ArrayDeque<>(entry.getValue()));
180+
} else {
181+
final Deque<String> newValues = newParams.get(entry.getKey());
182+
final Deque<String> oldValues = entry.getValue();
183+
oldValues.stream().filter(v -> !newValues.contains(v)).forEach(v-> newValues.add(v));
184+
}
185+
}
186+
return newParams;
187+
}
188+
163189
public static String getQueryParamEncoding(HttpServerExchange exchange) {
164190
String encoding = null;
165191
OptionMap undertowOptions = exchange.getConnection().getUndertowOptions();

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

Lines changed: 20 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,6 @@
1717
*/
1818
package io.undertow.servlet.util;
1919

20-
import io.undertow.server.Connectors;
21-
import io.undertow.server.HttpServerExchange;
22-
import io.undertow.servlet.handlers.ServletPathMatch;
23-
import io.undertow.servlet.handlers.ServletRequestContext;
24-
import io.undertow.servlet.spec.HttpServletRequestImpl;
25-
import io.undertow.servlet.spec.HttpServletResponseImpl;
26-
import io.undertow.servlet.spec.ServletContextImpl;
27-
import io.undertow.util.BadRequestException;
28-
import io.undertow.util.ParameterLimitException;
29-
import jakarta.servlet.ServletException;
30-
import java.util.Deque;
31-
import java.util.Map;
32-
3320
import static jakarta.servlet.AsyncContext.ASYNC_CONTEXT_PATH;
3421
import static jakarta.servlet.AsyncContext.ASYNC_MAPPING;
3522
import static jakarta.servlet.AsyncContext.ASYNC_PATH_INFO;
@@ -55,6 +42,21 @@
5542
import static jakarta.servlet.RequestDispatcher.INCLUDE_REQUEST_URI;
5643
import static jakarta.servlet.RequestDispatcher.INCLUDE_SERVLET_PATH;
5744

45+
import java.util.Deque;
46+
import java.util.Map;
47+
48+
import io.undertow.server.Connectors;
49+
import io.undertow.server.HttpServerExchange;
50+
import io.undertow.servlet.handlers.ServletPathMatch;
51+
import io.undertow.servlet.handlers.ServletRequestContext;
52+
import io.undertow.servlet.spec.HttpServletRequestImpl;
53+
import io.undertow.servlet.spec.HttpServletResponseImpl;
54+
import io.undertow.servlet.spec.ServletContextImpl;
55+
import io.undertow.util.BadRequestException;
56+
import io.undertow.util.ParameterLimitException;
57+
import io.undertow.util.QueryParameterUtils;
58+
import jakarta.servlet.ServletException;
59+
5860
/**
5961
* <p>Utility class to manage the dispatching parsing of the path. The methods
6062
* fill the exchange, request and response with the needed data for the
@@ -215,24 +217,6 @@ public static ServletPathMatch dispatchAsync(final String path,
215217
return pathMatch;
216218
}
217219

218-
private static Map<String, Deque<String>> mergeQueryParameters(final Map<String, Deque<String>> newParams, final Map<String, Deque<String>> oldParams) {
219-
for (Map.Entry<String, Deque<String>> entry : oldParams.entrySet()) {
220-
Deque<String> values = newParams.get(entry.getKey());
221-
if (values == null) {
222-
// add all the values as new params do not contain this key
223-
newParams.put(entry.getKey(), entry.getValue());
224-
} else {
225-
// merge values new params first
226-
for (String v : entry.getValue()) {
227-
if (!values.contains(v)) {
228-
values.add(v);
229-
}
230-
}
231-
}
232-
}
233-
return newParams;
234-
}
235-
236220
private static String assignRequestPath(final String path, final HttpServletRequestImpl requestImpl,
237221
final ServletContextImpl servletContext, final boolean include) throws ParameterLimitException, BadRequestException {
238222
final StringBuilder sb = new StringBuilder();
@@ -258,7 +242,11 @@ private static String assignRequestPath(final String path, final HttpServletRequ
258242
}
259243
// both forward and include merge parameters by spec
260244
if (!fake.getQueryString().isEmpty()) {
261-
requestImpl.setQueryParameters(mergeQueryParameters(fake.getQueryParameters(), requestImpl.getQueryParameters()));
245+
final Map<String, Deque<String>> merged = QueryParameterUtils.mergeQueryParameters(fake.getQueryParameters(), exchange.getQueryParameters());
246+
requestImpl.setQueryParameters(null);
247+
exchange.getQueryParameters().clear();
248+
exchange.getQueryParameters().putAll(merged);
249+
requestImpl.getQueryParameters();
262250
}
263251
return newRequestPath;
264252
}

0 commit comments

Comments
 (0)