Skip to content

Commit

Permalink
Maintain order of query params
Browse files Browse the repository at this point in the history
  • Loading branch information
@jguerra
jguerra committed Dec 16, 2024
1 parent b9497fa commit 2865b45
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 31 deletions.
55 changes: 24 additions & 31 deletions zuul-core/src/main/java/com/netflix/zuul/message/http/HttpQueryParams.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,10 +39,10 @@
public class HttpQueryParams implements Cloneable {
private final ListMultimap<String, String> delegate;
private final boolean immutable;
private final HashMap<String, Boolean> trailingEquals;
private final Map<String, Boolean> trailingEquals;

public HttpQueryParams() {
delegate = ArrayListMultimap.create();
delegate = LinkedListMultimap.create();
immutable = false;
trailingEquals = new HashMap<>();
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -106,10 +106,8 @@ else if (s.length() > 0) {
*/
public String getFirst(String name) {
List<String> values = delegate.get(name);
if (values != null) {
if (values.size() > 0) {
return values.get(0);
}
if (!values.isEmpty()) {
return values.get(0);
}
return null;
}
Expand All @@ -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));
Expand Down Expand Up @@ -164,25 +162,20 @@ public Set<String> keySet() {

public String toEncodedString() {
StringBuilder sb = new StringBuilder();
try {
for (Map.Entry<String, String> 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<String, String> 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();
}
Expand All @@ -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();
Expand Down
24 changes: 24 additions & 0 deletions zuul-core/src/test/java/com/netflix/zuul/message/http/HttpQueryParamsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -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());
}
}

0 comments on commit 2865b45

Please sign in to comment.