From 70df5446d3f0bc203363ad74e737fd772e8e9c2c Mon Sep 17 00:00:00 2001
From: JosJuice <josjuice@gmail.com>
Date: Wed, 4 Nov 2020 20:59:39 +0100
Subject: [PATCH] Android: Make the handling of SAF open modes more robust

---
 .../dolphinemu/utils/ContentHandler.java      |  4 +-
 .../jni/AndroidCommon/AndroidCommon.cpp       | 39 +++++++++++++------
 .../Android/jni/AndroidCommon/AndroidCommon.h |  9 +++++
 Source/Core/Common/File.cpp                   | 16 ++------
 4 files changed, 43 insertions(+), 25 deletions(-)

diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/ContentHandler.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/ContentHandler.java
index 83b9c585c9..dc4472a212 100644
--- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/ContentHandler.java
+++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/ContentHandler.java
@@ -17,7 +17,9 @@ public class ContentHandler
       return DolphinApplication.getAppContext().getContentResolver()
               .openFileDescriptor(Uri.parse(uri), mode).detachFd();
     }
-    catch (FileNotFoundException | NullPointerException e)
+    // Some content providers throw IllegalArgumentException for invalid modes,
+    // despite the documentation saying that invalid modes result in a FileNotFoundException
+    catch (FileNotFoundException | IllegalArgumentException | NullPointerException e)
     {
       return -1;
     }
diff --git a/Source/Android/jni/AndroidCommon/AndroidCommon.cpp b/Source/Android/jni/AndroidCommon/AndroidCommon.cpp
index 73405b6d8e..344e34c7b6 100644
--- a/Source/Android/jni/AndroidCommon/AndroidCommon.cpp
+++ b/Source/Android/jni/AndroidCommon/AndroidCommon.cpp
@@ -10,6 +10,7 @@
 
 #include <jni.h>
 
+#include "Common/Assert.h"
 #include "Common/StringUtil.h"
 #include "jni/AndroidCommon/IDCache.h"
 
@@ -42,21 +43,35 @@ std::vector<std::string> JStringArrayToVector(JNIEnv* env, jobjectArray array)
   return result;
 }
 
+bool IsPathAndroidContent(const std::string& uri)
+{
+  return StringBeginsWith(uri, "content://");
+}
+
+std::string OpenModeToAndroid(std::string mode)
+{
+  // The 'b' specifier is not supported. Since we're on POSIX, it's fine to just skip it.
+  if (!mode.empty() && mode.back() == 'b')
+    mode.pop_back();
+
+  if (mode == "r+")
+    mode = "rw";
+  else if (mode == "w+")
+    mode = "rwt";
+  else if (mode == "a+")
+    mode = "rwa";
+  else if (mode == "a")
+    mode = "wa";
+
+  return mode;
+}
+
 int OpenAndroidContent(const std::string& uri, const std::string& mode)
 {
   JNIEnv* env = IDCache::GetEnvForThread();
-  const jint fd = env->CallStaticIntMethod(IDCache::GetContentHandlerClass(),
-                                           IDCache::GetContentHandlerOpenFd(), ToJString(env, uri),
-                                           ToJString(env, mode));
-
-  // We can get an IllegalArgumentException when passing an invalid mode
-  if (env->ExceptionCheck())
-  {
-    env->ExceptionDescribe();
-    abort();
-  }
-
-  return fd;
+  return env->CallStaticIntMethod(IDCache::GetContentHandlerClass(),
+                                  IDCache::GetContentHandlerOpenFd(), ToJString(env, uri),
+                                  ToJString(env, mode));
 }
 
 bool DeleteAndroidContent(const std::string& uri)
diff --git a/Source/Android/jni/AndroidCommon/AndroidCommon.h b/Source/Android/jni/AndroidCommon/AndroidCommon.h
index ca8245182d..967933bdf8 100644
--- a/Source/Android/jni/AndroidCommon/AndroidCommon.h
+++ b/Source/Android/jni/AndroidCommon/AndroidCommon.h
@@ -12,5 +12,14 @@ std::string GetJString(JNIEnv* env, jstring jstr);
 jstring ToJString(JNIEnv* env, const std::string& str);
 std::vector<std::string> JStringArrayToVector(JNIEnv* env, jobjectArray array);
 
+// Returns true if the given path should be opened as Android content instead of a normal file.
+bool IsPathAndroidContent(const std::string& uri);
+
+// Turns a C/C++ style mode (e.g. "rb") into one which can be used with OpenAndroidContent.
+std::string OpenModeToAndroid(std::string mode);
+
+// Opens a given file and returns a file descriptor.
 int OpenAndroidContent(const std::string& uri, const std::string& mode);
+
+// Deletes a given file.
 bool DeleteAndroidContent(const std::string& uri);
diff --git a/Source/Core/Common/File.cpp b/Source/Core/Common/File.cpp
index a724cbb723..c601eb33f2 100644
--- a/Source/Core/Common/File.cpp
+++ b/Source/Core/Common/File.cpp
@@ -18,7 +18,6 @@
 #ifdef ANDROID
 #include <algorithm>
 
-#include "Common/StringUtil.h"
 #include "jni/AndroidCommon/AndroidCommon.h"
 #endif
 
@@ -66,24 +65,17 @@ void IOFile::Swap(IOFile& other) noexcept
 bool IOFile::Open(const std::string& filename, const char openmode[])
 {
   Close();
+
 #ifdef _WIN32
   m_good = _tfopen_s(&m_file, UTF8ToTStr(filename).c_str(), UTF8ToTStr(openmode).c_str()) == 0;
 #else
 #ifdef ANDROID
-  if (StringBeginsWith(filename, "content://"))
-  {
-    // The Java method which OpenAndroidContent passes the mode to does not support the b specifier.
-    // Since we're on POSIX, it's fine to just remove the b.
-    std::string mode_without_b(openmode);
-    mode_without_b.erase(std::remove(mode_without_b.begin(), mode_without_b.end(), 'b'),
-                         mode_without_b.end());
-    m_file = fdopen(OpenAndroidContent(filename, mode_without_b), mode_without_b.c_str());
-  }
+  if (IsPathAndroidContent(filename))
+    m_file = fdopen(OpenAndroidContent(filename, OpenModeToAndroid(openmode)), openmode);
   else
 #endif
-  {
     m_file = std::fopen(filename.c_str(), openmode);
-  }
+
   m_good = m_file != nullptr;
 #endif