From b70bbf366e61d442808c31ce1ddf943f83691df3 Mon Sep 17 00:00:00 2001 From: Mohamed Ibrahim Date: Wed, 1 Jul 2026 16:37:17 +0100 Subject: [PATCH] fix(android): isolate logger locale so it can't corrupt the host app Opening the Sharingan logger from a host that uses per-app locales (e.g. AppCompat set to Arabic/RTL) rendered the logger in an RTL frame and, worse, permanently corrupted the host: after closing the logger the host was stuck in English text with RTL layout (issue #38). Root cause: SharinganActivity is a plain ComponentActivity that neither pins its own locale nor guards the process-global one. On API 33+ the framework applied the host's per-app locale (ar -> RTL) to the logger, and the logger's config handling flipped the process-global Locale/LocaleList default to English, which leaked back to the host. The logger is a locale-neutral surface: always English + LTR, and it must never read or mutate the host's locale. Two-layer fix: - Activity: attachBaseContext pins an en/LTR Configuration via the non-mutating createConfigurationContext (never resources.updateConfiguration). - Compose: the stateless SharinganScreenContent forces LocalLayoutDirection = Ltr around its whole body, so the frame is LTR on every platform (including iOS) and in Studio previews. The global-locale guard was needed: an instrumented test proved that createConfigurationContext(en) resets LocaleList.getDefault() to English. SharinganActivity now snapshots the host's LocaleList in attachBaseContext and restores it (right after, and again in onDestroy), leaving the host's locale untouched. Verified red->green on an API 34 emulator with a new instrumented test that pins the process to Arabic, opens/closes the logger, and asserts the logger is LTR while open and every process-global JVM locale default is still Arabic after. The test is @SdkSuppress(minSdkVersion = 33) since it drives the API 33+ LocaleManager. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../sharingan/sample/LoggerLocaleLeakTest.kt | 107 ++++++++++++++++++ .../kotlin/dev/sharingan/SharinganActivity.kt | 31 +++++ .../dev/sharingan/ui/SharinganScreen.kt | 62 +++++----- 3 files changed, 173 insertions(+), 27 deletions(-) create mode 100644 sample/composeApp/src/androidInstrumentedTest/kotlin/dev/sharingan/sample/LoggerLocaleLeakTest.kt diff --git a/sample/composeApp/src/androidInstrumentedTest/kotlin/dev/sharingan/sample/LoggerLocaleLeakTest.kt b/sample/composeApp/src/androidInstrumentedTest/kotlin/dev/sharingan/sample/LoggerLocaleLeakTest.kt new file mode 100644 index 0000000..22cf322 --- /dev/null +++ b/sample/composeApp/src/androidInstrumentedTest/kotlin/dev/sharingan/sample/LoggerLocaleLeakTest.kt @@ -0,0 +1,107 @@ +package dev.sharingan.sample + +import android.app.LocaleManager +import android.content.Context +import android.os.LocaleList +import android.view.View +import androidx.test.core.app.ActivityScenario +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.filters.SdkSuppress +import androidx.test.platform.app.InstrumentationRegistry +import dev.sharingan.SharinganActivity +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import java.util.Locale + +/** + * Regression for issue #38: opening the Sharingan logger from a host app that + * uses per-app locales (Arabic/RTL) must not corrupt the host. The logger is a + * locale-neutral surface — English + LTR while open — and must leave every + * process-global locale knob exactly as the host set it. + * + * ``` + * ./gradlew :sample:composeApp:connectedDebugAndroidTest + * ``` + * + * We stand in for the AppCompat host by driving the framework per-app locale + * (`LocaleManager`, what `AppCompatDelegate.setApplicationLocales` calls under + * the hood on API 33+) directly in the process, since a test-APK host Activity + * can't be launched into the app-under-test's process and the leak we assert is + * process-global anyway. Requires API 33+ (the emulators CI runs on). + */ +@RunWith(AndroidJUnit4::class) +@SdkSuppress(minSdkVersion = 33) // LocaleManager is API 33+; skip (don't crash) below. +class LoggerLocaleLeakTest { + + private val instrumentation = InstrumentationRegistry.getInstrumentation() + private val context: Context get() = instrumentation.targetContext + private val localeManager get() = context.getSystemService(LocaleManager::class.java) + + private fun setAppLocale(tags: String) = instrumentation.runOnMainSync { + localeManager.applicationLocales = LocaleList.forLanguageTags(tags) + } + + @Before + fun pinHostToArabic() { + setAppLocale("ar") + // LocaleManager applies asynchronously; wait until the host is Arabic. + val deadline = System.currentTimeMillis() + 5_000 + while (localeManager.applicationLocales.toLanguageTags() != "ar" && + System.currentTimeMillis() < deadline + ) { + Thread.sleep(50) + } + assertEquals( + "precondition: host must be pinned to Arabic", + "ar", + localeManager.applicationLocales.toLanguageTags(), + ) + // The above only confirms system-server state. The process-local JVM + // default is what SharinganActivity snapshots/restores, and it updates + // separately — wait for it too, so we don't snapshot a stale value. + val jvmDeadline = System.currentTimeMillis() + 2_000 + while (LocaleList.getDefault()[0].language != "ar" && + System.currentTimeMillis() < jvmDeadline + ) { + Thread.sleep(50) + } + assertEquals( + "precondition: process default locale must be Arabic before launch", + "ar", + LocaleList.getDefault()[0].language, + ) + } + + @After + fun clearAppLocale() = setAppLocale("") + + @Test + fun `Given_Arabic_host_When_logger_opens_and_closes_Then_stays_LTR_and_host_locale_untouched`() { + // Open the logger, assert it renders LTR, then close it. + ActivityScenario.launch(SharinganActivity::class.java).use { logger -> + logger.onActivity { activity -> + assertEquals( + "logger frame must be LTR while open", + View.LAYOUT_DIRECTION_LTR, + activity.resources.configuration.layoutDirection, + ) + } + } + + // Sanity: system-server per-app locale is untouched. This lives in + // system_server, not our process, so the process-local fix can't corrupt + // it — it passes regardless of the bug and just guards the test setup. + assertEquals( + "system per-app locale changed unexpectedly (test setup sanity)", + "ar", + localeManager.applicationLocales.toLanguageTags(), + ) + // The REAL leak assertions: the process-global JVM defaults the buggy + // activity flipped to English must be back to the host's Arabic. + assertEquals("Locale.getDefault() leaked", "ar", Locale.getDefault().language) + assertEquals("LocaleList.getDefault() leaked", "ar", LocaleList.getDefault()[0].language) + } +} diff --git a/sharingan/src/androidMain/kotlin/dev/sharingan/SharinganActivity.kt b/sharingan/src/androidMain/kotlin/dev/sharingan/SharinganActivity.kt index d7c6d42..3e9a506 100644 --- a/sharingan/src/androidMain/kotlin/dev/sharingan/SharinganActivity.kt +++ b/sharingan/src/androidMain/kotlin/dev/sharingan/SharinganActivity.kt @@ -1,17 +1,43 @@ package dev.sharingan +import android.content.Context +import android.content.res.Configuration import android.os.Bundle +import android.os.LocaleList import androidx.activity.ComponentActivity import androidx.activity.compose.setContent import androidx.activity.enableEdgeToEdge import dev.sharingan.ui.SharinganScreen +import java.util.Locale /** * Hosts the Sharingan log browser. Launched by tapping the capture * notification or by calling [show]; apps never need to declare it — * it ships in the library manifest. + * + * The logger is a locale-neutral surface: always English + LTR regardless of + * the host's per-app locale, and it never leaks its locale back into the host + * (issue #38). [attachBaseContext] pins an English/LTR configuration for this + * activity via the non-mutating [Context.createConfigurationContext]; that call + * has the side effect of resetting the process-global [LocaleList] default to + * English, so we snapshot the host's default and restore it — both right after, + * and again in [onDestroy] — leaving the host's locale untouched. */ public class SharinganActivity : ComponentActivity() { + + private var hostLocales: LocaleList? = null + + override fun attachBaseContext(newBase: Context) { + hostLocales = LocaleList.getDefault() + val config = Configuration(newBase.resources.configuration).apply { + setLocale(Locale.ENGLISH) + setLayoutDirection(Locale.ENGLISH) + } + // Non-mutating: createConfigurationContext, never resources.updateConfiguration. + super.attachBaseContext(newBase.createConfigurationContext(config)) + hostLocales?.let(LocaleList::setDefault) + } + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) enableEdgeToEdge() @@ -19,4 +45,9 @@ public class SharinganActivity : ComponentActivity() { SharinganScreen() } } + + override fun onDestroy() { + hostLocales?.let(LocaleList::setDefault) + super.onDestroy() + } } diff --git a/sharingan/src/commonMain/kotlin/dev/sharingan/ui/SharinganScreen.kt b/sharingan/src/commonMain/kotlin/dev/sharingan/ui/SharinganScreen.kt index 2151534..a581884 100644 --- a/sharingan/src/commonMain/kotlin/dev/sharingan/ui/SharinganScreen.kt +++ b/sharingan/src/commonMain/kotlin/dev/sharingan/ui/SharinganScreen.kt @@ -9,6 +9,7 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.safeDrawing import androidx.compose.material3.Scaffold import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue @@ -18,6 +19,8 @@ import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalLayoutDirection +import androidx.compose.ui.unit.LayoutDirection import dev.sharingan.HttpEvent import dev.sharingan.Sharingan import dev.sharingan.SharinganEvent @@ -137,34 +140,39 @@ internal fun SharinganScreenContent( ) { val colors = LocalSharinganColors.current PlatformBackHandler(enabled = selectedEvent != null, onBack = onBack) - Scaffold( - modifier = modifier, - containerColor = colors.bg, - contentWindowInsets = WindowInsets.safeDrawing, - ) { innerPadding -> - Box( - Modifier - .padding(innerPadding) - .consumeWindowInsets(innerPadding) - .fillMaxSize(), - ) { - if (selectedEvent != null) { - DetailScreenContent(event = selectedEvent, onBack = onBack, onShare = onShareSingle) - } else { - HomeScreenContent( - state = homeState, - onSelectProtocol = onSelectProtocol, - onQueryChange = onQueryChange, - onChipChange = onChipChange, - onToggleRecording = onToggleRecording, - onOpenEvent = onOpenEvent, - onShareAll = onShareAll, - ) + // The logger is a locale-neutral surface — always LTR, on every platform + // (including iOS) and in Studio previews, regardless of the host's layout + // direction (issue #38). + CompositionLocalProvider(LocalLayoutDirection provides LayoutDirection.Ltr) { + Scaffold( + modifier = modifier, + containerColor = colors.bg, + contentWindowInsets = WindowInsets.safeDrawing, + ) { innerPadding -> + Box( + Modifier + .padding(innerPadding) + .consumeWindowInsets(innerPadding) + .fillMaxSize(), + ) { + if (selectedEvent != null) { + DetailScreenContent(event = selectedEvent, onBack = onBack, onShare = onShareSingle) + } else { + HomeScreenContent( + state = homeState, + onSelectProtocol = onSelectProtocol, + onQueryChange = onQueryChange, + onChipChange = onChipChange, + onToggleRecording = onToggleRecording, + onOpenEvent = onOpenEvent, + onShareAll = onShareAll, + ) + } + SharinganToast(toastMessage, Modifier.align(Alignment.BottomCenter)) } - SharinganToast(toastMessage, Modifier.align(Alignment.BottomCenter)) } - } - if (shareState != null) { - ShareSheet(state = shareState, onAction = onShareAction, onDismiss = onShareDismiss) + if (shareState != null) { + ShareSheet(state = shareState, onAction = onShareAction, onDismiss = onShareDismiss) + } } }