Fix file matching w/ full-backup rules xml
Documentation is pretty vague:
https://developer.android.com/guide/topics/data/autobackup#XMLSyntax.
But there were a couple of issues:
* It was prematurely returning false without consuming the rest of the
includes (cause of the bug linked).
* It was using string comparison for checking if a file is in a
directory, which ended up flagging directories such as "a/b" as
containing files "a/b.txt".
Reviewers,
* Please, pay full attention to test cases.
* Since this is code move + code change, set diff as 2..latest to check
changes to the function.
Bug: 110720194
Test: atest BackupUtilsTest
Test: Backup and restore app w/ multiple directory includes, verify
everything restored
Change-Id: Ic0fea43156ce8fb641af69ae73679289a20c291c
diff --git a/core/java/android/app/backup/BackupAgent.java b/core/java/android/app/backup/BackupAgent.java
index 9657922..ec2cf0c 100644
--- a/core/java/android/app/backup/BackupAgent.java
+++ b/core/java/android/app/backup/BackupAgent.java
@@ -43,7 +43,6 @@
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
-import java.util.Collection;
import java.util.LinkedList;
import java.util.Map;
import java.util.Set;
@@ -833,7 +832,7 @@
}
if (excludes != null &&
- isFileSpecifiedInPathList(destination, excludes)) {
+ BackupUtils.isFileSpecifiedInPathList(destination, excludes)) {
if (Log.isLoggable(FullBackup.TAG_XML_PARSER, Log.VERBOSE)) {
Log.v(FullBackup.TAG_XML_PARSER,
"onRestoreFile: \"" + destinationCanonicalPath + "\": listed in"
@@ -847,7 +846,8 @@
// it's a small list), we'll go through and look for it.
boolean explicitlyIncluded = false;
for (Set<PathWithRequiredFlags> domainIncludes : includes.values()) {
- explicitlyIncluded |= isFileSpecifiedInPathList(destination, domainIncludes);
+ explicitlyIncluded |=
+ BackupUtils.isFileSpecifiedInPathList(destination, domainIncludes);
if (explicitlyIncluded) {
break;
}
@@ -866,33 +866,6 @@
}
/**
- * @return True if the provided file is either directly in the provided list, or the provided
- * file is within a directory in the list.
- */
- private boolean isFileSpecifiedInPathList(File file,
- Collection<PathWithRequiredFlags> canonicalPathList) throws IOException {
- for (PathWithRequiredFlags canonical : canonicalPathList) {
- String canonicalPath = canonical.getPath();
- File fileFromList = new File(canonicalPath);
- if (fileFromList.isDirectory()) {
- if (file.isDirectory()) {
- // If they are both directories check exact equals.
- return file.equals(fileFromList);
- } else {
- // O/w we have to check if the file is within the directory from the list.
- return file.getCanonicalPath().startsWith(canonicalPath);
- }
- } else {
- if (file.equals(fileFromList)) {
- // Need to check the explicit "equals" so we don't end up with substrings.
- return true;
- }
- }
- }
- return false;
- }
-
- /**
* Only specialized platform agents should overload this entry point to support
* restores to crazy non-app locations.
* @hide
diff --git a/core/java/android/app/backup/BackupUtils.java b/core/java/android/app/backup/BackupUtils.java
new file mode 100644
index 0000000..8cf8a84
--- /dev/null
+++ b/core/java/android/app/backup/BackupUtils.java
@@ -0,0 +1,58 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.app.backup;
+
+import android.app.backup.FullBackup.BackupScheme.PathWithRequiredFlags;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collection;
+
+/** @hide */
+public class BackupUtils {
+
+ private BackupUtils() {}
+
+ /**
+ * Returns {@code true} if {@code file} is either directly in {@code canonicalPathList} or is a
+ * file contained in a directory in the list.
+ */
+ public static boolean isFileSpecifiedInPathList(
+ File file, Collection<PathWithRequiredFlags> canonicalPathList) throws IOException {
+ for (PathWithRequiredFlags canonical : canonicalPathList) {
+ String canonicalPath = canonical.getPath();
+ File fileFromList = new File(canonicalPath);
+ if (fileFromList.isDirectory()) {
+ if (file.isDirectory()) {
+ // If they are both directories check exact equals.
+ if (file.equals(fileFromList)) {
+ return true;
+ }
+ } else {
+ // O/w we have to check if the file is within the directory from the list.
+ if (file.toPath().startsWith(canonicalPath)) {
+ return true;
+ }
+ }
+ } else if (file.equals(fileFromList)) {
+ // Need to check the explicit "equals" so we don't end up with substrings.
+ return true;
+ }
+ }
+ return false;
+ }
+}
diff --git a/services/robotests/src/android/app/backup/BackupUtilsTest.java b/services/robotests/src/android/app/backup/BackupUtilsTest.java
new file mode 100644
index 0000000..04a2a14
--- /dev/null
+++ b/services/robotests/src/android/app/backup/BackupUtilsTest.java
@@ -0,0 +1,195 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package android.app.backup;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import android.app.backup.FullBackup.BackupScheme.PathWithRequiredFlags;
+import android.content.Context;
+import android.platform.test.annotations.Presubmit;
+
+import com.android.server.testing.FrameworkRobolectricTestRunner;
+import com.android.server.testing.SystemLoaderPackages;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RuntimeEnvironment;
+import org.robolectric.annotation.Config;
+import org.robolectric.annotation.internal.DoNotInstrument;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+@RunWith(FrameworkRobolectricTestRunner.class)
+@Config(manifest = Config.NONE, sdk = 26)
+@SystemLoaderPackages({"android.app.backup"})
+@Presubmit
+@DoNotInstrument
+public class BackupUtilsTest {
+ private Context mContext;
+
+ @Before
+ public void setUp() throws Exception {
+ mContext = RuntimeEnvironment.application;
+ }
+
+ @Test
+ public void testIsFileSpecifiedInPathList_whenFileAndPathListHasIt() throws Exception {
+ boolean isSpecified =
+ BackupUtils.isFileSpecifiedInPathList(file("a/b.txt"), paths(file("a/b.txt")));
+
+ assertThat(isSpecified).isTrue();
+ }
+
+ @Test
+ public void testIsFileSpecifiedInPathList_whenFileAndPathListHasItsDirectory()
+ throws Exception {
+ boolean isSpecified =
+ BackupUtils.isFileSpecifiedInPathList(file("a/b.txt"), paths(directory("a")));
+
+ assertThat(isSpecified).isTrue();
+ }
+
+ @Test
+ public void testIsFileSpecifiedInPathList_whenFileAndPathListHasOtherFile() throws Exception {
+ boolean isSpecified =
+ BackupUtils.isFileSpecifiedInPathList(file("a/b.txt"), paths(file("a/c.txt")));
+
+ assertThat(isSpecified).isFalse();
+ }
+
+ @Test
+ public void testIsFileSpecifiedInPathList_whenFileAndPathListEmpty() throws Exception {
+ boolean isSpecified = BackupUtils.isFileSpecifiedInPathList(file("a/b.txt"), paths());
+
+ assertThat(isSpecified).isFalse();
+ }
+
+ @Test
+ public void testIsFileSpecifiedInPathList_whenDirectoryAndPathListHasIt() throws Exception {
+ boolean isSpecified =
+ BackupUtils.isFileSpecifiedInPathList(directory("a"), paths(directory("a")));
+
+ assertThat(isSpecified).isTrue();
+ }
+
+ @Test
+ public void testIsFileSpecifiedInPathList_whenDirectoryAndPathListEmpty() throws Exception {
+ boolean isSpecified = BackupUtils.isFileSpecifiedInPathList(directory("a"), paths());
+
+ assertThat(isSpecified).isFalse();
+ }
+
+ @Test
+ public void testIsFileSpecifiedInPathList_whenDirectoryAndPathListHasParent() throws Exception {
+ boolean isSpecified =
+ BackupUtils.isFileSpecifiedInPathList(directory("a/b"), paths(directory("a")));
+
+ assertThat(isSpecified).isFalse();
+ }
+
+ @Test
+ public void testIsFileSpecifiedInPathList_whenFileAndPathListDoesntContainDirectory()
+ throws Exception {
+ boolean isSpecified =
+ BackupUtils.isFileSpecifiedInPathList(file("a/b.txt"), paths(directory("c")));
+
+ assertThat(isSpecified).isFalse();
+ }
+
+ @Test
+ public void testIsFileSpecifiedInPathList_whenFileAndPathListHasDirectoryWhoseNameIsPrefix()
+ throws Exception {
+ boolean isSpecified =
+ BackupUtils.isFileSpecifiedInPathList(file("a/b.txt"), paths(directory("a/b")));
+
+ assertThat(isSpecified).isFalse();
+ }
+
+ @Test
+ public void testIsFileSpecifiedInPathList_whenFileAndPathListHasDirectoryWhoseNameIsPrefix2()
+ throws Exception {
+ boolean isSpecified =
+ BackupUtils.isFileSpecifiedInPathList(
+ file("name/subname.txt"), paths(directory("nam")));
+
+ assertThat(isSpecified).isFalse();
+ }
+
+ @Test
+ public void
+ testIsFileSpecifiedInPathList_whenFileAndPathListContainsFirstNotRelatedAndSecondContainingDirectory()
+ throws Exception {
+ boolean isSpecified =
+ BackupUtils.isFileSpecifiedInPathList(
+ file("a/b.txt"), paths(directory("b"), directory("a")));
+
+ assertThat(isSpecified).isTrue();
+ }
+
+ @Test
+ public void
+ testIsFileSpecifiedInPathList_whenDirectoryAndPathListContainsFirstNotRelatedAndSecondSameDirectory()
+ throws Exception {
+ boolean isSpecified =
+ BackupUtils.isFileSpecifiedInPathList(
+ directory("a/b"), paths(directory("b"), directory("a/b")));
+
+ assertThat(isSpecified).isTrue();
+ }
+
+ @Test
+ public void
+ testIsFileSpecifiedInPathList_whenFileAndPathListContainsFirstNotRelatedFileAndSecondSameFile()
+ throws Exception {
+ boolean isSpecified =
+ BackupUtils.isFileSpecifiedInPathList(
+ file("a/b.txt"), paths(directory("b"), file("a/b.txt")));
+
+ assertThat(isSpecified).isTrue();
+ }
+
+ private File file(String path) throws IOException {
+ File file = new File(mContext.getDataDir(), path);
+ File parent = file.getParentFile();
+ parent.mkdirs();
+ file.createNewFile();
+ if (!file.isFile()) {
+ throw new IOException("Couldn't create file");
+ }
+ return file;
+ }
+
+ private File directory(String path) throws IOException {
+ File directory = new File(mContext.getDataDir(), path);
+ directory.mkdirs();
+ if (!directory.isDirectory()) {
+ throw new IOException("Couldn't create directory");
+ }
+ return directory;
+ }
+
+ private Collection<PathWithRequiredFlags> paths(File... files) {
+ return Stream.of(files)
+ .map(file -> new PathWithRequiredFlags(file.getPath(), 0))
+ .collect(Collectors.toList());
+ }
+}