Issue 1771 - Update RootTools

src/org/adblockplus/android/configurators/IptablesProxyConfigurator.java

Index: src/org/adblockplus/android/configurators/IptablesProxyConfigurator.java
diff --git a/src/org/adblockplus/android/configurators/IptablesProxyConfigurator.java b/src/org/adblockplus/android/configurators/IptablesProxyConfigurator.java
index 0017d5aa5089caf618c35ce77c8a5b5bfc9ee370..12aa1c525688df272560d4ae0cca80728d8fd2be 100644
--- a/src/org/adblockplus/android/configurators/IptablesProxyConfigurator.java
+++ b/src/org/adblockplus/android/configurators/IptablesProxyConfigurator.java
@@ -19,16 +19,23 @@ package org.adblockplus.android.configurators;
 
 import java.io.File;
 import java.io.FileNotFoundException;
+import java.io.IOException;
 import java.net.InetAddress;
+import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.Semaphore;
+import java.util.concurrent.TimeoutException;
 
 import org.adblockplus.android.Utils;
 
-import com.stericson.RootTools.RootTools;
-
 import android.content.Context;
 import android.util.Log;
 
+import com.stericson.RootTools.RootTools;
+import com.stericson.RootTools.exceptions.RootDeniedException;
+import com.stericson.RootTools.execution.Command;
+import com.stericson.RootTools.execution.Shell;
+
 /**
  * Proxy registration using RootTools and iptables.
  */
@@ -48,11 +55,28 @@ public class IptablesProxyConfigurator implements ProxyConfigurator
     this.context = context;
   }
 
+  private static List<String> sendShell(final String command, final int timeout) throws IOException, TimeoutException,

[Felix Dahlke, 2014/11/19 10:38:39]

I think "sendShell" is not a great name, I can see why they changed it. How
about going with "runRootCommand" or something similar here instead, for
clarity?

[René Jeschke, 2014/11/24 13:07:57]

Right, makes sense, will change this.

+      RootDeniedException
+  {
+    final CommandOutput cmd = new CommandOutput(0, DEFAULT_TIMEOUT, command);
+
+    Shell.runRootCommand(cmd);
+
+    cmd.waitForCompletion();
+
+    return cmd.output;
+  }
+
   @Override
   public boolean initialize()
   {
     try
     {
+      // If we don't set `handlerEnabled` to `false`, RootTools uses Handlers
+      // which get executed on the UI thread which in fact renders it useless
+      // for our purpose (as it either finished too late or blocks forever).
+      RootTools.handlerEnabled = false;
+
       if (!RootTools.isAccessGiven())
       {
         throw new IllegalStateException("No root access");
@@ -67,14 +91,14 @@ public class IptablesProxyConfigurator implements ProxyConfigurator
 
       final String path = ipt.getAbsolutePath();
 
-      RootTools.sendShell("chmod 700 " + path, DEFAULT_TIMEOUT);
+      sendShell("chmod 700 " + path, DEFAULT_TIMEOUT);
 
       boolean compatible = false;
       boolean version = false;
 
       final String command = path + " --version\n" + path + " -L -t nat -n\n";
 
-      final List<String> result = RootTools.sendShell(command, DEFAULT_TIMEOUT);
+      final List<String> result = sendShell(command, DEFAULT_TIMEOUT);
 
       for (final String line : result)
       {
@@ -117,7 +141,7 @@ public class IptablesProxyConfigurator implements ProxyConfigurator
       cmd.append(this.iptables);
       cmd.append(IPTABLES_ADD_HTTP.replace("{{PORT}}", String.valueOf(port)));
 
-      RootTools.sendShell(cmd.toString(), DEFAULT_TIMEOUT);
+      sendShell(cmd.toString(), DEFAULT_TIMEOUT);
 
       this.isRegistered = true;
 
@@ -136,7 +160,7 @@ public class IptablesProxyConfigurator implements ProxyConfigurator
   {
     try
     {
-      RootTools.sendShell(this.iptables + " -t nat -F OUTPUT", DEFAULT_TIMEOUT);
+      sendShell(this.iptables + " -t nat -F OUTPUT", DEFAULT_TIMEOUT);
     }
     catch (final Exception e)
     {
@@ -172,14 +196,14 @@ public class IptablesProxyConfigurator implements ProxyConfigurator
 
       final String path = ipt.getAbsolutePath();
 
-      RootTools.sendShell("chmod 700 " + path, DEFAULT_TIMEOUT);
+      sendShell("chmod 700 " + path, DEFAULT_TIMEOUT);
 
       boolean compatible = false;
       boolean version = false;
 
       String command = path + " --version\n" + path + " -L -t nat -n\n";
 
-      final List<String> result = RootTools.sendShell(command, DEFAULT_TIMEOUT);
+      final List<String> result = sendShell(command, DEFAULT_TIMEOUT);
 
       for (final String line : result)
       {
@@ -200,7 +224,7 @@ public class IptablesProxyConfigurator implements ProxyConfigurator
 
       command = path + " -L -t nat -n\n";
 
-      return RootTools.sendShell(command, DEFAULT_TIMEOUT);
+      return sendShell(command, DEFAULT_TIMEOUT);
     }
     catch (final Throwable t)
     {
@@ -231,4 +255,41 @@ public class IptablesProxyConfigurator implements ProxyConfigurator
   {
     return "[ProxyConfigurator: " + this.getType() + "]";
   }
+
+  private final static class CommandOutput extends Command

[Felix Dahlke, 2014/11/19 10:38:39]

Shouldn't this rather be "OutputCommand" (or anything else ending in "Command"),
since it's a command, not an "output"?

[René Jeschke, 2014/11/24 13:07:57]

Right, will also change this.

+  {
+    private final Semaphore completionNotify = new Semaphore(1);

[Felix Dahlke, 2014/11/19 10:38:39]

This confused me a little bit, being a verb (bit unusual for field names). How
about just "running" or something?

[René Jeschke, 2014/11/24 13:07:57]

"running" as name for a flag would mean sense, but I'm using a Semaphore here,
which is used for waiting/counting.

I will come up with something different, still.

[René Jeschke, 2015/01/23 13:37:36]

Ok, I changed this to running as I don't have a better name (except
'semaphore').

Thing is, 'running' is wrong, as running is either true or false. A semaphore
has no true/false it is an integer.

Still, maybe someone finds a better name in the future^^

[Felix Dahlke, 2015/01/27 08:45:36]

True, but I figure for a semaphore with one permit, it seems OK.

+
+    public List<String> output = new ArrayList<String>();
+
+    public CommandOutput(final int id, final int timeout, final String command)
+    {
+      super(id, timeout, command);
+
+      this.completionNotify.acquireUninterruptibly();
+    }
+
+    @Override
+    public void commandOutput(int id, String line)
+    {
+      this.output.add(line);
+    }
+
+    @Override
+    public void commandCompleted(int id, int exitCode)
+    {
+      this.completionNotify.release();
+    }
+
+    @Override
+    public void commandTerminated(int id, String reason)
+    {
+      this.completionNotify.release();
+    }
+
+    public void waitForCompletion()
+    {
+      this.completionNotify.acquireUninterruptibly();
+    }
+  }
 }