Merge "Revert "Skip restore of read-only directories"" into sc-v2-dev
This commit is contained in:
committed by
Android (Google) Code Review
commit
f31dd1557c
@@ -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();
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user