diff options
| -rw-r--r-- | AconfigFlags.bp | 10 | ||||
| -rw-r--r-- | core/java/android/app/backup/BackupAgent.java | 20 | ||||
| -rw-r--r-- | core/tests/coretests/src/android/app/backup/BackupAgentTest.java | 54 | ||||
| -rw-r--r-- | services/backup/Android.bp | 6 | ||||
| -rw-r--r-- | services/backup/flags.aconfig | 8 | ||||
| -rw-r--r-- | services/core/Android.bp | 1 | ||||
| -rw-r--r-- | services/tests/mockingservicestests/Android.bp | 1 |
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", |