Skip to content

Commit f3f9d92

Browse files
jdrueckertskaldarnar
jdrueckert
and
skaldarnar
authored
fix: do not crash on unexpected map format in GenericMapTypeHandler (#5062)
Co-authored-by: Tobias Nett <[email protected]>
1 parent ceff6a9 commit f3f9d92

File tree

2 files changed

+175
-15
lines changed

2 files changed

+175
-15
lines changed

‎subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandler.java‎

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
package org.terasology.persistence.typeHandling.coreTypes;
55

6+
import com.google.common.base.MoreObjects;
67
import com.google.common.base.Preconditions;
78
import com.google.common.collect.Maps;
89
import org.slf4j.Logger;
@@ -61,28 +62,53 @@ private PersistedData serializeEntry(Map.Entry<K, V> entry, PersistedDataSeriali
6162

6263
@Override
6364
public Optional<Map<K, V>> deserialize(PersistedData data) {
64-
if (!data.isArray()) {
65+
if (!data.isArray() || data.isValueMap()) {
66+
logger.warn("Incorrect map format detected: object instead of array.\n" + getUsageInfo(data));
6567
return Optional.empty();
6668
}
6769

6870
Map<K, V> result = Maps.newLinkedHashMap();
6971

7072
for (PersistedData entry : data.getAsArray()) {
7173
PersistedDataMap kvEntry = entry.getAsValueMap();
72-
final Optional<K> key = keyHandler.deserialize(kvEntry.get(KEY));
73-
74-
if (key.isPresent()) {
75-
final Optional<V> value = valueHandler.deserialize(kvEntry.get(VALUE));
76-
if (value.isPresent()) {
77-
result.put(key.get(), value.get());
78-
} else {
79-
logger.warn("Missing value for key '{}' with {} given entry '{}'", key.get(), valueHandler, kvEntry.get(VALUE));
80-
}
81-
} else {
82-
logger.warn("Missing field '{}' for entry '{}'", KEY, kvEntry);
74+
PersistedData rawKey = kvEntry.get(KEY);
75+
PersistedData rawValue = kvEntry.get(VALUE);
76+
if (rawKey == null || rawValue == null) {
77+
logger.warn("Incorrect map format detected: missing map entry with \"key\" or \"value\" key.\n" + getUsageInfo(data));
78+
return Optional.empty();
8379
}
80+
81+
final Optional<K> key = keyHandler.deserialize(rawKey);
82+
if (key.isEmpty()) {
83+
logger.warn("Could not deserialize key '{}' as '{}'", rawKey, keyHandler.getClass().getSimpleName());
84+
return Optional.empty();
85+
}
86+
87+
final Optional<V> value = valueHandler.deserialize(kvEntry.get(VALUE));
88+
if (value.isEmpty()) {
89+
logger.warn("Could not deserialize value '{}' as '{}'", rawValue, valueHandler.getClass().getSimpleName());
90+
return Optional.empty();
91+
}
92+
93+
result.put(key.get(), value.get());
8494
}
8595

8696
return Optional.of(result);
8797
}
98+
99+
private String getUsageInfo(PersistedData data) {
100+
return "Expected\n" +
101+
" \"mapName\": [\n" +
102+
" { \"key\": \"...\", \"value\": \"...\" }\n" +
103+
" ]\n" +
104+
"but found \n'{}'" + data + "'";
105+
}
106+
107+
@Override
108+
public String toString() {
109+
return MoreObjects.toStringHelper(this)
110+
.add("key", keyHandler)
111+
.add("value", valueHandler)
112+
.toString();
113+
}
88114
}

‎subsystems/TypeHandlerLibrary/src/test/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandlerTest.java‎

Lines changed: 137 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
package org.terasology.persistence.typeHandling.coreTypes;
55

6+
import org.junit.jupiter.api.DisplayName;
67
import org.junit.jupiter.api.Test;
78
import org.terasology.persistence.typeHandling.PersistedData;
89
import org.terasology.persistence.typeHandling.PersistedDataSerializer;
@@ -12,7 +13,6 @@
1213
import org.terasology.persistence.typeHandling.inMemory.PersistedString;
1314
import org.terasology.persistence.typeHandling.inMemory.arrays.PersistedValueArray;
1415

15-
import java.util.Collections;
1616
import java.util.List;
1717
import java.util.Map;
1818
import java.util.Optional;
@@ -25,14 +25,102 @@ class GenericMapTypeHandlerTest {
2525
private static final String TEST_KEY = "health:baseRegen";
2626
private static final long TEST_VALUE = -1;
2727

28+
/**
29+
* JSON equivalent:
30+
* <pre><code>
31+
* [
32+
* {
33+
* "key": "health:baseRegen",
34+
* "value": -1
35+
* }
36+
* ]
37+
* </code></pre>
38+
*/
2839
private final PersistedData testData = new PersistedValueArray(List.of(
2940
new PersistedMap(Map.of(
3041
GenericMapTypeHandler.KEY, new PersistedString(TEST_KEY),
3142
GenericMapTypeHandler.VALUE, new PersistedLong(TEST_VALUE)
3243
))
3344
));
3445

46+
/**
47+
* JSON equivalent:
48+
* <pre><code>
49+
* {
50+
* "health:baseRegen": -1
51+
* }
52+
* </code></pre>
53+
*/
54+
private final PersistedData testDataMalformatted = new PersistedValueArray(List.of(
55+
new PersistedMap(Map.of(
56+
TEST_KEY, new PersistedLong(TEST_VALUE)
57+
))
58+
));
59+
60+
/**
61+
* JSON equivalent:
62+
* <pre><code>
63+
* [
64+
* {
65+
* "not key": "health:baseRegen",
66+
* "value": -1
67+
* }
68+
* ]
69+
* </code></pre>
70+
*/
71+
private final PersistedData testDataMissingKeyEntry = new PersistedValueArray(List.of(
72+
new PersistedMap(Map.of(
73+
"not key", new PersistedString(TEST_KEY),
74+
GenericMapTypeHandler.VALUE, new PersistedLong(TEST_VALUE)
75+
))
76+
));
77+
78+
/**
79+
* JSON equivalent:
80+
* <pre><code>
81+
* [
82+
* {
83+
* "key": "health:baseRegen",
84+
* "not value": -1
85+
* }
86+
* ]
87+
* </code></pre>
88+
*/
89+
private final PersistedData testDataMissingValueEntry = new PersistedValueArray(List.of(
90+
new PersistedMap(Map.of(
91+
GenericMapTypeHandler.KEY, new PersistedString(TEST_KEY),
92+
"not value", new PersistedLong(TEST_VALUE)
93+
))
94+
));
95+
96+
/**
97+
* JSON equivalent:
98+
* <pre><code>
99+
* [
100+
* {
101+
* "key": "health:baseRegen",
102+
* "value": -1
103+
* },
104+
* {
105+
* "not key": "health:baseRegen",
106+
* "not value": -1
107+
* },
108+
* ]
109+
* </code></pre>
110+
*/
111+
private final PersistedData testDataValidAndInvalidMix = new PersistedValueArray(List.of(
112+
new PersistedMap(Map.of(
113+
GenericMapTypeHandler.KEY, new PersistedString(TEST_KEY),
114+
GenericMapTypeHandler.VALUE, new PersistedLong(TEST_VALUE)
115+
)),
116+
new PersistedMap(Map.of(
117+
"not key", new PersistedString(TEST_KEY),
118+
"not value", new PersistedLong(TEST_VALUE)
119+
))
120+
));
121+
35122
@Test
123+
@DisplayName("Data with valid formatting can be deserialized successfully.")
36124
void testDeserialize() {
37125
var th = new GenericMapTypeHandler<>(
38126
new StringTypeHandler(),
@@ -44,23 +132,69 @@ void testDeserialize() {
44132
}
45133

46134
@Test
135+
@DisplayName("Deserializing valid data with a mismatching value type handler fails deserialization (returns empty `Optional`)")
47136
void testDeserializeWithMismatchedValueHandler() {
48137
var th = new GenericMapTypeHandler<>(
49138
new StringTypeHandler(),
50139
new UselessTypeHandler<>()
51140
);
52141

53-
assertThat(th.deserialize(testData)).hasValue(Collections.emptyMap());
142+
assertThat(th.deserialize(testData)).isEmpty();
54143
}
55144

56145
@Test
146+
@DisplayName("Deserializing valid data with a mismatching key type handler fails deserialization (returns empty `Optional`)")
57147
void testDeserializeWithMismatchedKeyHandler() {
58148
var th = new GenericMapTypeHandler<>(
59149
new UselessTypeHandler<>(),
60150
new LongTypeHandler()
61151
);
62152

63-
assertThat(th.deserialize(testData)).hasValue(Collections.emptyMap());
153+
assertThat(th.deserialize(testData)).isEmpty();
154+
}
155+
156+
@Test
157+
@DisplayName("Incorrectly formatted data (without an outer array) fails deserialization (returns empty `Optional`)")
158+
void testDeserializeWithObjectInsteadOfArray() {
159+
var th = new GenericMapTypeHandler<>(
160+
new StringTypeHandler(),
161+
new LongTypeHandler()
162+
);
163+
164+
assertThat(th.deserialize(testDataMalformatted)).isEmpty();
165+
}
166+
167+
@Test
168+
@DisplayName("Incorrectly formatted data (without a map entry with key \"key\") fails deserialization (returns empty `Optional`)")
169+
void testDeserializeWithMissingKeyEntry() {
170+
var th = new GenericMapTypeHandler<>(
171+
new StringTypeHandler(),
172+
new LongTypeHandler()
173+
);
174+
175+
assertThat(th.deserialize(testDataMissingKeyEntry)).isEmpty();
176+
}
177+
178+
@Test
179+
@DisplayName("Incorrectly formatted data (without a map entry with key \"value\") fails deserialization (returns empty `Optional`)")
180+
void testDeserializeWithMissingValueEntry() {
181+
var th = new GenericMapTypeHandler<>(
182+
new StringTypeHandler(),
183+
new LongTypeHandler()
184+
);
185+
186+
assertThat(th.deserialize(testDataMissingValueEntry)).isEmpty();
187+
}
188+
189+
@Test
190+
@DisplayName("A map containing both, correctly and incorrectly formatted data, fails deserialization (returns empty `Optional`)")
191+
void testDeserializeWithValidAndInvalidEntries() {
192+
var th = new GenericMapTypeHandler<>(
193+
new StringTypeHandler(),
194+
new LongTypeHandler()
195+
);
196+
197+
assertThat(th.deserialize(testDataValidAndInvalidMix)).isEmpty();
64198
}
65199

66200
/** Never returns a value. */

0 commit comments

Comments
 (0)