Proxy configurators

src/org/adblockplus/android/ProxyService.java

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 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
@@ skipping 39 matching lines @@
 import android.util.Log;
 
 public class ProxyService extends Service implements OnSharedPreferenceChangeListener
 {
   private static final String TAG = Utils.getTag(ProxyService.class);
 
   private static final String LOCALHOST = "127.0.0.1";
 
   private static final int[] PORT_VARIANTS = new int[] {-1, 2020, 3030, 4040, 5050, 6060, 7070, 9090, 1234, 12345, 4321, 0};
 
-  // FIXME remove me

[Felix Dahlke, 2014/08/19 09:06:00]

Why not remove it?

[René Jeschke, 2014/08/19 10:41:33]

When I remove it, we don't have a way to enable request logging. That's why I
currently keep this thing. I'll open an issue for that.

[Felix Dahlke, 2014/08/19 12:37:26]

Can you remove the FIXME comment then? :)

[René Jeschke, 2014/08/19 12:57:36]

Done.

   private static final boolean LOG_REQUESTS = false;
 
   static final int ONGOING_NOTIFICATION_ID = R.string.app_name;
 
   private static final long POSITION_RIGHT = Build.VERSION.SDK_INT >= Build.VERSION_CODES.GINGERBREAD ? Long.MIN_VALUE : Long.MAX_VALUE;
 
   /**
    * Broadcasted when service starts or stops.
    */
   public static final String BROADCAST_STATE_CHANGED = "org.adblockplus.android.SERVICE_STATE_CHANGED";
 
   /**
    * Broadcasted if proxy fails to start.
    */
   public static final String BROADCAST_PROXY_FAILED = "org.adblockplus.android.PROXY_FAILURE";
 
   /**
    * Common command bridge
    */
-  public static final String COMMAND_BRIDGE_ACTION = "org.adblockplus.android.COMMAND_BRIDGE";
+  public static final String PROXY_STATE_CHANGED_ACTION = "org.adblockplus.android.PROXY_STATE_CHANGED";
 
   boolean hideIcon;
 
   protected ProxyServer proxy = null;
 
   protected int port;
 
   private final Properties proxyConfiguration = new Properties();
 
   private ProxyConfigurator proxyConfigurator = null;
 
   @SuppressLint("NewApi")
   @Override
   public void onCreate()
   {
     super.onCreate();
 
-    // FIXME 'StrictMode' is API9

[Felix Dahlke, 2014/08/19 09:06:00]

Isn't our conclusion so far that API 9 should be our minimum supported version?
(https://issues.adblockplus.org/ticket/246)

[René Jeschke, 2014/08/19 10:41:33]

Hm, there are devices out there that run API7 on a ARMv7 ... I thought we talked
about this ... we should talk about this again^^

[Felix Dahlke, 2014/08/19 12:37:26]

Still - we're using API9 all over the place, don't we? If we do want to be API7
compatible, ADT should point this out, so no real point in the FIXME comment
IMO.

[René Jeschke, 2014/08/19 12:57:36]

Thing is, as you disliked my tries to remove the warnings from this project, 90%
is noise here. I definitely do find this FIXME again ... we need another pass to
remove warnings!
 ... still removed

[Felix Dahlke, 2014/08/19 13:17:50]

Did I? I'm all for getting rid of warnings!

[René Jeschke, 2014/08/19 13:23:53]

Yeah, think it's been unrelated changes back then, we'll need an issue for that
:-D

[Felix Dahlke, 2014/08/19 13:28:51]

8)

     StrictMode.setThreadPolicy(new StrictMode.ThreadPolicy.Builder()
         .permitAll()
         .penaltyLog()
         .build());
 
     final SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this);
 
     // Try to read user proxy settings
     String proxyHost = System.getProperty("http.proxyHost");;
     String proxyPort = System.getProperty("http.proxyPort");
     final String proxyExcl = System.getProperty("http.nonProxyHosts");
     String proxyUser = null;
     String proxyPass = null;
 
     if (proxyHost == null || proxyPort == null)
     {
       // 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);
     }
 
     registerReceiver(this.proxyReceiver, new IntentFilter(ProxyService.BROADCAST_PROXY_FAILED));
-    registerReceiver(this.commandBridge, new IntentFilter(ProxyService.COMMAND_BRIDGE_ACTION));
+    registerReceiver(this.proxyStateChangedReceiver, new IntentFilter(ProxyService.PROXY_STATE_CHANGED_ACTION));
 
     final InetAddress inetAddress;
     try
     {
       inetAddress = InetAddress.getByName(LOCALHOST);
     }
     catch (final UnknownHostException e)
     {
       sendBroadcast(new Intent(BROADCAST_PROXY_FAILED)
-          .putExtra("msg", "Could not resolve localhost, this one is serious^^"));

[Felix Dahlke, 2014/08/19 09:06:00]

I can see your point, but I think we shouldn't have smilies in log messages :D

[René Jeschke, 2014/08/19 10:41:33]

Done.

+          .putExtra("msg", "Could not resolve 'localhost'"));
       return;
     }
 
     // Start proxy
     if (proxy == null)
     {
       // Select available port and bind to it, use previously selected port by default
       ServerSocket listen = null;
       String msg = null;
       for (final int p : PORT_VARIANTS)
@@ skipping 46 matching lines @@
       {
       case HTTP:
         proxyConfiguration.put("main.handlers", "urlmodifier adblock");
         proxyConfiguration.put("urlmodifier.class", "org.adblockplus.brazil.TransparentProxyHandler");
         break;
       case HTTPS:
         proxyConfiguration.put("main.handlers", "https adblock");
         proxyConfiguration.put("https.class", "org.adblockplus.brazil.SSLConnectionHandler");
         break;
       default:
-        // FIXME should we reset the settings here? (Can't happen currently)

[Felix Dahlke, 2014/08/19 09:06:00]

As with most TODO comments, I think this should become an issue :)

Not entirely sure what you mean by "reset the settings".

[René Jeschke, 2014/08/19 10:41:33]

Ah, just see that this TODO is now out-of-scope, removed.

         sendBroadcast(new Intent(BROADCAST_PROXY_FAILED)
             .putExtra("msg", "Unsupported proxy server type: " + this.getProxyRegistrationType().getProxyType()));
         return;
       }
       proxyConfiguration.put("adblock.class", "org.adblockplus.brazil.RequestHandler");
       if (LOG_REQUESTS)
         proxyConfiguration.put("adblock.proxylog", "yes");
 
       configureUserProxy(proxyConfiguration, proxyHost, proxyPort, proxyExcl, proxyUser, proxyPass);
 
       proxy = new ProxyServer();
       proxy.logLevel = Server.LOG_LOG;
       proxy.setup(listen, proxyConfiguration.getProperty("handler"), proxyConfiguration);
       proxy.start();
-    } // if (proxy == null) // Start proxy

[Felix Dahlke, 2014/08/19 09:06:00]

I think this comment is ridiculously likely to get out of sync with the actual
code above, I'd rather not introduce it.

[René Jeschke, 2014/08/19 10:41:33]

Done.

+    }
 
     prefs.registerOnSharedPreferenceChangeListener(this);
 
     // Lock service
     hideIcon = prefs.getBoolean(getString(R.string.pref_hideicon), getResources().getBoolean(R.bool.def_hideicon));
     startForeground(ONGOING_NOTIFICATION_ID, getNotification());
 
     sendStateChangedBroadcast();
     Log.i(TAG, "Service started");
   }
 
   @Override
   public int onStartCommand(final Intent intent, final int flags, final int startId)
   {
     return START_STICKY;
   }
 
   @Override
   public void onDestroy()
   {
     super.onDestroy();
 
     unregisterReceiver(this.proxyReceiver);
-    unregisterReceiver(this.commandBridge);
+    unregisterReceiver(this.proxyStateChangedReceiver);
 
     if (this.proxyConfigurator != null)
     {
       this.proxyConfigurator.unregisterProxy();
       this.proxyConfigurator.shutdown();
     }
 
     sendBroadcast(new Intent(BROADCAST_STATE_CHANGED).putExtra("enabled", false));
 
     // Stop proxy server
     if (proxy != null)
     {
       proxy.close();
       proxy = null;
     }
 
     // Release service lock
     stopForeground(true);
 
     Log.i(TAG, "Service stopped");
   }
 
   /**
    * Sets user proxy settings in proxy service properties.
    */
   private void configureUserProxy(final Properties config, final String proxyHost, final String proxyPort, final String proxyExcl,
       final String proxyUser, final String proxyPass)
   {
-    final ProxyRegistrationType regType = this.getProxyRegistrationType();

[Felix Dahlke, 2014/08/19 09:06:00]

How about moving this down to where it's used?

[René Jeschke, 2014/08/19 10:41:33]

Done.

-
     // Clean previous settings
     config.remove("adblock.proxyHost");
     config.remove("adblock.proxyPort");
     config.remove("adblock.auth");
     config.remove("adblock.proxyExcl");
     config.remove("https.proxyHost");
     config.remove("https.proxyPort");
     config.remove("https.auth");
 
-    if (regType == ProxyRegistrationType.REFLECTION)
+    final ProxyRegistrationType regType = this.getProxyRegistrationType();
+    if (regType == ProxyRegistrationType.NATIVE)
     {
       passProxySettings(proxyHost, proxyPort, proxyExcl);
     }
 
     // Check if there are any settings
     if (StringUtils.isEmpty(proxyHost))
     {
       return;
     }
 
@@ skipping 11 matching lines @@
     {
       p = Integer.valueOf(proxyPort);
     }
     catch (final NumberFormatException e)
     {
       return;
     }
 
     if (p == 0 || isLocalhost(proxyHost) && (p == port || p == 8080))
     {
-      if (regType == ProxyRegistrationType.REFLECTION)
+      if (regType == ProxyRegistrationType.NATIVE)
       {
         passProxySettings(null, null, null);
       }
       return;
     }
 
     config.put("adblock.proxyHost", proxyHost);
     config.put("adblock.proxyPort", proxyPort);
     if (regType.getProxyType() == ProxyServerType.HTTPS)
     {
@@ skipping 13 matching lines @@
       final String proxyAuth = "Basic " + new String(Base64.encode(proxyUser + ":" + proxyPass));
       config.put("adblock.auth", proxyAuth);
       if (regType.getProxyType() == ProxyServerType.HTTPS)
       {
         config.put("https.auth", proxyAuth);
       }
     }
   }
 
   /**
-   * Lean method to send a {@link BridgeCommand} without additional extras.
-   *
-   * @param context
-   *          the context to use for sending the broadcast
-   * @param command
-   *          the command
-   */
-  public static void sendBridgeCommand(final Context context, final BridgeCommand command)
-  {
-    context.sendBroadcast(
-        new Intent(COMMAND_BRIDGE_ACTION)
-            .putExtra("command", command.toString()));
-  }
-
-  /**
    * @return {@code true} if the given host string resolves to {@code localhost}
    */
   public static boolean isLocalhost(final String host)
   {
     try
     {
       if (StringUtils.isEmpty(host))
       {
         return false;
       }
 
       if (host.equals("127.0.0.1") || host.equalsIgnoreCase("localhost"))
       {
         return true;
       }
 
       return InetAddress.getByName(host).isLoopbackAddress();
     }
-    catch (Exception e)
+    catch (final Exception e)
     {
       return false;
     }
   }
 
   private void passProxySettings(final String proxyHost, final String proxyPort, final String proxyExcl)
   {
     try
     {
       final CrashHandler handler = (CrashHandler) Thread.getDefaultUncaughtExceptionHandler();
       handler.saveProxySettings(proxyHost, proxyPort, proxyExcl);
     }
     catch (final ClassCastException e)
     {
       // ignore - default handler in use
     }
   }
 
   @Override
   public void onSharedPreferenceChanged(final SharedPreferences sharedPreferences, final String key)
   {
-    // TODO verify this

[Felix Dahlke, 2014/08/19 09:06:00]

We should probably do that before pushing the code, eh?

[René Jeschke, 2014/08/19 10:41:33]

Right, it's the '!NATIVE_PROXY_SUPPORTED' thingy I don't fully understand, will
have a look into this again.

-    if (this.getProxyRegistrationType() != ProxyRegistrationType.REFLECTION)
+    if (this.getProxyRegistrationType() != ProxyRegistrationType.NATIVE)
     {
       final String keyHost = getString(R.string.pref_proxyhost);
       final String keyPort = getString(R.string.pref_proxyport);
       final String keyUser = getString(R.string.pref_proxyuser);
       final String keyPass = getString(R.string.pref_proxypass);
       if (key.equals(keyHost) || key.equals(keyPort) || key.equals(keyUser) || key.equals(keyPass))
       {
         final String proxyHost = sharedPreferences.getString(keyHost, null);
         final String proxyPort = sharedPreferences.getString(keyPort, null);
         final String proxyUser = sharedPreferences.getString(keyUser, null);
         final String proxyPass = sharedPreferences.getString(keyPass, null);
         if (proxy != null)
         {
           configureUserProxy(proxyConfiguration, proxyHost, proxyPort, null, proxyUser, proxyPass);
           proxy.restart(proxyConfiguration.getProperty("handler"));
         }
       }
     }
   }
 
   /**
    * @return the active proxy configuration's type or {@link ProxyRegistrationType#UNKNOWN} if none is configured
    */
-  public ProxyRegistrationType getProxyRegistrationType()
+  private ProxyRegistrationType getProxyRegistrationType()
   {
     return this.proxyConfigurator != null ? this.proxyConfigurator.getType() : ProxyRegistrationType.UNKNOWN;
   }
 
   /**
    * @return {@code true} if there is a valid proxy configurator and registration succeeded
    */
   public boolean isRegistered()
   {
     return this.proxyConfigurator != null && this.proxyConfigurator.isRegistered();
   }
 
   /**
-   * @return {@code true} if registration type is {@link ProxyRegistrationType#REFLECTION} and registration succeeded
+   * @return {@code true} if registration type is {@link ProxyRegistrationType#NATIVE} and registration succeeded
    */
   public boolean isNativeProxyAutoConfigured()
   {
-    return this.getProxyRegistrationType() == ProxyRegistrationType.REFLECTION && this.isRegistered();
+    return this.getProxyRegistrationType() == ProxyRegistrationType.NATIVE && this.isRegistered();
   }
 
   /**
    * @return {@code true} if registration type is {@link ProxyRegistrationType#MANUAL}
    */
   public boolean isManual()
   {
     return this.getProxyRegistrationType() == ProxyRegistrationType.MANUAL;
+  }
+
+  /**
+   * @return {@code true} if registration type is {@link ProxyRegistrationType#IPTABLES}
+   */
+  public boolean isIptables()
+  {
+    return this.getProxyRegistrationType() == ProxyRegistrationType.IPTABLES;
   }
 
   @SuppressLint("NewApi")
   private Notification getNotification()
   {
     final boolean filtering = AdblockPlus.getApplication().isFilteringEnabled();
 
     final int msgId;
     switch(this.getProxyRegistrationType())
     {
     case MANUAL:
       if (this.isRegistered())
       {
         msgId = filtering ? R.string.notif_wifi : R.string.notif_wifi_nofiltering;
       }
       else
       {
         msgId = filtering ? R.string.notif_waiting : R.string.notif_wifi_nofiltering;
       }
       break;
-    case REFLECTION:
+    case NATIVE:
       msgId = filtering ? R.string.notif_wifi : R.string.notif_wifi_nofiltering;
       break;
     case IPTABLES:
     case CYANOGENMOD:
       msgId = R.string.notif_all;
       break;
     default:
       msgId = R.string.notif_waiting;
       break;
     }
@@ skipping 69 matching lines @@
     {
       if (intent.getAction().equals(ProxyService.BROADCAST_PROXY_FAILED))
       {
         stopSelf();
       }
     }
   };
 
   /**
    * <p>
-   * A BR for simple message passing from various ProxyConfigurators.
+   * Proxy state change receiver.
    * </p>
-   * <p>
-   * The purpose of this BroadcastReceiver is to allow other components to send notifications to the service and to be prepared for upcoming service
-   * refactorings.
-   * </p>
-   */
-  private final BroadcastReceiver commandBridge = new BroadcastReceiver()
+   */
+  private final BroadcastReceiver proxyStateChangedReceiver = new BroadcastReceiver()
   {
     @Override
     public void onReceive(final Context context, final Intent intent)
     {
-      if (intent != null)
-      {
-        switch (BridgeCommand.fromString(intent.getStringExtra("command")))
-        {
-        case STATE_CHANGED:
-          final NotificationManager notificationManager = (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
-          notificationManager.notify(ONGOING_NOTIFICATION_ID, getNotification());
-          ProxyService.this.sendStateChangedBroadcast();
-          break;
-        default:
-          // ignore
-          break;
-        }
+      if (intent != null && PROXY_STATE_CHANGED_ACTION.equals(intent.getAction()))
+      {
+        final NotificationManager notificationManager = (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
+        notificationManager.notify(ONGOING_NOTIFICATION_ID, getNotification());
+        ProxyService.this.sendStateChangedBroadcast();
       }
     }
   };
 
   final class ProxyServer extends Server
   {
     @Override
     public void close()
     {
       try
@@ skipping 12 matching lines @@
     @Override
     public void log(final int level, final Object obj, final String message)
     {
       if (level <= logLevel)
       {
         Log.println(7 - level, obj != null ? obj.toString() : TAG, message);
       }
     }
   }
 }