diff --git a/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpQueryParams.java b/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpQueryParams.java index dba2492d30..2f3888c671 100644 --- a/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpQueryParams.java +++ b/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpQueryParams.java @@ -16,13 +16,13 @@ package com.netflix.zuul.message.http; import com.google.common.base.Strings; -import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.Iterables; +import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.ListMultimap; -import java.io.UnsupportedEncodingException; import java.net.URLDecoder; import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.util.Collection; import java.util.HashMap; import java.util.List; @@ -39,10 +39,10 @@ public class HttpQueryParams implements Cloneable { private final ListMultimap delegate; private final boolean immutable; - private final HashMap trailingEquals; + private final Map trailingEquals; public HttpQueryParams() { - delegate = ArrayListMultimap.create(); + delegate = LinkedListMultimap.create(); immutable = false; trailingEquals = new HashMap<>(); } @@ -70,8 +70,8 @@ public static HttpQueryParams parse(String queryString) { String value = s.substring(i + 1); try { - name = URLDecoder.decode(name, "UTF-8"); - value = URLDecoder.decode(value, "UTF-8"); + name = URLDecoder.decode(name, StandardCharsets.UTF_8); + value = URLDecoder.decode(value, StandardCharsets.UTF_8); } catch (Exception e) { // do nothing } @@ -84,11 +84,11 @@ public static HttpQueryParams parse(String queryString) { } } // key only - else if (s.length() > 0) { + else if (!s.isEmpty()) { String name = s; try { - name = URLDecoder.decode(name, "UTF-8"); + name = URLDecoder.decode(name, StandardCharsets.UTF_8); } catch (Exception e) { // do nothing } @@ -106,10 +106,8 @@ else if (s.length() > 0) { */ public String getFirst(String name) { List values = delegate.get(name); - if (values != null) { - if (values.size() > 0) { - return values.get(0); - } + if (!values.isEmpty()) { + return values.get(0); } return null; } @@ -124,7 +122,7 @@ public boolean contains(String name) { /** * Per https://tools.ietf.org/html/rfc7230#page-19, query params are to be treated as case sensitive. - * However, as an utility, this exists to allow us to do a case insensitive match on demand. + * However, as a utility, this exists to allow us to do a case insensitive match on demand. */ public boolean containsIgnoreCase(String name) { return delegate.containsKey(name) || delegate.containsKey(name.toLowerCase(Locale.ROOT)); @@ -164,25 +162,20 @@ public Set keySet() { public String toEncodedString() { StringBuilder sb = new StringBuilder(); - try { - for (Map.Entry entry : entries()) { - sb.append(URLEncoder.encode(entry.getKey(), "UTF-8")); - if (!Strings.isNullOrEmpty(entry.getValue())) { - sb.append('='); - sb.append(URLEncoder.encode(entry.getValue(), "UTF-8")); - } else if (isTrailingEquals(entry.getKey())) { - sb.append('='); - } - sb.append('&'); + for (Map.Entry entry : entries()) { + sb.append(URLEncoder.encode(entry.getKey(), StandardCharsets.UTF_8)); + if (!Strings.isNullOrEmpty(entry.getValue())) { + sb.append('='); + sb.append(URLEncoder.encode(entry.getValue(), StandardCharsets.UTF_8)); + } else if (isTrailingEquals(entry.getKey())) { + sb.append('='); } + sb.append('&'); + } - // Remove trailing '&'. - if (sb.length() > 0 && '&' == sb.charAt(sb.length() - 1)) { - sb.deleteCharAt(sb.length() - 1); - } - } catch (UnsupportedEncodingException e) { - // Won't happen. - e.printStackTrace(); + // Remove trailing '&'. + if (!sb.isEmpty() && '&' == sb.charAt(sb.length() - 1)) { + sb.deleteCharAt(sb.length() - 1); } return sb.toString(); } @@ -200,7 +193,7 @@ public String toString() { } // Remove trailing '&'. - if (sb.length() > 0 && '&' == sb.charAt(sb.length() - 1)) { + if (!sb.isEmpty() && '&' == sb.charAt(sb.length() - 1)) { sb.deleteCharAt(sb.length() - 1); } return sb.toString(); diff --git a/zuul-core/src/test/java/com/netflix/zuul/message/http/HttpQueryParamsTest.java b/zuul-core/src/test/java/com/netflix/zuul/message/http/HttpQueryParamsTest.java index d0dce57f84..94c40515b4 100644 --- a/zuul-core/src/test/java/com/netflix/zuul/message/http/HttpQueryParamsTest.java +++ b/zuul-core/src/test/java/com/netflix/zuul/message/http/HttpQueryParamsTest.java @@ -19,7 +19,10 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.List; import java.util.Locale; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; @@ -37,6 +40,17 @@ void testMultiples() { assertEquals("k1=v1&k1=v2&k2=v3", qp.toEncodedString()); } + @Test + void testDuplicateKv() { + HttpQueryParams qp = new HttpQueryParams(); + qp.add("k1", "v1"); + qp.add("k1", "v1"); + qp.add("k1", "v1"); + + assertEquals("k1=v1&k1=v1&k1=v1", qp.toEncodedString()); + assertEquals(List.of("v1", "v1", "v1"), qp.get("k1")); + } + @Test void testToEncodedString() { HttpQueryParams qp = new HttpQueryParams(); @@ -145,4 +159,14 @@ void parseKeysIgnoreCase() { assertTrue(queryParams.containsIgnoreCase(camelCaseKey)); } + + @Test + void maintainsOrderOnToString() { + String queryString = + IntStream.range(0, 100).mapToObj(i -> "k%d=v%d".formatted(i, i)).collect(Collectors.joining("&")); + HttpQueryParams queryParams = HttpQueryParams.parse(queryString); + assertEquals(queryString, queryParams.toEncodedString()); + assertEquals(queryString, queryParams.toString()); + assertEquals(queryString, queryParams.immutableCopy().toString()); + } }