Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d13c86c0 authored by Sarp Misoglu's avatar Sarp Misoglu
Browse files

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
parent c2b644f0

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