Merge pull request #807 from apache/feature/http-params-case

[WW-5370] Makes HttpParameters case-insensitive

core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java

@@ -29,20 +29,22 @@ import java.util.TreeMap;
 import java.util.TreeSet;
 
 @SuppressWarnings("unchecked")
-public class HttpParameters implements Map<String, Parameter>, Cloneable {
+public class HttpParameters implements Map<String, Parameter> {
 
     final private Map<String, Parameter> parameters;
 
     private HttpParameters(Map<String, Parameter> parameters) {
-        this.parameters = parameters;
+        this.parameters = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+        this.parameters.putAll(parameters);
     }
 
+    @SuppressWarnings("rawtypes")
     public static Builder create(Map requestParameterMap) {
         return new Builder(requestParameterMap);
     }
 
     public static Builder create() {
-        return new Builder(new HashMap<>());
+        return new Builder(new TreeMap<>(String.CASE_INSENSITIVE_ORDER));
     }
 
     public HttpParameters remove(Set<String> paramsToRemove) {
@@ -64,7 +66,10 @@ public class HttpParameters implements Map<String, Parameter>, Cloneable {
 
     /**
      * Access to this method can be potentially dangerous as it allows access to raw parameter values.
+     *
+     * @deprecated since 6.4.0, it will be removed with a new major release
      */
+    @Deprecated
     private Map<String, String[]> toMap() {
         final Map<String, String[]> result = new HashMap<>(parameters.size());
         for (Map.Entry<String, Parameter> entry : parameters.entrySet()) {
@@ -73,6 +78,12 @@ public class HttpParameters implements Map<String, Parameter>, Cloneable {
         return result;
     }
 
+    /**
+     * Appends all the parameters by overriding any existing params in a case-insensitive manner
+     *
+     * @param newParams A new params to append
+     * @return a current instance of {@link HttpParameters}
+     */
     public HttpParameters appendAll(Map<String, Parameter> newParams) {
         parameters.putAll(newParams);
         return this;
@@ -100,11 +111,11 @@ public class HttpParameters implements Map<String, Parameter>, Cloneable {
 
     @Override
     public Parameter get(Object key) {
-        if (parameters.containsKey(key)) {
-            return parameters.get(key);
-        } else {
-            return new Parameter.Empty(String.valueOf(key));
+        if (key == null) {
+            return new Parameter.Empty("null");
         }
+        Parameter val = parameters.get(key.toString());
+        return val != null ? val : new Parameter.Empty(key.toString());
     }
 
     @Override
@@ -177,8 +188,8 @@ public class HttpParameters implements Map<String, Parameter>, Cloneable {
 
         public HttpParameters build() {
             Map<String, Parameter> parameters = (parent == null)
-                    ? new HashMap<>()
-                    : new HashMap<>(parent.parameters);
+                ? new HashMap<>()
+                : new HashMap<>(parent.parameters);
 
             for (Map.Entry<String, Object> entry : requestParameterMap.entrySet()) {
                 String name = entry.getKey();
@@ -197,8 +208,9 @@ public class HttpParameters implements Map<String, Parameter>, Cloneable {
         * Alternate Builder method which avoids wrapping any parameters that are already
         * a {@link Parameter} element within another {@link Parameter} wrapper.
         *
-        * @return 
+        * @deprecated since 6.4.0, use {@link #build()} instead
          */
+        @Deprecated
         public HttpParameters buildNoNestedWrapping() {
             Map<String, Parameter> parameters = (parent == null)
                     ? new HashMap<>()

core/src/main/java/org/apache/struts2/dispatcher/Parameter.java

@@ -166,6 +166,19 @@ public interface Parameter {
                     "name='" + name + '\'' +
                     '}';
         }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (!(o instanceof Empty)) return false;
+            Empty empty = (Empty) o;
+            return Objects.equals(name, empty.name);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(name);
+        }
     }
 
 }

core/src/test/java/org/apache/struts2/dispatcher/HttpParametersTest.java

@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.dispatcher;
+
+import org.junit.Test;
+
+import java.util.HashMap;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class HttpParametersTest {
+
+    @Test
+    public void shouldGetBeCaseInsensitive() {
+        // given
+        HttpParameters params = HttpParameters.create(new HashMap<String, Object>() {{
+            put("param1", "value1");
+        }}).build();
+
+        // then
+        assertEquals("value1", params.get("Param1").getValue());
+        assertEquals("value1", params.get("paraM1").getValue());
+        assertEquals("value1", params.get("pAraM1").getValue());
+    }
+
+    @Test
+    public void shouldRemoveBeCaseInsensitive() {
+        // given
+        HttpParameters params = HttpParameters.create(new HashMap<String, Object>() {{
+            put("param1", "value1");
+        }}).build();
+
+        // then
+        assertFalse(params.remove("Param1").contains("param1"));
+        assertEquals(new Parameter.Empty("param1"), params.get("param1"));
+    }
+
+    @Test
+    public void shouldAppendSameParamsIgnoringCase() {
+        // given
+        HttpParameters params = HttpParameters.create(new HashMap<String, Object>() {{
+            put("param1", "value1");
+        }}).build();
+
+        // when
+        assertEquals("value1", params.get("param1").getValue());
+
+        params = params.appendAll(HttpParameters.create(new HashMap<String, String>() {{
+            put("Param1", "Value1");
+        }}).build());
+
+        // then
+        assertTrue(params.contains("param1"));
+        assertTrue(params.contains("Param1"));
+
+        assertEquals("Value1", params.get("param1").getValue());
+        assertEquals("Value1", params.get("Param1").getValue());
+    }
+
+}