kusalk commented on code in PR #807:
URL: https://github.com/apache/struts/pull/807#discussion_r1423587234
##########
core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java:
##########
@@ -29,25 +29,27 @@
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) {
for (String paramName : paramsToRemove) {
- parameters.remove(paramName);
+ parameters.entrySet().removeIf(p ->
p.getKey().equalsIgnoreCase(paramName));
Review Comment:
Do we still need this custom comparison?
##########
core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java:
##########
@@ -73,7 +78,14 @@ private Map<String, String[]> 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<String, Parameter> newParams) {
+ remove(newParams.keySet());
Review Comment:
Probably don't need this anymore either right?
##########
core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java:
##########
@@ -100,7 +112,7 @@ public boolean containsValue(Object value) {
@Override
public Parameter get(Object key) {
- if (parameters.containsKey(key)) {
+ if (key != null && contains(String.valueOf(key))) {
Review Comment:
Not sure this part makes sense, checking for the existence of String#valueOf
then retrieving using the object?
Do you mean something like this:
```
@Override
public Parameter get(Object key) {
if (key == null) {
return new Parameter.Empty("null");
}
Parameter val = parameters.get(key.toString());
return val != null ? val : new Parameter.Empty(key.toString());
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
