Merge "Add test for LocalCallingIdentity changes" into rvc-dev
diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
index 208a074..a014489 100644
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -19,6 +19,7 @@
#include "FuseDaemon.h"
#include <android-base/logging.h>
+#include <android-base/properties.h>
#include <android/log.h>
#include <android/trace.h>
#include <ctype.h>
@@ -75,7 +76,7 @@
// logging macros to avoid duplication.
#define TRACE_NODE(__node) \
- LOG(DEBUG) << __FUNCTION__ << " : " << #__node << " = [" << safe_name(__node) << "] "
+ LOG(VERBOSE) << __FUNCTION__ << " : " << #__node << " = [" << get_name(__node) << "] "
#define ATRACE_NAME(name) ScopedTrace ___tracer(name)
#define ATRACE_CALL() ATRACE_NAME(__FUNCTION__)
@@ -91,24 +92,21 @@
}
};
+const bool IS_OS_DEBUGABLE = android::base::GetIntProperty("ro.debuggable", 0);
+
#define FUSE_UNKNOWN_INO 0xffffffff
constexpr size_t MAX_READ_SIZE = 128 * 1024;
// Stolen from: UserHandle#getUserId
constexpr int PER_USER_RANGE = 100000;
-// Cache inode attributes for a 'short' time so that performance is decent and last modified time
-// stamps are not too stale
-constexpr double DEFAULT_ATTR_TIMEOUT_SECONDS = 10;
-// Ensure the VFS does not cache dentries, if it caches, the following scenario could occur:
-// 1. Process A has access to file A and does a lookup
-// 2. Process B does not have access to file A and does a lookup
-// (2) will succeed because the lookup request will not be sent from kernel to the FUSE daemon
-// and the kernel will respond from cache. Even if this by itself is not a security risk
-// because subsequent FUSE requests will fail if B does not have access to the resource.
-// It does cause indeterministic behavior because whether (2) succeeds or not depends on if
-// (1) occurred.
-// We prevent this caching by setting the entry_timeout value to 0.
-constexpr double DEFAULT_ENTRY_TIMEOUT_SECONDS = 0;
+// Cache inode attributes forever to improve performance
+// Whenver attributes could have changed on the lower filesystem outside the FUSE driver, we call
+// fuse_invalidate_entry_cache
+constexpr double DEFAULT_ATTR_TIMEOUT_SECONDS = std::numeric_limits<double>::max();
+// Cache dentries forever to improve performance
+// Whenver attributes could have changed on the lower filesystem outside the FUSE driver, we call
+// fuse_invalidate_entry_cache
+constexpr double DEFAULT_ENTRY_TIMEOUT_SECONDS = std::numeric_limits<double>::max();
/*
* In order to avoid double caching with fuse, call fadvise on the file handles
@@ -287,10 +285,19 @@
/* const */ char* zero_addr;
FAdviser fadviser;
+
+ std::atomic_bool* active;
};
-static inline string safe_name(node* n) {
- return n ? n->BuildSafePath() : "?";
+static inline string get_name(node* n) {
+ if (n) {
+ std::string name("node_path: " + n->BuildSafePath());
+ if (IS_OS_DEBUGABLE) {
+ name += " real_path: " + n->BuildPath();
+ }
+ return name;
+ }
+ return "?";
}
static inline __u64 ptr_to_id(void* ptr) {
@@ -356,7 +363,7 @@
static double get_attr_timeout(const string& path, uid_t uid, struct fuse* fuse, node* parent) {
if (fuse->IsRoot(parent) || is_android_path(path, fuse->path, uid)) {
// The /0 and /0/Android attrs can be always cached, as they never change
- return DBL_MAX;
+ return std::numeric_limits<double>::max();
} else {
return DEFAULT_ATTR_TIMEOUT_SECONDS;
}
@@ -365,7 +372,7 @@
static double get_entry_timeout(const string& path, uid_t uid, struct fuse* fuse, node* parent) {
if (fuse->IsRoot(parent) || is_android_path(path, fuse->path, uid)) {
// The /0 and /0/Android dentries can be always cached, as they are visible to all apps
- return DBL_MAX;
+ return std::numeric_limits<double>::max();
} else {
return DEFAULT_ENTRY_TIMEOUT_SECONDS;
}
@@ -423,6 +430,9 @@
FUSE_CAP_EXPORT_SUPPORT | FUSE_CAP_FLOCK_LOCKS);
conn->want |= conn->capable & mask;
conn->max_read = MAX_READ_SIZE;
+
+ struct fuse* fuse = reinterpret_cast<struct fuse*>(userdata);
+ fuse->active->store(true, std::memory_order_release);
}
static void pf_destroy(void* userdata) {
@@ -1491,6 +1501,7 @@
}
void FuseDaemon::InvalidateFuseDentryCache(const std::string& path) {
+ LOG(VERBOSE) << "Invalidating FUSE dentry cache";
if (active.load(std::memory_order_acquire)) {
string name;
fuse_ino_t parent;
@@ -1516,6 +1527,10 @@
FuseDaemon::FuseDaemon(JNIEnv* env, jobject mediaProvider) : mp(env, mediaProvider),
active(false), fuse(nullptr) {}
+bool FuseDaemon::IsStarted() const {
+ return active.load(std::memory_order_acquire);
+}
+
void FuseDaemon::Start(const int fd, const std::string& path) {
struct fuse_args args;
struct fuse_cmdline_opts opts;
@@ -1566,6 +1581,7 @@
return;
}
fuse_default.se = se;
+ fuse_default.active = &active;
se->fd = fd;
se->mountpoint = strdup(path.c_str());
@@ -1573,8 +1589,8 @@
// fuse_session_loop(se);
// Multi-threaded
LOG(INFO) << "Starting fuse...";
- active.store(true, std::memory_order_release);
fuse_session_loop_mt(se, &config);
+ fuse->active->store(false, std::memory_order_release);
LOG(INFO) << "Ending fuse...";
if (munmap(fuse_default.zero_addr, MAX_READ_SIZE)) {
@@ -1583,7 +1599,6 @@
fuse_opt_free_args(&args);
fuse_session_destroy(se);
- active.store(false, std::memory_order_relaxed);
LOG(INFO) << "Ended fuse";
return;
}
diff --git a/jni/FuseDaemon.h b/jni/FuseDaemon.h
index 1d25636..d9925d4 100644
--- a/jni/FuseDaemon.h
+++ b/jni/FuseDaemon.h
@@ -38,6 +38,11 @@
void Start(const int fd, const std::string& path);
/**
+ * Checks if the FUSE daemon is started.
+ */
+ bool IsStarted() const;
+
+ /**
* Check if file should be opened with FUSE
*/
bool ShouldOpenWithFuse(int fd, bool for_read, const std::string& path);
diff --git a/jni/com_android_providers_media_FuseDaemon.cpp b/jni/com_android_providers_media_FuseDaemon.cpp
index bede40a..fe4848f 100644
--- a/jni/com_android_providers_media_FuseDaemon.cpp
+++ b/jni/com_android_providers_media_FuseDaemon.cpp
@@ -50,6 +50,13 @@
daemon->Start(fd, utf_chars_path.c_str());
}
+bool com_android_providers_media_FuseDaemon_is_started(JNIEnv* env, jobject self,
+ jlong java_daemon) {
+ LOG(DEBUG) << "Checking if FUSE daemon started...";
+ const fuse::FuseDaemon* daemon = reinterpret_cast<fuse::FuseDaemon*>(java_daemon);
+ return daemon->IsStarted();
+}
+
void com_android_providers_media_FuseDaemon_delete(JNIEnv* env, jobject self, jlong java_daemon) {
LOG(DEBUG) << "Destroying the FUSE daemon...";
fuse::FuseDaemon* const daemon = reinterpret_cast<fuse::FuseDaemon*>(java_daemon);
@@ -85,6 +92,7 @@
return;
}
+ CHECK_EQ(pthread_getspecific(fuse::MediaProviderWrapper::gJniEnvKey), nullptr);
daemon->InvalidateFuseDentryCache(utf_chars_path.c_str());
}
// TODO(b/145741152): Throw exception
@@ -106,6 +114,8 @@
reinterpret_cast<void*>(com_android_providers_media_FuseDaemon_should_open_with_fuse)},
{"native_is_fuse_thread", "()Z",
reinterpret_cast<void*>(com_android_providers_media_FuseDaemon_is_fuse_thread)},
+ {"native_is_started", "(J)Z",
+ reinterpret_cast<void*>(com_android_providers_media_FuseDaemon_is_started)},
{"native_invalidate_fuse_dentry_cache", "(JLjava/lang/String;)V",
reinterpret_cast<void*>(
com_android_providers_media_FuseDaemon_invalidate_fuse_dentry_cache)}};
diff --git a/src/com/android/providers/media/LocalCallingIdentity.java b/src/com/android/providers/media/LocalCallingIdentity.java
index 1632b47..628f9cb 100644
--- a/src/com/android/providers/media/LocalCallingIdentity.java
+++ b/src/com/android/providers/media/LocalCallingIdentity.java
@@ -57,15 +57,15 @@
public final int pid;
public final int uid;
public final String packageNameUnchecked;
- public @Nullable String featureId;
+ public @Nullable String attributionTag;
private LocalCallingIdentity(Context context, int pid, int uid, String packageNameUnchecked,
- @Nullable String featureId) {
+ @Nullable String attributionTag) {
this.context = context;
this.pid = pid;
this.uid = uid;
this.packageNameUnchecked = packageNameUnchecked;
- this.featureId = featureId;
+ this.attributionTag = attributionTag;
}
/**
@@ -109,12 +109,12 @@
if (callingPackage == null) {
callingPackage = context.getOpPackageName();
}
- String callingFeatureId = provider.getCallingFeatureId();
- if (callingFeatureId == null) {
- callingFeatureId = context.getFeatureId();
+ String callingAttributionTag = provider.getCallingAttributionTag();
+ if (callingAttributionTag == null) {
+ callingAttributionTag = context.getAttributionTag();
}
return new LocalCallingIdentity(context, Binder.getCallingPid(), Binder.getCallingUid(),
- callingPackage, callingFeatureId);
+ callingPackage, callingAttributionTag);
}
public static LocalCallingIdentity fromExternal(Context context, int uid) {
@@ -134,8 +134,8 @@
}
public static LocalCallingIdentity fromExternal(Context context, int uid, String packageName,
- @Nullable String featureId) {
- return new LocalCallingIdentity(context, -1, uid, packageName, featureId);
+ @Nullable String attributionTag) {
+ return new LocalCallingIdentity(context, -1, uid, packageName, attributionTag);
}
public static LocalCallingIdentity fromSelf(Context context) {
@@ -144,11 +144,11 @@
android.os.Process.myPid(),
android.os.Process.myUid(),
context.getOpPackageName(),
- context.getFeatureId());
+ context.getAttributionTag());
ident.packageName = ident.packageNameUnchecked;
ident.packageNameResolved = true;
- // Use ident.featureId from context, hence no change
+ // Use ident.attributionTag from context, hence no change
ident.targetSdkVersion = Build.VERSION_CODES.CUR_DEVELOPMENT;
ident.targetSdkVersionResolved = true;
ident.hasPermission = ~(PERMISSION_IS_LEGACY_GRANTED | PERMISSION_IS_LEGACY_WRITE
@@ -335,7 +335,7 @@
if (context.checkPermission(ACCESS_MEDIA_LOCATION, pid, uid) == PERMISSION_DENIED
|| context.getSystemService(AppOpsManager.class).noteProxyOpNoThrow(
- permissionToOp(ACCESS_MEDIA_LOCATION), getPackageName(), uid, featureId, null)
+ permissionToOp(ACCESS_MEDIA_LOCATION), getPackageName(), uid, attributionTag, null)
!= MODE_ALLOWED) {
return true;
}
diff --git a/src/com/android/providers/media/MediaDocumentsProvider.java b/src/com/android/providers/media/MediaDocumentsProvider.java
index d7caf18..d90a5e0 100644
--- a/src/com/android/providers/media/MediaDocumentsProvider.java
+++ b/src/com/android/providers/media/MediaDocumentsProvider.java
@@ -142,10 +142,6 @@
public static final String METADATA_KEY_AUDIO = "android.media.metadata.audio";
public static final String METADATA_KEY_VIDEO = "android.media.metadata.video";
- // Video lat/long are just that. Lat/long. Unlike EXIF where the values are
- // in fact some funky string encoding. So we add our own contstant to convey coords.
- public static final String METADATA_VIDEO_LATITUDE = "android.media.metadata.video:latitude";
- public static final String METADATA_VIDEO_LONGITUTE = "android.media.metadata.video:longitude";
/*
* A mapping between media columns and metadata tag names. These keys of the
@@ -164,14 +160,10 @@
IMAGE_COLUMN_MAP.put(ImageColumns.WIDTH, ExifInterface.TAG_IMAGE_WIDTH);
IMAGE_COLUMN_MAP.put(ImageColumns.HEIGHT, ExifInterface.TAG_IMAGE_LENGTH);
IMAGE_COLUMN_MAP.put(ImageColumns.DATE_TAKEN, ExifInterface.TAG_DATETIME);
- IMAGE_COLUMN_MAP.put(ImageColumns.LATITUDE, ExifInterface.TAG_GPS_LATITUDE);
- IMAGE_COLUMN_MAP.put(ImageColumns.LONGITUDE, ExifInterface.TAG_GPS_LONGITUDE);
VIDEO_COLUMN_MAP.put(VideoColumns.DURATION, MediaMetadata.METADATA_KEY_DURATION);
VIDEO_COLUMN_MAP.put(VideoColumns.HEIGHT, ExifInterface.TAG_IMAGE_LENGTH);
VIDEO_COLUMN_MAP.put(VideoColumns.WIDTH, ExifInterface.TAG_IMAGE_WIDTH);
- VIDEO_COLUMN_MAP.put(VideoColumns.LATITUDE, METADATA_VIDEO_LATITUDE);
- VIDEO_COLUMN_MAP.put(VideoColumns.LONGITUDE, METADATA_VIDEO_LONGITUTE);
VIDEO_COLUMN_MAP.put(VideoColumns.DATE_TAKEN, MediaMetadata.METADATA_KEY_DATE);
AUDIO_COLUMN_MAP.put(AudioColumns.ARTIST, MediaMetadata.METADATA_KEY_ARTIST);
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 5a3c14a..02b9e66 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -4577,6 +4577,12 @@
Log.d(TAG, "Moving " + beforePath + " to " + afterPath);
try {
Os.rename(beforePath, afterPath);
+ if (!FuseDaemon.native_is_fuse_thread()) {
+ // If we are on a FUSE thread, we don't need to invalidate,
+ // (and *must* not, otherwise we'd crash) because the rename is already
+ // reflected in the lower filesystem
+ invalidateFuseDentry(beforePath);
+ }
} catch (ErrnoException e) {
throw new IllegalStateException(e);
}
@@ -5029,6 +5035,15 @@
return ExternalStorageServiceImpl.getFuseDaemon(volume.getId());
}
+ private void invalidateFuseDentry(String path) {
+ FuseDaemon daemon = getFuseDaemonForFile(new File(path));
+ if (daemon != null) {
+ daemon.invalidateFuseDentryCache(path);
+ } else {
+ Log.w(TAG, "Failed to invalidate FUSE dentry. Daemon unavailable for path " + path);
+ }
+ }
+
/**
* Replacement for {@link #openFileHelper(Uri, String)} which enforces any
* permissions applicable to the path before returning.
@@ -5180,6 +5195,12 @@
final File file = new File(path);
checkAccess(uri, extras, file, true);
file.delete();
+ if (!FuseDaemon.native_is_fuse_thread()) {
+ // If we are on a FUSE thread, we don't need to invalidate,
+ // (and *must* not, otherwise we'd crash) because the delete is already
+ // reflected in the lower filesystem
+ invalidateFuseDentry(path);
+ }
} catch (Exception e) {
Log.e(TAG, "Couldn't delete " + path, e);
}
diff --git a/src/com/android/providers/media/fuse/FuseDaemon.java b/src/com/android/providers/media/fuse/FuseDaemon.java
index c3f0e5f..95a160b 100644
--- a/src/com/android/providers/media/fuse/FuseDaemon.java
+++ b/src/com/android/providers/media/fuse/FuseDaemon.java
@@ -16,11 +16,13 @@
package com.android.providers.media.fuse;
+import android.os.ConditionVariable;
import android.os.ParcelFileDescriptor;
import android.util.Log;
import androidx.annotation.NonNull;
+import com.android.internal.annotations.GuardedBy;
import com.android.providers.media.MediaProvider;
import java.util.Objects;
@@ -30,11 +32,15 @@
*/
public final class FuseDaemon extends Thread {
public static final String TAG = "FuseDaemonThread";
+ private static final int POLL_INTERVAL_MS = 1000;
+ private static final int POLL_COUNT = 5;
+ private final Object mLock = new Object();
private final MediaProvider mMediaProvider;
private final int mFuseDeviceFd;
private final String mPath;
private final ExternalStorageServiceImpl mService;
+ @GuardedBy("mLock")
private long mPtr;
public FuseDaemon(@NonNull MediaProvider mediaProvider,
@@ -50,17 +56,18 @@
/** Starts a FUSE session. Does not return until the lower filesystem is unmounted. */
@Override
public void run() {
- mPtr = native_new(mMediaProvider);
- if (mPtr == 0) {
- return;
+ synchronized (mLock) {
+ mPtr = native_new(mMediaProvider);
+ if (mPtr == 0) {
+ throw new IllegalStateException("Unable to create native FUSE daemon");
+ }
}
Log.i(TAG, "Starting thread for " + getName() + " ...");
native_start(mPtr, mFuseDeviceFd, mPath); // Blocks
Log.i(TAG, "Exiting thread for " + getName() + " ...");
- // Cleanup
- if (mPtr != 0) {
+ synchronized (mLock) {
native_delete(mPtr);
mPtr = 0;
}
@@ -68,25 +75,50 @@
Log.i(TAG, "Exited thread for " + getName());
}
+ @Override
+ public void start() {
+ super.start();
+
+ // Wait for native_start
+ waitForStart();
+ }
+
+ private void waitForStart() {
+ int count = POLL_COUNT;
+ while (count-- > 0) {
+ synchronized (mLock) {
+ if (mPtr != 0 && native_is_started(mPtr)) {
+ return;
+ }
+ }
+ try {
+ Log.v(TAG, "Waiting " + POLL_INTERVAL_MS + "ms for FUSE start. Count " + count);
+ Thread.sleep(POLL_INTERVAL_MS);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ Log.e(TAG, "Interrupted while starting FUSE", e);
+ }
+ }
+ throw new IllegalStateException("Failed to start FUSE");
+ }
+
/** Waits for any running FUSE sessions to return. */
public void waitForExit() {
- Log.i(TAG, "Waiting 5s for thread " + getName() + " to exit...");
+ int waitMs = POLL_COUNT * POLL_INTERVAL_MS;
+ Log.i(TAG, "Waiting " + waitMs + "ms for FUSE " + getName() + " to exit...");
try {
- join(5000);
+ join(waitMs);
} catch (InterruptedException e) {
- Log.e(TAG, "Interrupted while waiting for thread " + getName()
- + " to exit. Terminating process", e);
- System.exit(1);
+ Thread.currentThread().interrupt();
+ throw new IllegalStateException("Interrupted while terminating FUSE " + getName());
}
if (isAlive()) {
- Log.i(TAG, "Failed to exit thread " + getName()
- + " successfully. Terminating process");
- System.exit(1);
+ throw new IllegalStateException("Failed to exit FUSE " + getName() + " successfully");
}
- Log.i(TAG, "Exited thread " + getName() + " successfully");
+ Log.i(TAG, "Exited FUSE " + getName() + " successfully");
}
/**
@@ -96,14 +128,26 @@
* @return {@code true} if the file should be opened via FUSE, {@code false} otherwise
*/
public boolean shouldOpenWithFuse(String path, boolean readLock, int fd) {
- return native_should_open_with_fuse(mPtr, path, readLock, fd);
+ synchronized (mLock) {
+ if (mPtr == 0) {
+ Log.i(TAG, "shouldOpenWithFuse failed, FUSE daemon unavailable");
+ return false;
+ }
+ return native_should_open_with_fuse(mPtr, path, readLock, fd);
+ }
}
/**
* Invalidates FUSE VFS dentry cache for {@code path}
*/
public void invalidateFuseDentryCache(String path) {
- native_invalidate_fuse_dentry_cache(mPtr, path);
+ synchronized (mLock) {
+ if (mPtr == 0) {
+ Log.i(TAG, "invalidateFuseDentryCache failed, FUSE daemon unavailable");
+ return;
+ }
+ native_invalidate_fuse_dentry_cache(mPtr, path);
+ }
}
private native long native_new(MediaProvider mediaProvider);
@@ -112,5 +156,6 @@
private native boolean native_should_open_with_fuse(long daemon, String path, boolean readLock,
int fd);
private native void native_invalidate_fuse_dentry_cache(long daemon, String path);
+ private native boolean native_is_started(long daemon);
public static native boolean native_is_fuse_thread();
}
diff --git a/src/com/android/providers/media/scan/ModernMediaScanner.java b/src/com/android/providers/media/scan/ModernMediaScanner.java
index a2333cb..81a6a34 100644
--- a/src/com/android/providers/media/scan/ModernMediaScanner.java
+++ b/src/com/android/providers/media/scan/ModernMediaScanner.java
@@ -117,6 +117,7 @@
import java.util.TimeZone;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
+import java.util.regex.Matcher;
import java.util.regex.Pattern;
/**
@@ -159,6 +160,11 @@
private static final Pattern PATTERN_INVISIBLE = Pattern.compile(
"(?i)^/storage/[^/]+(?:/[0-9]+)?(?:/Android/sandbox/([^/]+))?/Android/(?:data|obb)$");
+ private static final Pattern PATTERN_YEAR = Pattern.compile("([1-9][0-9][0-9][0-9])");
+
+ private static final Pattern PATTERN_ALBUM_ART = Pattern.compile(
+ "(?i)(?:(?:^folder|(?:^AlbumArt(?:(?:_\\{.*\\}_)?(?:small|large))?))(?:\\.jpg$)|(?:\\._.*))");
+
private final Context mContext;
private final DrmManagerClient mDrmClient;
@@ -693,7 +699,10 @@
return scanItemDirectory(existingId, file, attrs, mimeType, volumeName);
}
- final int mediaType = MimeUtils.resolveMediaType(mimeType);
+ int mediaType = MimeUtils.resolveMediaType(mimeType);
+ if (mediaType == FileColumns.MEDIA_TYPE_IMAGE && isFileAlbumArt(name)) {
+ mediaType = FileColumns.MEDIA_TYPE_NONE;
+ }
switch (mediaType) {
case FileColumns.MEDIA_TYPE_AUDIO:
return scanItemAudio(existingId, file, attrs, mimeType, volumeName);
@@ -1056,6 +1065,8 @@
return Optional.empty();
} else if (value instanceof String && ((String) value).equals("-1")) {
return Optional.empty();
+ } else if (value instanceof String && ((String) value).trim().length() == 0) {
+ return Optional.empty();
} else if (value instanceof Number && ((Number) value).intValue() == -1) {
return Optional.empty();
} else {
@@ -1096,6 +1107,7 @@
* the epoch, making our best guess from unrelated fields when offset
* information isn't directly available.
*/
+ @VisibleForTesting
static @NonNull Optional<Long> parseOptionalDateTaken(@NonNull ExifInterface exif,
long lastModifiedTime) {
final long originalTime = ExifUtils.getDateTimeOriginal(exif);
@@ -1181,6 +1193,21 @@
}
}
+ @VisibleForTesting
+ static @NonNull Optional<Integer> parseOptionalYear(@Nullable String value) {
+ final Optional<String> parsedValue = parseOptional(value);
+ if (parsedValue.isPresent()) {
+ final Matcher m = PATTERN_YEAR.matcher(parsedValue.get());
+ if (m.find()) {
+ return Optional.of(Integer.parseInt(m.group(1)));
+ } else {
+ return Optional.empty();
+ }
+ } else {
+ return Optional.empty();
+ }
+ }
+
private static @NonNull Optional<Integer> parseOptionalTrack(
@NonNull MediaMetadataRetriever mmr) {
final Optional<Integer> disc = parseOptionalNumerator(
@@ -1210,6 +1237,12 @@
if (fileMimeType.regionMatches(0, refinedMimeType, 0, refinedSplit + 1)) {
return Optional.of(refinedMimeType);
+ } else if ("video/mp4".equals(fileMimeType)
+ && "audio/mp4".equals(refinedMimeType)) {
+ // We normally only allow MIME types to be customized when the
+ // top-level type agrees, but this one very narrow case is added to
+ // support a music service that was writing "m4a" files as "mp4".
+ return Optional.of(refinedMimeType);
} else {
return Optional.empty();
}
@@ -1279,6 +1312,11 @@
return false;
}
+ @VisibleForTesting
+ static boolean isFileAlbumArt(String name) {
+ return PATTERN_ALBUM_ART.matcher(name).matches();
+ }
+
/**
* Test if this given {@link Uri} is a
* {@link android.provider.MediaStore.Audio.Playlists} item.
diff --git a/src/com/android/providers/media/scan/PlaylistResolver.java b/src/com/android/providers/media/scan/PlaylistResolver.java
index e9b6168..d546a80 100644
--- a/src/com/android/providers/media/scan/PlaylistResolver.java
+++ b/src/com/android/providers/media/scan/PlaylistResolver.java
@@ -106,7 +106,8 @@
while ((line = reader.readLine()) != null) {
if (!TextUtils.isEmpty(line) && !line.startsWith("#")) {
final int itemIndex = res.size() + 1;
- final File itemFile = parentPath.resolve(line).toFile();
+ final File itemFile = parentPath.resolve(
+ line.replace('\\', '/')).toFile();
try {
res.add(resolvePlaylistItem(resolver, uri, itemIndex, itemFile));
} catch (FileNotFoundException ignored) {
@@ -130,7 +131,8 @@
final Matcher matcher = PATTERN_PLS.matcher(line);
if (matcher.matches()) {
final int itemIndex = Integer.parseInt(matcher.group(1));
- final File itemFile = parentPath.resolve(matcher.group(2)).toFile();
+ final File itemFile = parentPath.resolve(
+ matcher.group(2).replace('\\', '/')).toFile();
try {
res.add(resolvePlaylistItem(resolver, uri, itemIndex, itemFile));
} catch (FileNotFoundException ignored) {
@@ -160,7 +162,8 @@
final String src = parser.getAttributeValue(null, ATTR_SRC);
if (src != null) {
final int itemIndex = res.size() + 1;
- final File itemFile = parentPath.resolve(src).toFile();
+ final File itemFile = parentPath.resolve(
+ src.replace('\\', '/')).toFile();
try {
res.add(resolvePlaylistItem(resolver, uri, itemIndex, itemFile));
} catch (FileNotFoundException ignored) {
@@ -179,7 +182,6 @@
@NonNull ContentResolver resolver, @NonNull Uri uri, int itemIndex, File itemFile)
throws IOException {
final Uri audioUri = MediaStore.Audio.Media.getContentUri(MediaStore.getVolumeName(uri));
- itemFile = new File(itemFile.getAbsolutePath().replace('\\', '/'));
try (Cursor cursor = resolver.query(audioUri,
new String[] { MediaColumns._ID }, MediaColumns.DATA + "=?",
new String[] { itemFile.getCanonicalPath() }, null)) {
diff --git a/src/com/android/providers/media/util/SQLiteQueryBuilder.java b/src/com/android/providers/media/util/SQLiteQueryBuilder.java
index fac5112..a8a3565 100644
--- a/src/com/android/providers/media/util/SQLiteQueryBuilder.java
+++ b/src/com/android/providers/media/util/SQLiteQueryBuilder.java
@@ -51,7 +51,7 @@
private static final String TAG = "SQLiteQueryBuilder";
private static final Pattern sAggregationPattern = Pattern.compile(
- "(?i)(AVG|COUNT|MAX|MIN|SUM|TOTAL|GROUP_CONCAT)\\((.+)\\)");
+ "(?i)(AVG|COUNT|MAX|MIN|SUM|TOTAL|GROUP_CONCAT|UNICODE)\\((.+)\\)");
private Map<String, String> mProjectionMap = null;
private Collection<Pattern> mProjectionGreylist = null;
@@ -721,6 +721,7 @@
}
private void enforceStrictToken(@NonNull String token) {
+ if (TextUtils.isEmpty(token)) return;
if (isTableOrColumn(token)) return;
if (SQLiteTokenizer.isFunction(token)) return;
if (SQLiteTokenizer.isType(token)) return;
diff --git a/tests/res/raw/test_audio_empty_title.mp3 b/tests/res/raw/test_audio_empty_title.mp3
new file mode 100644
index 0000000..1051f66
--- /dev/null
+++ b/tests/res/raw/test_audio_empty_title.mp3
Binary files differ
diff --git a/tests/src/com/android/providers/media/scan/LegacyMediaScannerTest.java b/tests/src/com/android/providers/media/scan/LegacyMediaScannerTest.java
index 9ecc6fc..b66a50b 100644
--- a/tests/src/com/android/providers/media/scan/LegacyMediaScannerTest.java
+++ b/tests/src/com/android/providers/media/scan/LegacyMediaScannerTest.java
@@ -54,4 +54,39 @@
} catch (UnsupportedOperationException expected) {
}
}
+
+ /**
+ * This implementation was copied verbatim from the legacy
+ * {@code frameworks/base/media/java/android/media/MediaScanner.java}.
+ */
+ static boolean isNonMediaFile(String path) {
+ // special case certain file names
+ // I use regionMatches() instead of substring() below
+ // to avoid memory allocation
+ final int lastSlash = path.lastIndexOf('/');
+ if (lastSlash >= 0 && lastSlash + 2 < path.length()) {
+ // ignore those ._* files created by MacOS
+ if (path.regionMatches(lastSlash + 1, "._", 0, 2)) {
+ return true;
+ }
+
+ // ignore album art files created by Windows Media Player:
+ // Folder.jpg, AlbumArtSmall.jpg, AlbumArt_{...}_Large.jpg
+ // and AlbumArt_{...}_Small.jpg
+ if (path.regionMatches(true, path.length() - 4, ".jpg", 0, 4)) {
+ if (path.regionMatches(true, lastSlash + 1, "AlbumArt_{", 0, 10) ||
+ path.regionMatches(true, lastSlash + 1, "AlbumArt.", 0, 9)) {
+ return true;
+ }
+ int length = path.length() - lastSlash - 1;
+ if ((length == 17 && path.regionMatches(
+ true, lastSlash + 1, "AlbumArtSmall", 0, 13)) ||
+ (length == 10
+ && path.regionMatches(true, lastSlash + 1, "Folder", 0, 6))) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
}
diff --git a/tests/src/com/android/providers/media/scan/ModernMediaScannerTest.java b/tests/src/com/android/providers/media/scan/ModernMediaScannerTest.java
index f1aa936..b874011 100644
--- a/tests/src/com/android/providers/media/scan/ModernMediaScannerTest.java
+++ b/tests/src/com/android/providers/media/scan/ModernMediaScannerTest.java
@@ -19,8 +19,10 @@
import static com.android.providers.media.scan.MediaScanner.REASON_UNKNOWN;
import static com.android.providers.media.scan.MediaScannerTest.stage;
import static com.android.providers.media.scan.ModernMediaScanner.isDirectoryHidden;
+import static com.android.providers.media.scan.ModernMediaScanner.isFileAlbumArt;
import static com.android.providers.media.scan.ModernMediaScanner.parseOptionalDateTaken;
import static com.android.providers.media.scan.ModernMediaScanner.parseOptionalMimeType;
+import static com.android.providers.media.scan.ModernMediaScanner.parseOptionalYear;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -55,6 +57,7 @@
import java.io.File;
import java.io.FileOutputStream;
+import java.util.Optional;
@RunWith(AndroidJUnit4.class)
public class ModernMediaScannerTest {
@@ -109,6 +112,21 @@
}
@Test
+ public void testOverrideMimeType_148316354() throws Exception {
+ // Radical file type shifting isn't allowed
+ assertEquals(Optional.empty(),
+ parseOptionalMimeType("video/mp4", "audio/mpeg"));
+
+ // One specific narrow type of shift (mp4 -> m4a) is allowed
+ assertEquals(Optional.of("audio/mp4"),
+ parseOptionalMimeType("video/mp4", "audio/mp4"));
+
+ // The other direction isn't allowed
+ assertEquals(Optional.empty(),
+ parseOptionalMimeType("audio/mp4", "video/mp4"));
+ }
+
+ @Test
public void testParseDateTaken_Complete() throws Exception {
final File file = File.createTempFile("test", ".jpg");
final ExifInterface exif = new ExifInterface(file);
@@ -205,6 +223,42 @@
assertFalse(parseOptionalDateTaken(exif, 0L).isPresent());
}
+ @Test
+ public void testParseYear_Invalid() throws Exception {
+ assertEquals(Optional.empty(), parseOptionalYear(null));
+ assertEquals(Optional.empty(), parseOptionalYear(""));
+ assertEquals(Optional.empty(), parseOptionalYear(" "));
+ assertEquals(Optional.empty(), parseOptionalYear("meow"));
+
+ assertEquals(Optional.empty(), parseOptionalYear("0"));
+ assertEquals(Optional.empty(), parseOptionalYear("00"));
+ assertEquals(Optional.empty(), parseOptionalYear("000"));
+ assertEquals(Optional.empty(), parseOptionalYear("0000"));
+
+ assertEquals(Optional.empty(), parseOptionalYear("1"));
+ assertEquals(Optional.empty(), parseOptionalYear("01"));
+ assertEquals(Optional.empty(), parseOptionalYear("001"));
+ assertEquals(Optional.empty(), parseOptionalYear("0001"));
+
+ // No sane way to determine year from two-digit date formats
+ assertEquals(Optional.empty(), parseOptionalYear("01-01-01"));
+
+ // Specific example from partner
+ assertEquals(Optional.empty(), parseOptionalYear("000 "));
+ }
+
+ @Test
+ public void testParseYear_Valid() throws Exception {
+ assertEquals(Optional.of(1900), parseOptionalYear("1900"));
+ assertEquals(Optional.of(2020), parseOptionalYear("2020"));
+ assertEquals(Optional.of(2020), parseOptionalYear(" 2020 "));
+ assertEquals(Optional.of(2020), parseOptionalYear("01-01-2020"));
+
+ // Specific examples from partner
+ assertEquals(Optional.of(1984), parseOptionalYear("1984-06-26T07:00:00Z"));
+ assertEquals(Optional.of(2016), parseOptionalYear("Thu, 01 Sep 2016 10:11:12.123456 -0500"));
+ }
+
private static void assertDirectoryHidden(File file) {
assertTrue(file.getAbsolutePath(), isDirectoryHidden(file));
}
@@ -487,4 +541,58 @@
assertEquals(expected, cursor.getCount());
}
}
+
+ @Test
+ public void testScan_audio_empty_title() throws Exception {
+ final File music = new File(mDir, "Music");
+ final File audio = new File(music, "audio.mp3");
+
+ music.mkdirs();
+ stage(R.raw.test_audio_empty_title, audio);
+
+ mModern.scanFile(audio, REASON_UNKNOWN);
+
+ try (Cursor cursor = mIsolatedResolver
+ .query(MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, null, null, null, null)) {
+ assertEquals(1, cursor.getCount());
+ cursor.moveToFirst();
+ assertEquals("audio", cursor.getString(cursor.getColumnIndex(MediaColumns.TITLE)));
+ }
+ }
+
+ @Test
+ public void testAlbumArtPattern() throws Exception {
+ for (String path : new String[] {
+ "/storage/emulated/0/._abc",
+ "/storage/emulated/0/a._abc",
+
+ "/storage/emulated/0/AlbumArtSmall.jpg",
+ "/storage/emulated/0/albumartsmall.jpg",
+
+ "/storage/emulated/0/AlbumArt_{}_Small.jpg",
+ "/storage/emulated/0/albumart_{a}_small.jpg",
+ "/storage/emulated/0/AlbumArt_{}_Large.jpg",
+ "/storage/emulated/0/albumart_{a}_large.jpg",
+
+ "/storage/emulated/0/Folder.jpg",
+ "/storage/emulated/0/folder.jpg",
+
+ "/storage/emulated/0/AlbumArt.jpg",
+ "/storage/emulated/0/albumart.jpg",
+ "/storage/emulated/0/albumart1.jpg",
+ }) {
+ final File file = new File(path);
+ final String name = file.getName();
+ assertEquals(LegacyMediaScannerTest.isNonMediaFile(path), isFileAlbumArt(name));
+ }
+
+ for (String path : new String[] {
+ "/storage/emulated/0/AlbumArtLarge.jpg",
+ "/storage/emulated/0/albumartlarge.jpg",
+ }) {
+ final File file = new File(path);
+ final String name = file.getName();
+ assertTrue(isFileAlbumArt(name));
+ }
+ }
}