android/ui: switch lateinint app instance to optional

updates tailscale/corp#12430

The app instance is set in onCreate, but this has a chance to race
with the intent handlers throwing a null pointer exception.

This should ultimately be corrected in the API interface on the caller side, on but
that fix runs deep and still requires care on the part of the caller.  This will,
at least, error out gracefully rather than crashing the application process at the
expense of an action potentially failing.

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
jonathan/npe_crash_1
Jonathan Nobels 2 years ago
parent 634d51c20b
commit 4a79eda410

@ -55,15 +55,15 @@ class App : UninitializedApp(), libtailscale.AppContext {
.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN) .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)
.build() .build()
private lateinit var appInstance: App private var appInstance: App? = null
/** /**
* Initializes the app (if necessary) and returns the singleton app instance. Always use this * Initializes the app (if necessary) and returns the singleton app instance. Always use this
* function to obtain an App reference to make sure the app initializes. * function to obtain an App reference to make sure the app initializes.
*/ */
@JvmStatic @JvmStatic
fun get(): App { fun get(): App? {
appInstance.initOnce() appInstance?.initOnce()
return appInstance return appInstance
} }
} }
@ -82,6 +82,8 @@ class App : UninitializedApp(), libtailscale.AppContext {
override fun onCreate() { override fun onCreate() {
super.onCreate() super.onCreate()
appInstance = this
createNotificationChannel( createNotificationChannel(
STATUS_CHANNEL_ID, STATUS_CHANNEL_ID,
getString(R.string.vpn_status), getString(R.string.vpn_status),
@ -92,7 +94,7 @@ class App : UninitializedApp(), libtailscale.AppContext {
getString(R.string.taildrop_file_transfers), getString(R.string.taildrop_file_transfers),
getString(R.string.notifications_delivered_when_a_file_is_received_using_taildrop), getString(R.string.notifications_delivered_when_a_file_is_received_using_taildrop),
NotificationManagerCompat.IMPORTANCE_DEFAULT) NotificationManagerCompat.IMPORTANCE_DEFAULT)
appInstance = this
setUnprotectedInstance(this) setUnprotectedInstance(this)
} }
@ -128,7 +130,8 @@ class App : UninitializedApp(), libtailscale.AppContext {
applicationScope.launch { applicationScope.launch {
Notifier.state.collect { state -> Notifier.state.collect { state ->
val ableToStartVPN = state > Ipn.State.NeedsMachineAuth val ableToStartVPN = state > Ipn.State.NeedsMachineAuth
// If VPN is stopped, show a disconnected notification. If it is running as a foregrround service, IPNService will show a connected notification. // If VPN is stopped, show a disconnected notification. If it is running as a foregrround
// service, IPNService will show a connected notification.
if (state == Ipn.State.Stopped) { if (state == Ipn.State.Stopped) {
notifyStatus(false) notifyStatus(false)
} }
@ -345,11 +348,11 @@ open class UninitializedApp : Application() {
// File for shared preferences that are not encrypted. // File for shared preferences that are not encrypted.
private const val UNENCRYPTED_PREFERENCES = "unencrypted" private const val UNENCRYPTED_PREFERENCES = "unencrypted"
private lateinit var appInstance: UninitializedApp private var appInstance: UninitializedApp? = null
lateinit var notificationManager: NotificationManagerCompat lateinit var notificationManager: NotificationManagerCompat
@JvmStatic @JvmStatic
fun get(): UninitializedApp { fun get(): UninitializedApp? {
return appInstance return appInstance
} }
} }

@ -27,13 +27,13 @@ open class IPNService : VpnService(), libtailscale.IPNService {
override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int = override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int =
when (intent?.action) { when (intent?.action) {
ACTION_STOP_VPN -> { ACTION_STOP_VPN -> {
App.get().setWantRunning(false) App.get()?.setWantRunning(false)
close() close()
START_NOT_STICKY START_NOT_STICKY
} }
ACTION_START_VPN -> { ACTION_START_VPN -> {
showForegroundNotification() showForegroundNotification()
App.get().setWantRunning(true) App.get()?.setWantRunning(true)
Libtailscale.requestVPN(this) Libtailscale.requestVPN(this)
START_STICKY START_STICKY
} }
@ -41,15 +41,15 @@ open class IPNService : VpnService(), libtailscale.IPNService {
// This means we were started by Android due to Always On VPN. // This means we were started by Android due to Always On VPN.
// We show a non-foreground notification because we weren't // We show a non-foreground notification because we weren't
// started as a foreground service. // started as a foreground service.
App.get().notifyStatus(true) App.get()?.notifyStatus(true)
App.get().setWantRunning(true) App.get()?.setWantRunning(true)
Libtailscale.requestVPN(this) Libtailscale.requestVPN(this)
START_STICKY START_STICKY
} }
else -> { else -> {
// This means that we were restarted after the service was killed // This means that we were restarted after the service was killed
// (potentially due to OOM). // (potentially due to OOM).
if (UninitializedApp.get().isAbleToStartVPN()) { if (UninitializedApp.get()?.isAbleToStartVPN() == true) {
showForegroundNotification() showForegroundNotification()
App.get() App.get()
Libtailscale.requestVPN(this) Libtailscale.requestVPN(this)
@ -78,7 +78,7 @@ open class IPNService : VpnService(), libtailscale.IPNService {
private fun showForegroundNotification() { private fun showForegroundNotification() {
startForeground( startForeground(
UninitializedApp.STATUS_NOTIFICATION_ID, UninitializedApp.STATUS_NOTIFICATION_ID,
UninitializedApp.get().buildStatusNotification(true)) UninitializedApp.get()?.buildStatusNotification(true))
} }
private fun configIntent(): PendingIntent { private fun configIntent(): PendingIntent {

@ -11,7 +11,6 @@ import android.content.RestrictionsManager
import android.content.pm.ActivityInfo import android.content.pm.ActivityInfo
import android.content.res.Configuration.SCREENLAYOUT_SIZE_LARGE import android.content.res.Configuration.SCREENLAYOUT_SIZE_LARGE
import android.content.res.Configuration.SCREENLAYOUT_SIZE_MASK import android.content.res.Configuration.SCREENLAYOUT_SIZE_MASK
import android.net.VpnService
import android.os.Bundle import android.os.Bundle
import android.provider.Settings import android.provider.Settings
import android.util.Log import android.util.Log
@ -117,7 +116,7 @@ class MainActivity : ComponentActivity() {
if (granted) { if (granted) {
Log.d("VpnPermission", "VPN permission granted") Log.d("VpnPermission", "VPN permission granted")
viewModel.setVpnPrepared(true) viewModel.setVpnPrepared(true)
App.get().startVPN() App.get()?.startVPN()
} else { } else {
Log.d("VpnPermission", "VPN permission denied") Log.d("VpnPermission", "VPN permission denied")
viewModel.setVpnPrepared(false) viewModel.setVpnPrepared(false)
@ -285,7 +284,7 @@ class MainActivity : ComponentActivity() {
private fun login(urlString: String) { private fun login(urlString: String) {
// Launch coroutine to listen for state changes. When the user completes login, relaunch // Launch coroutine to listen for state changes. When the user completes login, relaunch
// MainActivity to bring the app back to focus. // MainActivity to bring the app back to focus.
App.get().applicationScope.launch { App.get()?.applicationScope?.launch {
try { try {
Notifier.state.collect { state -> Notifier.state.collect { state ->
if (state > Ipn.State.NeedsMachineAuth) { if (state > Ipn.State.NeedsMachineAuth) {
@ -324,20 +323,20 @@ class MainActivity : ComponentActivity() {
override fun onResume() { override fun onResume() {
super.onResume() super.onResume()
App.get()?.let {
val restrictionsManager = val restrictionsManager =
this.getSystemService(Context.RESTRICTIONS_SERVICE) as RestrictionsManager this.getSystemService(Context.RESTRICTIONS_SERVICE) as RestrictionsManager
lifecycleScope.launch(Dispatchers.IO) { MDMSettings.update(App.get(), restrictionsManager) } lifecycleScope.launch(Dispatchers.IO) { MDMSettings.update(it, restrictionsManager) }
} }
override fun onStart() {
super.onStart()
} }
override fun onStop() { override fun onStop() {
super.onStop() super.onStop()
App.get()?.let {
val restrictionsManager = val restrictionsManager =
this.getSystemService(Context.RESTRICTIONS_SERVICE) as RestrictionsManager this.getSystemService(Context.RESTRICTIONS_SERVICE) as RestrictionsManager
lifecycleScope.launch(Dispatchers.IO) { MDMSettings.update(App.get(), restrictionsManager) } lifecycleScope.launch(Dispatchers.IO) { MDMSettings.update(it, restrictionsManager) }
}
} }
private fun openApplicationSettings() { private fun openApplicationSettings() {

@ -64,11 +64,17 @@ public class QuickToggleService extends TileService {
public void onClick() { public void onClick() {
boolean r; boolean r;
synchronized (lock) { synchronized (lock) {
r = UninitializedApp.get().isAbleToStartVPN(); UninitializedApp app = UninitializedApp.get();
if (app == null) {
return;
}
r = app.isAbleToStartVPN();
} }
if (r) { if (r) {
// Get the application to make sure it initializes // Get the application to make sure it initializes
App.get(); if (App.get() == null) {
return;
}
onTileClick(); onTileClick();
} else { } else {
// Start main activity. // Start main activity.

@ -28,8 +28,12 @@ public final class StartVPNWorker extends Worker {
@Override @Override
public Result doWork() { public Result doWork() {
UninitializedApp app = UninitializedApp.get(); UninitializedApp app = UninitializedApp.get();
boolean ableToStartVPN = app.isAbleToStartVPN(); if (app == null) {
if (ableToStartVPN) { android.util.Log.e("StartVPNWorker", "App is not yet initialized, returning failure.");
return Result.failure();
}
if (app.isAbleToStartVPN()) {
if (VpnService.prepare(app) == null) { if (VpnService.prepare(app) == null) {
// We're ready and have permissions, start the VPN // We're ready and have permissions, start the VPN
app.startVPN(); app.startVPN();

@ -23,7 +23,13 @@ public final class StopVPNWorker extends Worker {
@NonNull @NonNull
@Override @Override
public Result doWork() { public Result doWork() {
UninitializedApp.get().stopVPN(); UninitializedApp app = UninitializedApp.get();
if (app == null) {
android.util.Log.e("StopVPNWorker", "App is not yet initialized, returning failure.");
return Result.failure();
}
app.stopVPN();
return Result.success(); return Result.success();
} }
} }

@ -10,7 +10,6 @@ import androidx.work.CoroutineWorker
import androidx.work.Data import androidx.work.Data
import androidx.work.WorkerParameters import androidx.work.WorkerParameters
import com.tailscale.ipn.UninitializedApp.Companion.STATUS_CHANNEL_ID import com.tailscale.ipn.UninitializedApp.Companion.STATUS_CHANNEL_ID
import com.tailscale.ipn.UninitializedApp.Companion.STATUS_EXIT_NODE_FAILURE_NOTIFICATION_ID
import com.tailscale.ipn.ui.localapi.Client import com.tailscale.ipn.ui.localapi.Client
import com.tailscale.ipn.ui.model.Ipn import com.tailscale.ipn.ui.model.Ipn
import com.tailscale.ipn.ui.notifier.Notifier import com.tailscale.ipn.ui.notifier.Notifier
@ -18,40 +17,40 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job import kotlinx.coroutines.Job
class UseExitNodeWorker( class UseExitNodeWorker(appContext: Context, workerParams: WorkerParameters) :
appContext: Context, CoroutineWorker(appContext, workerParams) {
workerParams: WorkerParameters
) : CoroutineWorker(appContext, workerParams) {
override suspend fun doWork(): Result { override suspend fun doWork(): Result {
val app = UninitializedApp.get() val app = UninitializedApp.get() ?: return Result.failure()
suspend fun runAndGetResult(): String? { suspend fun runAndGetResult(): String? {
val exitNodeName = inputData.getString(EXIT_NODE_NAME) val exitNodeName = inputData.getString(EXIT_NODE_NAME)
val exitNodeId = if (exitNodeName.isNullOrEmpty()) { val exitNodeId =
if (exitNodeName.isNullOrEmpty()) {
null null
} else { } else {
if (!app.isAbleToStartVPN()) { if (app.isAbleToStartVPN() != true) {
return app.getString(R.string.vpn_is_not_ready_to_start) return app.getString(R.string.vpn_is_not_ready_to_start)
} }
val peers = val peers =
(Notifier.netmap.value (Notifier.netmap.value
?: run { return@runAndGetResult app.getString(R.string.tailscale_is_not_setup) }) ?: run {
.Peers ?: run { return@runAndGetResult app.getString(R.string.no_peers_found) } return@runAndGetResult app.getString(R.string.tailscale_is_not_setup)
})
.Peers
?: run {
return@runAndGetResult app.getString(R.string.no_peers_found)
}
val filteredPeers = peers.filter { val filteredPeers = peers.filter { it.displayName == exitNodeName }.toList()
it.displayName == exitNodeName
}.toList()
if (filteredPeers.isEmpty()) { if (filteredPeers.isEmpty()) {
return app.getString(R.string.no_peers_with_name_found, exitNodeName) return app.getString(R.string.no_peers_with_name_found, exitNodeName)
} else if (filteredPeers.size > 1) { } else if (filteredPeers.size > 1) {
return app.getString(R.string.multiple_peers_with_name_found, exitNodeName) return app.getString(R.string.multiple_peers_with_name_found, exitNodeName)
} else if (!filteredPeers[0].isExitNode) { } else if (!filteredPeers[0].isExitNode) {
return app.getString( return app.getString(R.string.peer_with_name_is_not_an_exit_node, exitNodeName)
R.string.peer_with_name_is_not_an_exit_node,
exitNodeName
)
} }
filteredPeers[0].StableID filteredPeers[0].StableID
@ -65,7 +64,8 @@ class UseExitNodeWorker(
val scope = CoroutineScope(Dispatchers.Default + Job()) val scope = CoroutineScope(Dispatchers.Default + Job())
var result: String? = null var result: String? = null
Client(scope).editPrefs(prefsOut) { Client(scope).editPrefs(prefsOut) {
result = if (it.isFailure) { result =
if (it.isFailure) {
it.exceptionOrNull()?.message it.exceptionOrNull()?.message
} else { } else {
null null
@ -88,7 +88,8 @@ class UseExitNodeWorker(
PendingIntent.getActivity( PendingIntent.getActivity(
app, 1, intent, PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE) app, 1, intent, PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE)
val notification = NotificationCompat.Builder(app, STATUS_CHANNEL_ID) val notification =
NotificationCompat.Builder(app, STATUS_CHANNEL_ID)
.setSmallIcon(R.drawable.ic_notification) .setSmallIcon(R.drawable.ic_notification)
.setContentTitle(app.getString(R.string.use_exit_node_intent_failed)) .setContentTitle(app.getString(R.string.use_exit_node_intent_failed))
.setContentText(result) .setContentText(result)

@ -44,7 +44,7 @@ class AlwaysNeverUserDecidesMDMSetting(key: String, localizedTitle: String) :
override fun getFrom(bundle: Bundle?, app: App): AlwaysNeverUserDecides { override fun getFrom(bundle: Bundle?, app: App): AlwaysNeverUserDecides {
val storedString = val storedString =
bundle?.getString(key) bundle?.getString(key)
?: App.get().getEncryptedPrefs().getString(key, null) ?: App.get()?.getEncryptedPrefs()?.getString(key, null)
?: "user-decides" ?: "user-decides"
return when (storedString) { return when (storedString) {
"always" -> { "always" -> {
@ -64,7 +64,7 @@ class ShowHideMDMSetting(key: String, localizedTitle: String) :
MDMSetting<ShowHide>(ShowHide.Show, key, localizedTitle) { MDMSetting<ShowHide>(ShowHide.Show, key, localizedTitle) {
override fun getFrom(bundle: Bundle?, app: App): ShowHide { override fun getFrom(bundle: Bundle?, app: App): ShowHide {
val storedString = val storedString =
bundle?.getString(key) ?: App.get().getEncryptedPrefs().getString(key, null) ?: "show" bundle?.getString(key) ?: App.get()?.getEncryptedPrefs()?.getString(key, null) ?: "show"
return when (storedString) { return when (storedString) {
"hide" -> { "hide" -> {
ShowHide.Hide ShowHide.Hide

@ -14,7 +14,11 @@ import com.tailscale.ipn.ui.util.AndroidTVUtil.isAndroidTV
object AndroidTVUtil { object AndroidTVUtil {
fun isAndroidTV(): Boolean { fun isAndroidTV(): Boolean {
val pm = UninitializedApp.get().packageManager // Not ideal, but this is invoked from a Composable, at which point,
// we *will* have an Uninitialized app instance. If this method is being
// called extremely early in the app lifecycle, you may get the wrong
// answer here.
val pm = UninitializedApp.get()?.packageManager ?: return false
return (pm.hasSystemFeature(PackageManager.FEATURE_TELEVISION) || return (pm.hasSystemFeature(PackageManager.FEATURE_TELEVISION) ||
pm.hasSystemFeature(PackageManager.FEATURE_LEANBACK)) pm.hasSystemFeature(PackageManager.FEATURE_LEANBACK))
} }

@ -152,11 +152,11 @@ open class IpnViewModel : ViewModel() {
} }
fun startVPN() { fun startVPN() {
UninitializedApp.get().startVPN() UninitializedApp.get()?.startVPN()
} }
fun stopVPN() { fun stopVPN() {
UninitializedApp.get().stopVPN() UninitializedApp.get()?.stopVPN()
} }
// Login/Logout // Login/Logout

Loading…
Cancel
Save