Prevent shell response crashes

app/src/main/java/com/topjohnwu/magisk/Global.java

@@ -3,22 +3,26 @@ package com.topjohnwu.magisk;
 import android.content.Context;
 import android.content.SharedPreferences;
 import android.preference.PreferenceManager;
+import android.text.TextUtils;
 
 import com.topjohnwu.magisk.module.Module;
 import com.topjohnwu.magisk.module.Repo;
 import com.topjohnwu.magisk.utils.CallbackHandler;
+import com.topjohnwu.magisk.utils.Shell;
+import com.topjohnwu.magisk.utils.Utils;
 import com.topjohnwu.magisk.utils.ValueSortedMap;
 
 import java.util.List;
 
 public class Global {
+
     public static class Constant {
         // No global constants now
     }
     public static class Info {
         public static double magiskVersion;
-        public static double remoteMagiskVersion = -1;
         public static String magiskVersionString = "(none)";
+        public static double remoteMagiskVersion = -1;
         public static String magiskLink;
         public static String releaseNoteLink;
         public static int SNCheckResult = -1;
@@ -43,11 +47,28 @@ public class Global {
         public static boolean shellLogging;
         public static boolean devLogging;
 
-        public static void init(Context context) {
+    }
-            SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context);
+
-            isDarkTheme = prefs.getBoolean("dark_theme", false);
+    public static void init(Context context) {
-            devLogging = prefs.getBoolean("developer_logging", false);
+        SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context);
-            shellLogging = prefs.getBoolean("shell_logging", false);
+        Configs.isDarkTheme = prefs.getBoolean("dark_theme", false);
+        Configs.devLogging = prefs.getBoolean("developer_logging", false);
+        Configs.shellLogging = prefs.getBoolean("shell_logging", false);
+        updateMagiskInfo();
+    }
+
+    static void updateMagiskInfo() {
+        List<String> ret = Shell.sh("getprop magisk.version");
+        if (Utils.isValidShellResponse(ret)) {
+            Info.magiskVersion = -1;
+        } else {
+            try {
+                Info.magiskVersionString = ret.get(0);
+                Info.magiskVersion = Double.parseDouble(ret.get(0));
+            } catch (NumberFormatException e) {
+                // Custom version don't need to receive updates
+                Info.magiskVersion = Double.POSITIVE_INFINITY;
+            }
         }
     }
 

app/src/main/java/com/topjohnwu/magisk/InstallFragment.java

@@ -46,7 +46,11 @@ public class InstallFragment extends Fragment implements CallbackHandler.EventLi
         currentVersionTitle.setText(getString(R.string.current_magisk_title, Global.Info.magiskVersionString));
         installTitle.setText(getString(R.string.install_magisk_title, Global.Info.remoteMagiskVersion));
         flashButton.setOnClickListener(v1 -> {
-            String bootImage = Global.Info.bootBlock;
+            String bootImage;
+            if (spinner.getSelectedItemPosition() > 0)
+                bootImage = Global.Data.blockList.get(spinner.getSelectedItemPosition() - 1);
+            else
+                bootImage = Global.Info.bootBlock;
             if (bootImage == null) {
                 bootImage = Global.Data.blockList.get(spinner.getSelectedItemPosition() - 1);
             }

app/src/main/java/com/topjohnwu/magisk/LogFragment.java

@@ -14,6 +14,7 @@ import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.design.widget.Snackbar;
 import android.support.v4.app.ActivityCompat;
+import android.text.TextUtils;
 import android.view.LayoutInflater;
 import android.view.Menu;
 import android.view.MenuInflater;
@@ -143,12 +144,14 @@ public class LogFragment extends Fragment {
                 case 0:
                     List<String> logList = Utils.readFile(MAGISK_LOG);
 
-                    StringBuilder llog = new StringBuilder(15 * 10 * 1024);
+                    if (Utils.isValidShellResponse(logList)) {
-                    for (String s : logList) {
+                        StringBuilder llog = new StringBuilder(15 * 10 * 1024);
-                        llog.append(s).append("\n");
+                        for (String s : logList) {
+                            llog.append(s).append("\n");
+                        }
+                        return llog.toString();
                     }
-
+                    return "";
-                    return llog.toString();
 
                 case 1:
                     Shell.su("echo > " + MAGISK_LOG);
@@ -182,21 +185,24 @@ public class LogFragment extends Fragment {
 
                     List<String> in = Utils.readFile(MAGISK_LOG);
 
-                    try (FileWriter out = new FileWriter(targetFile)) {
+                    if (Utils.isValidShellResponse(in)) {
-                        for (String line : in) {
+                        try (FileWriter out = new FileWriter(targetFile)) {
-                            out.write(line + "\n");
+                            for (String line : in)
+                                out.write(line + "\n");
+                            return true;
+                        } catch (IOException e) {
+                            e.printStackTrace();
+                            return false;
                         }
-                        return true;
-                    } catch (IOException e) {
-                        e.printStackTrace();
-                        return false;
                     }
+                    return false;
             }
             return null;
         }
 
         @Override
         protected void onPostExecute(Object o) {
+            if (o == null) return;
             boolean bool;
             String llog;
             switch (mode) {
@@ -204,7 +210,7 @@ public class LogFragment extends Fragment {
                 case 1:
                     llog = (String) o;
                     progressBar.setVisibility(View.GONE);
-                    if (llog.length() == 0)
+                    if (TextUtils.isEmpty(llog))
                         txtLog.setText(R.string.log_is_empty);
                     else
                         txtLog.setText(llog);

app/src/main/java/com/topjohnwu/magisk/SettingsActivity.java

@@ -115,8 +115,8 @@ public class SettingsActivity extends AppCompatActivity {
         }
 
         @Override
-        public void onStop() {
+        public void onPause() {
-            super.onStop();
+            super.onPause();
             prefs.unregisterOnSharedPreferenceChangeListener(this);
         }
 

app/src/main/java/com/topjohnwu/magisk/SplashActivity.java

@@ -17,7 +17,7 @@ public class SplashActivity extends AppCompatActivity {
         super.onCreate(savedInstanceState);
         SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(getApplicationContext());
 
-        Global.Configs.init(getApplicationContext());
+        Global.init(getApplicationContext());
 
         if (Global.Configs.isDarkTheme) {
             setTheme(R.style.AppTheme_dh);

app/src/main/java/com/topjohnwu/magisk/StatusFragment.java

@@ -59,10 +59,6 @@ public class StatusFragment extends Fragment implements CallbackHandler.EventLis
     @BindColor(android.R.color.transparent) int trans;
     int defaultColor;
 
-    static {
-        checkMagiskInfo();
-    }
-
     private AlertDialog updateMagisk;
 
     @Nullable
@@ -162,25 +158,10 @@ public class StatusFragment extends Fragment implements CallbackHandler.EventLis
         unbinder.unbind();
     }
 
-    private static void checkMagiskInfo() {
-        List<String> ret = Shell.sh("getprop magisk.version");
-        if (ret.get(0).length() == 0) {
-            Global.Info.magiskVersion = -1;
-        } else {
-            try {
-                Global.Info.magiskVersionString = ret.get(0);
-                Global.Info.magiskVersion = Double.parseDouble(ret.get(0));
-            } catch (NumberFormatException e) {
-                // Custom version don't need to receive updates
-                Global.Info.magiskVersion = Double.POSITIVE_INFINITY;
-            }
-        }
-    }
-
     private void updateUI() {
         int image, color;
 
-        checkMagiskInfo();
+        Global.updateMagiskInfo();
 
         if (Global.Info.magiskVersion < 0) {
             magiskVersionText.setText(R.string.magisk_version_error);
@@ -188,23 +169,26 @@ public class StatusFragment extends Fragment implements CallbackHandler.EventLis
             magiskVersionText.setText(getString(R.string.magisk_version, Global.Info.magiskVersionString));
         }
 
-        if (Shell.rootStatus == 1) {
+        switch (Shell.rootStatus) {
-            color = colorOK;
+            case 0:
-            image = R.drawable.ic_check_circle;
-            rootStatusText.setText(R.string.proper_root);
-            rootInfoText.setText(Shell.sh("su -v").get(0));
-
-        } else {
-            rootInfoText.setText(R.string.root_info_warning);
-            if (Shell.rootStatus == 0) {
                 color = colorBad;
                 image = R.drawable.ic_cancel;
                 rootStatusText.setText(R.string.not_rooted);
-            } else {
+                break;
+            case 1:
+                List<String> stats = Shell.sh("su -v");
+                if (Utils.isValidShellResponse(stats)) {
+                    color = colorOK;
+                    image = R.drawable.ic_check_circle;
+                    rootStatusText.setText(R.string.proper_root);
+                    rootInfoText.setText(stats.get(0));
+                    break;
+                }
+            case -1:
+            default:
                 color = colorNeutral;
                 image = R.drawable.ic_help;
                 rootStatusText.setText(R.string.root_error);
-            }
         }
         rootStatusContainer.setBackgroundColor(color);
         rootStatusText.setTextColor(color);

app/src/main/java/com/topjohnwu/magisk/receivers/RepoDlReceiver.java

@@ -4,7 +4,7 @@ import android.net.Uri;
 
 import com.topjohnwu.magisk.R;
 import com.topjohnwu.magisk.utils.Async;
-import com.topjohnwu.magisk.utils.Utils.ByteArrayInOutStream;
+import com.topjohnwu.magisk.utils.ByteArrayInOutStream;
 import com.topjohnwu.magisk.utils.ZipUtils;
 
 import java.io.OutputStream;

app/src/main/java/com/topjohnwu/magisk/utils/Async.java

@@ -218,8 +218,9 @@ public class Async {
 
         protected boolean unzipAndCheck() {
             ZipUtils.unzip(mCachedFile, mCachedFile.getParentFile(), "META-INF/com/google/android");
-            return Utils.readFile(mCachedFile.getParent() + "/META-INF/com/google/android/updater-script")
+            List<String> ret;
-                    .get(0).contains("#MAGISK");
+            ret = Utils.readFile(mCachedFile.getParent() + "/META-INF/com/google/android/updater-script");
+            return Utils.isValidShellResponse(ret) && ret.get(0).contains("#MAGISK");
         }
 
         @Override
@@ -252,6 +253,7 @@ public class Async {
                             "/META-INF/com/google/android/update-binary dummy 1 " + mCachedFile.getPath(),
                     "if [ $? -eq 0 ]; then echo true; else echo false; fi"
             );
+            if (!Utils.isValidShellResponse(ret)) return -1;
             Logger.dev("FlashZip: Console log:");
             for (String line : ret) {
                 Logger.dev(line);
@@ -293,7 +295,7 @@ public class Async {
             Utils.getAlertDialogBuilder(mContext)
                     .setTitle(R.string.reboot_title)
                     .setMessage(R.string.reboot_msg)
-                    .setPositiveButton(R.string.reboot, (dialogInterface1, i) -> Shell.sh("su -c reboot"))
+                    .setPositiveButton(R.string.reboot, (dialogInterface, i) -> Shell.sh("su -c reboot"))
                     .setNegativeButton(R.string.no_thanks, null)
                     .show();
         }
@@ -323,9 +325,8 @@ public class Async {
         protected Void doInBackground(Void... params) {
             if (Shell.rootAccess()) {
                 Global.Data.blockList = Shell.su("ls /dev/block | grep mmc");
-                if (Global.Info.bootBlock == null) {
+                if (Global.Info.bootBlock == null)
                     Global.Info.bootBlock = Utils.detectBootImage();
-                }
             }
             return null;
         }

app/src/main/java/com/topjohnwu/magisk/utils/ByteArrayInOutStream.java

@@ -0,0 +1,18 @@
+package com.topjohnwu.magisk.utils;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+
+public class ByteArrayInOutStream extends ByteArrayOutputStream {
+    public ByteArrayInputStream getInputStream() {
+        ByteArrayInputStream in = new ByteArrayInputStream(buf, 0, count);
+        count = 0;
+        buf = new byte[32];
+        return in;
+    }
+
+    public void setBuffer(byte[] buffer) {
+        buf = buffer;
+        count = buffer.length;
+    }
+}

app/src/main/java/com/topjohnwu/magisk/utils/Shell.java

@@ -170,6 +170,7 @@ public class Shell {
                     try {
                         // Process terminated, it means the interactive shell has some issues
                         process.exitValue();
+                        rootStatus = -1;
                         return null;
                     } catch (IllegalThreadStateException e) {
                         // Process still running, gobble output until done
@@ -183,17 +184,22 @@ public class Shell {
                                 break;
                             }
                         }
-                        try { STDOUT.join(100); } catch (InterruptedException err) { return null; }
+                        try { STDOUT.join(100); } catch (InterruptedException err) {
+                            rootStatus = -1;
+                            return null;
+                        }
                     }
                 }
             }
         } catch (IOException e) {
             if (!e.getMessage().contains("EPIPE")) {
                 Logger.dev("Shell: Root shell error...");
+                rootStatus = -1;
                 return null;
             }
         } catch(InterruptedException e) {
             Logger.dev("Shell: Root shell error...");
+            rootStatus = -1;
             return null;
         }
 

app/src/main/java/com/topjohnwu/magisk/utils/Utils.java

@@ -31,29 +31,32 @@ public class Utils {
 
     public static boolean itemExist(boolean root, String path) {
         String command = "if [ -e " + path + " ]; then echo true; else echo false; fi";
+        List<String> ret;
         if (Shell.rootAccess() && root) {
-            return Boolean.parseBoolean(Shell.su(command).get(0));
+            ret = Shell.su(command);
+            return isValidShellResponse(ret) && Boolean.parseBoolean(ret.get(0));
         } else {
             return new File(path).exists();
         }
     }
 
     public static boolean commandExists(String s) {
-        List<String> ret;
         String command = "if [ -z $(which " + s + ") ]; then echo false; else echo true; fi";
-        ret = Shell.sh(command);
+        List<String> ret = Shell.sh(command);
-        return Boolean.parseBoolean(ret.get(0));
+        return isValidShellResponse(ret) && Boolean.parseBoolean(ret.get(0));
     }
 
     public static boolean createFile(String path) {
         String folder = path.substring(0, path.lastIndexOf('/'));
         String command = "mkdir -p " + folder + " 2>/dev/null; touch " + path + " 2>/dev/null; if [ -f \"" + path + "\" ]; then echo true; else echo false; fi";
-        return Shell.rootAccess() && Boolean.parseBoolean(Shell.su(command).get(0));
+        List<String> ret = Shell.su(command);
+        return isValidShellResponse(ret) && Boolean.parseBoolean(ret.get(0));
     }
 
     public static boolean removeItem(String path) {
         String command = "rm -rf " + path + " 2>/dev/null; if [ -e " + path + " ]; then echo false; else echo true; fi";
-        return Shell.rootAccess() && Boolean.parseBoolean(Shell.su(command).get(0));
+        List<String> ret = Shell.su(command);
+        return isValidShellResponse(ret) && Boolean.parseBoolean(ret.get(0));
     }
 
     public static List<String> getModList(String path) {
@@ -118,9 +121,8 @@ public class Utils {
                 "echo \"${BOOTIMAGE##*/}\""
         };
         List<String> ret = Shell.su(commands);
-        if (!ret.isEmpty()) {
+        if (isValidShellResponse(ret))
             return ret.get(0);
-        }
         return null;
     }
 
@@ -136,17 +138,14 @@ public class Utils {
         return !TextUtils.isEmpty(string) && string.toString().toLowerCase().contains(nonNullLowercaseSearch);
     }
 
-    public static class ByteArrayInOutStream extends ByteArrayOutputStream {
+    public static boolean isValidShellResponse(List<String> list) {
-        public ByteArrayInputStream getInputStream() {
+        if (list != null && list.size() != 0) {
-            ByteArrayInputStream in = new ByteArrayInputStream(buf, 0, count);
+            // Check if all empty
-            count = 0;
+            for (String res : list) {
-            buf = new byte[32];
+                if (!TextUtils.isEmpty(res)) return true;
-            return in;
+            }
-        }
-        public void setBuffer(byte[] buffer) {
-            buf = buffer;
-            count = buffer.length;
         }
+        return false;
     }
 
 }

app/src/main/java/com/topjohnwu/magisk/utils/ZipUtils.java

@@ -3,8 +3,6 @@ package com.topjohnwu.magisk.utils;
 import android.content.Context;
 import android.util.Pair;
 
-import com.topjohnwu.magisk.utils.Utils.ByteArrayInOutStream;
-
 import org.spongycastle.asn1.ASN1InputStream;
 import org.spongycastle.asn1.ASN1ObjectIdentifier;
 import org.spongycastle.asn1.DEROutputStream;