Merge "Add unit tests for recent regressions" into rvc-dev
diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
index 42fdd05..aa647d5 100644
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -841,25 +841,30 @@
return;
}
- if (ri->isRedactionNeeded() || is_file_locked(fd, path)) {
- // We don't want to use the FUSE VFS cache in two cases:
- // 1. When redaction is needed because app A with EXIF access might access
- // a region that should have been redacted for app B without EXIF access, but app B on
- // a subsequent read, will be able to see the EXIF data because the read request for that
- // region will be served from cache and not get to the FUSE daemon
- // 2. When the file has a read or write lock on it. This means that the MediaProvider has
- // given an fd to the lower file system to an app. There are two cases where using the cache
- // in this case can be a problem:
- // a. Writing to a FUSE fd with caching enabled will use the write-back cache and a
- // subsequent read from the lower fs fd will not see the write.
- // b. Reading from a FUSE fd with caching enabled may not see the latest writes using the
- // lower fs fd because those writes did not go through the FUSE layer and reads from FUSE
- // after that write may be served from cache
- fi->direct_io = true;
- }
+ handle* h = nullptr;
+ {
+ std::lock_guard<std::recursive_mutex> guard(fuse->lock);
- handle* h = new handle(path, fd, ri.release(), /*owner_uid*/ -1, !fi->direct_io);
- node->AddHandle(h);
+ if (ri->isRedactionNeeded() || is_file_locked(fd, path)) {
+ // We don't want to use the FUSE VFS cache in two cases:
+ // 1. When redaction is needed because app A with EXIF access might access
+ // a region that should have been redacted for app B without EXIF access, but app B on
+ // a subsequent read, will be able to see the EXIF data because the read request for
+ // that region will be served from cache and not get to the FUSE daemon
+ // 2. When the file has a read or write lock on it. This means that the MediaProvider
+ // has given an fd to the lower file system to an app. There are two cases where using
+ // the cache in this case can be a problem:
+ // a. Writing to a FUSE fd with caching enabled will use the write-back cache and a
+ // subsequent read from the lower fs fd will not see the write.
+ // b. Reading from a FUSE fd with caching enabled may not see the latest writes using
+ // the lower fs fd because those writes did not go through the FUSE layer and reads from
+ // FUSE after that write may be served from cache
+ fi->direct_io = true;
+ }
+
+ h = new handle(path, fd, ri.release(), /*owner_uid*/ -1, !fi->direct_io);
+ node->AddHandle(h);
+ }
fi->fh = ptr_to_id(h);
fi->keep_cache = 1;
@@ -1468,6 +1473,7 @@
bool use_fuse = false;
if (active.load(std::memory_order_acquire)) {
+ std::lock_guard<std::recursive_mutex> guard(fuse->lock);
const node* node = node::LookupAbsolutePath(fuse->root, path);
if (node && node->HasCachedHandle()) {
use_fuse = true;
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 172aa41..c29e344 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -583,6 +583,8 @@
Environment.DIRECTORY_MOVIES,
Environment.DIRECTORY_DOWNLOADS,
Environment.DIRECTORY_DCIM,
+ Environment.DIRECTORY_AUDIOBOOKS,
+ Environment.DIRECTORY_DOCUMENTS,
};
private static boolean isDefaultDirectoryName(@Nullable String dirName) {
@@ -1909,6 +1911,7 @@
defaultPrimary = Environment.DIRECTORY_MUSIC;
allowedPrimary = Arrays.asList(
Environment.DIRECTORY_ALARMS,
+ Environment.DIRECTORY_AUDIOBOOKS,
Environment.DIRECTORY_MUSIC,
Environment.DIRECTORY_NOTIFICATIONS,
Environment.DIRECTORY_PODCASTS,
diff --git a/src/com/android/providers/media/fuse/ExternalStorageServiceImpl.java b/src/com/android/providers/media/fuse/ExternalStorageServiceImpl.java
index 4ce2d2b..3fd1b4f 100644
--- a/src/com/android/providers/media/fuse/ExternalStorageServiceImpl.java
+++ b/src/com/android/providers/media/fuse/ExternalStorageServiceImpl.java
@@ -28,6 +28,7 @@
import com.android.providers.media.MediaProvider;
+import java.io.File;
import java.util.HashMap;
import java.util.Map;
@@ -42,8 +43,8 @@
@Override
public void onStartSession(String sessionId, /* @SessionFlag */ int flag,
- @NonNull ParcelFileDescriptor deviceFd, @NonNull String upperFileSystemPath,
- @NonNull String lowerFileSystemPath) {
+ @NonNull ParcelFileDescriptor deviceFd, @NonNull File upperFileSystemPath,
+ @NonNull File lowerFileSystemPath) {
MediaProvider mediaProvider = getMediaProvider();
synchronized (sLock) {
@@ -55,7 +56,7 @@
// REMOUNT_MODE_PASS_THROUGH which guarantees that all /storage paths are bind
// mounts of the lower filesystem.
FuseDaemon daemon = new FuseDaemon(mediaProvider, this, deviceFd, sessionId,
- upperFileSystemPath);
+ upperFileSystemPath.getPath());
daemon.start();
sFuseDaemons.put(sessionId, daemon);
}
diff --git a/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java b/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
index 40ffba7..54caffe 100644
--- a/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
+++ b/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
@@ -31,7 +31,6 @@
import static com.android.tests.fused.lib.TestUtils.STR_DATA2;
import static com.android.tests.fused.lib.TestUtils.assertCanRenameFile;
import static com.android.tests.fused.lib.TestUtils.assertCanRenameDirectory;
-import static com.android.tests.fused.lib.TestUtils.adoptShellPermissionIdentity;
import static com.android.tests.fused.lib.TestUtils.allowAppOpsToUid;
import static com.android.tests.fused.lib.TestUtils.assertCantRenameDirectory;
import static com.android.tests.fused.lib.TestUtils.assertCantRenameFile;
@@ -43,7 +42,6 @@
import static com.android.tests.fused.lib.TestUtils.deleteRecursively;
import static com.android.tests.fused.lib.TestUtils.deleteWithMediaProvider;
import static com.android.tests.fused.lib.TestUtils.denyAppOpsToUid;
-import static com.android.tests.fused.lib.TestUtils.dropShellPermissionIdentity;
import static com.android.tests.fused.lib.TestUtils.executeShellCommand;
import static com.android.tests.fused.lib.TestUtils.getContentResolver;
import static com.android.tests.fused.lib.TestUtils.getFileMimeTypeFromDatabase;
@@ -107,13 +105,22 @@
static final File EXTERNAL_STORAGE_DIR = Environment.getExternalStorageDirectory();
+ // Default top-level directories
+ static final File ALARMS_DIR = new File(EXTERNAL_STORAGE_DIR, Environment.DIRECTORY_ALARMS);
+ static final File AUDIOBOOKS_DIR = new File(EXTERNAL_STORAGE_DIR,
+ Environment.DIRECTORY_AUDIOBOOKS);
static final File DCIM_DIR = new File(EXTERNAL_STORAGE_DIR, Environment.DIRECTORY_DCIM);
- static final File PICTURES_DIR = new File(EXTERNAL_STORAGE_DIR, Environment.DIRECTORY_PICTURES);
- static final File MUSIC_DIR = new File(EXTERNAL_STORAGE_DIR, Environment.DIRECTORY_MUSIC);
- static final File MOVIES_DIR = new File(EXTERNAL_STORAGE_DIR, Environment.DIRECTORY_MOVIES);
+ static final File DOCUMENTS_DIR = new File(EXTERNAL_STORAGE_DIR,
+ Environment.DIRECTORY_DOCUMENTS);
static final File DOWNLOAD_DIR = new File(EXTERNAL_STORAGE_DIR,
Environment.DIRECTORY_DOWNLOADS);
+ static final File MUSIC_DIR = new File(EXTERNAL_STORAGE_DIR, Environment.DIRECTORY_MUSIC);
+ static final File MOVIES_DIR = new File(EXTERNAL_STORAGE_DIR, Environment.DIRECTORY_MOVIES);
+ static final File NOTIFICATIONS_DIR = new File(EXTERNAL_STORAGE_DIR, Environment.DIRECTORY_NOTIFICATIONS);
+ static final File PICTURES_DIR = new File(EXTERNAL_STORAGE_DIR, Environment.DIRECTORY_PICTURES);
static final File PODCASTS_DIR = new File(EXTERNAL_STORAGE_DIR, Environment.DIRECTORY_PODCASTS);
+ static final File RINGTONES_DIR = new File(EXTERNAL_STORAGE_DIR, Environment.DIRECTORY_RINGTONES);
+
static final File ANDROID_DATA_DIR = new File(EXTERNAL_STORAGE_DIR, "Android/data");
static final File ANDROID_MEDIA_DIR = new File(EXTERNAL_STORAGE_DIR, "Android/media");
static final String TEST_DIRECTORY_NAME = "FilePathAccessTestDirectory";
@@ -121,7 +128,7 @@
static final File EXTERNAL_FILES_DIR = getContext().getExternalFilesDir(null);
static final File EXTERNAL_MEDIA_DIR = getContext().getExternalMediaDirs()[0];
- static final String MUSIC_FILE_NAME = "FilePathAccessTest_file.mp3";
+ static final String AUDIO_FILE_NAME = "FilePathAccessTest_file.mp3";
static final String VIDEO_FILE_NAME = "FilePathAccessTest_file.mp4";
static final String IMAGE_FILE_NAME = "FilePathAccessTest_file.jpg";
static final String NONMEDIA_FILE_NAME = "FilePathAccessTest_file.pdf";
@@ -151,7 +158,7 @@
*/
@Test
public void testTypePathConformity() throws Exception {
- // Only music files can be created in Music
+ // Only audio files can be created in Music
assertThrows(IOException.class, "Operation not permitted", () -> {
new File(MUSIC_DIR, NONMEDIA_FILE_NAME).createNewFile();
});
@@ -166,7 +173,7 @@
new File(MOVIES_DIR, NONMEDIA_FILE_NAME).createNewFile();
});
assertThrows(IOException.class, "Operation not permitted", () -> {
- new File(MOVIES_DIR, MUSIC_FILE_NAME).createNewFile();
+ new File(MOVIES_DIR, AUDIO_FILE_NAME).createNewFile();
});
assertThrows(IOException.class, "Operation not permitted", () -> {
new File(MOVIES_DIR, IMAGE_FILE_NAME).createNewFile();
@@ -176,28 +183,42 @@
new File(DCIM_DIR, NONMEDIA_FILE_NAME).createNewFile();
});
assertThrows(IOException.class, "Operation not permitted", () -> {
- new File(DCIM_DIR, MUSIC_FILE_NAME).createNewFile();
+ new File(DCIM_DIR, AUDIO_FILE_NAME).createNewFile();
});
// Only image and video files can be created in Pictures
assertThrows(IOException.class, "Operation not permitted", () -> {
new File(PICTURES_DIR, NONMEDIA_FILE_NAME).createNewFile();
});
assertThrows(IOException.class, "Operation not permitted", () -> {
- new File(PICTURES_DIR, MUSIC_FILE_NAME).createNewFile();
+ new File(PICTURES_DIR, AUDIO_FILE_NAME).createNewFile();
});
+ assertCanCreateFile(new File(ALARMS_DIR, AUDIO_FILE_NAME));
+ assertCanCreateFile(new File(AUDIOBOOKS_DIR, AUDIO_FILE_NAME));
assertCanCreateFile(new File(DCIM_DIR, IMAGE_FILE_NAME));
- assertCanCreateFile(new File(MUSIC_DIR, MUSIC_FILE_NAME));
- assertCanCreateFile(new File(MOVIES_DIR, VIDEO_FILE_NAME));
+ assertCanCreateFile(new File(DCIM_DIR, VIDEO_FILE_NAME));
+ assertCanCreateFile(new File(DOCUMENTS_DIR, AUDIO_FILE_NAME));
+ assertCanCreateFile(new File(DOCUMENTS_DIR, IMAGE_FILE_NAME));
+ assertCanCreateFile(new File(DOCUMENTS_DIR, NONMEDIA_FILE_NAME));
+ assertCanCreateFile(new File(DOCUMENTS_DIR, VIDEO_FILE_NAME));
+ assertCanCreateFile(new File(DOWNLOAD_DIR, AUDIO_FILE_NAME));
+ assertCanCreateFile(new File(DOWNLOAD_DIR, IMAGE_FILE_NAME));
assertCanCreateFile(new File(DOWNLOAD_DIR, NONMEDIA_FILE_NAME));
+ assertCanCreateFile(new File(DOWNLOAD_DIR, VIDEO_FILE_NAME));
+ assertCanCreateFile(new File(MOVIES_DIR, VIDEO_FILE_NAME));
+ assertCanCreateFile(new File(MUSIC_DIR, AUDIO_FILE_NAME));
+ assertCanCreateFile(new File(NOTIFICATIONS_DIR, AUDIO_FILE_NAME));
+ assertCanCreateFile(new File(PICTURES_DIR, IMAGE_FILE_NAME));
assertCanCreateFile(new File(PICTURES_DIR, VIDEO_FILE_NAME));
+ assertCanCreateFile(new File(PODCASTS_DIR, AUDIO_FILE_NAME));
+ assertCanCreateFile(new File(RINGTONES_DIR, AUDIO_FILE_NAME));
// No file whatsoever can be created in the top level directory
assertThrows(IOException.class, "Operation not permitted", () -> {
new File(EXTERNAL_STORAGE_DIR, NONMEDIA_FILE_NAME).createNewFile();
});
assertThrows(IOException.class, "Operation not permitted", () -> {
- new File(EXTERNAL_STORAGE_DIR, MUSIC_FILE_NAME).createNewFile();
+ new File(EXTERNAL_STORAGE_DIR, AUDIO_FILE_NAME).createNewFile();
});
assertThrows(IOException.class, "Operation not permitted", () -> {
new File(EXTERNAL_STORAGE_DIR, IMAGE_FILE_NAME).createNewFile();
@@ -928,9 +949,9 @@
@Test
public void testSystemGalleryAppHasNoFullAccessToAudio() throws Exception {
- final File otherAppAudioFile = new File(MUSIC_DIR, "other_" + MUSIC_FILE_NAME);
- final File topLevelAudioFile = new File(EXTERNAL_STORAGE_DIR, MUSIC_FILE_NAME);
- final File audioInAnObviouslyWrongPlace = new File(PICTURES_DIR, MUSIC_FILE_NAME);
+ final File otherAppAudioFile = new File(MUSIC_DIR, "other_" + AUDIO_FILE_NAME);
+ final File topLevelAudioFile = new File(EXTERNAL_STORAGE_DIR, AUDIO_FILE_NAME);
+ final File audioInAnObviouslyWrongPlace = new File(PICTURES_DIR, AUDIO_FILE_NAME);
try {
installApp(TEST_APP_A, false);
@@ -977,7 +998,7 @@
final File imageFile = new File(PICTURES_DIR, IMAGE_FILE_NAME);
final File videoFile = new File(PICTURES_DIR, VIDEO_FILE_NAME);
final File topLevelVideoFile = new File(EXTERNAL_STORAGE_DIR, VIDEO_FILE_NAME);
- final File musicFile = new File(MUSIC_DIR, MUSIC_FILE_NAME);
+ final File musicFile = new File(MUSIC_DIR, AUDIO_FILE_NAME);
try {
installApp(TEST_APP_A, false);
allowAppOpsToUid(Process.myUid(), SYSTEM_GALERY_APPOPS);
@@ -1259,7 +1280,7 @@
@Test
public void testManageExternalStorageCanCreateFilesAnywhere() throws Exception {
final File topLevelPdf = new File(EXTERNAL_STORAGE_DIR, NONMEDIA_FILE_NAME);
- final File musicFileInMovies = new File(MOVIES_DIR, MUSIC_FILE_NAME);
+ final File musicFileInMovies = new File(MOVIES_DIR, AUDIO_FILE_NAME);
final File imageFileInDcim = new File(DCIM_DIR, IMAGE_FILE_NAME);
try {
allowAppOpsToUid(Process.myUid(), OPSTR_MANAGE_EXTERNAL_STORAGE);
@@ -1301,7 +1322,7 @@
public void testManageExternalStorageCanDeleteOtherAppsContents() throws Exception {
final File otherAppPdf = new File(DOWNLOAD_DIR, "other" + NONMEDIA_FILE_NAME);
final File otherAppImage = new File(DCIM_DIR, "other" + IMAGE_FILE_NAME);
- final File otherAppMusic = new File(MUSIC_DIR, "other" + MUSIC_FILE_NAME);
+ final File otherAppMusic = new File(MUSIC_DIR, "other" + AUDIO_FILE_NAME);
try {
installApp(TEST_APP_A, false);
@@ -1336,7 +1357,7 @@
final File pdf = new File(DOWNLOAD_DIR, NONMEDIA_FILE_NAME);
final File pdfInObviouslyWrongPlace = new File(PICTURES_DIR, NONMEDIA_FILE_NAME);
final File topLevelPdf = new File(EXTERNAL_STORAGE_DIR, NONMEDIA_FILE_NAME);
- final File musicFile = new File(MUSIC_DIR, MUSIC_FILE_NAME);
+ final File musicFile = new File(MUSIC_DIR, AUDIO_FILE_NAME);
try {
installApp(TEST_APP_A, false);
@@ -1395,7 +1416,7 @@
public void testManageExternalStorageQueryOtherAppsFile() throws Exception {
final File otherAppPdf = new File(DOWNLOAD_DIR, "other" + NONMEDIA_FILE_NAME);
final File otherAppImg = new File(DCIM_DIR, "other" + IMAGE_FILE_NAME);
- final File otherAppMusic = new File(MUSIC_DIR, "other" + MUSIC_FILE_NAME);
+ final File otherAppMusic = new File(MUSIC_DIR, "other" + AUDIO_FILE_NAME);
try {
installApp(TEST_APP_A, false);
assertCreateFilesAs(TEST_APP_A, otherAppImg, otherAppMusic, otherAppPdf);
@@ -1418,7 +1439,7 @@
public void testQueryOtherAppsFiles() throws Exception {
final File otherAppPdf = new File(DOWNLOAD_DIR, "other" + NONMEDIA_FILE_NAME);
final File otherAppImg = new File(DCIM_DIR, "other" + IMAGE_FILE_NAME);
- final File otherAppMusic = new File(MUSIC_DIR, "other" + MUSIC_FILE_NAME);
+ final File otherAppMusic = new File(MUSIC_DIR, "other" + AUDIO_FILE_NAME);
try {
installApp(TEST_APP_A, false);
assertCreateFilesAs(TEST_APP_A, otherAppImg, otherAppMusic, otherAppPdf);
@@ -1438,7 +1459,7 @@
public void testSystemGalleryQueryOtherAppsFiles() throws Exception {
final File otherAppPdf = new File(DOWNLOAD_DIR, "other" + NONMEDIA_FILE_NAME);
final File otherAppImg = new File(DCIM_DIR, "other" + IMAGE_FILE_NAME);
- final File otherAppMusic = new File(MUSIC_DIR, "other" + MUSIC_FILE_NAME);
+ final File otherAppMusic = new File(MUSIC_DIR, "other" + AUDIO_FILE_NAME);
try {
installApp(TEST_APP_A, false);
assertCreateFilesAs(TEST_APP_A, otherAppImg, otherAppMusic, otherAppPdf);