Merge "Use passthrough if file supports transcoding"
diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
index 673771b..e1e7547 100755
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -390,12 +390,14 @@
}
}
-static double get_attr_timeout(const string& path, bool should_inval, node* node,
- struct fuse* fuse) {
- if (fuse->disable_dentry_cache || should_inval || is_package_owned_path(path, fuse->path)) {
+static double get_attr_timeout(const string& path, node* node, struct fuse* fuse) {
+ if (fuse->disable_dentry_cache || node->ShouldInvalidate() ||
+ is_package_owned_path(path, fuse->path)) {
// We set dentry timeout to 0 for the following reasons:
// 1. The dentry cache was completely disabled
- // 2. Case-insensitive lookups need to invalidate other case-insensitive dentry matches
+ // 2.1 Case-insensitive lookups need to invalidate other case-insensitive dentry matches
+ // 2.2 Nodes supporting transforms need to be invalidated, so that subsequent lookups by a
+ // uid requiring a transform is guaranteed to come to the FUSE daemon.
// 3. With app data isolation enabled, app A should not guess existence of app B from the
// Android/{data,obb}/<package> paths, hence we prevent the kernel from caching that
// information.
@@ -404,8 +406,7 @@
return std::numeric_limits<double>::max();
}
-static double get_entry_timeout(const string& path, bool should_inval, node* node,
- struct fuse* fuse) {
+static double get_entry_timeout(const string& path, node* node, struct fuse* fuse) {
string media_path = fuse->GetEffectiveRootPath() + "/Android/media";
if (path.find(media_path, 0) == 0) {
// Installd might delete Android/media/<package> dirs when app data is cleared.
@@ -413,7 +414,7 @@
// dir via FUSE.
return 0;
}
- return get_attr_timeout(path, should_inval, node, fuse);
+ return get_attr_timeout(path, node, fuse);
}
static std::string get_path(node* node) {
@@ -433,7 +434,7 @@
return NULL;
}
- bool should_inval = false;
+ bool should_invalidate = false;
bool transforms_complete = true;
int transforms = 0;
string io_path;
@@ -441,7 +442,7 @@
if (S_ISREG(e->attr.st_mode)) {
// Handle potential file transforms
std::unique_ptr<mediaprovider::fuse::FileLookupResult> file_lookup_result =
- fuse->mp->FileLookup(path, req->ctx.uid);
+ fuse->mp->FileLookup(path, req->ctx.uid, req->ctx.pid);
if (!file_lookup_result) {
// Fail lookup if we can't fetch FileLookupResult for path
@@ -453,37 +454,34 @@
transforms = file_lookup_result->transforms;
io_path = file_lookup_result->io_path;
transforms_complete = file_lookup_result->transforms_complete;
+ // Invalidate if the inode supports transforms so that we always get a lookup into userspace
+ should_invalidate = file_lookup_result->transforms_supported;
// Update size with io_path size if io_path is not same as path
if (!io_path.empty() && (io_path != path) && (lstat(io_path.c_str(), &e->attr) < 0)) {
*error_code = errno;
return NULL;
}
-
- // Invalidate if there are any transforms so that we always get a lookup into userspace
- should_inval = should_inval || transforms;
}
node = parent->LookupChildByName(name, true /* acquire */, transforms);
if (!node) {
- node = ::node::Create(parent, name, io_path, transforms_complete, transforms, &fuse->lock,
- &fuse->tracker);
+ node = ::node::Create(parent, name, io_path, should_invalidate, transforms_complete,
+ transforms, &fuse->lock, &fuse->tracker);
} else if (!mediaprovider::fuse::containsMount(path, std::to_string(getuid() / PER_USER_RANGE))) {
- should_inval = should_inval || node->HasCaseInsensitiveMatch();
// Only invalidate a path if it does not contain mount.
// Invalidate both names to ensure there's no dentry left in the kernel after the following
// operations:
// 1) touch foo, touch FOO, unlink *foo*
// 2) touch foo, touch FOO, unlink *FOO*
// Invalidating lookup_name fixes (1) and invalidating node_name fixes (2)
- // |should_inval| invalidates lookup_name by using 0 timeout below and we explicitly
+ // SetShouldInvalidate invalidates lookup_name by using 0 timeout below and we explicitly
// invalidate node_name if different case
// Note that we invalidate async otherwise we will deadlock the kernel
if (name != node->GetName()) {
- should_inval = true;
// Record that we have made a case insensitive lookup, this allows us invalidate nodes
// correctly on subsequent lookups for the case of |node|
- node->SetCaseInsensitiveMatch();
+ node->SetShouldInvalidate();
// Make copies of the node name and path so we're not attempting to acquire
// any node locks from the invalidation thread. Depending on timing, we may end
@@ -503,8 +501,8 @@
// reuse inode numbers.
e->generation = 0;
e->ino = fuse->ToInode(node);
- e->entry_timeout = get_entry_timeout(path, should_inval, node, fuse);
- e->attr_timeout = get_attr_timeout(path, should_inval, node, fuse);
+ e->entry_timeout = get_entry_timeout(path, node, fuse);
+ e->attr_timeout = get_attr_timeout(path, node, fuse);
return node;
}
@@ -693,10 +691,7 @@
if (lstat(path.c_str(), &s) < 0) {
fuse_reply_err(req, errno);
} else {
- fuse_reply_attr(
- req, &s,
- get_attr_timeout(path, node->GetTransforms() || node->HasCaseInsensitiveMatch(),
- node, fuse));
+ fuse_reply_attr(req, &s, get_attr_timeout(path, node, fuse));
}
}
@@ -725,8 +720,15 @@
fd = h->fd;
} else {
const struct fuse_ctx* ctx = fuse_req_ctx(req);
- int status = fuse->mp->IsOpenAllowed(path, ctx->uid, true);
- if (status) {
+ std::unique_ptr<FileOpenResult> result = fuse->mp->OnFileOpen(
+ path, path, ctx->uid, ctx->pid, true /* for_write */, false /* redact */);
+
+ if (!result) {
+ fuse_reply_err(req, EFAULT);
+ return;
+ }
+
+ if (result->status) {
fuse_reply_err(req, EACCES);
return;
}
@@ -791,9 +793,7 @@
}
lstat(path.c_str(), attr);
- fuse_reply_attr(req, attr,
- get_attr_timeout(path, node->GetTransforms() || node->HasCaseInsensitiveMatch(),
- node, fuse));
+ fuse_reply_attr(req, attr, get_attr_timeout(path, node, fuse));
}
static void pf_canonical_path(fuse_req_t req, fuse_ino_t ino)
@@ -1110,9 +1110,17 @@
// TODO: If transform, disallow write
// Force permission check with the build path because the MediaProvider database might not be
// aware of the io_path
- int status = fuse->mp->IsOpenAllowed(build_path, ctx->uid, is_requesting_write(fi->flags));
- if (status) {
- fuse_reply_err(req, status);
+ bool for_write = is_requesting_write(fi->flags);
+ // We don't redact if the caller was granted write permission for this file
+ std::unique_ptr<FileOpenResult> result = fuse->mp->OnFileOpen(
+ build_path, io_path, ctx->uid, ctx->pid, for_write, !for_write /* redact */);
+ if (!result) {
+ fuse_reply_err(req, EFAULT);
+ return;
+ }
+
+ if (result->status) {
+ fuse_reply_err(req, result->status);
return;
}
@@ -1134,23 +1142,9 @@
return;
}
- // We don't redact if the caller was granted write permission for this file
- std::unique_ptr<RedactionInfo> ri;
- if (is_requesting_write(fi->flags)) {
- ri = std::make_unique<RedactionInfo>();
- } else {
- ri = fuse->mp->GetRedactionInfo(build_path, io_path, req->ctx.uid, req->ctx.pid);
- }
-
- if (!ri) {
- close(fd);
- fuse_reply_err(req, EFAULT);
- return;
- }
-
int keep_cache = 1;
- handle* h = create_handle_for_node(fuse, io_path, fd, req->ctx.uid, node, ri.release(),
- &keep_cache);
+ handle* h = create_handle_for_node(fuse, io_path, fd, req->ctx.uid, node,
+ result->redaction_info.release(), &keep_cache);
fi->fh = ptr_to_id(h);
fi->keep_cache = keep_cache;
fi->direct_io = !h->cached;
@@ -1626,7 +1620,15 @@
return;
}
- status = fuse->mp->IsOpenAllowed(path, req->ctx.uid, for_write);
+ std::unique_ptr<FileOpenResult> result = fuse->mp->OnFileOpen(
+ path, path, req->ctx.uid, req->ctx.pid, for_write, false /* redact */);
+ if (!result) {
+ status = EFAULT;
+ }
+
+ if (result->status) {
+ status = EACCES;
+ }
}
fuse_reply_err(req, status);
diff --git a/jni/MediaProviderWrapper.cpp b/jni/MediaProviderWrapper.cpp
index 27f9339..99fe081 100644
--- a/jni/MediaProviderWrapper.cpp
+++ b/jni/MediaProviderWrapper.cpp
@@ -56,33 +56,17 @@
return false;
}
-std::unique_ptr<RedactionInfo> getRedactionInfoInternal(JNIEnv* env, jobject media_provider_object,
- jmethodID mid_get_redaction_ranges,
- uid_t uid, pid_t tid, const string& path,
- const string& io_path) {
- ScopedLocalRef<jstring> j_path(env, env->NewStringUTF(path.c_str()));
- ScopedLocalRef<jstring> j_io_path(env, env->NewStringUTF(io_path.c_str()));
- ScopedLocalRef<jlongArray> redaction_ranges_local_ref(
- env, static_cast<jlongArray>(
- env->CallObjectMethod(media_provider_object, mid_get_redaction_ranges,
- j_path.get(), j_io_path.get(), uid, tid)));
- ScopedLongArrayRO redaction_ranges(env, redaction_ranges_local_ref.get());
-
- if (CheckForJniException(env)) {
- return nullptr;
+/**
+ * Auxiliary for caching class fields
+ */
+static jfieldID CacheField(JNIEnv* env, jclass clazz, const char field_name[], const char type[]) {
+ jfieldID fid;
+ string actual_field_name(field_name);
+ fid = env->GetFieldID(clazz, actual_field_name.c_str(), type);
+ if (!fid) {
+ LOG(FATAL) << "Error caching field: " << field_name << type;
}
-
- std::unique_ptr<RedactionInfo> ri;
- if (redaction_ranges.size() % 2) {
- LOG(ERROR) << "Error while calculating redaction ranges: array length is uneven";
- } else if (redaction_ranges.size() > 0) {
- ri = std::make_unique<RedactionInfo>(redaction_ranges.size() / 2, redaction_ranges.get());
- } else {
- // No ranges to redact
- ri = std::make_unique<RedactionInfo>();
- }
-
- return ri;
+ return fid;
}
int insertFileInternal(JNIEnv* env, jobject media_provider_object, jmethodID mid_insert_file,
@@ -107,18 +91,6 @@
return res;
}
-int isOpenAllowedInternal(JNIEnv* env, jobject media_provider_object, jmethodID mid_is_open_allowed,
- const string& path, uid_t uid, bool for_write) {
- ScopedLocalRef<jstring> j_path(env, env->NewStringUTF(path.c_str()));
- int res = env->CallIntMethod(media_provider_object, mid_is_open_allowed, j_path.get(), uid,
- for_write);
-
- if (CheckForJniException(env)) {
- return EFAULT;
- }
- return res;
-}
-
void scanFileInternal(JNIEnv* env, jobject media_provider_object, jmethodID mid_scan_file,
const string& path) {
ScopedLocalRef<jstring> j_path(env, env->NewStringUTF(path.c_str()));
@@ -263,14 +235,13 @@
media_provider_class_ = reinterpret_cast<jclass>(env->NewGlobalRef(media_provider_class_));
// Cache methods - Before calling a method, make sure you cache it here
- mid_get_redaction_ranges_ =
- CacheMethod(env, "getRedactionRanges", "(Ljava/lang/String;Ljava/lang/String;II)[J",
- /*is_static*/ false);
mid_insert_file_ = CacheMethod(env, "insertFileIfNecessary", "(Ljava/lang/String;I)I",
/*is_static*/ false);
mid_delete_file_ = CacheMethod(env, "deleteFile", "(Ljava/lang/String;I)I", /*is_static*/ false);
- mid_is_open_allowed_ = CacheMethod(env, "isOpenAllowed", "(Ljava/lang/String;IZ)I",
- /*is_static*/ false);
+ mid_on_file_open_ = CacheMethod(env, "onFileOpen",
+ "(Ljava/lang/String;Ljava/lang/String;IIZZ)Lcom/android/"
+ "providers/media/FileOpenResult;",
+ /*is_static*/ false);
mid_scan_file_ = CacheMethod(env, "scanFile", "(Ljava/lang/String;)V",
/*is_static*/ false);
mid_is_mkdir_or_rmdir_allowed_ = CacheMethod(env, "isDirectoryCreationOrDeletionAllowed",
@@ -295,18 +266,35 @@
/*is_static*/ false);
mid_file_lookup_ =
CacheMethod(env, "onFileLookup",
- "(Ljava/lang/String;I)Lcom/android/providers/media/FileLookupResult;",
+ "(Ljava/lang/String;II)Lcom/android/providers/media/FileLookupResult;",
/*is_static*/ false);
+ // FileLookupResult
file_lookup_result_class_ = env->FindClass("com/android/providers/media/FileLookupResult");
if (!file_lookup_result_class_) {
LOG(FATAL) << "Could not find class FileLookupResult";
}
file_lookup_result_class_ =
reinterpret_cast<jclass>(env->NewGlobalRef(file_lookup_result_class_));
- fid_file_lookup_transforms_ = CacheFileLookupField(env, "transforms", "I");
- fid_file_lookup_transforms_complete_ = CacheFileLookupField(env, "transformsComplete", "Z");
- fid_file_lookup_io_path_ = CacheFileLookupField(env, "ioPath", "Ljava/lang/String;");
+ fid_file_lookup_transforms_ = CacheField(env, file_lookup_result_class_, "transforms", "I");
+ fid_file_lookup_uid_ = CacheField(env, file_lookup_result_class_, "uid", "I");
+ fid_file_lookup_transforms_complete_ =
+ CacheField(env, file_lookup_result_class_, "transformsComplete", "Z");
+ fid_file_lookup_transforms_supported_ =
+ CacheField(env, file_lookup_result_class_, "transformsSupported", "Z");
+ fid_file_lookup_io_path_ =
+ CacheField(env, file_lookup_result_class_, "ioPath", "Ljava/lang/String;");
+
+ // FileOpenResult
+ file_open_result_class_ = env->FindClass("com/android/providers/media/FileOpenResult");
+ if (!file_open_result_class_) {
+ LOG(FATAL) << "Could not find class FileOpenResult";
+ }
+ file_open_result_class_ = reinterpret_cast<jclass>(env->NewGlobalRef(file_open_result_class_));
+ fid_file_open_status_ = CacheField(env, file_open_result_class_, "status", "I");
+ fid_file_open_uid_ = CacheField(env, file_open_result_class_, "uid", "I");
+ fid_file_open_redaction_ranges_ =
+ CacheField(env, file_open_result_class_, "redactionRanges", "[J");
}
MediaProviderWrapper::~MediaProviderWrapper() {
@@ -315,24 +303,6 @@
env->DeleteGlobalRef(media_provider_class_);
}
-std::unique_ptr<RedactionInfo> MediaProviderWrapper::GetRedactionInfo(const string& path,
- const string& io_path,
- uid_t uid, pid_t tid) {
- if (shouldBypassMediaProvider(uid) || !GetBoolProperty(kPropRedactionEnabled, true)) {
- return std::make_unique<RedactionInfo>();
- }
-
- // Default value in case JNI thread was being terminated, causes the read to fail.
- std::unique_ptr<RedactionInfo> res = nullptr;
-
- JNIEnv* env = MaybeAttachCurrentThread();
- auto ri = getRedactionInfoInternal(env, media_provider_object_, mid_get_redaction_ranges_, uid,
- tid, path, io_path);
- res = std::move(ri);
-
- return res;
-}
-
int MediaProviderWrapper::InsertFile(const string& path, uid_t uid) {
if (uid == ROOT_UID) {
return 0;
@@ -352,14 +322,48 @@
return deleteFileInternal(env, media_provider_object_, mid_delete_file_, path, uid);
}
-int MediaProviderWrapper::IsOpenAllowed(const string& path, uid_t uid, bool for_write) {
+std::unique_ptr<FileOpenResult> MediaProviderWrapper::OnFileOpen(const string& path,
+ const string& io_path, uid_t uid,
+ pid_t tid, bool for_write,
+ bool redact) {
+ JNIEnv* env = MaybeAttachCurrentThread();
if (shouldBypassMediaProvider(uid)) {
- return 0;
+ return std::make_unique<FileOpenResult>(0, uid, new RedactionInfo());
}
- JNIEnv* env = MaybeAttachCurrentThread();
- return isOpenAllowedInternal(env, media_provider_object_, mid_is_open_allowed_, path, uid,
- for_write);
+ ScopedLocalRef<jstring> j_path(env, env->NewStringUTF(path.c_str()));
+ ScopedLocalRef<jstring> j_io_path(env, env->NewStringUTF(io_path.c_str()));
+ ScopedLocalRef<jobject> j_res_file_open_object(
+ env, env->CallObjectMethod(media_provider_object_, mid_on_file_open_, j_path.get(),
+ j_io_path.get(), uid, tid, for_write, redact));
+
+ if (CheckForJniException(env)) {
+ return nullptr;
+ }
+
+ int status = env->GetIntField(j_res_file_open_object.get(), fid_file_open_status_);
+ int original_uid = env->GetIntField(j_res_file_open_object.get(), fid_file_open_uid_);
+
+ if (redact) {
+ ScopedLocalRef<jlongArray> redaction_ranges_local_ref(
+ env, static_cast<jlongArray>(env->GetObjectField(j_res_file_open_object.get(),
+ fid_file_open_redaction_ranges_)));
+ ScopedLongArrayRO redaction_ranges(env, redaction_ranges_local_ref.get());
+
+ std::unique_ptr<RedactionInfo> ri;
+ if (redaction_ranges.size() % 2) {
+ LOG(ERROR) << "Error while calculating redaction ranges: array length is uneven";
+ } else if (redaction_ranges.size() > 0) {
+ ri = std::make_unique<RedactionInfo>(redaction_ranges.size() / 2,
+ redaction_ranges.get());
+ } else {
+ // No ranges to redact
+ ri = std::make_unique<RedactionInfo>();
+ }
+ return std::make_unique<FileOpenResult>(status, original_uid, ri.release());
+ } else {
+ return std::make_unique<FileOpenResult>(status, original_uid, new RedactionInfo());
+ }
}
void MediaProviderWrapper::ScanFile(const string& path) {
@@ -476,7 +480,7 @@
}
std::unique_ptr<FileLookupResult> MediaProviderWrapper::FileLookup(const std::string& path,
- uid_t uid) {
+ uid_t uid, pid_t tid) {
JNIEnv* env = MaybeAttachCurrentThread();
ScopedLocalRef<jstring> j_path(env, env->NewStringUTF(path.c_str()));
@@ -489,15 +493,19 @@
}
int transforms = env->GetIntField(j_res_file_lookup_object.get(), fid_file_lookup_transforms_);
+ int original_uid = env->GetIntField(j_res_file_lookup_object.get(), fid_file_lookup_uid_);
bool transforms_complete = env->GetBooleanField(j_res_file_lookup_object.get(),
fid_file_lookup_transforms_complete_);
+ bool transforms_supported = env->GetBooleanField(j_res_file_lookup_object.get(),
+ fid_file_lookup_transforms_supported_);
ScopedLocalRef<jstring> j_io_path(
env,
(jstring)env->GetObjectField(j_res_file_lookup_object.get(), fid_file_lookup_io_path_));
ScopedUtfChars j_io_path_utf(env, j_io_path.get());
- std::unique_ptr<FileLookupResult> file_lookup_result = std::make_unique<FileLookupResult>(
- transforms, transforms_complete, string(j_io_path_utf.c_str()));
+ std::unique_ptr<FileLookupResult> file_lookup_result =
+ std::make_unique<FileLookupResult>(transforms, original_uid, transforms_complete,
+ transforms_supported, string(j_io_path_utf.c_str()));
return file_lookup_result;
}
@@ -535,27 +543,11 @@
mid = env->GetMethodID(media_provider_class_, actual_method_name.c_str(), signature);
}
if (!mid) {
- // SHOULD NOT HAPPEN!
LOG(FATAL) << "Error caching method: " << method_name << signature;
}
return mid;
}
-/**
- * Finds FileLookupResult field and adds it to fields map so it can be quickly accessed later.
- */
-jfieldID MediaProviderWrapper::CacheFileLookupField(JNIEnv* env, const char field_name[],
- const char type[]) {
- jfieldID fid;
- string actual_field_name(field_name);
- fid = env->GetFieldID(file_lookup_result_class_, actual_field_name.c_str(), type);
- if (!fid) {
- // SHOULD NOT HAPPEN!
- LOG(FATAL) << "Error caching field: " << field_name << type;
- }
- return fid;
-}
-
void MediaProviderWrapper::DetachThreadFunction(void* unused) {
int detach = gJavaVm->DetachCurrentThread();
CHECK_EQ(detach, 0);
diff --git a/jni/MediaProviderWrapper.h b/jni/MediaProviderWrapper.h
index 31d9d1b..fb1cf58 100644
--- a/jni/MediaProviderWrapper.h
+++ b/jni/MediaProviderWrapper.h
@@ -17,6 +17,7 @@
#ifndef MEDIAPROVIDER_FUSE_MEDIAPROVIDERWRAPPER_H_
#define MEDIAPROVIDER_FUSE_MEDIAPROVIDERWRAPPER_H_
+#include <android-base/logging.h>
#include <jni.h>
#include <sys/types.h>
@@ -35,20 +36,41 @@
namespace mediaprovider {
namespace fuse {
+/** Represents file open result from MediaProvider */
+struct FileOpenResult {
+ FileOpenResult(const int status, const int uid, const RedactionInfo* redaction_info)
+ : status(status), uid(uid), redaction_info(redaction_info) {}
+
+ const int status;
+ const int uid;
+ std::unique_ptr<const RedactionInfo> redaction_info;
+};
+
/**
* Represents transform info for a file, containing the transforms, the transforms completion
* status and the ioPath. Provided by MediaProvider.java via a JNI call.
*/
struct FileLookupResult {
- FileLookupResult(int transforms, bool transforms_complete, const std::string& io_path)
- : transforms(transforms), transforms_complete(transforms_complete), io_path(io_path) {}
+ FileLookupResult(int transforms, uid_t uid, bool transforms_complete, bool transforms_supported,
+ const std::string& io_path)
+ : transforms(transforms),
+ uid(uid),
+ transforms_complete(transforms_complete),
+ transforms_supported(transforms_supported),
+ io_path(io_path) {
+ if (transforms != 0) {
+ CHECK(transforms_supported);
+ }
+ }
/**
* This field is not to be interpreted, it is determined and populated from MediaProvider
* via a JNI call.
*/
const int transforms;
+ const uid_t uid;
const bool transforms_complete;
+ const bool transforms_supported;
const std::string io_path;
};
@@ -113,12 +135,18 @@
/**
* Determines if the given UID is allowed to open the file denoted by the given path.
*
- * @param path the path of the file to be opened
+ * Also computes and returns the RedactionInfo for a given file and |uid|
+ *
+ * @param path path of the requested file that will be used for database operations
+ * @param io_path path of the requested file that will be used for IO
* @param uid UID of the calling app
+ * @param tid UID of the calling app
* @param for_write specifies if the file is to be opened for write
- * @return 0 upon success or errno value upon failure.
+ * @param redact specifies whether to attempt redaction
+ * @return FileOpenResult containing status, uid and redaction_info
*/
- int IsOpenAllowed(const std::string& path, uid_t uid, bool for_write);
+ std::unique_ptr<FileOpenResult> OnFileOpen(const std::string& path, const std::string& io_path,
+ uid_t uid, pid_t tid, bool for_write, bool redact);
/**
* Potentially triggers a scan of the file before closing it and reconciles it with the
@@ -193,7 +221,7 @@
/**
* Returns FileLookupResult to determine transform info for a path and uid.
*/
- std::unique_ptr<FileLookupResult> FileLookup(const std::string& path, uid_t uid);
+ std::unique_ptr<FileLookupResult> FileLookup(const std::string& path, uid_t uid, pid_t tid);
/** Transforms from src to dst file */
bool Transform(const std::string& src, const std::string& dst, int transforms, uid_t uid);
@@ -224,13 +252,13 @@
private:
jclass file_lookup_result_class_;
+ jclass file_open_result_class_;
jclass media_provider_class_;
jobject media_provider_object_;
/** Cached MediaProvider method IDs **/
- jmethodID mid_get_redaction_ranges_;
jmethodID mid_insert_file_;
jmethodID mid_delete_file_;
- jmethodID mid_is_open_allowed_;
+ jmethodID mid_on_file_open_;
jmethodID mid_scan_file_;
jmethodID mid_is_mkdir_or_rmdir_allowed_;
jmethodID mid_is_opendir_allowed_;
@@ -244,8 +272,14 @@
jmethodID mid_file_lookup_;
/** Cached FileLookupResult field IDs **/
jfieldID fid_file_lookup_transforms_;
+ jfieldID fid_file_lookup_uid_;
jfieldID fid_file_lookup_transforms_complete_;
+ jfieldID fid_file_lookup_transforms_supported_;
jfieldID fid_file_lookup_io_path_;
+ /** Cached FileOpenResult field IDs **/
+ jfieldID fid_file_open_status_;
+ jfieldID fid_file_open_uid_;
+ jfieldID fid_file_open_redaction_ranges_;
/**
* Auxiliary for caching MediaProvider methods.
@@ -253,11 +287,6 @@
jmethodID CacheMethod(JNIEnv* env, const char method_name[], const char signature[],
bool is_static);
- /**
- * Auxiliary for caching FileLookupResult fields.
- */
- jfieldID CacheFileLookupField(JNIEnv* env, const char field_name[], const char type[]);
-
// Attaches the current thread (if necessary) and returns the JNIEnv
// associated with it.
static JNIEnv* MaybeAttachCurrentThread();
diff --git a/jni/node-inl.h b/jni/node-inl.h
index 1fa62b0..2b577bf 100644
--- a/jni/node-inl.h
+++ b/jni/node-inl.h
@@ -119,13 +119,14 @@
public:
// Creates a new node with the specified parent, name and lock.
static node* Create(node* parent, const std::string& name, const std::string& io_path,
- bool transforms_complete, const int transforms, std::recursive_mutex* lock,
- NodeTracker* tracker) {
+ bool should_invalidate, bool transforms_complete, const int transforms,
+ std::recursive_mutex* lock, NodeTracker* tracker) {
// Place the entire constructor under a critical section to make sure
// node creation, tracking (if enabled) and the addition to a parent are
// atomic.
std::lock_guard<std::recursive_mutex> guard(*lock);
- return new node(parent, name, io_path, transforms_complete, transforms, lock, tracker);
+ return new node(parent, name, io_path, should_invalidate, transforms_complete, transforms,
+ lock, tracker);
}
// Creates a new root node. Root nodes have no parents by definition
@@ -133,7 +134,8 @@
static node* CreateRoot(const std::string& path, std::recursive_mutex* lock,
NodeTracker* tracker) {
std::lock_guard<std::recursive_mutex> guard(*lock);
- node* root = new node(nullptr, path, path, true, 0, lock, tracker);
+ node* root = new node(nullptr, path, path, false /* should_invalidate */,
+ true /* transforms_complete */, 0, lock, tracker);
// The root always has one extra reference to avoid it being
// accidentally collected.
@@ -303,14 +305,14 @@
return false;
}
- bool HasCaseInsensitiveMatch() const {
+ bool ShouldInvalidate() const {
std::lock_guard<std::recursive_mutex> guard(*lock_);
- return has_case_insensitive_match_;
+ return should_invalidate_;
}
- void SetCaseInsensitiveMatch() {
+ void SetShouldInvalidate() {
std::lock_guard<std::recursive_mutex> guard(*lock_);
- has_case_insensitive_match_ = true;
+ should_invalidate_ = true;
}
bool HasRedactedCache() const {
@@ -346,8 +348,9 @@
static const node* LookupAbsolutePath(const node* root, const std::string& absolute_path);
private:
- node(node* parent, const std::string& name, const std::string& io_path, bool transforms_complete,
- const int transforms, std::recursive_mutex* lock, NodeTracker* tracker)
+ node(node* parent, const std::string& name, const std::string& io_path,
+ const bool should_invalidate, const bool transforms_complete, const int transforms,
+ std::recursive_mutex* lock, NodeTracker* tracker)
: name_(name),
io_path_(io_path),
transforms_complete_(transforms_complete),
@@ -355,7 +358,7 @@
refcount_(0),
parent_(nullptr),
has_redacted_cache_(false),
- has_case_insensitive_match_(false),
+ should_invalidate_(should_invalidate),
deleted_(false),
lock_(lock),
tracker_(tracker) {
@@ -366,6 +369,10 @@
if (parent != nullptr) {
AddToParent(parent);
}
+ // If the node requires transforms, we MUST never cache it in the VFS
+ if (transforms) {
+ CHECK(should_invalidate_);
+ }
}
// Acquires a reference to a node. This maps to the "lookup count" specified
@@ -499,7 +506,7 @@
// List of directory handles associated with this node. Guarded by |lock_|.
std::vector<std::unique_ptr<dirhandle>> dirhandles_;
bool has_redacted_cache_;
- bool has_case_insensitive_match_;
+ bool should_invalidate_;
bool deleted_;
std::recursive_mutex* lock_;
diff --git a/jni/node_test.cpp b/jni/node_test.cpp
index 8c3406e..6496f80 100644
--- a/jni/node_test.cpp
+++ b/jni/node_test.cpp
@@ -32,8 +32,9 @@
typedef std::unique_ptr<node, decltype(&NodeTest::destroy)> unique_node_ptr;
unique_node_ptr CreateNode(node* parent, const std::string& path, const int transforms = 0) {
- return unique_node_ptr(node::Create(parent, path, "", true, transforms, &lock_, &tracker_),
- &NodeTest::destroy);
+ return unique_node_ptr(
+ node::Create(parent, path, "", true, true, transforms, &lock_, &tracker_),
+ &NodeTest::destroy);
}
static class node* ForChild(class node* node, const std::string& name,
@@ -67,7 +68,7 @@
}
TEST_F(NodeTest, TestRelease) {
- node* node = node::Create(nullptr, "/path", "", true, 0, &lock_, &tracker_);
+ node* node = node::Create(nullptr, "/path", "", false, true, 0, &lock_, &tracker_);
acquire(node);
acquire(node);
ASSERT_EQ(3, GetRefCount(node));
@@ -277,10 +278,10 @@
unique_node_ptr parent = CreateNode(nullptr, "/path");
// This is the tree that we intend to delete.
- node* child = node::Create(parent.get(), "subdir", "", true, 0, &lock_, &tracker_);
- node::Create(child, "s1", "", true, 0, &lock_, &tracker_);
- node* subchild2 = node::Create(child, "s2", "", true, 0, &lock_, &tracker_);
- node::Create(subchild2, "sc2", "", true, 0, &lock_, &tracker_);
+ node* child = node::Create(parent.get(), "subdir", "", false, true, 0, &lock_, &tracker_);
+ node::Create(child, "s1", "", false, true, 0, &lock_, &tracker_);
+ node* subchild2 = node::Create(child, "s2", "", false, true, 0, &lock_, &tracker_);
+ node::Create(subchild2, "sc2", "", false, true, 0, &lock_, &tracker_);
ASSERT_EQ(child, parent->LookupChildByName("subdir", false /* acquire */));
node::DeleteTree(child);
diff --git a/src/com/android/providers/media/FileLookupResult.java b/src/com/android/providers/media/FileLookupResult.java
index d5531fc..4dc4d03 100644
--- a/src/com/android/providers/media/FileLookupResult.java
+++ b/src/com/android/providers/media/FileLookupResult.java
@@ -22,12 +22,17 @@
*/
public final class FileLookupResult {
public final int transforms;
+ public final int uid;
public final boolean transformsComplete;
+ public final boolean transformsSupported;
public final String ioPath;
- public FileLookupResult(int transforms, boolean transformsComplete, String ioPath) {
+ public FileLookupResult(int transforms, int uid, boolean transformsComplete,
+ boolean transformsSupported, String ioPath) {
this.transforms = transforms;
+ this.uid = uid;
this.transformsComplete = transformsComplete;
+ this.transformsSupported = transformsSupported;
this.ioPath = ioPath;
}
}
diff --git a/src/com/android/providers/media/FileOpenResult.java b/src/com/android/providers/media/FileOpenResult.java
new file mode 100644
index 0000000..7777b8b
--- /dev/null
+++ b/src/com/android/providers/media/FileOpenResult.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2021 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.providers.media;
+
+/**
+ * Wrapper class which contains the result of an open.
+ */
+public final class FileOpenResult {
+ public final int status;
+ public final int uid;
+ public final long[] redactionRanges;
+
+ public FileOpenResult(int status, int uid, long[] redactionRanges) {
+ this.status = status;
+ this.uid = uid;
+ this.redactionRanges = redactionRanges;
+ }
+}
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 5685dbb..2390047 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -241,28 +241,28 @@
static final Pattern PATTERN_SELECTION_ID = Pattern.compile(
"(?:image_id|video_id)\\s*=\\s*(\\d+)");
- /** File supports transforms and uid requires transcoding */
+ /** File access by uid requires the transcoding transform */
private static final int FLAG_TRANSFORM_TRANSCODING = 1;
- /** File supports transforms */
- private static final int FLAG_TRANSFORM_SUPPORTED = 1 << 30;
/**
* These directory names aren't declared in Environment as final variables, and so we need to
* have the same values in separate final variables in order to have them considered constant
* expressions.
+ * These directory names are intentionally in lower case to ease the case insensitive path
+ * comparison.
*/
- private static final String DIRECTORY_MUSIC = "Music";
- private static final String DIRECTORY_PODCASTS = "Podcasts";
- private static final String DIRECTORY_RINGTONES = "Ringtones";
- private static final String DIRECTORY_ALARMS = "Alarms";
- private static final String DIRECTORY_NOTIFICATIONS = "Notifications";
- private static final String DIRECTORY_PICTURES = "Pictures";
- private static final String DIRECTORY_MOVIES = "Movies";
- private static final String DIRECTORY_DOWNLOADS = "Download";
- private static final String DIRECTORY_DCIM = "DCIM";
- private static final String DIRECTORY_DOCUMENTS = "Documents";
- private static final String DIRECTORY_AUDIOBOOKS = "Audiobooks";
- private static final String DIRECTORY_ANDROID = "Android";
+ private static final String DIRECTORY_MUSIC_LOWER_CASE = "music";
+ private static final String DIRECTORY_PODCASTS_LOWER_CASE = "podcasts";
+ private static final String DIRECTORY_RINGTONES_LOWER_CASE = "ringtones";
+ private static final String DIRECTORY_ALARMS_LOWER_CASE = "alarms";
+ private static final String DIRECTORY_NOTIFICATIONS_LOWER_CASE = "notifications";
+ private static final String DIRECTORY_PICTURES_LOWER_CASE = "pictures";
+ private static final String DIRECTORY_MOVIES_LOWER_CASE = "movies";
+ private static final String DIRECTORY_DOWNLOADS_LOWER_CASE = "download";
+ private static final String DIRECTORY_DCIM_LOWER_CASE = "dcim";
+ private static final String DIRECTORY_DOCUMENTS_LOWER_CASE = "documents";
+ private static final String DIRECTORY_AUDIOBOOKS_LOWER_CASE = "audiobooks";
+ private static final String DIRECTORY_ANDROID_LOWER_CASE = "android";
private static final String DIRECTORY_MEDIA = "media";
private static final String DIRECTORY_THUMBNAILS = ".thumbnails";
@@ -305,6 +305,7 @@
// Stolen from: UserHandle#getUserId
private static final int PER_USER_RANGE = 100000;
+ private static final int MY_UID = android.os.Process.myUid();
private static final boolean PROP_CROSS_USER_ALLOWED =
SystemProperties.getBoolean("external_storage.cross_user.enabled", false);
@@ -337,8 +338,8 @@
// WARNING/TODO (b/173505864): This will be replaced by signature APIs in S
private static final String DOWNLOADS_PROVIDER_AUTHORITY = "downloads";
- @GuardedBy("mShouldRedactThreadIds")
- private final LongArray mShouldRedactThreadIds = new LongArray();
+ @GuardedBy("mPendingOpenInfo")
+ private final Map<Integer, PendingOpenInfo> mPendingOpenInfo = new ArrayMap<>();
@GuardedBy("mNonHiddenPaths")
private final LRUCache<String, Integer> mNonHiddenPaths = new LRUCache<>(NON_HIDDEN_CACHE_SIZE);
@@ -1387,20 +1388,31 @@
* for transform lookup query for a file and uid.
*
* @param path file path to get transforms for
- * @param uid app requesting IO
+ * @param uid app requesting IO form kernel
+ * @param tid FUSE thread id handling IO request from kernel
*
* Called from JNI in jni/MediaProviderWrapper.cpp
*/
@Keep
- public FileLookupResult onFileLookupForFuse(String path, int uid) {
+ public FileLookupResult onFileLookupForFuse(String path, int uid, int tid) {
+ uid = getBinderUidForFuse(uid, tid);
String ioPath = "";
boolean transformsComplete = true;
- int transforms = getTransformsForFuse(path, uid);
- if (transforms != 0) {
- ioPath = getIoPathForFuse(path, uid);
- transformsComplete = Objects.equals(path, ioPath);
+ boolean transformsSupported = mTranscodeHelper.supportsTranscode(path);
+ int transforms = 0;
+
+ if (transformsSupported) {
+ // TODO(b/170974147): Avoid duplicate shouldTranscode calls in getTransformsForFuse and
+ // getIoPathForFuse
+ transforms = getTransformsForFuse(path, uid);
+ if (transforms != 0) {
+ ioPath = getIoPathForFuse(path, uid);
+ transformsComplete = false;
+ }
}
- return new FileLookupResult(transforms, transformsComplete, ioPath);
+
+ return new FileLookupResult(transforms, uid, transformsComplete, transformsSupported,
+ ioPath);
}
/**
@@ -1430,15 +1442,24 @@
* @see {@link transformForFuse}
*/
private int getTransformsForFuse(String path, int uid) {
- int result = 0;
- if (mTranscodeHelper.supportsTranscode(path)) {
- result |= FLAG_TRANSFORM_SUPPORTED;
-
- if (mTranscodeHelper.shouldTranscode(path, uid, null /* bundle */)) {
- result |= FLAG_TRANSFORM_TRANSCODING;
- }
+ if (mTranscodeHelper.shouldTranscode(path, uid, null /* bundle */)) {
+ return FLAG_TRANSFORM_TRANSCODING;
}
- return result;
+ return 0;
+ }
+
+ public int getBinderUidForFuse(int uid, int tid) {
+ if (uid != MY_UID) {
+ return uid;
+ }
+
+ synchronized (mPendingOpenInfo) {
+ PendingOpenInfo info = mPendingOpenInfo.get(tid);
+ if (info == null) {
+ return uid;
+ }
+ return info.uid;
+ }
}
/**
@@ -2015,12 +2036,12 @@
private ArrayList<String> getIncludedDefaultDirectories() {
final ArrayList<String> includedDefaultDirs = new ArrayList<>();
if (checkCallingPermissionVideo(/*forWrite*/ true, null)) {
- includedDefaultDirs.add(DIRECTORY_DCIM);
- includedDefaultDirs.add(DIRECTORY_PICTURES);
- includedDefaultDirs.add(DIRECTORY_MOVIES);
+ includedDefaultDirs.add(Environment.DIRECTORY_DCIM);
+ includedDefaultDirs.add(Environment.DIRECTORY_PICTURES);
+ includedDefaultDirs.add(Environment.DIRECTORY_MOVIES);
} else if (checkCallingPermissionImages(/*forWrite*/ true, null)) {
- includedDefaultDirs.add(DIRECTORY_DCIM);
- includedDefaultDirs.add(DIRECTORY_PICTURES);
+ includedDefaultDirs.add(Environment.DIRECTORY_DCIM);
+ includedDefaultDirs.add(Environment.DIRECTORY_PICTURES);
}
return includedDefaultDirs;
}
@@ -2398,10 +2419,11 @@
return OsConstants.EPERM;
}
+ // TODO(b/177049768): We shouldn't use getExternalStorageDirectory for these checks.
final File directoryAndroid = new File(Environment.getExternalStorageDirectory(),
- DIRECTORY_ANDROID);
+ DIRECTORY_ANDROID_LOWER_CASE);
final File directoryAndroidMedia = new File(directoryAndroid, DIRECTORY_MEDIA);
- if (directoryAndroidMedia.getAbsolutePath().equals(oldPath)) {
+ if (directoryAndroidMedia.getAbsolutePath().equalsIgnoreCase(oldPath)) {
// Don't allow renaming 'Android/media' directory.
// Android/[data|obb] are bind mounted and these paths don't go through FUSE.
Log.e(TAG, errorMessage + oldPath + " is a default folder in app external "
@@ -2980,7 +3002,7 @@
final String[] relativePath = values.getAsString(MediaColumns.RELATIVE_PATH).split("/");
final String primary = (relativePath.length > 0) ? relativePath[0] : null;
if (!validPath) {
- validPath = allowedPrimary.contains(primary);
+ validPath = containsIgnoreCase(allowedPrimary, primary);
}
// Next, consider allowing paths when referencing a related item
@@ -3119,7 +3141,7 @@
final String secondary =
(relativePathSegments.length > 1) ? relativePathSegments[1] : "";
- if (DIRECTORY_ANDROID.equalsIgnoreCase(primary)
+ if (DIRECTORY_ANDROID_LOWER_CASE.equalsIgnoreCase(primary)
&& PRIVATE_SUBDIRECTORIES_ANDROID.contains(
secondary.toLowerCase(Locale.ROOT))) {
throw new IllegalArgumentException(
@@ -5429,9 +5451,10 @@
private List<File> getThumbnailDirectories(String volumeName) throws FileNotFoundException {
final File volumePath = getVolumePath(volumeName);
return Arrays.asList(
- FileUtils.buildPath(volumePath, DIRECTORY_MUSIC, DIRECTORY_THUMBNAILS),
- FileUtils.buildPath(volumePath, DIRECTORY_MOVIES, DIRECTORY_THUMBNAILS),
- FileUtils.buildPath(volumePath, DIRECTORY_PICTURES, DIRECTORY_THUMBNAILS));
+ FileUtils.buildPath(volumePath, Environment.DIRECTORY_MUSIC, DIRECTORY_THUMBNAILS),
+ FileUtils.buildPath(volumePath, Environment.DIRECTORY_MOVIES, DIRECTORY_THUMBNAILS),
+ FileUtils.buildPath(volumePath, Environment.DIRECTORY_PICTURES,
+ DIRECTORY_THUMBNAILS));
}
private void invalidateThumbnails(Uri uri) {
@@ -6623,20 +6646,22 @@
return new File(filePath);
}
- private ParcelFileDescriptor openWithTranscode(String filePath, int uid, int modeBits)
- throws FileNotFoundException {
- String transcodePath = mTranscodeHelper.getIoPath(filePath, uid);
- File transcodeFile = new File(transcodePath);
+ private ParcelFileDescriptor openWithFuse(String filePath, int uid, int modeBits,
+ boolean shouldRedact, boolean shouldTranscode) throws FileNotFoundException {
+ Log.d(TAG, "Open with FUSE. FilePath: " + filePath + ". Uid: " + uid
+ + ". ShouldRedact: " + shouldRedact + ". ShouldTranscode: " + shouldTranscode);
- if (mTranscodeHelper.isTranscodeFileCached(uid, filePath, transcodePath)) {
- Log.d(TAG, "Using FUSE with transcode cache for " + filePath + " Uid: " + uid);
- return FileUtils.openSafely(getFuseFile(transcodeFile), modeBits);
- } else if (mTranscodeHelper.transcode(filePath, transcodePath, uid)) {
- // TODO(b/174655855): We should transcode lazily and just return the opened fd here
- Log.d(TAG, "Using FUSE with transcode for " + filePath + " Uid: " + uid);
- return FileUtils.openSafely(getFuseFile(transcodeFile), modeBits);
- } else {
- throw new FileNotFoundException("Failed to transcode " + filePath);
+ int tid = android.os.Process.myTid();
+ synchronized (mPendingOpenInfo) {
+ mPendingOpenInfo.put(tid, new PendingOpenInfo(uid, shouldRedact));
+ }
+
+ try {
+ return FileUtils.openSafely(getFuseFile(new File(filePath)), modeBits);
+ } finally {
+ synchronized (mPendingOpenInfo) {
+ mPendingOpenInfo.remove(tid);
+ }
}
}
@@ -6773,25 +6798,11 @@
if (redactionInfo.redactionRanges.length > 0) {
// If fuse is enabled, we can provide an fd that points to the fuse
// file system and handle redaction in the fuse handler when the caller reads.
- Log.i(TAG, "Redacting with new FUSE for " + filePath + ". Uid: " + uid);
- long tid = android.os.Process.myTid();
- synchronized (mShouldRedactThreadIds) {
- mShouldRedactThreadIds.add(tid);
- }
-
- try {
- if (shouldTranscode) {
- pfd = openWithTranscode(filePath, uid, modeBits);
- } else {
- pfd = FileUtils.openSafely(getFuseFile(file), modeBits);
- }
- } finally {
- synchronized (mShouldRedactThreadIds) {
- mShouldRedactThreadIds.remove(mShouldRedactThreadIds.indexOf(tid));
- }
- }
+ pfd = openWithFuse(filePath, uid, modeBits, true /* shouldRedact */,
+ shouldTranscode);
} else if (shouldTranscode) {
- pfd = openWithTranscode(filePath, uid, modeBits);
+ pfd = openWithFuse(filePath, uid, modeBits, false /* shouldRedact */,
+ shouldTranscode);
} else {
FuseDaemon daemon = null;
try {
@@ -6809,15 +6820,15 @@
// we return an upper filesystem fd (via FUSE) to avoid file corruption
// resulting from cache inconsistencies between the upper and lower
// filesystem caches
- Log.w(TAG, "Using FUSE for " + filePath + ". Uid: " + uid);
- pfd = FileUtils.openSafely(getFuseFile(file), modeBits);
+ pfd = openWithFuse(filePath, uid, modeBits, false /* shouldRedact */,
+ shouldTranscode);
try {
lowerFsFd.close();
} catch (IOException e) {
Log.w(TAG, "Failed to close lower filesystem fd " + file.getPath(), e);
}
} else {
- Log.i(TAG, "Using lower FS for " + filePath + ". Uid: " + uid);
+ Log.i(TAG, "Open with lower FS for " + filePath + ". Uid: " + uid);
if (forWrite) {
// When opening for write on the lower filesystem, invalidate the VFS dentry
// so subsequent open/getattr calls will return correctly.
@@ -7076,6 +7087,15 @@
}
}
+ private static final class PendingOpenInfo {
+ public final int uid;
+ public final boolean shouldRedact;
+ public PendingOpenInfo(int uid, boolean shouldRedact) {
+ this.uid = uid;
+ this.shouldRedact = shouldRedact;
+ }
+ }
+
/**
* Calculates the ranges that need to be redacted for the given file and user that wants to
* access the file.
@@ -7086,12 +7106,9 @@
* @return Ranges that should be redacted.
*
* @throws IOException if an error occurs while calculating the redaction ranges
- *
- * Called from JNI in jni/MediaProviderWrapper.cpp
*/
- @Keep
@NonNull
- public long[] getRedactionRangesForFuse(String path, String ioPath, int uid, int tid)
+ private long[] getRedactionRangesFromFuse(String path, String ioPath, int uid, int tid)
throws IOException {
// |ioPath| might refer to a transcoded file path (which is not indexed in the db)
// |path| will always refer to a valid _data column
@@ -7099,18 +7116,21 @@
// we want to get redaction ranges from the transcoded file and *not* the original file
final File file = new File(ioPath);
- // When we're calculating redaction ranges for MediaProvider, it means we're actually
- // calculating redaction ranges for another app that called to MediaProvider through Binder.
- // If the tid is in mShouldRedactThreadIds, we should redact, otherwise, we don't redact
- if (uid == android.os.Process.myUid()) {
- boolean shouldRedact = false;
- synchronized (mShouldRedactThreadIds) {
- shouldRedact = mShouldRedactThreadIds.indexOf(tid) != -1;
- }
- if (shouldRedact) {
- return getRedactionRanges(file).redactionRanges;
- } else {
- return new long[0];
+ // When calculating redaction ranges initiated from MediaProvider, the redaction policy
+ // is slightly different from the FUSE initiated opens redaction policy. targetSdk=29 from
+ // MediaProvider requires redaction, but targetSdk=29 apps from FUSE don't require redaction
+ // Hence, we check the mPendingOpenInfo object (populated when opens are initiated from
+ // MediaProvider) if there's a pending open from MediaProvider with matching tid and uid and
+ // use the shouldRedact decision there if there's one.
+ synchronized (mPendingOpenInfo) {
+ PendingOpenInfo info = mPendingOpenInfo.get(tid);
+ if (info != null && info.uid == uid) {
+ boolean shouldRedact = info.shouldRedact;
+ if (shouldRedact) {
+ return getRedactionRanges(file).redactionRanges;
+ } else {
+ return new long[0];
+ }
}
}
@@ -7240,23 +7260,27 @@
* Called from JNI in jni/MediaProviderWrapper.cpp
*/
@Keep
- public int isOpenAllowedForFuse(String path, int uid, boolean forWrite) {
+ public FileOpenResult onFileOpenForFuse(String path, String ioPath, int uid, int tid,
+ boolean forWrite, boolean redact) {
+ uid = getBinderUidForFuse(uid, tid);
+
final LocalCallingIdentity token =
clearLocalCallingIdentity(getCachedCallingIdentityForFuse(uid));
try {
if (isPrivatePackagePathNotAccessibleByCaller(path)) {
Log.e(TAG, "Can't open a file in another app's external directory!");
- return OsConstants.ENOENT;
+ return new FileOpenResult(OsConstants.ENOENT, uid, new long[0]);
}
if (shouldBypassFuseRestrictions(forWrite, path)) {
- return 0;
+ return new FileOpenResult(0 /* status */, uid,
+ redact ? getRedactionRangesFromFuse(path, ioPath, uid, tid) : new long[0]);
}
// Legacy apps that made is this far don't have the right storage permission and hence
// are not allowed to access anything other than their external app directory
if (isCallingPackageRequestingLegacy()) {
- return OsConstants.EACCES;
+ return new FileOpenResult(OsConstants.EACCES /* status */, uid, new long[0]);
}
final Uri contentUri = FileUtils.getContentUriForPath(path);
@@ -7283,18 +7307,20 @@
if (isPending && !isPendingFromFuse(new File(path))) {
requireOwnershipForItem(ownerPackageName, fileUri);
}
- return 0;
- } catch (FileNotFoundException e) {
+ return new FileOpenResult(0 /* status */, uid,
+ redact ? getRedactionRangesFromFuse(path, ioPath, uid, tid) : new long[0]);
+ } catch (IOException e) {
// We are here because
// * App doesn't have read permission to the requested path, hence queryForSingleItem
// couldn't return a valid db row, or,
// * There is no db row corresponding to the requested path, which is more unlikely.
- // In both of these cases, it means that app doesn't have access permission to the file.
+ // * getRedactionRangesFromFuse couldn't fetch the redaction info correctly
+ // In all of these cases, it means that app doesn't have access permission to the file.
Log.e(TAG, "Couldn't find file: " + path, e);
- return OsConstants.EACCES;
+ return new FileOpenResult(OsConstants.EACCES /* status */, uid, new long[0]);
} catch (IllegalStateException | SecurityException e) {
Log.e(TAG, "Permission to access file: " + path + " is denied");
- return OsConstants.EACCES;
+ return new FileOpenResult(OsConstants.EACCES /* status */, uid, new long[0]);
} finally {
restoreLocalCallingIdentity(token);
}
@@ -7328,21 +7354,23 @@
throw new IllegalStateException("Couldn't get volume name for " + filePath);
}
Uri uri = Files.getContentUri(volName);
- final String topLevelDir = extractTopLevelDir(filePath);
+ String topLevelDir = extractTopLevelDir(filePath);
if (topLevelDir == null) {
// If the file path doesn't match the external storage directory, we use the files URI
// as default and let #insert enforce the restrictions
return uri;
}
+ topLevelDir = topLevelDir.toLowerCase(Locale.ROOT);
+
switch (topLevelDir) {
- case DIRECTORY_PODCASTS:
- case DIRECTORY_RINGTONES:
- case DIRECTORY_ALARMS:
- case DIRECTORY_NOTIFICATIONS:
- case DIRECTORY_AUDIOBOOKS:
+ case DIRECTORY_PODCASTS_LOWER_CASE:
+ case DIRECTORY_RINGTONES_LOWER_CASE:
+ case DIRECTORY_ALARMS_LOWER_CASE:
+ case DIRECTORY_NOTIFICATIONS_LOWER_CASE:
+ case DIRECTORY_AUDIOBOOKS_LOWER_CASE:
uri = Audio.Media.getContentUri(volName);
break;
- case DIRECTORY_MUSIC:
+ case DIRECTORY_MUSIC_LOWER_CASE:
if (MimeUtils.isPlaylistMimeType(mimeType)) {
uri = Audio.Playlists.getContentUri(volName);
} else if (!MimeUtils.isSubtitleMimeType(mimeType)) {
@@ -7350,7 +7378,7 @@
uri = Audio.Media.getContentUri(volName);
}
break;
- case DIRECTORY_MOVIES:
+ case DIRECTORY_MOVIES_LOWER_CASE:
if (MimeUtils.isPlaylistMimeType(mimeType)) {
uri = Audio.Playlists.getContentUri(volName);
} else if (!MimeUtils.isSubtitleMimeType(mimeType)) {
@@ -7358,16 +7386,16 @@
uri = Video.Media.getContentUri(volName);
}
break;
- case DIRECTORY_DCIM:
- case DIRECTORY_PICTURES:
+ case DIRECTORY_DCIM_LOWER_CASE:
+ case DIRECTORY_PICTURES_LOWER_CASE:
if (MimeUtils.isImageMimeType(mimeType)) {
uri = Images.Media.getContentUri(volName);
} else {
uri = Video.Media.getContentUri(volName);
}
break;
- case DIRECTORY_DOWNLOADS:
- case DIRECTORY_DOCUMENTS:
+ case DIRECTORY_DOWNLOADS_LOWER_CASE:
+ case DIRECTORY_DOCUMENTS_LOWER_CASE:
break;
default:
Log.w(TAG, "Forgot to handle a top level directory in getContentUriForFile?");
@@ -7375,6 +7403,15 @@
return uri;
}
+ private boolean containsIgnoreCase(@Nullable List<String> stringsList, @Nullable String item) {
+ if (item == null || stringsList == null) return false;
+
+ for (String current : stringsList) {
+ if (item.equalsIgnoreCase(current)) return true;
+ }
+ return false;
+ }
+
private boolean fileExists(@NonNull String absolutePath) {
// We don't care about specific columns in the match,
// we just want to check IF there's a match
diff --git a/src/com/android/providers/media/TranscodeHelper.java b/src/com/android/providers/media/TranscodeHelper.java
index e8d050e..95d6ba0 100644
--- a/src/com/android/providers/media/TranscodeHelper.java
+++ b/src/com/android/providers/media/TranscodeHelper.java
@@ -39,7 +39,6 @@
import android.content.ContentValues;
import android.content.Context;
import android.content.pm.PackageManager;
-import android.content.pm.PackageManager.NameNotFoundException;
import android.content.pm.PackageManager.Property;
import android.content.res.XmlResourceParser;
import android.database.Cursor;
@@ -85,6 +84,7 @@
import java.io.RandomAccessFile;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
+import java.util.Locale;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
@@ -487,8 +487,9 @@
final String cameraRelativePath =
String.format("%s/%s/", Environment.DIRECTORY_DCIM, DIRECTORY_CAMERA);
- return !isTranscodeFile(path) && name.endsWith(".mp4") &&
- cameraRelativePath.equalsIgnoreCase(FileUtils.extractRelativePath(path));
+ return !isTranscodeFile(path) && name.toLowerCase(Locale.ROOT).endsWith(".mp4")
+ && path.startsWith("/storage/emulated/")
+ && cameraRelativePath.equalsIgnoreCase(FileUtils.extractRelativePath(path));
}
/**
@@ -517,13 +518,16 @@
identity.setApplicationMediaCapabilitiesFlags(capabilitiesToFlags(capability));
return capability.isVideoMimeTypeSupported(MediaFormat.MIMETYPE_VIDEO_HEVC);
- } catch (NameNotFoundException | UnsupportedOperationException e) {
+ } catch (PackageManager.NameNotFoundException
+ | ApplicationMediaCapabilities.FormatNotFoundException
+ | UnsupportedOperationException e) {
return false;
}
}
@ApplicationMediaCapabilitiesFlags
- private int capabilitiesToFlags(ApplicationMediaCapabilities capability) {
+ private int capabilitiesToFlags(ApplicationMediaCapabilities capability)
+ throws ApplicationMediaCapabilities.FormatNotFoundException {
int flags = 0;
if (capability.isVideoMimeTypeSupported(MediaFormat.MIMETYPE_VIDEO_HEVC)) {
flags |= FLAG_HEVC;
@@ -638,7 +642,8 @@
FileColumns._VIDEO_CODEC_TYPE,
MediaStore.MediaColumns.WIDTH,
MediaStore.MediaColumns.HEIGHT,
- MediaStore.MediaColumns.BITRATE
+ MediaStore.MediaColumns.BITRATE,
+ MediaStore.MediaColumns.CAPTURE_FRAMERATE
};
try (Cursor c = queryFileForTranscode(path, resolverInfoProjection)) {
if (c != null && c.moveToNext()) {
@@ -646,19 +651,22 @@
int width = c.getInt(1);
int height = c.getInt(2);
int bitRate = c.getInt(3);
+ float framerate = c.getFloat(4);
// TODO(b/169849854): Get this info from Manifest, for now if app got here it
// definitely doesn't support hevc
ApplicationMediaCapabilities capability =
new ApplicationMediaCapabilities.Builder().build();
+ MediaFormat sourceFormat = MediaFormat.createVideoFormat(
+ codecType, width, height);
+ sourceFormat.setFloat(MediaFormat.KEY_FRAME_RATE, framerate);
MediaFormatResolver resolver = new MediaFormatResolver()
- .setSourceVideoFormatHint(MediaFormat.createVideoFormat(
- codecType, width, height))
+ .setSourceVideoFormatHint(sourceFormat)
.setClientCapabilities(capability);
- MediaFormat format = resolver.resolveVideoFormat();
- format.setInteger(MediaFormat.KEY_BIT_RATE, bitRate);
+ MediaFormat resolvedFormat = resolver.resolveVideoFormat();
+ resolvedFormat.setInteger(MediaFormat.KEY_BIT_RATE, bitRate);
- return format;
+ return resolvedFormat;
}
}
throw new IllegalStateException("Couldn't get video format info from database for " + path);
diff --git a/tests/client/src/com/android/providers/media/client/LegacyProviderMigrationTest.java b/tests/client/src/com/android/providers/media/client/LegacyProviderMigrationTest.java
index 9d012f6..8014b35 100644
--- a/tests/client/src/com/android/providers/media/client/LegacyProviderMigrationTest.java
+++ b/tests/client/src/com/android/providers/media/client/LegacyProviderMigrationTest.java
@@ -53,6 +53,7 @@
import android.webkit.MimeTypeMap;
import androidx.annotation.NonNull;
+import androidx.test.filters.FlakyTest;
import androidx.test.InstrumentationRegistry;
import androidx.test.runner.AndroidJUnit4;
@@ -82,6 +83,7 @@
* {@link MediaColumns#IS_FAVORITE} should be retained.
*/
@RunWith(AndroidJUnit4.class)
+@FlakyTest(bugId = 176977253)
public class LegacyProviderMigrationTest {
private static final String TAG = "LegacyTest";
diff --git a/tests/client/src/com/android/providers/media/client/PerformanceTest.java b/tests/client/src/com/android/providers/media/client/PerformanceTest.java
index 5f685cc..30c815d 100644
--- a/tests/client/src/com/android/providers/media/client/PerformanceTest.java
+++ b/tests/client/src/com/android/providers/media/client/PerformanceTest.java
@@ -88,15 +88,15 @@
// Verify that core actions finished within 30ms deadline
final long actionDeadline = 30;
- assertTrue(timers.actionInsert.getAverageDurationMillis() < actionDeadline);
- assertTrue(timers.actionUpdate.getAverageDurationMillis() < actionDeadline);
- assertTrue(timers.actionDelete.getAverageDurationMillis() < actionDeadline);
+ assertThat(timers.actionInsert.getAverageDurationMillis()).isLessThan(actionDeadline);
+ assertThat(timers.actionUpdate.getAverageDurationMillis()).isLessThan(actionDeadline);
+ assertThat(timers.actionDelete.getAverageDurationMillis()).isLessThan(actionDeadline);
// Verify that external notifications finished within 30ms deadline
final long notifyDeadline = 30;
- assertTrue(timers.notifyInsert.getAverageDurationMillis() < notifyDeadline);
- assertTrue(timers.notifyUpdate.getAverageDurationMillis() < notifyDeadline);
- assertTrue(timers.notifyDelete.getAverageDurationMillis() < notifyDeadline);
+ assertThat(timers.notifyInsert.getAverageDurationMillis()).isLessThan(notifyDeadline);
+ assertThat(timers.notifyUpdate.getAverageDurationMillis()).isLessThan(notifyDeadline);
+ assertThat(timers.notifyDelete.getAverageDurationMillis()).isLessThan(notifyDeadline);
}
private void doSingle(Uri collection, Timers timers) throws Exception {
@@ -167,15 +167,15 @@
// Verify that core actions finished within 30ms deadline
final long actionDeadline = 30 * COUNT_BULK;
- assertTrue(timers.actionInsert.getAverageDurationMillis() < actionDeadline);
- assertTrue(timers.actionUpdate.getAverageDurationMillis() < actionDeadline);
- assertTrue(timers.actionDelete.getAverageDurationMillis() < actionDeadline);
+ assertThat(timers.actionInsert.getAverageDurationMillis()).isLessThan(actionDeadline);
+ assertThat(timers.actionUpdate.getAverageDurationMillis()).isLessThan(actionDeadline);
+ assertThat(timers.actionDelete.getAverageDurationMillis()).isLessThan(actionDeadline);
// Verify that external notifications finished within 100ms deadline
final long notifyDeadline = 100;
- assertTrue(timers.notifyInsert.getAverageDurationMillis() < notifyDeadline);
- assertTrue(timers.notifyUpdate.getAverageDurationMillis() < notifyDeadline);
- assertTrue(timers.notifyDelete.getAverageDurationMillis() < notifyDeadline);
+ assertThat(timers.notifyInsert.getAverageDurationMillis()).isLessThan(notifyDeadline);
+ assertThat(timers.notifyUpdate.getAverageDurationMillis()).isLessThan(notifyDeadline);
+ assertThat(timers.notifyDelete.getAverageDurationMillis()).isLessThan(notifyDeadline);
}
private void doBulk(Uri collection, Timers timers) throws Exception {
diff --git a/tests/src/com/android/providers/media/MediaProviderForFuseTest.java b/tests/src/com/android/providers/media/MediaProviderForFuseTest.java
index 752f474..486def1 100644
--- a/tests/src/com/android/providers/media/MediaProviderForFuseTest.java
+++ b/tests/src/com/android/providers/media/MediaProviderForFuseTest.java
@@ -95,12 +95,11 @@
file.createNewFile();
// We can write our file
- Truth.assertThat(sMediaProvider.isOpenAllowedForFuse(
- file.getPath(), sTestUid, true)).isEqualTo(0);
-
- // We should have no redaction
- Truth.assertThat(sMediaProvider.getRedactionRangesForFuse(
- file.getPath(), file.getPath(), sTestUid, 0)).isEqualTo(new long[0]);
+ FileOpenResult result = sMediaProvider.onFileOpenForFuse(
+ file.getPath(), file.getPath(), sTestUid, 0 /* tid */, true /* forWrite */,
+ false /* redact */);
+ Truth.assertThat(result.status).isEqualTo(0);
+ Truth.assertThat(result.redactionRanges).isEqualTo(new long[0]);
// We can rename our file
final File renamed = new File(sTestDir, "renamed" + System.nanoTime() + ".jpg");
diff --git a/tests/transcode/src/com/android/providers/media/transcode/TranscodeTest.java b/tests/transcode/src/com/android/providers/media/transcode/TranscodeTest.java
index 83ad729..db02a8f 100644
--- a/tests/transcode/src/com/android/providers/media/transcode/TranscodeTest.java
+++ b/tests/transcode/src/com/android/providers/media/transcode/TranscodeTest.java
@@ -52,7 +52,6 @@
import org.junit.After;
import org.junit.Assume;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -353,7 +352,6 @@
* Tests that transcode cache is reused after ContentResolver transcode
* @throws Exception
*/
- @Ignore("b/174655855")
@Test
public void testTranscodedCacheReuse_ContentResolver() throws Exception {
File modernFile = new File(DIR_CAMERA, HEVC_FILE_NAME);
@@ -373,7 +371,6 @@
* and file path opens
* @throws Exception
*/
- @Ignore("b/174655855")
@Test
public void testTranscodedCacheReuse_ContentResolverFilePath() throws Exception {
File modernFile = new File(DIR_CAMERA, HEVC_FILE_NAME);