[
https://issues.apache.org/jira/browse/COLLECTIONS-872?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christopher Molin updated COLLECTIONS-872:
------------------------------------------
Description:
When using MultiKeyMap, the keys are stored as-is (as references).
This leads to the following problem:
# Create a MultiKeyMap, with key(s) of a type (class/{-}record{-}) which has
some fields.
# Use multiKeyMap.put(T... keys, V value), to create an entry in the Map, to
map the keys to a value
# Use multiKeyMap.get(T... keys), to verify that the mapping exists and
returns the expected value.
# Modify/alter any of the objects used as a key. It is enough to change the
value of any member field of any of the objects.
# Use multiKeyMap.get(T... keys) again, however, now there is no mapping for
these keys!
# Use multiKeyMap.get(T... keys) with the new modified/altered objects, and it
will return the expected value
This is potentially a major issue - especially for those unaware of this
behavior!
This is a problem since the introduction in v1.17
Potential Solution:
Calculate the Hash of the keys, and use those as keys. This way, would not be
subject to external modification.
Minimal reproducible test:
{code:java}
package io.github.chrimle.example.tests;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import org.apache.commons.collections4.map.MultiKeyMap;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
public class CollectionTests {
public static final String KEY_1 = "key1";
public static final String KEY_2 = "key2";
public static final String KEY_3 = "key3";
private MultiKeyMap<Object, String> multiKeyMap = new MultiKeyMap<>();
public class ExampleObject {
private String field = "originalValue";
public String getField() {
return field;
}
public void setField(String field) {
this.field = field;
}
@Override
public int hashCode() {
return Objects.hashCode(field);
}
@Override
public boolean equals(Object obj) {
if (obj == null) {
return false;
}
if (obj.getClass() != this.getClass()) {
return false;
}
if (this == obj) {
return true;
}
ExampleObject object = (ExampleObject) obj;
if (this.getField() == null) {
return object.getField() == null;
} else {
return this.getField().equals(object.getField());
}
}
}
@Test
public void name() {
// Both objects have the 'field' value set to "originalValue"
ExampleObject originalExampleObject = new ExampleObject();
ExampleObject modifiedExampleObject = new ExampleObject();
// Put original mapping
Assertions.assertNull(multiKeyMap.put(KEY_1, KEY_2, KEY_3,
modifiedExampleObject, "value"));
// Both mappings are correct
Assertions.assertEquals("value", multiKeyMap.get(KEY_1, KEY_2, KEY_3,
modifiedExampleObject));
Assertions.assertEquals("value", multiKeyMap.get(KEY_1, KEY_2, KEY_3,
originalExampleObject));
// Modify 'modifiedExampleObject'
modifiedExampleObject.setField("newValue");
// Modified mapping SHOULD NOT work
Assertions.assertNull(multiKeyMap.get(KEY_1, KEY_2, KEY_3,
modifiedExampleObject));
// Original mapping SHOULD work
Assertions.assertEquals("value", multiKeyMap.get(KEY_1, KEY_2, KEY_3,
originalExampleObject));
}
}
{code}
was:
When using MultiKeyMap, the keys are stored as-is (as references).
This leads to the following problem:
# Create a MultiKeyMap, with key(s) of a type (class/{-}record{-}) which has
some fields.
# Use multiKeyMap.put(T... keys, V value), to create an entry in the Map, to
map the keys to a value
# Use multiKeyMap.get(T... keys), to verify that the mapping exists and
returns the expected value.
# Modify/alter any of the objects used as a key. It is enough to change the
value of any member field of any of the objects.
# Use multiKeyMap.get(T... keys) again, however, now there is no mapping for
these keys!
# Use multiKeyMap.get(T... keys) with the new modified/altered objects, and it
will return the expected value
This is potentially a major issue - especially for those unaware of this
behavior!
This is a problem since the introduction in v1.17
Potential Solution:
Calculate the Hash of the keys, and use those as keys. This way, would not be
subject to external modification.
> MultiKeyMap: Key-objects are stored as reference, making it possible to
> modify the Map externally
> -------------------------------------------------------------------------------------------------
>
> Key: COLLECTIONS-872
> URL: https://issues.apache.org/jira/browse/COLLECTIONS-872
> Project: Commons Collections
> Issue Type: Bug
> Components: Map
> Affects Versions: 2.0, 2.1, 2.1.1, 3.0, 3.1, 3.2, 3.2.1, 3.2.2,
> 4.0-alpha1, 4.0, 4.1, 4.2, 4.3, 4.4, 4.5.0-M1, 4.5.0-M2
> Reporter: Christopher Molin
> Priority: Critical
>
> When using MultiKeyMap, the keys are stored as-is (as references).
> This leads to the following problem:
> # Create a MultiKeyMap, with key(s) of a type (class/{-}record{-}) which has
> some fields.
> # Use multiKeyMap.put(T... keys, V value), to create an entry in the Map, to
> map the keys to a value
> # Use multiKeyMap.get(T... keys), to verify that the mapping exists and
> returns the expected value.
> # Modify/alter any of the objects used as a key. It is enough to change the
> value of any member field of any of the objects.
> # Use multiKeyMap.get(T... keys) again, however, now there is no mapping for
> these keys!
> # Use multiKeyMap.get(T... keys) with the new modified/altered objects, and
> it will return the expected value
> This is potentially a major issue - especially for those unaware of this
> behavior!
>
> This is a problem since the introduction in v1.17
>
> Potential Solution:
> Calculate the Hash of the keys, and use those as keys. This way, would not be
> subject to external modification.
>
> Minimal reproducible test:
> {code:java}
> package io.github.chrimle.example.tests;
> import java.util.HashMap;
> import java.util.Map;
> import java.util.Objects;
> import org.apache.commons.collections4.map.MultiKeyMap;
> import org.junit.jupiter.api.Assertions;
> import org.junit.jupiter.api.Test;
> public class CollectionTests {
> public static final String KEY_1 = "key1";
> public static final String KEY_2 = "key2";
> public static final String KEY_3 = "key3";
> private MultiKeyMap<Object, String> multiKeyMap = new MultiKeyMap<>();
> public class ExampleObject {
> private String field = "originalValue";
> public String getField() {
> return field;
> }
> public void setField(String field) {
> this.field = field;
> }
> @Override
> public int hashCode() {
> return Objects.hashCode(field);
> }
> @Override
> public boolean equals(Object obj) {
> if (obj == null) {
> return false;
> }
> if (obj.getClass() != this.getClass()) {
> return false;
> }
> if (this == obj) {
> return true;
> }
> ExampleObject object = (ExampleObject) obj;
> if (this.getField() == null) {
> return object.getField() == null;
> } else {
> return this.getField().equals(object.getField());
> }
> }
> }
> @Test
> public void name() {
> // Both objects have the 'field' value set to "originalValue"
> ExampleObject originalExampleObject = new ExampleObject();
> ExampleObject modifiedExampleObject = new ExampleObject();
> // Put original mapping
> Assertions.assertNull(multiKeyMap.put(KEY_1, KEY_2, KEY_3,
> modifiedExampleObject, "value"));
> // Both mappings are correct
> Assertions.assertEquals("value", multiKeyMap.get(KEY_1, KEY_2, KEY_3,
> modifiedExampleObject));
> Assertions.assertEquals("value", multiKeyMap.get(KEY_1, KEY_2, KEY_3,
> originalExampleObject));
> // Modify 'modifiedExampleObject'
> modifiedExampleObject.setField("newValue");
> // Modified mapping SHOULD NOT work
> Assertions.assertNull(multiKeyMap.get(KEY_1, KEY_2, KEY_3,
> modifiedExampleObject));
> // Original mapping SHOULD work
> Assertions.assertEquals("value", multiKeyMap.get(KEY_1, KEY_2, KEY_3,
> originalExampleObject));
> }
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
