From 13d972d6eeaf998f7199f0d39446aff72fd67423 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sun, 10 Dec 2023 13:14:55 +0100 Subject: [PATCH 1/4] WW-5370 Makes HttpParameters case-insensitive --- .../struts2/dispatcher/HttpParameters.java | 31 ++++++--- .../dispatcher/HttpParametersTest.java | 65 +++++++++++++++++++ 2 files changed, 88 insertions(+), 8 deletions(-) create mode 100644 core/src/test/java/org/apache/struts2/dispatcher/HttpParametersTest.java diff --git a/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java b/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java index b0ab784abf..f35d475837 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java @@ -29,7 +29,7 @@ import java.util.TreeSet; @SuppressWarnings("unchecked") -public class HttpParameters implements Map, Cloneable { +public class HttpParameters implements Map { final private Map parameters; @@ -37,6 +37,7 @@ private HttpParameters(Map parameters) { this.parameters = parameters; } + @SuppressWarnings("rawtypes") public static Builder create(Map requestParameterMap) { return new Builder(requestParameterMap); } @@ -47,7 +48,7 @@ public static Builder create() { public HttpParameters remove(Set paramsToRemove) { for (String paramName : paramsToRemove) { - parameters.remove(paramName); + parameters.entrySet().removeIf(p -> p.getKey().equalsIgnoreCase(paramName)); } return this; } @@ -59,12 +60,15 @@ public HttpParameters remove(final String paramToRemove) { } public boolean contains(String name) { - return parameters.containsKey(name); + return parameters.keySet().stream().anyMatch(p -> p.equalsIgnoreCase(name)); } /** * 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 toMap() { final Map result = new HashMap<>(parameters.size()); for (Map.Entry entry : parameters.entrySet()) { @@ -73,7 +77,14 @@ private Map toMap() { 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 newParams) { + remove(newParams.keySet()); parameters.putAll(newParams); return this; } @@ -100,8 +111,11 @@ public boolean containsValue(Object value) { @Override public Parameter get(Object key) { - if (parameters.containsKey(key)) { - return parameters.get(key); + if (key != null && contains(String.valueOf(key))) { + return parameters.entrySet().stream() + .filter(p -> p.getKey().equalsIgnoreCase(String.valueOf(key))) + .findFirst().map(Entry::getValue) + .orElse(new Parameter.Empty(String.valueOf(key))); } else { return new Parameter.Empty(String.valueOf(key)); } @@ -177,8 +191,8 @@ public Builder withComparator(Comparator orderedComparator) { public HttpParameters build() { Map parameters = (parent == null) - ? new HashMap<>() - : new HashMap<>(parent.parameters); + ? new HashMap<>() + : new HashMap<>(parent.parameters); for (Map.Entry entry : requestParameterMap.entrySet()) { String name = entry.getKey(); @@ -197,8 +211,9 @@ public HttpParameters build() { * 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 parameters = (parent == null) ? new HashMap<>() diff --git a/core/src/test/java/org/apache/struts2/dispatcher/HttpParametersTest.java b/core/src/test/java/org/apache/struts2/dispatcher/HttpParametersTest.java new file mode 100644 index 0000000000..7c2efbc125 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/dispatcher/HttpParametersTest.java @@ -0,0 +1,65 @@ +/* + * 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.assertTrue; + +public class HttpParametersTest { + + @Test + public void shouldGetBeCaseInsensitive() { + // given + HttpParameters params = HttpParameters.create(new HashMap() {{ + 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 shouldAppendSameParamsIgnoringCase() { + // given + HttpParameters params = HttpParameters.create(new HashMap() {{ + put("param1", "value1"); + }}).build(); + + // when + assertEquals("value1", params.get("param1").getValue()); + + params = params.appendAll(HttpParameters.create(new HashMap() {{ + 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()); + } + +} \ No newline at end of file From c40978f8cd29d510f6b9d214db141dc6becc7342 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Tue, 12 Dec 2023 08:52:57 +0100 Subject: [PATCH 2/4] WW-5370 Uses TreeMap with case-insensitive comparator --- .../apache/struts2/dispatcher/HttpParameters.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java b/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java index f35d475837..e44f062739 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java @@ -34,7 +34,8 @@ public class HttpParameters implements Map { final private Map parameters; private HttpParameters(Map parameters) { - this.parameters = parameters; + this.parameters = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + this.parameters.putAll(parameters); } @SuppressWarnings("rawtypes") @@ -43,7 +44,7 @@ public static Builder create(Map requestParameterMap) { } public static Builder create() { - return new Builder(new HashMap<>()); + return new Builder(new TreeMap<>(String.CASE_INSENSITIVE_ORDER)); } public HttpParameters remove(Set paramsToRemove) { @@ -60,7 +61,7 @@ public HttpParameters remove(final String paramToRemove) { } public boolean contains(String name) { - return parameters.keySet().stream().anyMatch(p -> p.equalsIgnoreCase(name)); + return parameters.containsKey(name); } /** @@ -112,10 +113,7 @@ public boolean containsValue(Object value) { @Override public Parameter get(Object key) { if (key != null && contains(String.valueOf(key))) { - return parameters.entrySet().stream() - .filter(p -> p.getKey().equalsIgnoreCase(String.valueOf(key))) - .findFirst().map(Entry::getValue) - .orElse(new Parameter.Empty(String.valueOf(key))); + return parameters.get(key); } else { return new Parameter.Empty(String.valueOf(key)); } From 4eaab8a79d3674f4e519ca726f96fd7576180799 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Tue, 12 Dec 2023 09:46:38 +0100 Subject: [PATCH 3/4] WW-5370 Simplifies code --- .../apache/struts2/dispatcher/HttpParameters.java | 9 ++++----- .../struts2/dispatcher/HttpParametersTest.java | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java b/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java index e44f062739..a2d4675e00 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java @@ -49,7 +49,7 @@ public static Builder create() { public HttpParameters remove(Set paramsToRemove) { for (String paramName : paramsToRemove) { - parameters.entrySet().removeIf(p -> p.getKey().equalsIgnoreCase(paramName)); + parameters.remove(paramName); } return this; } @@ -85,7 +85,6 @@ private Map toMap() { * @return a current instance of {@link HttpParameters} */ public HttpParameters appendAll(Map newParams) { - remove(newParams.keySet()); parameters.putAll(newParams); return this; } @@ -112,10 +111,10 @@ public boolean containsValue(Object value) { @Override public Parameter get(Object key) { - if (key != null && contains(String.valueOf(key))) { - return parameters.get(key); + if (key != null) { + return parameters.get(String.valueOf(key)); } else { - return new Parameter.Empty(String.valueOf(key)); + return null; } } diff --git a/core/src/test/java/org/apache/struts2/dispatcher/HttpParametersTest.java b/core/src/test/java/org/apache/struts2/dispatcher/HttpParametersTest.java index 7c2efbc125..b8b0a3038a 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/HttpParametersTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/HttpParametersTest.java @@ -23,6 +23,8 @@ import java.util.HashMap; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; public class HttpParametersTest { @@ -40,6 +42,18 @@ public void shouldGetBeCaseInsensitive() { assertEquals("value1", params.get("pAraM1").getValue()); } + @Test + public void shouldRemoveBeCaseInsensitive() { + // given + HttpParameters params = HttpParameters.create(new HashMap() {{ + put("param1", "value1"); + }}).build(); + + // then + assertFalse(params.remove("Param1").contains("param1")); + assertNull(params.get("param1")); + } + @Test public void shouldAppendSameParamsIgnoringCase() { // given From 102e040e092916163640deddf1114a3e28fcb429 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Tue, 12 Dec 2023 10:00:31 +0100 Subject: [PATCH 4/4] WW-5370 Adds proper logic to handle null --- .../apache/struts2/dispatcher/HttpParameters.java | 8 ++++---- .../org/apache/struts2/dispatcher/Parameter.java | 13 +++++++++++++ .../struts2/dispatcher/HttpParametersTest.java | 3 +-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java b/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java index a2d4675e00..54f465eea1 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java @@ -111,11 +111,11 @@ public boolean containsValue(Object value) { @Override public Parameter get(Object key) { - if (key != null) { - return parameters.get(String.valueOf(key)); - } else { - return null; + if (key == null) { + return new Parameter.Empty("null"); } + Parameter val = parameters.get(key.toString()); + return val != null ? val : new Parameter.Empty(key.toString()); } @Override diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Parameter.java b/core/src/main/java/org/apache/struts2/dispatcher/Parameter.java index b8a2270830..edae6c3a17 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Parameter.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Parameter.java @@ -166,6 +166,19 @@ public String toString() { "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); + } } } diff --git a/core/src/test/java/org/apache/struts2/dispatcher/HttpParametersTest.java b/core/src/test/java/org/apache/struts2/dispatcher/HttpParametersTest.java index b8b0a3038a..aea4a7b007 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/HttpParametersTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/HttpParametersTest.java @@ -24,7 +24,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; public class HttpParametersTest { @@ -51,7 +50,7 @@ public void shouldRemoveBeCaseInsensitive() { // then assertFalse(params.remove("Param1").contains("param1")); - assertNull(params.get("param1")); + assertEquals(new Parameter.Empty("param1"), params.get("param1")); } @Test