From 76188263491df21fecfab1c3ea2b118c84e224b8 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 17 Oct 2013 12:37:01 +0200 Subject: [PATCH] 419687 - HttpClient's query parameters must be case sensitive. Modified class Fields to take a boolean parameter that defines whether it is case sensitive or not, and updated HttpRequest to use a case sensitive Fields instance for the query parameters. --- .../org/eclipse/jetty/client/HttpRequest.java | 2 +- .../jetty/client/HttpClientURITest.java | 26 +++++++ .../java/org/eclipse/jetty/util/Fields.java | 72 ++++++++++++++----- 3 files changed, 82 insertions(+), 18 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java index 2e4cbec1c5c..3ea2e0e6c22 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java @@ -54,7 +54,7 @@ public class HttpRequest implements Request private static final AtomicLong ids = new AtomicLong(); private final HttpFields headers = new HttpFields(); - private final Fields params = new Fields(); + private final Fields params = new Fields(true); private final Map attributes = new HashMap<>(); private final List requestListeners = new ArrayList<>(); private final List responseListeners = new ArrayList<>(); diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java index 610f891cfd7..3c88ea14495 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java @@ -312,4 +312,30 @@ public class HttpClientURITest extends AbstractHttpClientServerTest Assert.assertEquals(HttpStatus.OK_200, response.getStatus()); } + + @Test + public void testCaseSensitiveParameterName() throws Exception + { + final String name1 = "a"; + final String name2 = "A"; + start(new AbstractHandler() + { + @Override + public void handle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + Assert.assertEquals(name1, request.getParameter(name1)); + Assert.assertEquals(name2, request.getParameter(name2)); + } + }); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scheme) + .path("/path?" + name1 + "=" + name1) + .param(name2, name2) + .timeout(5, TimeUnit.SECONDS) + .send(); + + Assert.assertEquals(HttpStatus.OK_200, response.getStatus()); + } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Fields.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Fields.java index 7436dfcb7d3..65815579b48 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/Fields.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Fields.java @@ -29,20 +29,33 @@ import java.util.Set; /** *

A container for name/value pairs, known as fields.

- *

A {@link Field} is composed of a case-insensitive name string and + *

A {@link Field} is composed of a name string that can be case-sensitive + * or case-insensitive (by specifying the option at the constructor) and * of a case-sensitive set of value strings.

*

The implementation of this class is not thread safe.

*/ public class Fields implements Iterable { + private final boolean caseSensitive; private final Map fields; /** - *

Creates an empty modifiable {@link Fields} instance.

+ *

Creates an empty, modifiable, case insensitive {@link Fields} instance.

* @see #Fields(Fields, boolean) */ public Fields() { + this(false); + } + + /** + *

Creates an empty, modifiable, case insensitive {@link Fields} instance.

+ * @param caseSensitive whether this {@link Fields} instance must be case sensitive + * @see #Fields(Fields, boolean) + */ + public Fields(boolean caseSensitive) + { + this.caseSensitive = caseSensitive; fields = new LinkedHashMap<>(); } @@ -55,6 +68,7 @@ public class Fields implements Iterable */ public Fields(Fields original, boolean immutable) { + this.caseSensitive = original.caseSensitive; Map copy = new LinkedHashMap<>(); copy.putAll(original.fields); fields = immutable ? Collections.unmodifiableMap(copy) : copy; @@ -68,7 +82,18 @@ public class Fields implements Iterable if (obj == null || getClass() != obj.getClass()) return false; Fields that = (Fields)obj; - return fields.equals(that.fields); + if (size() != that.size()) + return false; + if (caseSensitive != that.caseSensitive) + return false; + for (Map.Entry entry : fields.entrySet()) + { + String name = entry.getKey(); + Field value = entry.getValue(); + if (!value.equals(that.get(name), caseSensitive)) + return false; + } + return true; } @Override @@ -88,13 +113,18 @@ public class Fields implements Iterable return result; } + private String normalizeName(String name) + { + return caseSensitive ? name : name.toLowerCase(Locale.ENGLISH); + } + /** * @param name the field name * @return the {@link Field} with the given name, or null if no such field exists */ public Field get(String name) { - return fields.get(name.trim().toLowerCase(Locale.ENGLISH)); + return fields.get(normalizeName(name)); } /** @@ -105,10 +135,9 @@ public class Fields implements Iterable */ public void put(String name, String value) { - name = name.trim(); // Preserve the case for the field name Field field = new Field(name, value); - fields.put(name.toLowerCase(Locale.ENGLISH), field); + fields.put(normalizeName(name), field); } /** @@ -119,7 +148,7 @@ public class Fields implements Iterable public void put(Field field) { if (field != null) - fields.put(field.name().toLowerCase(Locale.ENGLISH), field); + fields.put(normalizeName(field.name()), field); } /** @@ -131,17 +160,18 @@ public class Fields implements Iterable */ public void add(String name, String value) { - name = name.trim(); - Field field = fields.get(name.toLowerCase(Locale.ENGLISH)); + String key = normalizeName(name); + Field field = fields.get(key); if (field == null) { + // Preserve the case for the field name field = new Field(name, value); - fields.put(name.toLowerCase(Locale.ENGLISH), field); + fields.put(key, field); } else { field = new Field(field.name(), field.values(), value); - fields.put(name.toLowerCase(Locale.ENGLISH), field); + fields.put(key, field); } } @@ -153,8 +183,7 @@ public class Fields implements Iterable */ public Field remove(String name) { - name = name.trim(); - return fields.remove(name.toLowerCase(Locale.ENGLISH)); + return fields.remove(normalizeName(name)); } /** @@ -219,6 +248,17 @@ public class Fields implements Iterable System.arraycopy(moreValues, 0, this.values, values.length, moreValues.length); } + public boolean equals(Field that, boolean caseSensitive) + { + if (this == that) + return true; + if (that == null) + return false; + if (caseSensitive) + return equals(that); + return name.equalsIgnoreCase(that.name) && Arrays.equals(values, that.values); + } + @Override public boolean equals(Object obj) { @@ -227,15 +267,13 @@ public class Fields implements Iterable if (obj == null || getClass() != obj.getClass()) return false; Field that = (Field)obj; - // Field names must be lowercase, thus we lowercase them before transmission, but keep them as is - // internally. That's why we've to compare them case insensitive. - return name.equalsIgnoreCase(that.name) && Arrays.equals(values, that.values); + return name.equals(that.name) && Arrays.equals(values, that.values); } @Override public int hashCode() { - int result = name.toLowerCase(Locale.ENGLISH).hashCode(); + int result = name.hashCode(); result = 31 * result + Arrays.hashCode(values); return result; }