* Plugins:

- Check plugin key against all installed plugins
      - Reword some error messages
    * VersionComparator: add '-' and '_' as valid separators
      to better support plugin and java version checking

apps/routerconsole/java/src/net/i2p/router/web/PluginStarter.java

@@ -5,9 +5,12 @@ import java.io.IOException;
 import java.lang.reflect.Method;
 import java.net.URL;
 import java.net.URLClassLoader;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Properties;
 import java.util.StringTokenizer;
 
@@ -220,15 +223,45 @@ public class PluginStarter implements Runnable {
             DataHelper.loadProps(rv, cfgFile);
         } catch (IOException ioe) {}
 
+        List<String> names = getPlugins();
+        for (String name : names) {
+            String prop = PREFIX + name + ENABLED;
+            if (rv.getProperty(prop) == null)
+                rv.setProperty(prop, "true");
+        }
+        return rv;
+    }
+
+    /**
+     *  all installed plugins whether enabled or not
+     */
+    public static List<String> getPlugins() {
+        List<String> rv = new ArrayList();
         File pluginDir = new File(I2PAppContext.getGlobalContext().getAppDir(), PluginUpdateHandler.PLUGIN_DIR);
         File[] files = pluginDir.listFiles();
         if (files == null)
             return rv;
         for (int i = 0; i < files.length; i++) {
-            String name = files[i].getName();
+            if (files[i].isDirectory())
-            String prop = PREFIX + name + ENABLED;
+                rv.add(files[i].getName());
-            if (files[i].isDirectory() && rv.getProperty(prop) == null)
+        }
-                rv.setProperty(prop, "true");
+        return rv;
+    }
+
+    /**
+     *  The signing keys from all the plugins
+     *  @return Map of key to keyname
+     *  Last one wins if a dup (installer should prevent dups)
+     */
+    public static Map<String, String> getPluginKeys(I2PAppContext ctx) {
+        Map<String, String> rv = new HashMap();
+        List<String> names = getPlugins();
+        for (String name : names) {
+            Properties props = pluginProperties(ctx, name);
+            String pubkey = props.getProperty("key");
+            String keyName = props.getProperty("keyName");
+            if (pubkey != null && keyName != null && pubkey.length() == 172 && keyName.length() > 0)
+                rv.put(pubkey, keyName);
         }
         return rv;
     }

apps/routerconsole/java/src/net/i2p/router/web/PluginUpdateHandler.java

@@ -2,6 +2,7 @@ package net.i2p.router.web;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Map;
 import java.util.Properties;
 
 import net.i2p.CoreVersion;
@@ -184,6 +185,14 @@ public class PluginUpdateHandler extends UpdateHandler {
                 return;
             }
 
+            // add all existing plugin keys, so any conflicts with existing keys
+            // will be discovered and rejected
+            Map<String, String> existingKeys = PluginStarter.getPluginKeys(_context);
+            for (Map.Entry<String, String> e : existingKeys.entrySet()) {
+                // ignore dups/bad keys
+                up.addKey(e.getKey(), e.getValue());
+            }
+
             if (up.haveKey(pubkey)) {
                 // the key is already in the TrustedUpdate keyring
                 if (!up.verify(f)) {
@@ -285,14 +294,14 @@ public class PluginUpdateHandler extends UpdateHandler {
                 if (minVersion != null &&
                     (new VersionComparator()).compare(minVersion, oldVersion) > 0) {
                     to.delete();
-                    updateStatus("<b>" + _("Plugin update requires installed version {0} or higher", minVersion) + "</b>");
+                    updateStatus("<b>" + _("Plugin update requires installed plugin version {0} or higher", minVersion) + "</b>");
                     return;
                 }
                 String maxVersion = ConfigClientsHelper.stripHTML(props, "max-installed-version");
                 if (maxVersion != null &&
                     (new VersionComparator()).compare(maxVersion, oldVersion) < 0) {
                     to.delete();
-                    updateStatus("<b>" + _("Plugin update requires installed version {0} or lower", maxVersion) + "</b>");
+                    updateStatus("<b>" + _("Plugin update requires installed plugin version {0} or lower", maxVersion) + "</b>");
                     return;
                 }
 
@@ -306,7 +315,7 @@ public class PluginUpdateHandler extends UpdateHandler {
             } else {
                 if (Boolean.valueOf(props.getProperty("update-only")).booleanValue()) {
                     to.delete();
-                    updateStatus("<b>" + _("Plugin is for upgrades only, but the plugin is not installed", url) + "</b>");
+                    updateStatus("<b>" + _("Plugin is for upgrades only, but the plugin is not installed") + "</b>");
                     return;
                 }
                 if (!destDir.mkdir()) {
@@ -319,16 +328,16 @@ public class PluginUpdateHandler extends UpdateHandler {
             // Finally, extract the zip to the plugin directory
             if (!FileUtil.extractZip(to, destDir)) {
                 to.delete();
-                updateStatus("<b>" + _("Unzip of plugin in plugin directory {0} failed", destDir.getAbsolutePath()) + "</b>");
+                updateStatus("<b>" + _("Failed to install plugin in {0}", destDir.getAbsolutePath()) + "</b>");
                 return;
             }
 
             to.delete();
             if (Boolean.valueOf(props.getProperty("dont-start-at-install")).booleanValue()) {
                 if (Boolean.valueOf(props.getProperty("router-restart-required")).booleanValue())
-                    updateStatus("<b>" + _("Plugin {0} successfully installed, router restart required", appName) + "</b>");
+                    updateStatus("<b>" + _("Plugin {0} installed, router restart required", appName) + "</b>");
                 else {
-                    updateStatus("<b>" + _("Plugin {0} successfully installed", appName) + "</b>");
+                    updateStatus("<b>" + _("Plugin {0} installed", appName) + "</b>");
                     Properties pluginProps = PluginStarter.pluginProperties();
                     pluginProps.setProperty(PluginStarter.PREFIX + appName + PluginStarter.ENABLED, "false");
                     PluginStarter.storePluginProperties(pluginProps);
@@ -337,11 +346,11 @@ public class PluginUpdateHandler extends UpdateHandler {
                 // start everything
                 try {
                     if (PluginStarter.startPlugin(_context, appName))
-                        updateStatus("<b>" + _("Plugin {0} started", appName) + "</b>");
+                        updateStatus("<b>" + _("Plugin {0} installed and started", appName) + "</b>");
                     else
-                        updateStatus("<b>" + _("Failed to start plugin {0}, check logs", appName) + "</b>");
+                        updateStatus("<b>" + _("Plugin {0} installed but failed to start, check logs", appName) + "</b>");
                 } catch (Exception e) {
-                    updateStatus("<b>" + _("Failed to start plugin {0}:", appName) + ' ' + e + "</b>");
+                    updateStatus("<b>" + _("Plugin {0} installed but failed to start", appName) + ": " + e + "</b>");
                 }
             }
         }
@@ -350,7 +359,7 @@ public class PluginUpdateHandler extends UpdateHandler {
         public void transferFailed(String url, long bytesTransferred, long bytesRemaining, int currentAttempt) {
             File f = new File(_updateFile);
             f.delete();
-            updateStatus("<b>" + _("Plugin download from {0} failed", url) + "</b>");
+            updateStatus("<b>" + _("Failed to download plugin from {0}", url) + "</b>");
         }
     }
     

core/java/src/net/i2p/util/VersionComparator.java

@@ -5,7 +5,8 @@ import java.util.StringTokenizer;
 
 /**
  * Compares versions.
- * Characters other than [0-9.] are ignored.
+ * Characters other than [0-9.-_] are ignored.
+ * I2P only uses '.' but Sun Java uses '_' and plugins may use any of '.-_'
  * Moved from TrustedUpdate.java
  * @since 0.7.10
  */
@@ -15,8 +16,8 @@ public class VersionComparator implements Comparator<String> {
         // try it the easy way first
         if (l.equals(r))
             return 0;
-        StringTokenizer lTokens = new StringTokenizer(sanitize(l), ".");
+        StringTokenizer lTokens = new StringTokenizer(sanitize(l), VALID_SEPARATOR_CHARS);
-        StringTokenizer rTokens = new StringTokenizer(sanitize(r), ".");
+        StringTokenizer rTokens = new StringTokenizer(sanitize(r), VALID_SEPARATOR_CHARS);
 
         while (lTokens.hasMoreTokens() && rTokens.hasMoreTokens()) {
             String lNumber = lTokens.nextToken();
@@ -48,7 +49,8 @@ public class VersionComparator implements Comparator<String> {
         return left - right;
     }
 
-    private static final String VALID_VERSION_CHARS = "0123456789.";
+    private static final String VALID_SEPARATOR_CHARS = ".-_";
+    private static final String VALID_VERSION_CHARS = "0123456789" + VALID_SEPARATOR_CHARS;
 
     private static final String sanitize(String versionString) {
         StringBuilder versionStringBuilder = new StringBuilder(versionString);