From 70a98115d828f17d8c997b43347b4ce989130bce Mon Sep 17 00:00:00 2001
From: Jesse Sterr <jsterr@google.com>
Date: Tue, 30 Apr 2024 07:36:03 -0700
Subject: [PATCH] Fixed a potential `NullPointerException` in
 `ImmutableMap.Builder` on 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
---
 .../google/common/collect/ImmutableMapTest.java  | 10 ++++++++++
 .../google/common/collect/ImmutableMapTest.java  | 10 ++++++++++
 .../com/google/common/collect/ImmutableMap.java  | 16 +++++++++++++---
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/android/guava-tests/test/com/google/common/collect/ImmutableMapTest.java b/android/guava-tests/test/com/google/common/collect/ImmutableMapTest.java
index 95f690e96ade..126b01c1fb6a 100644
--- a/android/guava-tests/test/com/google/common/collect/ImmutableMapTest.java
+++ b/android/guava-tests/test/com/google/common/collect/ImmutableMapTest.java
@@ -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 =
diff --git a/guava-tests/test/com/google/common/collect/ImmutableMapTest.java b/guava-tests/test/com/google/common/collect/ImmutableMapTest.java
index 2e6ef0270942..fbf31e793369 100644
--- a/guava-tests/test/com/google/common/collect/ImmutableMapTest.java
+++ b/guava-tests/test/com/google/common/collect/ImmutableMapTest.java
@@ -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 =
diff --git a/guava/src/com/google/common/collect/ImmutableMap.java b/guava/src/com/google/common/collect/ImmutableMap.java
index 91585fec5535..d3b7cd627c50 100644
--- a/guava/src/com/google/common/collect/ImmutableMap.java
+++ b/guava/src/com/google/common/collect/ImmutableMap.java
@@ -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,
@@ -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
@@ -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()];