ABP/Android proxy service

src/org/adblockplus/android/ProxyService.java

 package org.adblockplus.android;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.lang.reflect.Method;
 import java.net.InetAddress;
 import java.net.ServerSocket;
 import java.util.List;
 import java.util.Properties;
 import java.util.concurrent.TimeoutException;
 
 import sunlabs.brazil.server.Server;
 import sunlabs.brazil.util.Base64;
@@ skipping 16 matching lines @@
 import android.os.IBinder;
 import android.preference.PreferenceManager;
 import android.util.Log;
 import android.widget.Toast;
 
 import com.stericson.RootTools.RootTools;
 import com.stericson.RootTools.RootToolsException;
 
 public class ProxyService extends Service implements OnSharedPreferenceChangeListener
 {
+  private static final String LOCALHOST = "127.0.0.1";
+  /**
+   * Indicates that system supports native proxy configuration.
+   */
+  public static boolean hasNativeProxy = Build.VERSION.SDK_INT >= 12; // Honeycomb 3.1
+
   static
   {
     RootTools.debugMode = false;
   }
 
   private static final String TAG = "ProxyService";
+  private static final boolean logRequests = false;
 
   private final static int DEFAULT_TIMEOUT = 3000;
   private final static int NO_TRAFFIC_TIMEOUT = 5 * 60 * 1000; // 5 minutes
 
   final static int ONGOING_NOTIFICATION_ID = R.string.app_name;
   private final static int NOTRAFFIC_NOTIFICATION_ID = R.string.app_name + 3;
 
   /**
    * Broadcasted when service starts or stops.
    */
@@ skipping 10 matching lines @@
   private PendingIntent contentIntent;
 
   private Handler notrafficHandler;
 
   protected ProxyServer proxy = null;
   protected int port;
 
   /**
    * Indicates that service is working with root privileges.
    */
-  private boolean isTransparent = false;
+  private boolean transparent = false;
   /**
    * Indicates that service has autoconfigured Android proxy settings (version
    * 3.1+).
    */
-  private boolean isNativeProxy = false;
+  private boolean nativeProxy = false;
 
   private String iptables = null;
 
   @Override
   public void onCreate()
   {
     super.onCreate();
 
     // Get port for local proxy
     SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this);
     String p = prefs.getString(getString(R.string.pref_port), null);
     try
     {
       port = p != null ? Integer.valueOf(p) : getResources().getInteger(R.integer.def_port);
     }
     catch (NumberFormatException e)
     {
       Toast.makeText(this, getString(R.string.msg_badport) + ": " + p, Toast.LENGTH_LONG).show();
       port = getResources().getInteger(R.integer.def_port);
     }
 
     // Try to read user proxy settings
     String proxyHost = null;
     String proxyPort = null;
     String proxyExcl = null;
     String proxyUser = null;
     String proxyPass = null;
 
-    if (Build.VERSION.SDK_INT >= 12) // Honeycomb 3.1

[Andrey Novikov, 2012/11/09 09:23:47]

What exactly are you referencing?

[Felix Dahlke, 2012/11/09 14:40:46]

I think it was AdvancedPreferences.hasNativeProxy. If the >= 3.1 checks are
always used to figure out if that's the case, I think this check should only be
done once, and set a central hasNativeProxy variable or something like that.

What if the condition for native proxy support is going to become more
complicated, e.g. because support is removed in a future version? We'll have to
find all >= 3.1 checks in the code and hope that they were to test for native
proxy support.

+    if (hasNativeProxy)
     {
       // Read system settings
       proxyHost = System.getProperty("http.proxyHost");
       proxyPort = System.getProperty("http.proxyPort");
       proxyExcl = System.getProperty("http.nonProxyHosts");
 
       Log.d(TAG, "PRX: " + proxyHost + ":" + proxyPort + "(" + proxyExcl + ")");
       String[] px = ProxySettings.getUserProxy(getApplicationContext()); // not used but left for future reference

[Felix Dahlke, 2012/11/08 16:37:50]

Future reference? It is in fact used below, if only for logging.

[Felix Dahlke, 2012/11/09 14:40:46]

I see, I misunderstood that then. I thought you meant reference somewhere in the
source code.

       if (px != null)
         Log.d(TAG, "PRX: " + px[0] + ":" + px[1] + "(" + px[2] + ")");
     }
     else
     {
       // Read application settings
       proxyHost = prefs.getString(getString(R.string.pref_proxyhost), null);
       proxyPort = prefs.getString(getString(R.string.pref_proxyport), null);
       proxyUser = prefs.getString(getString(R.string.pref_proxyuser), null);
       proxyPass = prefs.getString(getString(R.string.pref_proxypass), null);
     }
 
     // Check for root privileges and try to install transparent proxy
     if (RootTools.isAccessGiven())
     {
       try
       {
-        iptables = getIptables();

[Andrey Novikov, 2012/11/09 09:23:47]

Done.

-        if (iptables != null)
-        {
-          StringBuffer cmd = new StringBuffer();
-          int uid = getPackageManager().getPackageInfo(getPackageName(), 0).applicationInfo.uid;
-          cmd.append(iptables);
-          cmd.append(IPTABLES_RETURN.replace("{{UID}}", String.valueOf(uid)));
-          cmd.append(iptables);
-          cmd.append(IPTABLES_ADD_HTTP.replace("{{PORT}}", String.valueOf(port)));
-          String rules = cmd.toString();
-          RootTools.sendShell(rules, DEFAULT_TIMEOUT);
-          isTransparent = true;
-        }
+        initIptables();
+
+        StringBuffer cmd = new StringBuffer();
+        int uid = getPackageManager().getPackageInfo(getPackageName(), 0).applicationInfo.uid;
+        cmd.append(iptables);
+        cmd.append(IPTABLES_RETURN.replace("{{UID}}", String.valueOf(uid)));
+        cmd.append(iptables);
+        cmd.append(IPTABLES_ADD_HTTP.replace("{{PORT}}", String.valueOf(port)));
+        String rules = cmd.toString();
+        RootTools.sendShell(rules, DEFAULT_TIMEOUT);
+        transparent = true;
+      }
+      catch (FileNotFoundException e)
+      {
+        // ignore - this is "normal" case
       }
       catch (NameNotFoundException e)
       {
-        e.printStackTrace();

[Felix Dahlke, 2012/11/08 16:37:50]

Here and below, I'd prefer Log.e().

[Andrey Novikov, 2012/11/09 09:23:47]

Done.

+        Log.e(TAG, "Failed to initialize iptables", e);
       }
       catch (IOException e)
       {
-        e.printStackTrace();
+        Log.e(TAG, "Failed to initialize iptables", e);
       }
       catch (RootToolsException e)
       {
-        e.printStackTrace();
+        Log.e(TAG, "Failed to initialize iptables", e);
       }
       catch (TimeoutException e)
       {
-        e.printStackTrace();
-      }
-    }
-
-    if (!isTransparent)
+        Log.e(TAG, "Failed to initialize iptables", e);
+      }
+    }
+
+    if (!transparent)
     {
       // Try to set native proxy
-      isNativeProxy = ProxySettings.setConnectionProxy(getApplicationContext(), "127.0.0.1", port, "");
-
-      if (isNativeProxy)
+      nativeProxy = ProxySettings.setConnectionProxy(getApplicationContext(), LOCALHOST, port, "");
+
+      if (nativeProxy)
       {
         registerReceiver(connectionReceiver, new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION));
         registerReceiver(connectionReceiver, new IntentFilter("android.net.wifi.LINK_CONFIGURATION_CHANGED"));
       }
     }
 
     // Start engine
     AdblockPlus.getApplication().startEngine();
 
     registerReceiver(proxyReceiver, new IntentFilter(ProxyService.BROADCAST_PROXY_FAILED));
@@ skipping 12 matching lines @@
       {
         sendBroadcast(new Intent(BROADCAST_PROXY_FAILED).putExtra("msg", e.getMessage()));
         Log.e(TAG, null, e);
         return;
       }
 
       Properties config = new Properties();
       config.put("handler", "main");
       config.put("main.prefix", "");
       config.put("main.class", "sunlabs.brazil.server.ChainHandler");
-      if (isTransparent)
+      if (transparent)
       {
         config.put("main.handlers", "urlmodifier adblock");
         config.put("urlmodifier.class", "org.adblockplus.brazil.TransparentProxyHandler");
       }
       else
       {
         config.put("main.handlers", "https adblock");
         config.put("https.class", "org.adblockplus.brazil.SSLConnectionHandler");
       }
       config.put("adblock.class", "org.adblockplus.brazil.RequestHandler");
-      // config.put("adblock.proxylog", "yes");
+      if (logRequests)
+        config.put("adblock.proxylog", "yes");
 
       configureUserProxy(config, proxyHost, proxyPort, proxyExcl, proxyUser, proxyPass);
 
       proxy = new ProxyServer();
       proxy.logLevel = Server.LOG_DIAGNOSTIC;
       proxy.setup(listen, config.getProperty("handler"), config);
       proxy.start();
     }
 
     prefs.registerOnSharedPreferenceChangeListener(this);
 
-    String msg = getString(isTransparent ? R.string.notif_all : isNativeProxy ? R.string.notif_wifi : R.string.notif_waiting);
-    if (!isTransparent && !isNativeProxy)
+    String msg = getString(transparent ? R.string.notif_all : nativeProxy ? R.string.notif_wifi : R.string.notif_waiting);
+    if (!transparent && !nativeProxy)
     {
       // Initiate no traffic check
       notrafficHandler = new Handler();
       notrafficHandler.postDelayed(noTraffic, NO_TRAFFIC_TIMEOUT);
     }
     // Lock service
     ongoingNotification = new Notification();
     ongoingNotification.when = 0;
     contentIntent = PendingIntent.getActivity(this, 0, new Intent(this, Preferences.class).addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP | Intent.FLAG_ACTIVITY_NEW_TASK), 0);
     ongoingNotification.icon = R.drawable.ic_stat_blocking;
     ongoingNotification.setLatestEventInfo(getApplicationContext(), getText(R.string.app_name), msg, contentIntent);
     startForeground(ONGOING_NOTIFICATION_ID, ongoingNotification);
 
-    sendBroadcast(new Intent(BROADCAST_STATE_CHANGED).putExtra("enabled", true).putExtra("port", port).putExtra("manual", !isTransparent && !isNativeProxy));
+    sendBroadcast(new Intent(BROADCAST_STATE_CHANGED).putExtra("enabled", true).putExtra("port", port).putExtra("manual", !transparent && !nativeProxy));
     Log.i(TAG, "Service started");
   }
 
   @Override
   public void onDestroy()
   {
     super.onDestroy();
 
     stopNoTrafficCheck(false);
 
     unregisterReceiver(matchesReceiver);
     unregisterReceiver(proxyReceiver);
 
     // Stop IP redirecting
-    if (isTransparent)
+    if (transparent)
     {
       new Thread() {
         @Override
         public void run()
         {
           try
           {
             RootTools.sendShell(iptables + " -t nat -F OUTPUT", DEFAULT_TIMEOUT);
           }
           catch (Exception e)
           {
-            e.printStackTrace();

[Felix Dahlke, 2012/11/08 16:37:50]

Log.e()?

[Andrey Novikov, 2012/11/09 09:23:47]

Done.

+            Log.e(TAG, "Failed to clear iptables", e);
           }
         }
       }.start();
     }
 
     // Clear native proxy
-    if (isNativeProxy)
+    if (nativeProxy)
     {
       unregisterReceiver(connectionReceiver);
       clearConnectionProxy();
     }
 
     sendBroadcast(new Intent(BROADCAST_STATE_CHANGED).putExtra("enabled", false));
 
     // Stop proxy server
     proxy.close();
 
@@ skipping 31 matching lines @@
   /**
    * Sets user proxy settings in proxy service properties.
    */
   private void configureUserProxy(Properties config, String proxyHost, String proxyPort, String proxyExcl, String proxyUser, String proxyPass)
   {
     // Clean previous settings
     config.remove("adblock.proxyHost");
     config.remove("adblock.proxyPort");
     config.remove("adblock.auth");
     config.remove("adblock.proxyExcl");
-    if (!isTransparent)
+    if (!transparent)
     {
       config.remove("https.proxyHost");
       config.remove("https.proxyPort");
       config.remove("https.auth");
     }
 
-    if (isNativeProxy)
+    if (nativeProxy)
       passProxySettings(proxyHost, proxyPort, proxyExcl);
 
     // Check if there are any settings
     if (proxyHost == null || "".equals(proxyHost))
       return;
 
     // Check for dirty proxy settings - this indicated previous crash:
     // proxy points to ourselves
     // proxy port is null, 0 or not a number
     // proxy is 127.0.0.1:8080
     if (proxyPort == null)
       return;
     int p = 0;
     try
     {
       p = Integer.valueOf(proxyPort);
     }
     catch (NumberFormatException e)
     {
       return;
     }
     if (p == 0 || isLocalHost(proxyHost) && (p == port || p == 8080))
     {
-      if (isNativeProxy)
+      if (nativeProxy)
         passProxySettings(null, null, null);
       return;
     }
 
     config.put("adblock.proxyHost", proxyHost);
     config.put("adblock.proxyPort", proxyPort);
-    if (!isTransparent)
+    if (!transparent)
     {
       config.put("https.proxyHost", proxyHost);
       config.put("https.proxyPort", proxyPort);
     }
 
     // TODO Not implemented in our proxy but needed to restore settings
     if (proxyExcl != null)
       config.put("adblock.proxyExcl", proxyExcl);
 
     if (proxyUser != null && !"".equals(proxyUser) && proxyPass != null && !"".equals(proxyPass))
     {
       // Base64 encode user:password
       String proxyAuth = "Basic " + new String(Base64.encode(proxyUser + ":" + proxyPass));
       config.put("adblock.auth", proxyAuth);
-      if (!isTransparent)
+      if (!transparent)
         config.put("https.auth", proxyAuth);
     }
   }
 
   private void passProxySettings(String proxyHost, String proxyPort, String proxyExcl)
   {
     try
     {
       CrashHandler handler = (CrashHandler) Thread.getDefaultUncaughtExceptionHandler();
       handler.saveProxySettings(proxyHost, proxyPort, proxyExcl);
     }
     catch (ClassCastException e)
     {
       // ignore - default handler in use
     }
   }
 
   @Override
   public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, String key)
   {
-    if (Build.VERSION.SDK_INT < 12) // Honeycomb 3.1

[Felix Dahlke, 2012/11/08 16:37:50]

Like before, I think there's a global variable or method indicating this.

[Felix Dahlke, 2012/11/09 14:40:46]

See my reasoning above. If we're doing this check always for the same purpose
(which I suspect), it should be done just once, and stored in an appropriately
named variable (or done multiple times, but by an appropriately named method, I
don't mind)

-    {
-      if (key.equals(getString(R.string.pref_proxyhost)) || key.equals(getString(R.string.pref_proxyport)) || key.equals(getString(R.string.pref_proxyuser))
-          || key.equals(getString(R.string.pref_proxypass)))
-      {
-        String proxyHost = sharedPreferences.getString(getString(R.string.pref_proxyhost), null);
-        String proxyPort = sharedPreferences.getString(getString(R.string.pref_proxyport), null);
-        String proxyUser = sharedPreferences.getString(getString(R.string.pref_proxyuser), null);
-        String proxyPass = sharedPreferences.getString(getString(R.string.pref_proxypass), null);
+    if (hasNativeProxy)
+    {
+      String ketHost = getString(R.string.pref_proxyhost);
+      String keyPort = getString(R.string.pref_proxyport);
+      String keyUser = getString(R.string.pref_proxyuser);
+      String keyPass = getString(R.string.pref_proxypass);
+      if (key.equals(ketHost) || key.equals(keyPort) || key.equals(keyUser) || key.equals(keyPass))
+      {
+        String proxyHost = sharedPreferences.getString(ketHost, null);
+        String proxyPort = sharedPreferences.getString(keyPort, null);
+        String proxyUser = sharedPreferences.getString(keyUser, null);
+        String proxyPass = sharedPreferences.getString(keyPass, null);
         if (proxy != null)
         {
           configureUserProxy(proxy.props, proxyHost, proxyPort, null, proxyUser, proxyPass);
           proxy.restart(proxy.props.getProperty("handler"));
         }
       }
     }
   }
 
   public boolean isTransparent()
   {
-    return isTransparent;

[Felix Dahlke, 2012/11/08 16:37:50]

I believe the bean conventions says that boolean getter methods should be named
isVariable and the field just variable. Just some nit picking of course. The
same would go for isNativeProxy.

[Andrey Novikov, 2012/11/09 09:23:47]

Done.

+    return transparent;
   }
 
   public boolean isNativeProxy()
   {
-    return isNativeProxy;
+    return nativeProxy;
   }
 
   /**
    * Checks if specified host is local.
    */
   private static final boolean isLocalHost(String host)
   {
     if (host == null)
       return false;
 
     try
     {
-      if (host != null)
-      {
-        if (host.equalsIgnoreCase("localhost"))
-          return true;
-
-        String className = "android.net.NetworkUtils";
-        Class<?> c = Class.forName(className);
-        /*
-         * InetAddress address = NetworkUtils.numericToInetAddress(host);
-         */
-        Method method = c.getMethod("numericToInetAddress", String.class);
-        InetAddress address = (InetAddress) method.invoke(null, host);
-
-        if (address.isLoopbackAddress())
-          return true;
-      }
+      if (host.equalsIgnoreCase("localhost"))
+        return true;
+
+      String className = "android.net.NetworkUtils";
+      Class<?> c = Class.forName(className);
+      /*
+       * InetAddress address = NetworkUtils.numericToInetAddress(host);
+       */
+      Method method = c.getMethod("numericToInetAddress", String.class);
+      InetAddress address = (InetAddress) method.invoke(null, host);
+
+      if (address.isLoopbackAddress())
+        return true;
     }
     catch (Exception e)
     {

[Felix Dahlke, 2012/11/08 16:37:50]

Comment or logging please.

[Andrey Novikov, 2012/11/09 09:23:47]

Done.

+      Log.w(TAG, null, e);
     }
     return false;
   }
 
   /**
-   * Returns path to iptables executable.
-   */
-  public String getIptables() throws IOException, RootToolsException, TimeoutException

[Felix Dahlke, 2012/11/08 16:37:50]

Bean convention again: Can you think of another prefix than "get"? Especially
since it actually modifies the filesystem. How about something like
"findIptablesExecutable()"? "find" is conventionally used for retrieval methods
that could return null.

[Andrey Novikov, 2012/11/12 08:53:12]

Done.

+   * Initializes iptables executable.
+   * 
+   * @throws FileNotFoundException If iptables initialization failed due to provided reasons.
+   */
+  private void initIptables() throws IOException, RootToolsException, TimeoutException, FileNotFoundException
   {
     if (!RootTools.isAccessGiven())
-      return null;
+      throw new FileNotFoundException("No root access");
 
     File ipt = getFileStreamPath("iptables");
 
     if (!ipt.exists())
-      return null;

[Felix Dahlke, 2012/11/08 16:37:50]

I think this is worth some logging, would help debugging.

[Andrey Novikov, 2012/11/09 09:23:47]

Done.

-
+    {
+      Log.e(TAG, "No iptables excutable found");
+      throw new FileNotFoundException("No iptables executable");
+    }
+    
     String path = ipt.getAbsolutePath();
 
     RootTools.sendShell("chmod 700 " + path, DEFAULT_TIMEOUT);

[Andrey Novikov, 2012/11/09 09:23:47]

We do not want any others to be able to execute it.

[Felix Dahlke, 2012/11/09 14:40:46]

See above, I thought it was using a system wide executable.

 
     boolean compatible = false;
     boolean version = false;
 
     String command = path + " --version\n" + path + " -L -t nat -n\n";
 
     List<String> result = RootTools.sendShell(command, DEFAULT_TIMEOUT);
     for (String line : result)
     {
       if (line.contains("OUTPUT"))
         compatible = true;
       if (line.contains("v1.4."))
         version = true;
     }
 
     if (!compatible || !version)
-      return null;
-
-    return path;
+    {
+      Log.e(TAG, "Incompatible iptables excutable");
+      throw new FileNotFoundException("Incompatible iptables excutable");
+    }
+
+    iptables = path;
   }
   
   public List<String> getIptablesOutput()
   {
     if (iptables == null)
       return null;
 
     String command = iptables + " -L -t nat -n\n";
     try
     {
       return RootTools.sendShell(command, DEFAULT_TIMEOUT);
     }
     catch (Exception e)
     {
       Log.e(TAG, "Failed to get iptables configuration", e);
       return null;
     }
   }
 
   /**
    * Stops no traffic check, optionally resetting notification message.
    * 
    * @param changeStatus
-   *          true if notification message should me set to normal operating
+   *          true if notification message should be set to normal operating
    *          mode
    */
   private void stopNoTrafficCheck(boolean changeStatus)
   {
     if (notrafficHandler != null)
     {
       notrafficHandler.removeCallbacks(noTraffic);
       if (changeStatus)
       {
         NotificationManager notificationManager = (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
@@ skipping 79 matching lines @@
       String action = intent.getAction();
       Log.i(TAG, "Action: " + action);
       if (ConnectivityManager.CONNECTIVITY_ACTION.equals(action))
       {
         NetworkInfo info = intent.getParcelableExtra(ConnectivityManager.EXTRA_NETWORK_INFO);
         String typeName = info.getTypeName();
         String subtypeName = info.getSubtypeName();
         boolean available = info.isAvailable();
         Log.i(TAG, "Network Type: " + typeName + ", subtype: " + subtypeName + ", available: " + available);
         if (info.getType() == ConnectivityManager.TYPE_WIFI)
-          ProxySettings.setConnectionProxy(getApplicationContext(), "127.0.0.1", port, "");
+          ProxySettings.setConnectionProxy(getApplicationContext(), LOCALHOST, port, "");
       }
       else if ("android.net.wifi.LINK_CONFIGURATION_CHANGED".equals(action))
       {
         Object lp = intent.getParcelableExtra("linkProperties");
         Method method;
         try
         {
           /*
            * lp.getHttpProxy();
            */
           method = lp.getClass().getMethod("getHttpProxy");
           Object pp = method.invoke(lp);
 
           String[] userProxy = ProxySettings.getUserProxy(pp);
           if (userProxy != null && Integer.valueOf(userProxy[1]) != port)
           {
             Log.i(TAG, "User has set new proxy: " + userProxy[0] + ":" + userProxy[1] + "(" + userProxy[2] + ")");
             if (proxy != null)
             {
               configureUserProxy(proxy.props, userProxy[0], userProxy[1], userProxy[2], null, null);
               proxy.restart(proxy.props.getProperty("handler"));
             }
           }
         }
         catch (Exception e)
         {
           // This should not happen
-          e.printStackTrace();
+          Log.e(TAG, null, e);
         }
 
       }
     }
   };
 
   final class ProxyServer extends Server
   {
     @Override
     public void close()
     {
       try
       {
         listen.close();
         this.interrupt();
         this.join();
       }
       catch (Exception e)
       {

[Felix Dahlke, 2012/11/08 16:37:50]

Logging or a comment?

+        // ignore - it always happens
       }
       log(LOG_WARNING, null, "server stopped");
     }
 
     @Override
     public void log(int level, Object obj, String message)
     {
       if (level <= logLevel)
       {
         Log.println(7 - level, obj != null ? obj.toString() : TAG, message);
       }
     }
   }
 }