Issue 1771 - Update RootTools

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

 /*
- * This file is part of Adblock Plus <http://adblockplus.org/>,
- * Copyright (C) 2006-2014 Eyeo GmbH
+ * This file is part of Adblock Plus <https://adblockplus.org/>,
+ * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
@@ skipping 34 matching lines @@
 
   private final Context context;
   private String iptables;
   private boolean isRegistered = false;
 
   public IptablesProxyConfigurator(final Context context)
   {
     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.

+  private static List<String> runRootCommand(final String command, final int timeout) throws IOException, TimeoutException,
       RootDeniedException
   {
-    final CommandOutput cmd = new CommandOutput(0, DEFAULT_TIMEOUT, command);
+    final CapturingOutputCommand cmd = new CapturingOutputCommand(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");
       }
 
-      final File ipt = this.context.getFileStreamPath("iptables");
-
-      if (!ipt.exists())
-      {
-        throw new FileNotFoundException("No iptables executable");
-      }
-
-      final String path = ipt.getAbsolutePath();
-
-      sendShell("chmod 700 " + path, DEFAULT_TIMEOUT);
+      final String path = getIptablesExecutablePath();
+
+      runRootCommand("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 = sendShell(command, DEFAULT_TIMEOUT);
+      final List<String> result = runRootCommand(command, DEFAULT_TIMEOUT);
 
       for (final String line : result)
       {
         if (line.contains("OUTPUT"))
         {
           compatible = true;
         }
         if (line.contains("v1.4."))
         {
           version = true;
         }
       }
 
       if (!(compatible && version))
       {
         throw new IllegalStateException("Incompatible iptables excutable");
       }
 
       this.iptables = path;
 
       return true;
     }
     catch (final Exception e)
     {
       return false;
     }
   }
 
+  private String getIptablesExecutablePath() throws FileNotFoundException
+  {
+    File iptablesExecutable = new File("/system/bin/iptables");
+    if (!iptablesExecutable.exists())
+    {
+      Log.i(TAG, "iptables not found on the system, using embedded binary");
+      iptablesExecutable = context.getFileStreamPath("iptables");
+    }
+    if (!iptablesExecutable.exists())
+      throw new FileNotFoundException("No iptables executable");
+    return iptablesExecutable.getAbsolutePath();
+  }
+
   @Override
   public boolean registerProxy(final InetAddress address, final int port)
   {
     try
     {
       final int uid = this.context.getPackageManager().getPackageInfo(this.context.getPackageName(), 0).applicationInfo.uid;
 
       final StringBuilder cmd = new StringBuilder();
       cmd.append(this.iptables);
       cmd.append(IPTABLES_RETURN.replace("{{UID}}", String.valueOf(uid)));
       cmd.append('\n');
       cmd.append(this.iptables);
       cmd.append(IPTABLES_ADD_HTTP.replace("{{PORT}}", String.valueOf(port)));
 
-      sendShell(cmd.toString(), DEFAULT_TIMEOUT);
+      runRootCommand(cmd.toString(), DEFAULT_TIMEOUT);
 
       this.isRegistered = true;
 
       return true;
     }
     catch (final Exception e)
     {
       // I leave this logging message for now, passing 'init' and failing 'register' definitely is a failure
       Log.e(TAG, "Couldn't register proxy using iptables.", e);
       return false;
     }
   }
 
   @Override
   public void unregisterProxy()
   {
     try
     {
-      sendShell(this.iptables + " -t nat -F OUTPUT", DEFAULT_TIMEOUT);
+      runRootCommand(this.iptables + " -t nat -F OUTPUT", DEFAULT_TIMEOUT);
     }
     catch (final Exception e)
     {
       Log.w(TAG, "Failed to unregister proxy using iptables.", e);
     }
     finally
     {
       this.isRegistered = false;
     }
   }
@@ skipping 15 matching lines @@
 
       final File ipt = context.getFileStreamPath("iptables");
 
       if (!ipt.exists())
       {
         throw new FileNotFoundException("No iptables executable");
       }
 
       final String path = ipt.getAbsolutePath();
 
-      sendShell("chmod 700 " + path, DEFAULT_TIMEOUT);
+      runRootCommand("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 = sendShell(command, DEFAULT_TIMEOUT);
+      final List<String> result = runRootCommand(command, DEFAULT_TIMEOUT);
 
       for (final String line : result)
       {
         if (line.contains("OUTPUT"))
         {
           compatible = true;
         }
         if (line.contains("v1.4."))
         {
           version = true;
         }
       }
 
       if (!(compatible && version))
       {
         throw new IllegalStateException("Incompatible iptables excutable");
       }
 
       command = path + " -L -t nat -n\n";
 
-      return sendShell(command, DEFAULT_TIMEOUT);
+      return runRootCommand(command, DEFAULT_TIMEOUT);
     }
     catch (final Throwable t)
     {
       return null;
     }
   }
 
   @Override
   public ProxyRegistrationType getType()
   {
@@ skipping 11 matching lines @@
   {
     return false;
   }
 
   @Override
   public String toString()
   {
     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.

+  private final static class CapturingOutputCommand extends Command
+  {
+    private final Semaphore running = new Semaphore(1);
 
     public List<String> output = new ArrayList<String>();
 
-    public CommandOutput(final int id, final int timeout, final String command)
+    public CapturingOutputCommand(final int id, final int timeout, final String command)
     {
       super(id, timeout, command);
 
-      this.completionNotify.acquireUninterruptibly();
+      this.running.acquireUninterruptibly();
     }
 
     @Override
     public void commandOutput(int id, String line)
     {
       this.output.add(line);
     }
 
     @Override
     public void commandCompleted(int id, int exitCode)
     {
-      this.completionNotify.release();
+      this.running.release();
     }
 
     @Override
     public void commandTerminated(int id, String reason)
     {
-      this.completionNotify.release();
+      this.running.release();
     }
 
     public void waitForCompletion()
     {
-      this.completionNotify.acquireUninterruptibly();
+      this.running.acquireUninterruptibly();
     }
   }
 }