ToolsInstaller: safer state machine

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

3 files changed, 42 insertions, 39 deletions

diff --git a/app/src/main/java/com/wireguard/android/preference/ToolsInstallerPreference.java b/app/src/main/java/com/wireguard/android/preference/ToolsInstallerPreference.java
index f8cab060..2b63ba0e 100644
--- a/app/src/main/java/com/wireguard/android/preference/ToolsInstallerPreference.java
+++ b/app/src/main/java/com/wireguard/android/preference/ToolsInstallerPreference.java
@@ -11,9 +11,11 @@ import android.support.annotation.NonNull;
 import android.support.v7.preference.Preference;
 import android.system.OsConstants;
 import android.util.AttributeSet;
+import android.util.Log;
 
 import com.wireguard.android.Application;
 import com.wireguard.android.R;
+import com.wireguard.android.util.ToolsInstaller;
 
 /**
  * Preference implementing a button that asynchronously runs {@code ToolsInstaller} and displays the
@@ -43,28 +45,34 @@ public class ToolsInstallerPreference extends Preference {
         Application.getAsyncWorker().supplyAsync(Application.getToolsInstaller()::areInstalled).whenComplete(this::onCheckResult);
     }
 
-    private void onCheckResult(final Integer result, final Throwable throwable) {
-        setState(throwable == null && result == OsConstants.EALREADY ?
-                State.ALREADY : initialState());
+    private void onCheckResult(final int state, final Throwable throwable) {
+        if (throwable != null || state == ToolsInstaller.ERROR)
+            setState(State.INITIAL);
+        else if ((state & ToolsInstaller.YES) == ToolsInstaller.YES)
+            setState(State.ALREADY);
+        else if ((state & (ToolsInstaller.MAGISK | ToolsInstaller.NO)) == (ToolsInstaller.MAGISK | ToolsInstaller.NO))
+            setState(State.INITIAL_MAGISK);
+        else if ((state & (ToolsInstaller.SYSTEM | ToolsInstaller.NO)) == (ToolsInstaller.SYSTEM | ToolsInstaller.NO))
+            setState(State.INITIAL_SYSTEM);
+        else
+            setState(State.INITIAL);
     }
 
     @Override
     protected void onClick() {
-        setState(workingState());
+        setState(State.WORKING);
         Application.getAsyncWorker().supplyAsync(Application.getToolsInstaller()::install).whenComplete(this::onInstallResult);
     }
 
     private void onInstallResult(final Integer result, final Throwable throwable) {
-        final State nextState;
         if (throwable != null)
-            nextState = State.FAILURE;
-        else if (result == OsConstants.EXIT_SUCCESS)
-            nextState = successState();
-        else if (result == OsConstants.EALREADY)
-            nextState = State.ALREADY;
+            setState(State.FAILURE);
+        else if ((result & (ToolsInstaller.YES | ToolsInstaller.MAGISK)) == (ToolsInstaller.YES | ToolsInstaller.MAGISK))
+            setState(State.SUCCESS_MAGISK);
+        else if ((result & (ToolsInstaller.YES | ToolsInstaller.SYSTEM)) == (ToolsInstaller.YES | ToolsInstaller.SYSTEM))
+            setState(State.SUCCESS_SYSTEM);
         else
-            nextState = State.FAILURE;
-        setState(nextState);
+            setState(State.FAILURE);
     }
 
     private void setState(@NonNull final State state) {
@@ -76,26 +84,15 @@ public class ToolsInstallerPreference extends Preference {
         notifyChanged();
     }
 
-    private State initialState() {
-        return Application.getToolsInstaller().willInstallAsMagiskModule(false) ? State.INITIAL_MAGISK : State.INITIAL_SYSTEM;
-    }
-    private State workingState() {
-        return Application.getToolsInstaller().willInstallAsMagiskModule(false) ? State.WORKING_MAGISK : State.WORKING_SYSTEM;
-    }
-    private State successState() {
-        return Application.getToolsInstaller().willInstallAsMagiskModule(false) ? State.SUCCESS_MAGISK : State.SUCCESS_SYSTEM;
-    }
-
     private enum State {
         INITIAL(R.string.tools_installer_initial, true),
         ALREADY(R.string.tools_installer_already, false),
         FAILURE(R.string.tools_installer_failure, true),
+        WORKING(R.string.tools_installer_working, false),
         INITIAL_SYSTEM(R.string.tools_installer_initial_system, true),
         SUCCESS_SYSTEM(R.string.tools_installer_success_system, false),
-        WORKING_SYSTEM(R.string.tools_installer_working_system, false),
         INITIAL_MAGISK(R.string.tools_installer_initial_magisk, true),
-        SUCCESS_MAGISK(R.string.tools_installer_success_magisk, false),
-        WORKING_MAGISK(R.string.tools_installer_working_magisk, false);
+        SUCCESS_MAGISK(R.string.tools_installer_success_magisk, false);
 
         private final int messageResourceId;
         private final boolean shouldEnableView;
diff --git a/app/src/main/java/com/wireguard/android/util/ToolsInstaller.java b/app/src/main/java/com/wireguard/android/util/ToolsInstaller.java
index dbd3085e..724d93b0 100644
--- a/app/src/main/java/com/wireguard/android/util/ToolsInstaller.java
+++ b/app/src/main/java/com/wireguard/android/util/ToolsInstaller.java
@@ -25,6 +25,12 @@ import java.util.List;
  */
 
 public final class ToolsInstaller {
+    public static final int ERROR = 0x0;
+    public static final int YES = 0x1;
+    public static final int NO = 0x2;
+    public static final int MAGISK = 0x4;
+    public static final int SYSTEM = 0x8;
+
     private static final String[][] EXECUTABLES = {
             {"libwg.so", "wg"},
             {"libwg-quick.so", "wg-quick"},
@@ -60,9 +66,8 @@ public final class ToolsInstaller {
     }
 
     public int areInstalled() throws NoRootException {
-        willInstallAsMagiskModule(true);
         if (INSTALL_DIR == null)
-            return OsConstants.ENOENT;
+            return ERROR;
         final StringBuilder script = new StringBuilder();
         for (final String[] names : EXECUTABLES) {
             script.append(String.format("cmp -s '%s' '%s' && ",
@@ -71,9 +76,13 @@ public final class ToolsInstaller {
         }
         script.append("exit ").append(OsConstants.EALREADY).append(';');
         try {
-            return Application.getRootShell().run(null, script.toString());
+            final int ret = Application.getRootShell().run(null, script.toString());
+            if (ret == OsConstants.EALREADY)
+                return willInstallAsMagiskModule() ? YES | MAGISK : YES | SYSTEM;
+            else
+                return willInstallAsMagiskModule() ? NO | MAGISK : NO | SYSTEM;
         } catch (final IOException ignored) {
-            return OsConstants.EXIT_FAILURE;
+            return ERROR;
         }
     }
 
@@ -97,11 +106,9 @@ public final class ToolsInstaller {
         }
     }
 
-    public boolean willInstallAsMagiskModule(boolean checkForIt) {
+    private boolean willInstallAsMagiskModule() {
         synchronized (lock) {
             if (installAsMagiskModule == null) {
-                if (!checkForIt)
-                    throw new RuntimeException("Expected to already know whether this is a Magisk system");
                 try {
                     installAsMagiskModule = Application.getRootShell().run(null, "[ -d /sbin/.core/mirror -a -d /sbin/.core/img -a ! -f /cache/.disable_magisk ]") == OsConstants.EXIT_SUCCESS;
                 } catch (final Exception ignored) {
@@ -123,9 +130,9 @@ public final class ToolsInstaller {
                     new File(nativeLibraryDir, names[0]), destination, destination, destination));
         }
         try {
-            return Application.getRootShell().run(null, script.toString());
+            return Application.getRootShell().run(null, script.toString()) == 0 ? YES | SYSTEM : ERROR;
         } catch (final IOException ignored) {
-            return OsConstants.EXIT_FAILURE;
+            return ERROR;
         }
     }
 
@@ -144,14 +151,14 @@ public final class ToolsInstaller {
         script.append("trap - INT TERM EXIT;");
 
         try {
-            return Application.getRootShell().run(null, script.toString());
+            return Application.getRootShell().run(null, script.toString()) == 0 ? YES | MAGISK : ERROR;
         } catch (final IOException ignored) {
-            return OsConstants.EXIT_FAILURE;
+            return ERROR;
         }
     }
 
     public int install() throws NoRootException {
-        return willInstallAsMagiskModule(true) ? installMagisk() : installSystem();
+        return willInstallAsMagiskModule() ? installMagisk() : installSystem();
     }
 
     public int symlink() throws NoRootException {
diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
index 7abbcc3d..cbf0542e 100644
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@@ -70,13 +70,12 @@
     <string name="tools_installer_already">wg and wg-quick are already installed</string>
     <string name="tools_installer_failure">Unable to install command-line tools (no root?)</string>
     <string name="tools_installer_initial">Install optional tools for scripting</string>
+    <string name="tools_installer_working">Installing wg and wg-quick</string>
     <string name="tools_installer_initial_system">Install optional tools for scripting into the system partition</string>
     <string name="tools_installer_initial_magisk">Install optional tools for scripting as Magisk module</string>
     <string name="tools_installer_success_system">wg and wg-quick installed into the system partition</string>
     <string name="tools_installer_success_magisk">wg and wg-quick installed as a Magisk module (reboot required)</string>
     <string name="tools_installer_title">Install command line tools</string>
-    <string name="tools_installer_working_system">Installing wg and wg-quick into the system partition</string>
-    <string name="tools_installer_working_magisk">Installing wg and wg-quick as a Magisk module</string>
     <string name="tunnel_create_error">Unable to create tunnel: %s</string>
     <string name="tunnel_create_success">Successfully created tunnel “%s”</string>
     <string name="tunnel_rename_error">Unable to rename tunnel: %s</string>