Author: markt
Date: Mon Mar 6 15:28:47 2017
New Revision: 1785667
URL: http://svn.apache.org/viewvc?rev=1785667&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60808
Ensure that the Map returned by ServletRequest.getParameterMap() is fully
immutable.
Based on a patch provided by woosan.
Added:
tomcat/trunk/test/org/apache/catalina/util/TestParameterMap.java
Modified:
tomcat/trunk/java/org/apache/catalina/util/ParameterMap.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/catalina/util/ParameterMap.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/ParameterMap.java?rev=1785667&r1=1785666&r2=1785667&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/util/ParameterMap.java (original)
+++ tomcat/trunk/java/org/apache/catalina/util/ParameterMap.java Mon Mar 6
15:28:47 2017
@@ -16,13 +16,17 @@
*/
package org.apache.catalina.util;
+import java.io.Serializable;
+import java.util.Collection;
+import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;
+import java.util.Set;
import org.apache.tomcat.util.res.StringManager;
/**
- * Extended implementation of <strong>HashMap</strong> that includes a
+ * Implementation of <strong>java.util.Map</strong> that includes a
* <code>locked</code> property. This class can be used to safely expose
* Catalina internal parameter map objects to user classes without having
* to clone them in order to avoid modifications. When first created, a
@@ -33,20 +37,22 @@ import org.apache.tomcat.util.res.String
*
* @author Craig R. McClanahan
*/
-public final class ParameterMap<K,V> extends LinkedHashMap<K,V> {
+public final class ParameterMap<K,V> implements Map<K,V>, Serializable {
- private static final long serialVersionUID = 1L;
+ private static final long serialVersionUID = 2L;
+
+ private final Map<K,V> delegatedMap;
+
+ private final Map<K,V> unmodifiableDelegatedMap;
- // ----------------------------------------------------------- Constructors
/**
* Construct a new, empty map with the default initial capacity and
* load factor.
*/
public ParameterMap() {
-
- super();
-
+ delegatedMap = new LinkedHashMap<>();
+ unmodifiableDelegatedMap = Collections.unmodifiableMap(delegatedMap);
}
@@ -57,9 +63,8 @@ public final class ParameterMap<K,V> ext
* @param initialCapacity The initial capacity of this map
*/
public ParameterMap(int initialCapacity) {
-
- super(initialCapacity);
-
+ delegatedMap = new LinkedHashMap<>(initialCapacity);
+ unmodifiableDelegatedMap = Collections.unmodifiableMap(delegatedMap);
}
@@ -71,9 +76,8 @@ public final class ParameterMap<K,V> ext
* @param loadFactor The load factor of this map
*/
public ParameterMap(int initialCapacity, float loadFactor) {
-
- super(initialCapacity, loadFactor);
-
+ delegatedMap = new LinkedHashMap<>(initialCapacity, loadFactor);
+ unmodifiableDelegatedMap = Collections.unmodifiableMap(delegatedMap);
}
@@ -83,15 +87,11 @@ public final class ParameterMap<K,V> ext
* @param map Map whose contents are duplicated in the new map
*/
public ParameterMap(Map<K,V> map) {
-
- super(map);
-
+ delegatedMap = new LinkedHashMap<>(map);
+ unmodifiableDelegatedMap = Collections.unmodifiableMap(delegatedMap);
}
- // ------------------------------------------------------------- Properties
-
-
/**
* The current lock state of this parameter map.
*/
@@ -102,9 +102,7 @@ public final class ParameterMap<K,V> ext
* @return the locked state of this parameter map.
*/
public boolean isLocked() {
-
- return (this.locked);
-
+ return locked;
}
@@ -114,102 +112,145 @@ public final class ParameterMap<K,V> ext
* @param locked The new locked state
*/
public void setLocked(boolean locked) {
-
this.locked = locked;
-
}
/**
* The string manager for this package.
*/
- private static final StringManager sm =
- StringManager.getManager("org.apache.catalina.util");
-
-
- // --------------------------------------------------------- Public Methods
-
+ private static final StringManager sm =
StringManager.getManager("org.apache.catalina.util");
/**
- * Remove all mappings from this map.
+ * {@inheritDoc}
*
* @exception IllegalStateException if this map is currently locked
*/
@Override
public void clear() {
-
- if (locked)
- throw new IllegalStateException
- (sm.getString("parameterMap.locked"));
- super.clear();
-
+ checkLocked();
+ delegatedMap.clear();
}
/**
- * Associate the specified value with the specified key in this map. If
- * the map previously contained a mapping for this key, the old value is
- * replaced.
- *
- * @param key Key with which the specified value is to be associated
- * @param value Value to be associated with the specified key
- *
- * @return The previous value associated with the specified key, or
- * <code>null</code> if there was no mapping for key
+ * {@inheritDoc}
*
* @exception IllegalStateException if this map is currently locked
*/
@Override
public V put(K key, V value) {
-
- if (locked)
- throw new IllegalStateException
- (sm.getString("parameterMap.locked"));
- return (super.put(key, value));
-
+ checkLocked();
+ return delegatedMap.put(key, value);
}
/**
- * Copy all of the mappings from the specified map to this one. These
- * mappings replace any mappings that this map had for any of the keys
- * currently in the specified Map.
- *
- * @param map Mappings to be stored into this map
+ * {@inheritDoc}
*
* @exception IllegalStateException if this map is currently locked
*/
@Override
public void putAll(Map<? extends K,? extends V> map) {
-
- if (locked)
- throw new IllegalStateException
- (sm.getString("parameterMap.locked"));
- super.putAll(map);
-
+ checkLocked();
+ delegatedMap.putAll(map);
}
/**
- * Remove the mapping for this key from the map if present.
- *
- * @param key Key whose mapping is to be removed from the map
- *
- * @return The previous value associated with the specified key, or
- * <code>null</code> if there was no mapping for that key
+ * {@inheritDoc}
*
* @exception IllegalStateException if this map is currently locked
*/
@Override
public V remove(Object key) {
+ checkLocked();
+ return delegatedMap.remove(key);
+ }
+
+
+ private void checkLocked() {
+ if (locked) {
+ throw new
IllegalStateException(sm.getString("parameterMap.locked"));
+ }
+ }
+
+
+ @Override
+ public int size() {
+ return delegatedMap.size();
+ }
+
+
+ @Override
+ public boolean isEmpty() {
+ return delegatedMap.isEmpty();
+ }
+
+
+ @Override
+ public boolean containsKey(Object key) {
+ return delegatedMap.containsKey(key);
+ }
- if (locked)
- throw new IllegalStateException
- (sm.getString("parameterMap.locked"));
- return (super.remove(key));
+ @Override
+ public boolean containsValue(Object value) {
+ return delegatedMap.containsValue(value);
}
+ @Override
+ public V get(Object key) {
+ return delegatedMap.get(key);
+ }
+
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * Returns an <strong>unmodifiable</strong> {@link Set} view of the keys
+ * contained in this map if it is locked.
+ */
+ @Override
+ public Set<K> keySet() {
+ if (locked) {
+ return unmodifiableDelegatedMap.keySet();
+ }
+
+ return delegatedMap.keySet();
+ }
+
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * Returns an <strong>unmodifiable</strong> {@link Collection} view of the
+ * values contained in this map if it is locked.
+ */
+ @Override
+ public Collection<V> values() {
+ if (locked) {
+ return unmodifiableDelegatedMap.values();
+ }
+
+ return delegatedMap.values();
+ }
+
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * Returns an <strong>unmodifiable</strong> {@link Set} view of the
mappings
+ * contained in this map if it is locked.
+ */
+ @Override
+ public Set<java.util.Map.Entry<K, V>> entrySet() {
+ if (locked) {
+ return unmodifiableDelegatedMap.entrySet();
+ }
+
+ return delegatedMap.entrySet();
+ }
}
Added: tomcat/trunk/test/org/apache/catalina/util/TestParameterMap.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/util/TestParameterMap.java?rev=1785667&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/util/TestParameterMap.java (added)
+++ tomcat/trunk/test/org/apache/catalina/util/TestParameterMap.java Mon Mar 6
15:28:47 2017
@@ -0,0 +1,310 @@
+/*
+ * 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.catalina.util;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestParameterMap {
+
+ private static final String[] TEST_PARAM_VALUES_1 = { "value1" };
+ private static final String[] TEST_PARAM_VALUES_2 = { "value2" };
+ private static final String[] TEST_PARAM_VALUES_2_UPDATED = {
"value2-updated" };
+ private static final String[] TEST_PARAM_VALUES_3 = { "value3" };
+ private static final String[] TEST_PARAM_VALUES_REPLACED = { "replaced" };
+
+ private Map<String, String[]> paramMap;
+
+ @Before
+ public void setUp() {
+ paramMap = new ParameterMap<>();
+
+ paramMap.put("param1", TEST_PARAM_VALUES_1);
+ paramMap.put("param2", TEST_PARAM_VALUES_2);
+ paramMap.put("param3", TEST_PARAM_VALUES_3);
+
+ Assert.assertTrue(paramMap.containsKey("param1"));
+ Assert.assertArrayEquals(TEST_PARAM_VALUES_1, paramMap.get("param1"));
+ Assert.assertTrue(paramMap.containsKey("param2"));
+ Assert.assertArrayEquals(TEST_PARAM_VALUES_2, paramMap.get("param2"));
+ Assert.assertTrue(paramMap.containsKey("param3"));
+ Assert.assertArrayEquals(TEST_PARAM_VALUES_3, paramMap.get("param3"));
+
+ final Set<String> keySet = paramMap.keySet();
+ Assert.assertTrue(keySet.contains("param1"));
+ Assert.assertTrue(keySet.contains("param2"));
+ Assert.assertTrue(keySet.contains("param3"));
+
+ paramMap.put("param2", TEST_PARAM_VALUES_2_UPDATED);
+ paramMap.remove("param3");
+
+ Assert.assertTrue(paramMap.containsKey("param1"));
+ Assert.assertArrayEquals(TEST_PARAM_VALUES_1, paramMap.get("param1"));
+ Assert.assertTrue(paramMap.containsKey("param2"));
+ Assert.assertArrayEquals(TEST_PARAM_VALUES_2_UPDATED,
paramMap.get("param2"));
+ Assert.assertFalse(paramMap.containsKey("param3"));
+ Assert.assertNull(paramMap.get("param3"));
+
+ Assert.assertTrue(keySet.contains("param1"));
+ Assert.assertTrue(keySet.contains("param2"));
+ Assert.assertFalse(keySet.contains("param3"));
+ }
+
+ @After
+ public void tearDown() {
+ Assert.assertTrue(paramMap.containsKey("param1"));
+ Assert.assertArrayEquals(TEST_PARAM_VALUES_1, paramMap.get("param1"));
+ Assert.assertTrue(paramMap.containsKey("param2"));
+ Assert.assertArrayEquals(TEST_PARAM_VALUES_2_UPDATED,
paramMap.get("param2"));
+ Assert.assertFalse(paramMap.containsKey("param3"));
+ Assert.assertNull(paramMap.get("param3"));
+ }
+
+ @Test
+ public void testMapImmutabilityAfterLocked() {
+ ((ParameterMap<String, String[]>) paramMap).setLocked(true);
+
+ try {
+ String[] updatedParamValues22 = new String[] { "value2-updated-2"
};
+ paramMap.put("param2", updatedParamValues22);
+ Assert.fail("ParameterMap is not locked.");
+ } catch (IllegalStateException expectedException) {
+ }
+
+ try {
+ String[] updatedParamValues22 = new String[] { "value2-updated-2"
};
+ paramMap.putIfAbsent("param22", updatedParamValues22);
+ Assert.fail("ParameterMap is not locked.");
+ } catch (IllegalStateException expectedException) {
+ }
+
+ try {
+ final Map<String, String[]> additionalParams = new HashMap<>();
+ additionalParams.put("param4", new String[] { "value4" });
+ paramMap.putAll(additionalParams);
+ Assert.fail("ParameterMap is not locked.");
+ } catch (IllegalStateException expectedException) {
+ }
+
+ try {
+ paramMap.merge("param2", new String[] { "value2-merged" }, (a, b)
-> (b));
+ Assert.fail("ParameterMap is not locked.");
+ } catch (IllegalStateException expectedException) {
+ }
+
+ try {
+ paramMap.remove("param2");
+ Assert.fail("ParameterMap is not locked.");
+ } catch (IllegalStateException expectedException) {
+ }
+
+ try {
+ paramMap.remove("param2", TEST_PARAM_VALUES_2_UPDATED);
+ Assert.fail("ParameterMap is not locked.");
+ } catch (IllegalStateException expectedException) {
+ }
+
+ try {
+ paramMap.replace("param2", new String[] { "value2-replaced" });
+ Assert.fail("ParameterMap is not locked.");
+ } catch (IllegalStateException expectedException) {
+ }
+
+ try {
+ paramMap.replace("param2", TEST_PARAM_VALUES_2_UPDATED, new
String[] { "value2-replaced" });
+ Assert.fail("ParameterMap is not locked.");
+ } catch (IllegalStateException expectedException) {
+ }
+
+ try {
+ paramMap.replaceAll((a, b) -> TEST_PARAM_VALUES_REPLACED);
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ paramMap.clear();
+ Assert.fail("ParameterMap is not locked.");
+ } catch (IllegalStateException expectedException) {
+ }
+ }
+
+ @Test
+ public void testKeySetImmutabilityAfterLocked() {
+ ((ParameterMap<String, String[]>) paramMap).setLocked(true);
+
+ final Set<String> keySet = paramMap.keySet();
+
+ try {
+ keySet.add("param4");
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ keySet.remove("param2");
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ keySet.removeIf((a) -> "param2".equals(a));
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ keySet.removeAll(Arrays.asList("param1", "param2"));
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ keySet.retainAll(Collections.emptyList());
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ keySet.clear();
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+ }
+
+ @Test
+ public void testValuesImmutabilityAfterLocked() {
+ ((ParameterMap<String, String[]>) paramMap).setLocked(true);
+
+ final Collection<String[]> valuesCol = paramMap.values();
+
+ try {
+ valuesCol.add(new String[] { "value4" });
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ List<String[]> list = new ArrayList<>();
+ list.add(new String[] { "value4" });
+ valuesCol.addAll(list);
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ valuesCol.remove(TEST_PARAM_VALUES_1);
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ valuesCol.removeIf((a) -> true);
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ List<String[]> list = new ArrayList<>();
+ list.add(TEST_PARAM_VALUES_1);
+ valuesCol.removeAll(list);
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ valuesCol.retainAll(Collections.emptyList());
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ valuesCol.clear();
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+ }
+
+ @Test
+ public void testEntrySetImmutabilityAfterLocked() {
+ ((ParameterMap<String, String[]>) paramMap).setLocked(true);
+
+ final Set<Map.Entry<String, String[]>> entrySet = paramMap.entrySet();
+
+ try {
+ final Map<String, String[]> anotherParamsMap = new HashMap<>();
+ anotherParamsMap.put("param4", new String[] { "value4" });
+ Map.Entry<String, String[]> anotherEntry =
anotherParamsMap.entrySet().iterator().next();
+ entrySet.add(anotherEntry);
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ final Map<String, String[]> anotherParamsMap = new HashMap<>();
+ anotherParamsMap.put("param4", new String[] { "value4" });
+ anotherParamsMap.put("param5", new String[] { "value5" });
+ entrySet.addAll(anotherParamsMap.entrySet());
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ final Map.Entry<String, String[]> entry =
entrySet.iterator().next();
+ entrySet.remove(entry);
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ entrySet.removeIf((a) -> true);
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ Set<Map.Entry<String, String[]>> anotherEntrySet = new
HashSet<>(entrySet);
+ entrySet.removeAll(anotherEntrySet);
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ entrySet.retainAll(Collections.emptySet());
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+
+ try {
+ entrySet.clear();
+ Assert.fail("ParameterMap is not locked.");
+ } catch (UnsupportedOperationException expectedException) {
+ }
+ }
+}
\ No newline at end of file
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1785667&r1=1785666&r2=1785667&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Mar 6 15:28:47 2017
@@ -157,6 +157,11 @@
that meant multiple attempts to read the same entry from a JAR in
succession would fail for the second and subsequent attempts. (markt)
</fix>
+ <fix>
+ <bug>60808</bug>: Ensure that the <code>Map</code> returned by
+ <code>ServletRequest.getParameterMap()</code> is fully immutable. Based
+ on a patch provided by woosan. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
