Merge "Revert "Skip restore of read-only directories"" into sc-v2-dev

services/backup/java/com/android/server/backup/restore/FullRestoreEngine.java

@@ -25,10 +25,8 @@ import static com.android.server.backup.UserBackupManagerService.OP_TYPE_RESTORE
 import static com.android.server.backup.UserBackupManagerService.SHARED_BACKUP_AGENT_PACKAGE;
 import static com.android.server.backup.internal.BackupHandler.MSG_RESTORE_OPERATION_TIMEOUT;
 
-import android.annotation.NonNull;
 import android.app.ApplicationThreadConstants;
 import android.app.IBackupAgent;
-import android.app.backup.BackupAgent;
 import android.app.backup.BackupManager;
 import android.app.backup.FullBackup;
 import android.app.backup.IBackupManagerMonitor;
@@ -41,12 +39,10 @@ import android.content.pm.Signature;
 import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
 import android.provider.Settings;
-import android.system.OsConstants;
 import android.text.TextUtils;
 import android.util.Slog;
 
 import com.android.internal.annotations.GuardedBy;
-import com.android.internal.annotations.VisibleForTesting;
 import com.android.server.LocalServices;
 import com.android.server.backup.BackupAgentTimeoutParameters;
 import com.android.server.backup.BackupRestoreTask;
@@ -60,7 +56,6 @@ import com.android.server.backup.utils.FullBackupRestoreObserverUtils;
 import com.android.server.backup.utils.RestoreUtils;
 import com.android.server.backup.utils.TarBackupReader;
 
-import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -137,7 +132,6 @@ public class FullRestoreEngine extends RestoreEngine {
     @GuardedBy("mPipesLock")
     private boolean mPipesClosed;
     private final BackupEligibilityRules mBackupEligibilityRules;
-    private FileMetadata mReadOnlyParent = null;
 
     public FullRestoreEngine(UserBackupManagerService backupManagerService,
             BackupRestoreTask monitorTask, IFullBackupRestoreObserver observer,
@@ -160,21 +154,6 @@ public class FullRestoreEngine extends RestoreEngine {
         mBackupEligibilityRules = backupEligibilityRules;
     }
 
-    @VisibleForTesting
-    FullRestoreEngine() {
-        mIsAdbRestore = false;
-        mAllowApks = false;
-        mEphemeralOpToken = 0;
-        mUserId = 0;
-        mBackupEligibilityRules = null;
-        mAgentTimeoutParameters = null;
-        mBuffer = null;
-        mBackupManagerService = null;
-        mMonitor = null;
-        mMonitorTask = null;
-        mOnlyPackage = null;
-    }
-
     public IBackupAgent getAgent() {
         return mAgent;
     }
@@ -414,11 +393,6 @@ public class FullRestoreEngine extends RestoreEngine {
                         okay = false;
                     }
 
-                    if (shouldSkipReadOnlyDir(info)) {
-                        // b/194894879: We don't support restore of read-only dirs.
-                        okay = false;
-                    }
-
                     // At this point we have an agent ready to handle the full
                     // restore data as well as a pipe for sending data to
                     // that agent.  Tell the agent to start reading from the
@@ -595,45 +569,6 @@ public class FullRestoreEngine extends RestoreEngine {
         return (info != null);
     }
 
-    boolean shouldSkipReadOnlyDir(FileMetadata info) {
-        if (isValidParent(mReadOnlyParent, info)) {
-            // This file has a read-only parent directory, we shouldn't
-            // restore it.
-            return true;
-        } else {
-            // We're now in a different branch of the file tree, update the parent
-            // value.
-            if (isReadOnlyDir(info)) {
-                // Current directory is read-only. Remember it so that we can skip all
-                // of its contents.
-                mReadOnlyParent = info;
-                Slog.w(TAG, "Skipping restore of " + info.path + " and its contents as "
-                        + "read-only dirs are currently not supported.");
-                return true;
-            } else {
-                mReadOnlyParent = null;
-            }
-        }
-
-        return false;
-    }
-
-    private static boolean isValidParent(FileMetadata parentDir, @NonNull FileMetadata childDir) {
-        return parentDir != null
-                && childDir.packageName.equals(parentDir.packageName)
-                && childDir.domain.equals(parentDir.domain)
-                && childDir.path.startsWith(getPathWithTrailingSeparator(parentDir.path));
-    }
-
-    private static String getPathWithTrailingSeparator(String path) {
-        return path.endsWith(File.separator) ? path : path + File.separator;
-    }
-
-    private static boolean isReadOnlyDir(FileMetadata file) {
-        // Check if owner has 'write' bit in the file's mode value (see 'man -7 inode' for details).
-        return file.type == BackupAgent.TYPE_DIRECTORY && (file.mode & OsConstants.S_IWUSR) == 0;
-    }
-
     private void setUpPipes() throws IOException {
         synchronized (mPipesLock) {
             mPipes = ParcelFileDescriptor.createPipe();

services/tests/servicestests/src/com/android/server/backup/restore/FullRestoreEngineTest.java

@@ -1,150 +0,0 @@
-/*
- * Copyright (C) 2022 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 com.android.server.backup.restore;
-
-import static com.google.common.truth.Truth.assertWithMessage;
-
-import android.app.backup.BackupAgent;
-import android.platform.test.annotations.Presubmit;
-import android.system.OsConstants;
-
-import androidx.test.runner.AndroidJUnit4;
-
-import com.android.server.backup.FileMetadata;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-
-@Presubmit
-@RunWith(AndroidJUnit4.class)
-public class FullRestoreEngineTest {
-    private static final String DEFAULT_PACKAGE_NAME = "package";
-    private static final String DEFAULT_DOMAIN_NAME = "domain";
-    private static final String NEW_PACKAGE_NAME = "new_package";
-    private static final String NEW_DOMAIN_NAME = "new_domain";
-
-    private FullRestoreEngine mRestoreEngine;
-
-    @Before
-    public void setUp() {
-        mRestoreEngine = new FullRestoreEngine();
-    }
-
-    @Test
-    public void shouldSkipReadOnlyDir_skipsAllReadonlyDirsAndTheirChildren() {
-        // Create the file tree.
-        TestFile[] testFiles = new TestFile[] {
-                TestFile.dir("root"),
-                TestFile.file("root/auth_token"),
-                TestFile.dir("root/media"),
-                TestFile.file("root/media/picture1.png"),
-                TestFile.file("root/push_token.txt"),
-                TestFile.dir("root/read-only-dir-1").markReadOnly().expectSkipped(),
-                TestFile.dir("root/read-only-dir-1/writable-subdir").expectSkipped(),
-                TestFile.file("root/read-only-dir-1/writable-subdir/writable-file").expectSkipped(),
-                TestFile.dir("root/read-only-dir-1/writable-subdir/read-only-subdir-2")
-                        .markReadOnly().expectSkipped(),
-                TestFile.file("root/read-only-dir-1/writable-file").expectSkipped(),
-                TestFile.file("root/random-stuff.txt"),
-                TestFile.dir("root/database"),
-                TestFile.file("root/database/users.db"),
-                TestFile.dir("root/read-only-dir-2").markReadOnly().expectSkipped(),
-                TestFile.file("root/read-only-dir-2/writable-file-1").expectSkipped(),
-                TestFile.file("root/read-only-dir-2/writable-file-2").expectSkipped(),
-        };
-
-        assertCorrectItemsAreSkipped(testFiles);
-    }
-
-    @Test
-    public void shouldSkipReadOnlyDir_onlySkipsChildrenUnderTheSamePackage() {
-        TestFile[] testFiles = new TestFile[]{
-                TestFile.dir("read-only-dir").markReadOnly().expectSkipped(),
-                TestFile.file("read-only-dir/file").expectSkipped(),
-                TestFile.file("read-only-dir/file-from-different-package")
-                        .setPackage(NEW_PACKAGE_NAME),
-        };
-
-        assertCorrectItemsAreSkipped(testFiles);
-    }
-
-    @Test
-    public void shouldSkipReadOnlyDir_onlySkipsChildrenUnderTheSameDomain() {
-        TestFile[] testFiles = new TestFile[]{
-                TestFile.dir("read-only-dir").markReadOnly().expectSkipped(),
-                TestFile.file("read-only-dir/file").expectSkipped(),
-                TestFile.file("read-only-dir/file-from-different-domain")
-                        .setDomain(NEW_DOMAIN_NAME),
-        };
-
-        assertCorrectItemsAreSkipped(testFiles);
-    }
-
-    private void assertCorrectItemsAreSkipped(TestFile[] testFiles) {
-        // Verify all directories marked with .expectSkipped are skipped.
-        for (TestFile testFile : testFiles) {
-            boolean actualExcluded = mRestoreEngine.shouldSkipReadOnlyDir(testFile.mMetadata);
-            boolean expectedExcluded = testFile.mShouldSkip;
-            assertWithMessage(testFile.mMetadata.path).that(actualExcluded).isEqualTo(
-                    expectedExcluded);
-        }
-    }
-
-    private static class TestFile {
-        private final FileMetadata mMetadata;
-        private boolean mShouldSkip;
-
-        static TestFile dir(String path) {
-            return new TestFile(path, BackupAgent.TYPE_DIRECTORY);
-        }
-
-        static TestFile file(String path) {
-            return new TestFile(path, BackupAgent.TYPE_FILE);
-        }
-
-        TestFile markReadOnly() {
-            mMetadata.mode = 0;
-            return this;
-        }
-
-        TestFile expectSkipped() {
-            mShouldSkip = true;
-            return this;
-        }
-
-        TestFile setPackage(String packageName) {
-            mMetadata.packageName = packageName;
-            return this;
-        }
-
-        TestFile setDomain(String domain) {
-            mMetadata.domain = domain;
-            return this;
-        }
-
-        private TestFile(String path, int type) {
-            FileMetadata metadata = new FileMetadata();
-            metadata.path = path;
-            metadata.type = type;
-            metadata.packageName = DEFAULT_PACKAGE_NAME;
-            metadata.domain = DEFAULT_DOMAIN_NAME;
-            metadata.mode = OsConstants.S_IWUSR; // Mark as writable.
-            mMetadata = metadata;
-        }
-    }
-}