From f28fe8e09129e6aa505b2e0c570b23d250e12652 Mon Sep 17 00:00:00 2001
From: Firefly <service@t-firefly.com>
Date: Fri, 16 Oct 2015 12:01:31 -0700
Subject: [PATCH] [frameworks/base] Sync extras bundle comparison can throw NPE

BUG: 23591205
Change-Id: Ic6404c0befe70c34b078e0eae6a627826173d82c
(cherry picked from commit 9ad2c8403354a985258c098681067e74b9e2f638)
(cherry picked from commit c0f39c1ece72a05c796f7ba30b7a2b5b580d5025)
---
 .../java/android/content/PeriodicSync.java    |  6 +-
 .../android/server/content/SyncManager.java   |  4 +-
 .../server/content/SyncManagerTest.java       | 64 +++++++++++++++++++
 3 files changed, 71 insertions(+), 3 deletions(-)
 create mode 100644 frameworks/base/services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java

diff --git a/frameworks/base/core/java/android/content/PeriodicSync.java b/frameworks/base/core/java/android/content/PeriodicSync.java
index 3efd89aa38..0441cccdd4 100644
--- a/frameworks/base/core/java/android/content/PeriodicSync.java
+++ b/frameworks/base/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;
             }
         }
diff --git a/frameworks/base/services/core/java/com/android/server/content/SyncManager.java b/frameworks/base/services/core/java/com/android/server/content/SyncManager.java
index 52d5e831c0..2147ada278 100644
--- a/frameworks/base/services/core/java/com/android/server/content/SyncManager.java
+++ b/frameworks/base/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;
 
@@ -3196,7 +3197,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;
             }
         }
@@ -3204,7 +3205,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) {
diff --git a/frameworks/base/services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java b/frameworks/base/services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java
new file mode 100644
index 0000000000..be6861c436
--- /dev/null
+++ b/frameworks/base/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 */));
+    }
+}