Merge "Clear pipe after onRestoreFile is called" into main (1bac6b42) · Commits · e / os / android_frameworks_base

AconfigFlags.bp

+9 −1

Original line number	Diff line number	Diff line
		@@ -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}",
		@@ -1093,3 +1094,10 @@ java_aconfig_library {
		aconfig_declarations: "android.crashrecovery.flags-aconfig",
		defaults: ["framework-minus-apex-aconfig-java-defaults"],
		}

		// Backup
		java_aconfig_library {
		name: "backup_flags_lib",
		aconfig_declarations: "backup_flags",
		defaults: ["framework-minus-apex-aconfig-java-defaults"],
		}

core/java/android/app/backup/BackupAgent.java

+20 −0

Original line number	Diff line number	Diff line
		@@ -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();

core/tests/coretests/src/android/app/backup/BackupAgentTest.java

+50 −4

Original line number	Diff line number	Diff line
		@@ -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.
		}
		}
		}

services/backup/Android.bp

+0 −6

Original line number	Diff line number	Diff line
		@@ -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",
		}

services/backup/flags.aconfig

+8 −0

Original line number	Diff line number	Diff line
		@@ -25,3 +25,11 @@ flag {
		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