DataChangedJournal: Disallow null files/directories.

Passing a null File into DataChangedJournal would always have
thrown NPE if various methods (toString(), equals(), hashCode(),
addPackage()) were called later, but by that time the origin
(stacktrace) of the offending null value would have been lost.

This CL changes the class to detect this error earlier, namely
in the ctor.

In addition, this CL changes newJournal(File journalDirectory)
to not accept null; previously, if null was passed, then the
journal would have been created in the wrong directory, which
would then have led to incorrect behavior of listJournals:
 - listJournals(null) would have thrown NPE
 - listJournals(nonNullJournalDirectory) would not have found
   the journal.

I convinced myself through code inspection that this error case
is not currently possible, but this was not straightforward:
UserBackupManagerService.mJournalDir is passed to DCJ.newJournal()
from writeToJournalLocked() which is called from dataChangedImpl()
which is called from mPackageTrackingReceiver, which is luckily
registered only after mJournalDir was initialized fairly late in the
non-test ctor); should this become buggy in future, then after this
CL it should be more likely to be discovered.

Bug: 162022005
Test: Manually checked no non-test callers pass in non-null.
Test: atest FrameworksServicesTests:com.android.server.backup.DataChangedJournalTest

Change-Id: I1756add05f3d65e455b2e61448f98ceceb5b39f1

2 files changed, 19 insertions, 5 deletions

diff --git a/services/backup/java/com/android/server/backup/DataChangedJournal.java b/services/backup/java/com/android/server/backup/DataChangedJournal.java
index e0bb02bdee43..c8a98554b045 100644
--- a/services/backup/java/com/android/server/backup/DataChangedJournal.java
+++ b/services/backup/java/com/android/server/backup/DataChangedJournal.java
@@ -16,6 +16,7 @@
 
 package com.android.server.backup;
 
+import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.util.Slog;
 
@@ -27,6 +28,7 @@ import java.io.IOException;
 import java.io.RandomAccessFile;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 import java.util.function.Consumer;
 
 /**
@@ -50,8 +52,8 @@ public final class DataChangedJournal {
     /**
      * Constructs an instance that reads from and writes to the given file.
      */
-    DataChangedJournal(File file) {
-        mFile = file;
+    DataChangedJournal(@NonNull File file) {
+        mFile = Objects.requireNonNull(file);
     }
 
 
@@ -133,9 +135,10 @@ public final class DataChangedJournal {
      * @return The journal.
      * @throws IOException if there is an IO error creating the file.
      */
-    static DataChangedJournal newJournal(File journalDirectory) throws IOException {
-        return new DataChangedJournal(
-                File.createTempFile(FILE_NAME_PREFIX, null, journalDirectory));
+    static DataChangedJournal newJournal(@NonNull File journalDirectory) throws IOException {
+        Objects.requireNonNull(journalDirectory);
+        File file = File.createTempFile(FILE_NAME_PREFIX, null, journalDirectory);
+        return new DataChangedJournal(file);
     }
 
     /**
diff --git a/services/tests/servicestests/src/com/android/server/backup/DataChangedJournalTest.java b/services/tests/servicestests/src/com/android/server/backup/DataChangedJournalTest.java
index 7aa111250cac..9daa4ba3c9c8 100644
--- a/services/tests/servicestests/src/com/android/server/backup/DataChangedJournalTest.java
+++ b/services/tests/servicestests/src/com/android/server/backup/DataChangedJournalTest.java
@@ -39,6 +39,7 @@ import org.mockito.MockitoAnnotations;
 import java.io.DataInputStream;
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.IOException;
 import java.util.function.Consumer;
 
 @SmallTest
@@ -134,6 +135,16 @@ public class DataChangedJournalTest {
         assertThat(folder.listFiles()).hasLength(1);
     }
 
+    @Test(expected = NullPointerException.class)
+    public void newJournal_nullJournalDir() throws IOException {
+        DataChangedJournal.newJournal(null);
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void nullFile() {
+        new DataChangedJournal(null);
+    }
+
     @Test
     public void toString_isSameAsFileToString() throws Exception {
         assertThat(mJournal.toString()).isEqualTo(mFile.toString());