From 1b4955e49f06a791a1b5f3f7333ab631cab5a293 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=A4=80=ED=98=95?= <158379358+junhyeong9812@users.noreply.github.com> Date: Wed, 30 Sep 2026 19:47:30 +0900 Subject: [PATCH] Avoid phantom keys in LinkedCaseInsensitiveMap.computeIfAbsent computeIfAbsent registered the case-insensitive key before invoking the mapping function. If the function returned null or threw an exception, no mapping was recorded in the target map, but the key registration was left behind. The map then reported containsKey(key) as true while size() was 0, keySet() was empty, and get(key) returned null. A later insertion with a different casing also reused the stale casing of the failed call, since the existing-key branch resolved to the registered key. The case-insensitive key is now only looked up up front. For a new key, it is registered from within the mapping function once a non-null value has been computed, which is still before the entry is inserted. A null result or an exception therefore leaves both maps untouched, while a removeEldestEntry override that evicts the new entry right away still removes the registration, as it does for put. The existing-key path keeps computing under the stored casing. Closes gh-37351 Signed-off-by: junhyeong9812 --- .../util/LinkedCaseInsensitiveMap.java | 15 ++++-- .../util/LinkedCaseInsensitiveMapTests.java | 46 +++++++++++++++++++ 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/LinkedCaseInsensitiveMap.java b/spring-core/src/main/java/org/springframework/util/LinkedCaseInsensitiveMap.java index b221ca9a265..0a2da8c73f0 100644 --- a/spring-core/src/main/java/org/springframework/util/LinkedCaseInsensitiveMap.java +++ b/spring-core/src/main/java/org/springframework/util/LinkedCaseInsensitiveMap.java @@ -222,17 +222,22 @@ public class LinkedCaseInsensitiveMap implements Map @Override public @Nullable V computeIfAbsent(String key, Function mappingFunction) { - String oldKey = this.caseInsensitiveKeys.putIfAbsent(convertKey(key), key); + String convertedKey = convertKey(key); + String oldKey = this.caseInsensitiveKeys.get(convertedKey); if (oldKey != null) { V oldKeyValue = this.targetMap.get(oldKey); if (oldKeyValue != null) { return oldKeyValue; } - else { - key = oldKey; - } + return this.targetMap.computeIfAbsent(oldKey, mappingFunction); } - return this.targetMap.computeIfAbsent(key, mappingFunction); + return this.targetMap.computeIfAbsent(key, k -> { + V value = mappingFunction.apply(k); + if (value != null) { + this.caseInsensitiveKeys.putIfAbsent(convertedKey, k); + } + return value; + }); } @Override diff --git a/spring-core/src/test/java/org/springframework/util/LinkedCaseInsensitiveMapTests.java b/spring-core/src/test/java/org/springframework/util/LinkedCaseInsensitiveMapTests.java index 43840c6168d..6b7e1a86f04 100644 --- a/spring-core/src/test/java/org/springframework/util/LinkedCaseInsensitiveMapTests.java +++ b/spring-core/src/test/java/org/springframework/util/LinkedCaseInsensitiveMapTests.java @@ -22,6 +22,7 @@ import java.util.Map; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** * Tests for {@link LinkedCaseInsensitiveMap}. @@ -106,6 +107,7 @@ class LinkedCaseInsensitiveMapTests { assertThat(map.put("null", null)).isEqualTo("value"); assertThat(map.computeIfAbsent("NULL", s -> "value")).isEqualTo("value"); assertThat(map.get("null")).isEqualTo("value"); + assertThat(map.keySet()).containsExactly("Key", "null"); } @Test @@ -115,6 +117,50 @@ class LinkedCaseInsensitiveMapTests { assertThat(map.computeIfAbsent("Key", key -> "value3")).isEqualTo("value1"); } + @Test + void computeIfAbsentWithNullComputedValue() { + assertThat(map.computeIfAbsent("Key", key -> null)).isNull(); + assertThat(map).isEmpty(); + assertThat(map.containsKey("key")).isFalse(); + } + + @Test + void computeIfAbsentAfterNullComputedValueUsesGivenKey() { + assertThat(map.computeIfAbsent("Key", key -> null)).isNull(); + assertThat(map.computeIfAbsent("KEY", key -> "value")).isEqualTo("value"); + assertThat(map.keySet()).containsExactly("KEY"); + } + + @Test + void computeIfAbsentWithFailingMappingFunction() { + assertThatIllegalStateException().isThrownBy(() -> + map.computeIfAbsent("Key", key -> { throw new IllegalStateException(); })); + assertThat(map).isEmpty(); + assertThat(map.containsKey("key")).isFalse(); + } + + @Test + void computeIfAbsentWithNullComputedValueForExistingNullValue() { + assertThat(map.put("Key", null)).isNull(); + assertThat(map.computeIfAbsent("KEY", key -> null)).isNull(); + assertThat(map).hasSize(1); + assertThat(map.containsKey("key")).isTrue(); + assertThat(map.keySet()).containsExactly("Key"); + } + + @Test + void computeIfAbsentWithImmediatelyEvictedEntry() { + LinkedCaseInsensitiveMap evictingMap = new LinkedCaseInsensitiveMap<>() { + @Override + protected boolean removeEldestEntry(Map.Entry eldest) { + return true; + } + }; + assertThat(evictingMap.computeIfAbsent("Key", key -> "value")).isEqualTo("value"); + assertThat(evictingMap).isEmpty(); + assertThat(evictingMap.containsKey("key")).isFalse(); + } + @Test void mapClone() { assertThat(map.put("key", "value1")).isNull();