DO NOT MERGE Sync extras bundle comparison can throw NPE
BUG: 23591205
Change-Id: Ic6404c0befe70c34b078e0eae6a627826173d82c
(cherry picked from commit 9ad2c8403354a985258c098681067e74b9e2f638)
diff --git a/core/java/android/content/PeriodicSync.java b/core/java/android/content/PeriodicSync.java
index 3efd89a..0441ccc 100644
--- a/core/java/android/content/PeriodicSync.java
+++ b/core/java/android/content/PeriodicSync.java
@@ -21,6 +21,8 @@
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 @@
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/services/core/java/com/android/server/content/SyncManager.java b/services/core/java/com/android/server/content/SyncManager.java
index 334bc18..b61f90e 100644
--- a/services/core/java/com/android/server/content/SyncManager.java
+++ b/services/core/java/com/android/server/content/SyncManager.java
@@ -104,6 +104,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Random;
import java.util.Set;
@@ -3370,7 +3371,7 @@
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;
}
}
@@ -3378,7 +3379,6 @@
}
/**
- * 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/services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java b/services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java
new file mode 100644
index 0000000..be6861c
--- /dev/null
+++ b/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 */));
+ }
+}
