Skip to content

Commit

Permalink
Fixed a potential NullPointerException in ImmutableMap.Builder on…
Browse files Browse the repository at this point in the history
… a rare code path.

When the `Builder` entries length does not match the `size` and `orderEntriesByValue` is used in combination with `buildKeepingLast` and there are no duplicate keys, then the size used for sorting incorrectly becomes the length of the entries array instead of the size tracked by the `Builder`. This can lead to the `sort` method trying to access null entries from the array.

RELNOTES=`collect`: Fixed a potential `NullPointerException` in `ImmutableMap.Builder` on a rare code path.
PiperOrigin-RevId: 629407343
  • Loading branch information
Jesse Sterr authored and Google Java Core Libraries committed Apr 30, 2024
1 parent e075e89 commit 70a9811
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,16 @@ public void testBuilder_orderEntriesByValue_keepingLast() {
}
}

@GwtIncompatible // we haven't implemented this
public void testBuilder_orderEntriesByValueAfterExactSizeBuild_keepingLastWithoutDuplicates() {
ImmutableMap.Builder<String, Integer> builder =
new Builder<String, Integer>(3)
.orderEntriesByValue(Ordering.natural())
.put("three", 3)
.put("one", 1);
assertMapEquals(builder.buildKeepingLast(), "one", 1, "three", 3);
}

@GwtIncompatible // we haven't implemented this
public void testBuilder_orderEntriesByValue_keepingLast_builderSizeFieldPreserved() {
ImmutableMap.Builder<String, Integer> builder =
Expand Down
10 changes: 10 additions & 0 deletions guava-tests/test/com/google/common/collect/ImmutableMapTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,16 @@ public void testBuilder_orderEntriesByValue_keepingLast() {
}
}

@GwtIncompatible // we haven't implemented this
public void testBuilder_orderEntriesByValueAfterExactSizeBuild_keepingLastWithoutDuplicates() {
ImmutableMap.Builder<String, Integer> builder =
new Builder<String, Integer>(3)
.orderEntriesByValue(Ordering.natural())
.put("three", 3)
.put("one", 1);
assertMapEquals(builder.buildKeepingLast(), "one", 1, "three", 3);
}

@GwtIncompatible // we haven't implemented this
public void testBuilder_orderEntriesByValue_keepingLast_builderSizeFieldPreserved() {
ImmutableMap.Builder<String, Integer> builder =
Expand Down
16 changes: 13 additions & 3 deletions guava/src/com/google/common/collect/ImmutableMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -562,8 +562,11 @@ private ImmutableMap<K, V> build(boolean throwIfDuplicateKeys) {
if (!throwIfDuplicateKeys) {
// We want to retain only the last-put value for any given key, before sorting.
// This could be improved, but orderEntriesByValue is rather rarely used anyway.
nonNullEntries = lastEntryForEachKey(nonNullEntries, size);
localSize = nonNullEntries.length;
Entry<K, V>[] lastEntryForEachKey = lastEntryForEachKey(nonNullEntries, size);
if (lastEntryForEachKey != null) {
nonNullEntries = lastEntryForEachKey;
localSize = lastEntryForEachKey.length;
}
}
Arrays.sort(
nonNullEntries,
Expand Down Expand Up @@ -640,6 +643,13 @@ ImmutableMap<K, V> buildJdkBacked() {
}
}

/**
* Scans the first {@code size} elements of {@code entries} looking for duplicate keys. If
* duplicates are found, a new correctly-sized array is returned with the same elements (up to
* {@code size}), except containing only the last occurrence of each duplicate key. Otherwise
* {@code null} is returned.
*/
@CheckForNull
private static <K, V> Entry<K, V>[] lastEntryForEachKey(Entry<K, V>[] entries, int size) {
Set<K> seen = new HashSet<>();
BitSet dups = new BitSet(); // slots that are overridden by a later duplicate key
Expand All @@ -649,7 +659,7 @@ private static <K, V> Entry<K, V>[] lastEntryForEachKey(Entry<K, V>[] entries, i
}
}
if (dups.isEmpty()) {
return entries;
return null;
}
@SuppressWarnings({"rawtypes", "unchecked"})
Entry<K, V>[] newEntries = new Entry[size - dups.cardinality()];
Expand Down

0 comments on commit 70a9811

Please sign in to comment.