Sync extras bundle comparison can throw NPE am: c0f39c1ece am: 34b700a7c2 am: a0d20db02f

am: 6d13650c74

* commit '6d13650c74b7cab047e43a4a3a0b880a832dd2db':
  Sync extras bundle comparison can throw NPE

core/java/android/content/PeriodicSync.java

@@ -21,6 +21,8 @@ import android.os.Bundle;
 import android.os.Parcel;
 import android.accounts.Account;
 
+import java.util.Objects;
+
 /**
  * Value type that contains information about a periodic sync.
  */
@@ -144,7 +146,9 @@ public class PeriodicSync implements Parcelable {
             if (!b2.containsKey(key)) {
                 return false;
             }
-            if (!b1.get(key).equals(b2.get(key))) {
+            // Null check. According to ContentResolver#validateSyncExtrasBundle null-valued keys
+            // are allowed in the bundle.
+            if (!Objects.equals(b1.get(key), b2.get(key))) {
                 return false;
             }
         }

services/core/java/com/android/server/content/SyncManager.java

@@ -100,6 +100,7 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Random;
 import java.util.Set;
 
@@ -3194,7 +3195,7 @@ public class SyncManager {
             if (!smaller.containsKey(key)) {
                 return false;
             }
-            if (!bigger.get(key).equals(smaller.get(key))) {
+            if (!Objects.equals(bigger.get(key), smaller.get(key))) {
                 return false;
             }
         }
@@ -3202,7 +3203,6 @@ public class SyncManager {
     }
 
     /**
-     * TODO: Get rid of this when we separate sync settings extras from dev specified extras.
      * @return true if the provided key is used by the SyncManager in scheduling the sync.
      */
     private static boolean isSyncSetting(String key) {

services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java

@@ -0,0 +1,64 @@
+package com.android.server.content;
+
+import android.os.Bundle;
+
+import junit.framework.TestCase;
+
+public class SyncManagerTest extends TestCase {
+
+    final String KEY_1 = "key_1";
+    final String KEY_2 = "key_2";
+
+    public void testSyncExtrasEquals_WithNull() throws Exception {
+        Bundle b1 = new Bundle();
+        Bundle b2 = new Bundle();
+
+        b1.putString(KEY_1, null);
+        b2.putString(KEY_1, null);
+
+        assertTrue("Null extra not properly compared between bundles.",
+                SyncManager.syncExtrasEquals(b1, b2, false /* don't care about system extras */));
+    }
+
+    public void testSyncExtrasEqualsBigger_WithNull() throws Exception {
+        Bundle b1 = new Bundle();
+        Bundle b2 = new Bundle();
+
+        b1.putString(KEY_1, null);
+        b2.putString(KEY_1, null);
+
+        b1.putString(KEY_2, "bla");
+        b2.putString(KEY_2, "bla");
+
+        assertTrue("Extras not properly compared between bundles.",
+                SyncManager.syncExtrasEquals(b1, b2, false /* don't care about system extras */));
+    }
+
+    public void testSyncExtrasEqualsFails_differentValues() throws Exception {
+        Bundle b1 = new Bundle();
+        Bundle b2 = new Bundle();
+
+        b1.putString(KEY_1, null);
+        b2.putString(KEY_1, null);
+
+        b1.putString(KEY_2, "bla");
+        b2.putString(KEY_2, "ble");  // different key
+
+        assertFalse("Extras considered equal when they are different.",
+                SyncManager.syncExtrasEquals(b1, b2, false /* don't care about system extras */));
+    }
+
+    public void testSyncExtrasEqualsFails_differentNulls() throws Exception {
+        Bundle b1 = new Bundle();
+        Bundle b2 = new Bundle();
+
+        b1.putString(KEY_1, null);
+        b2.putString(KEY_1, "bla");  // different key
+
+        b1.putString(KEY_2, "ble");
+        b2.putString(KEY_2, "ble");
+
+        assertFalse("Extras considered equal when they are different.",
+                SyncManager.syncExtrasEquals(b1, b2, false /* don't care about system extras */));
+    }
+}