From e2cda3d8f074abe49991e7b7b757dd9ca42a26aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 11 Oct 2026 21:29:47 +0200 Subject: [PATCH 1/6] UI: Forget the views' pending requests when recreating them WipeRequesterToken() only reset the screen's token, after CreateViews() had already handed it to the new views, so nothing was ever forgotten - not even in ~Screen(), which saw a token of -1. A text input dialog open across a RecreateViews() (on the RetroAchievements screen, a slow login failing) would then write through a deleted PopupTextInputChoice, possibly into the field that took its place: #21708. Requests made while creating views now get their own token, forgotten before the views are deleted. Screen-level requests keep the screen's token. RequestManager: ForgetRequestsWithToken() now locks, and also drops responses that arrived but weren't processed yet. ProcessRequests() calls the callbacks one at a time without holding the lock, so a callback that closes a screen drops the rest of that screen's responses. Co-Authored-By: Claude Opus 5.5 --- Common/System/Request.cpp | 57 ++++++++++++++++++++++----------------- Common/System/Request.h | 17 ++++++------ Common/UI/Screen.cpp | 17 +++++++++--- Common/UI/Screen.h | 10 ++++--- Common/UI/UIScreen.cpp | 8 +++--- unittest/UnitTest.cpp | 54 ++++++++++++++++++++++++++++++++++++- 6 files changed, 119 insertions(+), 44 deletions(-) diff --git a/Common/System/Request.cpp b/Common/System/Request.cpp index 9b3a0ce080..858cf688f5 100644 --- a/Common/System/Request.cpp +++ b/Common/System/Request.cpp @@ -86,6 +86,7 @@ bool RequestManager::MakeSystemRequest(SystemRequestType type, RequesterToken to } void RequestManager::ForgetRequestsWithToken(RequesterToken token) { + std::lock_guard guard(callbackMutex_); for (auto &iter : callbackMap_) { if (iter.second.token == token) { INFO_LOG(Log::System, "Forgetting about requester with token %d", token); @@ -93,23 +94,28 @@ void RequestManager::ForgetRequestsWithToken(RequesterToken token) { iter.second.failedCallback = nullptr; } } + // Also the ones that finished but haven't been processed yet. + std::lock_guard responseGuard(responseMutex_); + for (auto &response : pendingResponses_) { + if (response.token == token) { + response.callback = nullptr; + response.failedCallback = nullptr; + } + } } +// Responses can contain passwords, so we don't log them. void RequestManager::PostSystemSuccess(int requestId, std::string_view responseString, int responseValue) { std::lock_guard guard(callbackMutex_); auto iter = callbackMap_.find(requestId); if (iter == callbackMap_.end()) { - ERROR_LOG(Log::System, "PostSystemSuccess: Unexpected request ID %d (responseString=%.*s)", requestId, (int)responseString.size(), responseString.data()); + ERROR_LOG(Log::System, "PostSystemSuccess: Unexpected request ID %d", requestId); return; } std::lock_guard responseGuard(responseMutex_); - PendingSuccess response; - response.callback = iter->second.callback; - response.responseString = responseString; - response.responseValue = responseValue; - pendingSuccesses_.push_back(response); - DEBUG_LOG(Log::System, "PostSystemSuccess: Request %d (%.*s, %d)", requestId, (int)responseString.size(), responseString.data(), responseValue); + pendingResponses_.push_back({ true, std::string(responseString), responseValue, iter->second.callback, nullptr, iter->second.token }); + DEBUG_LOG(Log::System, "PostSystemSuccess: Request %d (%d chars, %d)", requestId, (int)responseString.size(), responseValue); callbackMap_.erase(iter); } @@ -124,35 +130,38 @@ void RequestManager::PostSystemFailure(int requestId, int responseValue) { WARN_LOG(Log::System, "PostSystemFailure: Request %d failed", requestId); std::lock_guard responseGuard(responseMutex_); - PendingFailure response; - response.failedCallback = iter->second.failedCallback; - response.responseValue = responseValue; - pendingFailures_.push_back(response); + pendingResponses_.push_back({ false, std::string(), responseValue, nullptr, iter->second.failedCallback, iter->second.token }); callbackMap_.erase(iter); } void RequestManager::ProcessRequests() { - std::lock_guard guard(responseMutex_); - for (auto &iter : pendingSuccesses_) { - if (iter.callback) { - iter.callback(iter.responseString.c_str(), iter.responseValue); + // One at a time, without holding the lock while calling. A callback can make new requests, or close a screen, + // which forgets the callbacks of the responses still in the queue. + while (true) { + PendingResponse response; + { + std::lock_guard guard(responseMutex_); + if (pendingResponses_.empty()) { + break; + } + response = std::move(pendingResponses_.front()); + pendingResponses_.pop_front(); + } + if (response.success) { + if (response.callback) { + response.callback(response.responseString, response.responseValue); + } + } else if (response.failedCallback) { + response.failedCallback(response.responseValue); } } - pendingSuccesses_.clear(); - for (auto &iter : pendingFailures_) { - if (iter.failedCallback) { - iter.failedCallback(iter.responseValue); - } - } - pendingFailures_.clear(); } void RequestManager::Clear() { std::lock_guard guard(callbackMutex_); std::lock_guard responseGuard(responseMutex_); - pendingSuccesses_.clear(); - pendingFailures_.clear(); + pendingResponses_.clear(); callbackMap_.clear(); } diff --git a/Common/System/Request.h b/Common/System/Request.h index cbcd742fbf..41450f7355 100644 --- a/Common/System/Request.h +++ b/Common/System/Request.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include @@ -64,21 +65,19 @@ private: std::map callbackMap_; std::mutex callbackMutex_; - struct PendingSuccess { + // A finished request, waiting for ProcessRequests to call its callback. + struct PendingResponse { + bool success; std::string responseString; - int responseValue; - RequestCallback callback; - }; - - struct PendingFailure { - RequestFailedCallback failedCallback; int responseValue; // can have error codes for example. + RequestCallback callback; + RequestFailedCallback failedCallback; + RequesterToken token; }; // Let's start at 10 to get a recognizably valid ID in logs. int idCounter_ = 10; - std::vector pendingSuccesses_; - std::vector pendingFailures_; + std::deque pendingResponses_; std::mutex responseMutex_; RequesterToken tokenGen_ = 20000; diff --git a/Common/UI/Screen.cpp b/Common/UI/Screen.cpp index e886ce5ab1..0e31b1f82e 100644 --- a/Common/UI/Screen.cpp +++ b/Common/UI/Screen.cpp @@ -27,18 +27,27 @@ void Screen::focusChanged(ScreenFocusChange focusChange) { } int Screen::GetRequesterToken() { - if (token_ < 0) { - token_ = g_requestManager.GenerateRequesterToken(); + int &token = creatingViews_ ? viewsToken_ : token_; + if (token < 0) { + token = g_requestManager.GenerateRequesterToken(); + } + return token; +} + +void Screen::ForgetViewRequests() { + if (viewsToken_ >= 0) { + g_requestManager.ForgetRequestsWithToken(viewsToken_); + viewsToken_ = -1; } - return token_; } Screen::~Screen() { screenManager_ = nullptr; + // To avoid expired callbacks getting called. if (token_ >= 0) { - // To avoid expired callbacks getting called. g_requestManager.ForgetRequestsWithToken(token_); } + ForgetViewRequests(); } const char *DialogResultToString(DialogResult result) { diff --git a/Common/UI/Screen.h b/Common/UI/Screen.h index 03f4284021..485c28645d 100644 --- a/Common/UI/Screen.h +++ b/Common/UI/Screen.h @@ -138,14 +138,18 @@ public: virtual bool WantsTextInput() const { return false; } protected: + // Requests made while creating views (by the views themselves, or with callbacks that capture them) get a separate + // token, so they can be forgotten when the views are deleted, while the screen's own requests stay alive. int GetRequesterToken(); - void WipeRequesterToken() { - token_ = -1; - } + void SetCreatingViews(bool creating) { creatingViews_ = creating; } + // Drops the callbacks of pending requests made by the views, which are about to be deleted. + void ForgetViewRequests(); private: ScreenManager *screenManager_ = nullptr; int token_ = -1; + int viewsToken_ = -1; + bool creatingViews_ = false; DISALLOW_COPY_AND_ASSIGN(Screen); }; diff --git a/Common/UI/UIScreen.cpp b/Common/UI/UIScreen.cpp index 4817514066..d29d2dead9 100644 --- a/Common/UI/UIScreen.cpp +++ b/Common/UI/UIScreen.cpp @@ -39,9 +39,14 @@ void UIScreen::DoRecreateViews() { root_->PersistData(UI::PERSIST_SAVE, "root", persisted); } + // Pending requests from the views (like an open text input dialog) would call back into deleted views. + ForgetViewRequests(); + delete root_; root_ = nullptr; + SetCreatingViews(true); CreateViews(); + SetCreatingViews(false); UI::View *defaultView = root_ ? root_->GetDefaultFocusView() : nullptr; if (defaultView && defaultView->GetVisibility() == UI::V_VISIBLE) { defaultView->SetFocus(UI::FocusFlags::CAUSE_OTHER); @@ -59,9 +64,6 @@ void UIScreen::DoRecreateViews() { root_->SubviewFocused(focused); } } - - // NOTE: We also wipe the requester token. It's possible that views were created with the old token, so any pending requests from them must be invalidated. - WipeRequesterToken(); } bool UIScreen::touch(const TouchInput &touch) { diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 51d92272a3..fd25357f62 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -122,6 +122,7 @@ #include "GPU/Math3D.h" #include "Common/File/AndroidContentURI.h" +#include "Common/System/Request.h" #include "unittest/JitHarness.h" #include "unittest/TestVertexJit.h" @@ -158,7 +159,15 @@ std::vector System_GetCameraDeviceList() { return std::vector cb) { cb(false, ""); } @@ -3270,6 +3279,48 @@ static bool TestQueueBuf() { return true; } +static bool TestRequestManager() { + RequestManager rm; + std::vector results; + auto ask = [&](RequesterToken token, std::string name) { + rm.MakeSystemRequest(SystemRequestType::INPUT_TEXT_MODAL, token, [&results, name](std::string_view value, int) { + results.push_back(name + "=" + std::string(value)); + }, nullptr, "", "", 0); + return g_lastInputBoxRequestId; + }; + + RequesterToken tokenA = rm.GenerateRequesterToken(); + RequesterToken tokenB = rm.GenerateRequesterToken(); + + // Forgotten before the answer arrives. + int a1 = ask(tokenA, "a1"); + rm.ForgetRequestsWithToken(tokenA); + rm.PostSystemSuccess(a1, "x"); + // Forgotten after the answer arrived, but before it was processed. + int a2 = ask(tokenA, "a2"); + int b1 = ask(tokenB, "b1"); + rm.PostSystemSuccess(a2, "y"); + rm.PostSystemSuccess(b1, "z"); + rm.ForgetRequestsWithToken(tokenA); + rm.ProcessRequests(); + EXPECT_EQ_INT((int)results.size(), 1); + EXPECT_TRUE(results[0] == "b1=z"); + + // A callback that forgets its requester drops the rest of its responses in the same batch. + results.clear(); + int b2 = rm.MakeSystemRequest(SystemRequestType::INPUT_TEXT_MODAL, tokenB, [&](std::string_view, int) { + results.push_back("b2"); + rm.ForgetRequestsWithToken(tokenB); + }, nullptr, "", "", 0) ? g_lastInputBoxRequestId : -1; + int b3 = ask(tokenB, "b3"); + rm.PostSystemSuccess(b2, ""); + rm.PostSystemSuccess(b3, ""); + rm.ProcessRequests(); + EXPECT_EQ_INT((int)results.size(), 1); + EXPECT_TRUE(results[0] == "b2"); + return true; +} + TestItem availableTests[] = { #if PPSSPP_ARCH(ARM64) || PPSSPP_ARCH(AMD64) || PPSSPP_ARCH(X86) TEST_ITEM(Arm64Emitter), @@ -3347,6 +3398,7 @@ TestItem availableTests[] = { TEST_ITEM(FatShortNames), TEST_ITEM(GEDumpGameID), TEST_ITEM(QueueBuf), + TEST_ITEM(RequestManager), }; int main(int argc, const char *argv[]) { From 60719830949bad9e68c0de217d3d95b0249c662d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 11 Oct 2026 21:29:47 +0200 Subject: [PATCH 2/6] Android: Mask password input boxes, don't log entered text The passwordMasking flag of System_InputBoxGetString() never reached Java, and both sides logged the entered text, including RetroAchievements passwords. Co-Authored-By: Claude Opus 5.5 --- android/jni/app-android.cpp | 6 +++-- .../src/org/ppsspp/ppsspp/PpssppActivity.java | 24 ++++++++++++------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index a082653eed..40dedba731 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -966,7 +966,8 @@ bool System_MakeRequest(SystemRequestType type, int requestId, const std::string return true; case SystemRequestType::INPUT_TEXT_MODAL: { - std::string serialized = StringFromFormat("%d:@:%s:@:%s", requestId, param1.c_str(), param2.c_str()); + // The flags go before the strings, so the free text comes last. + std::string serialized = StringFromFormat("%d:@:%d:@:%s:@:%s", requestId, param3 ? 1 : 0, param1.c_str(), param2.c_str()); PushCommand("inputbox", serialized); return true; } @@ -1038,7 +1039,8 @@ bool System_MakeRequest(SystemRequestType type, int requestId, const std::string extern "C" void JNICALL Java_org_ppsspp_ppsspp_NativeApp_sendRequestResult(JNIEnv *env, jclass, jint jrequestID, jboolean result, jstring jvalue, jint jintValue) { std::string value = jvalue ? GetJavaString(env, jvalue) : "(no value)"; - INFO_LOG(Log::System, "Received result of request %d from Java: %d: %d '%s'", jrequestID, (int)result, jintValue, value.c_str()); + // Don't log the value, it can be a password. + INFO_LOG(Log::System, "Received result of request %d from Java: %d: %d (%d chars)", jrequestID, (int)result, jintValue, (int)value.size()); if (result) { g_requestManager.PostSystemSuccess(jrequestID, value, jintValue); } else { diff --git a/android/src/org/ppsspp/ppsspp/PpssppActivity.java b/android/src/org/ppsspp/ppsspp/PpssppActivity.java index 831106396e..a702b1fd4c 100644 --- a/android/src/org/ppsspp/ppsspp/PpssppActivity.java +++ b/android/src/org/ppsspp/ppsspp/PpssppActivity.java @@ -1478,7 +1478,7 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList } // The return value is sent to C++ via requestID. - public void inputBox(final int requestId, final String title, String defaultText, String defaultAction) { + public void inputBox(final int requestId, final String title, String defaultText, boolean passwordMasking, String defaultAction) { final FrameLayout fl = new FrameLayout(this); final EditText input = new EditText(this); input.setGravity(Gravity.CENTER); @@ -1487,7 +1487,11 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList editBoxLayout.setMargins(2, 20, 2, 20); fl.addView(input, editBoxLayout); - input.setInputType(InputType.TYPE_CLASS_TEXT); + if (passwordMasking) { + input.setInputType(InputType.TYPE_CLASS_TEXT | InputType.TYPE_TEXT_VARIATION_PASSWORD); + } else { + input.setInputType(InputType.TYPE_CLASS_TEXT); + } input.setImeOptions(EditorInfo.IME_ACTION_DONE); input.setText(defaultText); input.setFocusableInTouchMode(true); @@ -1692,12 +1696,13 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList } else if (command.equals("inputbox")) { String title = "Input"; String defString = ""; - String[] param = params.split(":@:", 3); + String[] param = params.split(":@:", 4); int requestID = Integer.parseInt(param[0]); - if (param.length > 1 && !param[1].isEmpty()) - title = param[1]; - if (param.length > 2) - defString = param[2]; + boolean passwordMasking = param.length > 1 && param[1].equals("1"); + if (param.length > 2 && !param[2].isEmpty()) + title = param[2]; + if (param.length > 3) + defString = param[3]; // Workaround for issue #13363 to fix Split/Second game start - it requires text input // but we don't support it on VR devices. if (isVRDevice()) { @@ -1705,8 +1710,9 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList NativeApp.sendRequestResult(requestID, false, defString, 0); return true; } - Log.i(TAG, "Launching inputbox: #" + requestID + " " + title + " " + defString); - inputBox(requestID, title, defString, "OK"); + // Don't log defString, it can be a password. + Log.i(TAG, "Launching inputbox: #" + requestID + " " + title); + inputBox(requestID, title, defString, passwordMasking, "OK"); return true; } else if (command.equals("vibrate")) { int milliseconds = -1; From 2aa3e8d6d393db02d33d6c8a50ed731e1c0ecb55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 11 Oct 2026 21:37:19 +0200 Subject: [PATCH 3/6] Android: Always answer native requests An input box whose show() threw, a second input box open at onDestroy, and browse commands dropped for a destroyed activity or failing in processCommand were never answered, leaving the native side waiting (a game's OSK forever). The image proxy also crashed the app on a picker result without a URI, a SecurityException or an OOM. Input boxes now answer exactly once, instead of a success followed by an ignored failure. Co-Authored-By: Claude Opus 5.5 --- .../ppsspp/ImageResultProxyActivity.java | 9 ++- .../src/org/ppsspp/ppsspp/PpssppActivity.java | 59 ++++++++++++------- 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/android/src/org/ppsspp/ppsspp/ImageResultProxyActivity.java b/android/src/org/ppsspp/ppsspp/ImageResultProxyActivity.java index 3a778fe67e..8024009fcf 100644 --- a/android/src/org/ppsspp/ppsspp/ImageResultProxyActivity.java +++ b/android/src/org/ppsspp/ppsspp/ImageResultProxyActivity.java @@ -23,6 +23,11 @@ public class ImageResultProxyActivity extends AppCompatActivity { private String copyAndDownscaleToCache(Uri selectedImage) { Log.i(TAG, "Selected image: " + selectedImage); + if (selectedImage == null) { + // Some pickers only return a ClipData. + Log.e(TAG, "No image URI in the result"); + return null; + } File cacheDir = getExternalCacheDir(); if (cacheDir == null) { Log.e(TAG, "External cache directory not available"); @@ -104,7 +109,9 @@ public class ImageResultProxyActivity extends AppCompatActivity { String returnPath = tempFile.getAbsolutePath(); Log.i(TAG, "Return path: " + returnPath); return returnPath; - } catch (IOException e) { + } catch (IOException | RuntimeException | OutOfMemoryError e) { + // We're in the app's process, so anything uncaught here (SecurityException from a provider, a huge bitmap) + // takes the whole app down. Log.e(TAG, "Error processing image: " + e.getMessage()); e.printStackTrace(); return null; diff --git a/android/src/org/ppsspp/ppsspp/PpssppActivity.java b/android/src/org/ppsspp/ppsspp/PpssppActivity.java index a702b1fd4c..1289afa0d6 100644 --- a/android/src/org/ppsspp/ppsspp/PpssppActivity.java +++ b/android/src/org/ppsspp/ppsspp/PpssppActivity.java @@ -125,8 +125,8 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList private final ArrayList inputPlayers = new ArrayList<>(); private PowerSaveModeReceiver mPowerSaveModeReceiver = null; - // The open input box, if any. - private AlertDialog inputDialog = null; + // The open input boxes. There can be two, if a game's OSK pops up over a UI one. + private final ArrayList inputDialogs = new ArrayList<>(); private NativeSurfaceManager surfaceManager = null; private static LocationHelper mLocationHelper; private static InfraredHelper mInfraredHelper; @@ -1000,10 +1000,10 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList mPowerSaveModeReceiver = null; } // Otherwise its window leaks, and its request is never answered. Dismissing answers it as cancelled. - if (inputDialog != null) { - inputDialog.dismiss(); - inputDialog = null; + for (AlertDialog dlg : new ArrayList<>(inputDialogs)) { + dlg.dismiss(); } + inputDialogs.clear(); // After finish(), Android is free to deliver onDestroy late, after it has already created and resumed a // new activity if the user was quick to relaunch. That one found initialized still set and adopted the @@ -1499,22 +1499,25 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList input.selectAll(); //input.setSelection(input.getText().length()); + // Answered exactly once: OK and Done are followed by a dismiss. + final boolean[] answered = { false }; AlertDialog.Builder builder = createDialogBuilder() .setView(fl) .setTitle(title) .setPositiveButton(defaultAction, (d, which) -> { Log.i(TAG, "input box successful"); + answered[0] = true; NativeApp.sendRequestResult(requestId, true, input.getText().toString(), 0); // Dismiss happens automatically. }) .setNegativeButton("Cancel", (d, which) -> d.cancel()); builder.setOnDismissListener( d -> { Log.i(TAG, "input box dismissed"); - // This will be ignored if we already sent a success. - NativeApp.sendRequestResult(requestId, false, "", 0); - if (inputDialog == d) { - inputDialog = null; + if (!answered[0]) { + answered[0] = true; + NativeApp.sendRequestResult(requestId, false, "", 0); } + inputDialogs.remove(d); // Also comes after onDestroy dismissed it. if (!isDestroyed()) { updateSystemUiVisibility(); @@ -1525,7 +1528,10 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList input.setOnEditorActionListener((v, actionId, event) -> { if (actionId == EditorInfo.IME_ACTION_DONE) { Log.i(TAG, "input box successful via Keyboard Done"); - NativeApp.sendRequestResult(requestId, true, input.getText().toString(), 0); + if (!answered[0]) { + answered[0] = true; + NativeApp.sendRequestResult(requestId, true, input.getText().toString(), 0); + } // We must dismiss the dialog manually here dlg.dismiss(); @@ -1540,10 +1546,15 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList } try { dlg.show(); - inputDialog = dlg; + inputDialogs.add(dlg); input.requestFocus(); } catch (Exception e) { NativeApp.reportException(e, "AlertDialog"); + // The dismiss listener won't run, and the native side (maybe a game's OSK) waits for an answer. + if (!answered[0]) { + answered[0] = true; + NativeApp.sendRequestResult(requestId, false, "", 0); + } } } @@ -2013,6 +2024,19 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList } } + // For commands that carry a native request ID (always first in the parameter), answers it as failed, since the + // native side waits for an answer. + private static void failRequestCommand(String command, String parameter) { + if (!command.equals("inputbox") && !command.equals("browse_image") && !command.startsWith("browse_file") && !command.equals("browse_folder")) { + return; + } + try { + NativeApp.sendRequestResult(Integer.parseInt(parameter.split(":@:", 2)[0]), false, "", 0); + } catch (NumberFormatException e) { + Log.e(TAG, "Bad request ID for command '" + command + "'"); + } + } + // called by the C++ code through JNI. Dispatch anything we can't directly handle // on the gfx thread to the UI thread. @Keep @@ -2022,18 +2046,13 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList // Posted before this activity was destroyed, but runs after. if (isDestroyed()) { Log.w(TAG, "Dropping command '" + command + "' for a destroyed activity"); - if (command.equals("inputbox")) { - // The native side waits for an answer. - try { - NativeApp.sendRequestResult(Integer.parseInt(parameter.split(":@:", 2)[0]), false, "", 0); - } catch (NumberFormatException e) { - Log.e(TAG, "Bad inputbox parameter: " + parameter); - } - } + failRequestCommand(command, parameter); return; } if (!processCommand(command, parameter)) { - Log.e(TAG, "processCommand failed: cmd: '" + command + "' param: '" + parameter + "'"); + // Not logging the parameter, an inputbox one can contain a password. + Log.e(TAG, "processCommand failed: cmd: '" + command + "'"); + failRequestCommand(command, parameter); } }); } From 468b83a7389b29736c8988bf53f00848c0d32e34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 11 Oct 2026 21:37:19 +0200 Subject: [PATCH 4/6] Android: Fix JNI string encoding, thread safety, password in debug log GetJavaString() returned Java's modified UTF-8, so input like emoji reached C++ as invalid UTF-8. Commands to Java now get converted the other way. Request IDs are made on several threads, and the permissions map was written on the Java UI thread and read on the render thread. The frame command debug log no longer prints params, which could hold a password. Co-Authored-By: Claude Opus 5.5 --- Common/System/Request.h | 6 ++-- android/jni/app-android.cpp | 56 +++++++++++++++++++++++++++++-------- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/Common/System/Request.h b/Common/System/Request.h index 41450f7355..80461f7756 100644 --- a/Common/System/Request.h +++ b/Common/System/Request.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -76,11 +77,12 @@ private: }; // Let's start at 10 to get a recognizably valid ID in logs. - int idCounter_ = 10; + // Atomic since requests are made from several threads (the CPU thread for the OSK and the microphone). + std::atomic idCounter_{ 10 }; std::deque pendingResponses_; std::mutex responseMutex_; - RequesterToken tokenGen_ = 20000; + std::atomic tokenGen_{ 20000 }; }; const char *RequestTypeAsString(SystemRequestType type); diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index 40dedba731..00b1f1f806 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -83,6 +83,7 @@ struct JNIEnv {}; #include "Common/Input/KeyCodes.h" #include "Common/Profiler/Profiler.h" #include "Common/Math/math_util.h" +#include "Common/Data/Encoding/Utf8.h" #include "Common/Data/Text/Parsers.h" #include "Common/VR/PPSSPPVR.h" #include "Common/GPU/Vulkan/VulkanLoader.h" @@ -177,8 +178,15 @@ static std::atomic renderer_inited{false}; static bool sustainedPerfSupported = false; static std::string g_installerName; +// Written on the Java UI thread, read on the render thread. +static std::mutex permissionsLock; static std::map permissions; +static void SetPermissionStatus(SystemPermission permission, PermissionStatus status) { + std::lock_guard guard(permissionsLock); + permissions[permission] = status; +} + static GraphicsContext *graphicsContext; #define MessageBox(a, b, c, d) __android_log_print(ANDROID_LOG_INFO, APP_NAME, "%s %s", (b), (c)); @@ -520,12 +528,30 @@ std::vector Android_GetNativeCrashHistory(int maxEntries) { return crashHistory; } +// GetStringUTFChars returns Java's modified UTF-8, where characters outside the BMP (emoji) become two 3-byte +// surrogates, so we go through UTF-16. std::string GetJavaString(JNIEnv *env, jstring jstr) { if (!jstr) return ""; - const char *str = env->GetStringUTFChars(jstr, nullptr); - std::string cpp_string = std::string(str); - env->ReleaseStringUTFChars(jstr, str); + const jsize len = env->GetStringLength(jstr); + const jchar *chars = env->GetStringChars(jstr, nullptr); + if (!chars) + return ""; + std::string cpp_string; + cpp_string.reserve(len); + char buf[4]; + for (jsize i = 0; i < len; i++) { + uint32_t c = chars[i]; + if (c >= 0xD800 && c < 0xDC00 && i + 1 < len && chars[i + 1] >= 0xDC00 && chars[i + 1] < 0xE000) { + c = 0x10000 + ((c - 0xD800) << 10) + (chars[i + 1] - 0xDC00); + i++; + } else if (c >= 0xD800 && c < 0xE000) { + // Unpaired surrogate. + c = 0xFFFD; + } + cpp_string.append(buf, UTF8::encode(buf, c)); + } + env->ReleaseStringChars(jstr, chars); return cpp_string; } @@ -1063,7 +1089,9 @@ PermissionStatus System_GetPermissionStatus(SystemPermission permission) { if (androidVersion < 23) { return PERMISSION_STATUS_GRANTED; } else { - return permissions[permission]; + std::lock_guard guard(permissionsLock); + auto iter = permissions.find(permission); + return iter != permissions.end() ? iter->second : PERMISSION_STATUS_UNKNOWN; } } @@ -1262,28 +1290,28 @@ extern "C" void JNICALL Java_org_ppsspp_ppsspp_NativeApp_sendMessageFromJava(JNI } else if (msg == "permission_pending") { if (prm == "storage") { INFO_LOG(Log::System, "STORAGE PERMISSION: PENDING"); - permissions[SYSTEM_PERMISSION_STORAGE] = PERMISSION_STATUS_PENDING; + SetPermissionStatus(SYSTEM_PERMISSION_STORAGE, PERMISSION_STATUS_PENDING); } else if (prm == "local_network") { INFO_LOG(Log::System, "LOCAL NETWORK PERMISSION: PENDING"); - permissions[SYSTEM_PERMISSION_LOCAL_NETWORK] = PERMISSION_STATUS_PENDING; + SetPermissionStatus(SYSTEM_PERMISSION_LOCAL_NETWORK, PERMISSION_STATUS_PENDING); } } else if (msg == "permission_denied") { if (prm == "storage") { INFO_LOG(Log::System, "STORAGE PERMISSION: DENIED"); - permissions[SYSTEM_PERMISSION_STORAGE] = PERMISSION_STATUS_DENIED; + SetPermissionStatus(SYSTEM_PERMISSION_STORAGE, PERMISSION_STATUS_DENIED); } else if (prm == "local_network") { INFO_LOG(Log::System, "LOCAL NETWORK PERMISSION: DENIED"); - permissions[SYSTEM_PERMISSION_LOCAL_NETWORK] = PERMISSION_STATUS_DENIED; + SetPermissionStatus(SYSTEM_PERMISSION_LOCAL_NETWORK, PERMISSION_STATUS_DENIED); } else { WARN_LOG(Log::System, "UNKNOWN PERMISSION GRANTED: %s", prm.c_str()); } } else if (msg == "permission_granted") { if (prm == "storage") { INFO_LOG(Log::System, "STORAGE PERMISSION: GRANTED"); - permissions[SYSTEM_PERMISSION_STORAGE] = PERMISSION_STATUS_GRANTED; + SetPermissionStatus(SYSTEM_PERMISSION_STORAGE, PERMISSION_STATUS_GRANTED); } else if (prm == "local_network") { INFO_LOG(Log::System, "LOCAL NETWORK PERMISSION: GRANTED"); - permissions[SYSTEM_PERMISSION_LOCAL_NETWORK] = PERMISSION_STATUS_GRANTED; + SetPermissionStatus(SYSTEM_PERMISSION_LOCAL_NETWORK, PERMISSION_STATUS_GRANTED); } else { WARN_LOG(Log::System, "UNKNOWN PERMISSION GRANTED: %s", prm.c_str()); } @@ -1517,10 +1545,14 @@ static void ProcessFrameCommands() { if (!frameCommands.empty()) { INFO_LOG(Log::System, "Processing %zu frame commands", frameCommands.size()); for (const FrameCommand &frameCmd : frameCommands) { - DEBUG_LOG(Log::System, "frameCommand '%s' '%s'", frameCmd.command.c_str(), frameCmd.params.c_str()); + // Not logging the params, an inputbox one can contain a password. + DEBUG_LOG(Log::System, "frameCommand '%s'", frameCmd.command.c_str()); + // The params can contain any text, like a title the game gave its OSK. + std::string params; + ConvertUTF8ToJavaModifiedUTF8(¶ms, frameCmd.params); jstring cmd = env->NewStringUTF(frameCmd.command.c_str()); - jstring param = env->NewStringUTF(frameCmd.params.c_str()); + jstring param = env->NewStringUTF(params.c_str()); env->CallVoidMethod(activity, postCommand, cmd, param); env->DeleteLocalRef(cmd); env->DeleteLocalRef(param); From 78f389bd8079a240933be0d44eefd482317eedbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 11 Oct 2026 21:37:20 +0200 Subject: [PATCH 5/6] Close the file used to detect a background image's format Co-Authored-By: Claude Opus 5.5 --- UI/GameSettingsScreen.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/UI/GameSettingsScreen.cpp b/UI/GameSettingsScreen.cpp index e20b5ff7ff..f53fffa8a5 100644 --- a/UI/GameSettingsScreen.cpp +++ b/UI/GameSettingsScreen.cpp @@ -1632,6 +1632,9 @@ void GameSettingsScreen::OnChangeBackground(UI::EventParams &e) { if (f != nullptr && 8 == fread(buffer, 1, ARRAY_SIZE(buffer), f)) { type = DetectImageFileType(buffer, ARRAY_SIZE(buffer)); } + if (f) { + fclose(f); + } std::string filename; switch (type) { From 04f5994ea624b1c1e0b477e07dd04d96bb7a2ae4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 12 Oct 2026 12:43:36 +0200 Subject: [PATCH 6/6] Create .nomedia files in the NAND directory when installing firmware Avoids Android looking for images. --- Core/Util/PSARUnpack.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Core/Util/PSARUnpack.cpp b/Core/Util/PSARUnpack.cpp index 6f7d1d1c5d..9efbfae9d7 100644 --- a/Core/Util/PSARUnpack.cpp +++ b/Core/Util/PSARUnpack.cpp @@ -1353,6 +1353,11 @@ bool UnpackUpdaterFromMountedDisc(const Path &outputDir, const PSARUnpackOptions return false; } + // Write a nomedia file to avoid Android indexing the files. +#if PPSSPP_PLATFORM(ANDROID) + File::CreateEmptyFile(outputDir / ".nomedia"); +#endif + INFO_LOG(Log::Loader, "Found a %d byte updater on the mounted disc", (int)psar.size()); return UnpackPSAR(psar.data(), psar.size(), outputDir, options, stats, error); } @@ -1488,6 +1493,12 @@ bool AutoInstallFirmwareFromDisc() { } const Path nandRoot = GetSysDirectory(DIRECTORY_NAND); + + // Write a nomedia file to avoid Android indexing the files. +#if PPSSPP_PLATFORM(ANDROID) + File::CreateEmptyFile(nandRoot / ".nomedia"); +#endif + InstalledFirmwareInfo installed; ReadInstalledFirmwareInfo(nandRoot, &installed, false);