Clear pipe after onRestoreFile is called

The BackupAgent can override onRestoreFile and ignore the contents of
the pipe. Currently, the FullBackupEngine assumes that the data is
always consumed and when it's not the pipe buffer gets full and the
restore is blocked indefinitely for the whole device.

Test: atest -v android.app.backup.BackupAgentTest#doRestoreFile_consumesAllBytesInBuffer
Bug: 320633449
Change-Id: Ic48a11a8134786d9427ac0ff1636bef561eb6a2a

7 files changed, 87 insertions, 13 deletions

diff --git a/AconfigFlags.bp b/AconfigFlags.bp
index 98b62b3e2046..2c78f935c00d 100644
--- a/AconfigFlags.bp
+++ b/AconfigFlags.bp
@@ -60,6 +60,7 @@ aconfig_srcjars = [
     ":android.webkit.flags-aconfig-java{.generated_srcjars}",
     ":android.widget.flags-aconfig-java{.generated_srcjars}",
     ":audio-framework-aconfig",
+    ":backup_flags_lib{.generated_srcjars}",
     ":camera_platform_flags_core_java_lib{.generated_srcjars}",
     ":com.android.hardware.input-aconfig-java{.generated_srcjars}",
     ":com.android.input.flags-aconfig-java{.generated_srcjars}",
@@ -1092,4 +1093,11 @@ java_aconfig_library {
     name: "android.crashrecovery.flags-aconfig-java",
     aconfig_declarations: "android.crashrecovery.flags-aconfig",
     defaults: ["framework-minus-apex-aconfig-java-defaults"],
-}
\ No newline at end of file
+}
+
+// Backup
+java_aconfig_library {
+    name: "backup_flags_lib",
+    aconfig_declarations: "backup_flags",
+    defaults: ["framework-minus-apex-aconfig-java-defaults"],
+}
diff --git a/core/java/android/app/backup/BackupAgent.java b/core/java/android/app/backup/BackupAgent.java
index 6b558d07c059..ffbd80ce5824 100644
--- a/core/java/android/app/backup/BackupAgent.java
+++ b/core/java/android/app/backup/BackupAgent.java
@@ -43,12 +43,14 @@ import android.util.Log;
 
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.infra.AndroidFuture;
+import com.android.server.backup.Flags;
 
 import libcore.io.IoUtils;
 
 import org.xmlpull.v1.XmlPullParserException;
 
 import java.io.File;
+import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.lang.annotation.Retention;
@@ -1283,6 +1285,14 @@ public abstract class BackupAgent extends ContextWrapper {
                 // And bring live SharedPreferences instances up to date
                 reloadSharedPreferences();
 
+                // It's possible that onRestoreFile was overridden and that the agent did not
+                // consume all the data for this file from the pipe. We need to clear the pipe,
+                // otherwise the framework can get stuck trying to write to a full pipe or
+                // onRestoreFile could be called with the previous file's data left in the pipe.
+                if (Flags.enableClearPipeAfterRestoreFile()) {
+                    clearUnconsumedDataFromPipe(data, size);
+                }
+
                 Binder.restoreCallingIdentity(ident);
                 try {
                     callbackBinder.opCompleteForUser(getBackupUserId(), token, 0);
@@ -1296,6 +1306,16 @@ public abstract class BackupAgent extends ContextWrapper {
             }
         }
 
+        private static void clearUnconsumedDataFromPipe(ParcelFileDescriptor data, long size) {
+            try (FileInputStream in = new FileInputStream(data.getFileDescriptor())) {
+                if (in.available() > 0) {
+                    in.skip(size);
+                }
+            } catch (IOException e) {
+                Log.w(TAG, "Failed to clear unconsumed data from pipe.", e);
+            }
+        }
+
         @Override
         public void doRestoreFinished(int token, IBackupManager callbackBinder) {
             final long ident = Binder.clearCallingIdentity();
diff --git a/core/tests/coretests/src/android/app/backup/BackupAgentTest.java b/core/tests/coretests/src/android/app/backup/BackupAgentTest.java
index 9e8e2d6a2446..cd5deb6c4293 100644
--- a/core/tests/coretests/src/android/app/backup/BackupAgentTest.java
+++ b/core/tests/coretests/src/android/app/backup/BackupAgentTest.java
@@ -20,24 +20,33 @@ import static com.google.common.truth.Truth.assertThat;
 
 import static org.mockito.Mockito.when;
 
+import android.app.IBackupAgent;
 import android.app.backup.BackupAgent.IncludeExcludeRules;
 import android.app.backup.BackupAnnotations.BackupDestination;
 import android.app.backup.BackupAnnotations.OperationType;
 import android.app.backup.FullBackup.BackupScheme.PathWithRequiredFlags;
+import android.content.Context;
 import android.os.ParcelFileDescriptor;
 import android.os.UserHandle;
 import android.platform.test.annotations.Presubmit;
+import android.platform.test.flag.junit.SetFlagsRule;
 import android.util.ArraySet;
 
 import androidx.test.runner.AndroidJUnit4;
 
+import com.android.server.backup.Flags;
+
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.Map;
 import java.util.Set;
@@ -49,9 +58,12 @@ public class BackupAgentTest {
     private static final UserHandle USER_HANDLE = new UserHandle(15);
     private static final String DATA_TYPE_BACKED_UP = "test data type";
 
+    @Mock IBackupManager mIBackupManager;
     @Mock FullBackup.BackupScheme mBackupScheme;
+    @Mock Context mContext;
 
-    private BackupAgent mBackupAgent;
+    @Rule
+    public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();
 
     @Before
     public void setUp() {
@@ -67,11 +79,11 @@ public class BackupAgentTest {
         excludePaths.add(path);
         IncludeExcludeRules expectedRules = new IncludeExcludeRules(includePaths, excludePaths);
 
-        mBackupAgent = getAgentForBackupDestination(BackupDestination.CLOUD);
+        BackupAgent backupAgent = getAgentForBackupDestination(BackupDestination.CLOUD);
         when(mBackupScheme.maybeParseAndGetCanonicalExcludePaths()).thenReturn(excludePaths);
         when(mBackupScheme.maybeParseAndGetCanonicalIncludePaths()).thenReturn(includePaths);
 
-        IncludeExcludeRules rules = mBackupAgent.getIncludeExcludeRules(mBackupScheme);
+        IncludeExcludeRules rules = backupAgent.getIncludeExcludeRules(mBackupScheme);
         assertThat(rules).isEqualTo(expectedRules);
     }
 
@@ -137,6 +149,31 @@ public class BackupAgentTest {
                 0).getSuccessCount()).isEqualTo(1);
     }
 
+    @Test
+    public void doRestoreFile_agentOverrideIgnoresFile_consumesAllBytesInBuffer() throws Exception {
+        mSetFlagsRule.enableFlags(Flags.FLAG_ENABLE_CLEAR_PIPE_AFTER_RESTORE_FILE);
+        BackupAgent agent = new TestRestoreIgnoringFullBackupAgent();
+        agent.attach(mContext);
+        agent.onCreate(USER_HANDLE, BackupDestination.CLOUD, OperationType.RESTORE);
+        IBackupAgent agentBinder = (IBackupAgent) agent.onBind();
+
+        ParcelFileDescriptor[] pipes = ParcelFileDescriptor.createPipe();
+        FileOutputStream writeSide = new FileOutputStream(
+                pipes[1].getFileDescriptor());
+        writeSide.write("Hello".getBytes(StandardCharsets.UTF_8));
+
+        agentBinder.doRestoreFile(pipes[0], /* length= */ 5, BackupAgent.TYPE_FILE,
+                FullBackup.FILES_TREE_TOKEN, /* path= */ "hello_file", /* mode= */
+                0666, /* mtime= */ 12345, /* token= */ 6789, mIBackupManager);
+
+        try (FileInputStream in = new FileInputStream(pipes[0].getFileDescriptor())) {
+            assertThat(in.available()).isEqualTo(0);
+        } finally {
+            pipes[0].close();
+            pipes[1].close();
+        }
+    }
+
     private BackupAgent getAgentForBackupDestination(@BackupDestination int backupDestination) {
         BackupAgent agent = new TestFullBackupAgent();
         agent.onCreate(USER_HANDLE, backupDestination);
@@ -144,7 +181,6 @@ public class BackupAgentTest {
     }
 
     private static class TestFullBackupAgent extends BackupAgent {
-
         @Override
         public void onBackup(ParcelFileDescriptor oldState, BackupDataOutput data,
                 ParcelFileDescriptor newState) throws IOException {
@@ -162,4 +198,14 @@ public class BackupAgentTest {
             // Left empty as this is a full backup agent.
         }
     }
+
+    private static class TestRestoreIgnoringFullBackupAgent extends TestFullBackupAgent {
+
+        @Override
+        protected void onRestoreFile(ParcelFileDescriptor data, long size,
+                int type, String domain, String path, long mode, long mtime)
+                throws IOException {
+            // Ignore the file and don't consume any data.
+        }
+    }
 }
diff --git a/services/backup/Android.bp b/services/backup/Android.bp
index d08a97e2a573..2a85eb66f6c5 100644
--- a/services/backup/Android.bp
+++ b/services/backup/Android.bp
@@ -21,7 +21,6 @@ java_library_static {
     libs: ["services.core"],
     static_libs: [
         "app-compat-annotations",
-        "backup_flags_lib",
     ],
     lint: {
         baseline_filename: "lint-baseline.xml",
@@ -33,8 +32,3 @@ aconfig_declarations {
     package: "com.android.server.backup",
     srcs: ["flags.aconfig"],
 }
-
-java_aconfig_library {
-    name: "backup_flags_lib",
-    aconfig_declarations: "backup_flags",
-}
diff --git a/services/backup/flags.aconfig b/services/backup/flags.aconfig
index 1416c888f790..6a63b3a9db24 100644
--- a/services/backup/flags.aconfig
+++ b/services/backup/flags.aconfig
@@ -24,4 +24,12 @@ flag {
     description: "Enables the write buffer to pipes to be of maximum size."
     bug: "265976737"
     is_fixed_read_only: true
+}
+
+flag {
+    name: "enable_clear_pipe_after_restore_file"
+    namespace: "onboarding"
+    description: "Enables clearing the pipe buffer after restoring a single file to a BackupAgent."
+    bug: "320633449"
+    is_fixed_read_only: true
 }
\ No newline at end of file
diff --git a/services/core/Android.bp b/services/core/Android.bp
index 8e35b7455d02..89896c3735b6 100644
--- a/services/core/Android.bp
+++ b/services/core/Android.bp
@@ -211,7 +211,6 @@ java_library_static {
         "com_android_wm_shell_flags_lib",
         "com.android.server.utils_aconfig-java",
         "service-jobscheduler-deviceidle.flags-aconfig-java",
-        "backup_flags_lib",
         "policy_flags_lib",
         "net_flags_lib",
         "stats_flags_lib",
diff --git a/services/tests/mockingservicestests/Android.bp b/services/tests/mockingservicestests/Android.bp
index 321d945e9c15..d9283063b7fc 100644
--- a/services/tests/mockingservicestests/Android.bp
+++ b/services/tests/mockingservicestests/Android.bp
@@ -46,7 +46,6 @@ android_test {
         "androidx.test.espresso.core",
         "androidx.test.espresso.contrib",
         "androidx.test.ext.truth",
-        "backup_flags_lib",
         "flag-junit",
         "frameworks-base-testutils",
         "hamcrest-library",