UrlLauncher:

- Use arrays for exec
- Randomize temp file name
- Require quotes around args containing spaces in routerconsole.browser property
- Add debug logging
- Add chromium-browser to the default list
- Parse and use full command line from Windows registry
- Replace %1 with url in registry line and routerconsole.browser property
ShellCommand:
- Switch to i2p logging

apps/systray/java/src/net/i2p/apps/systray/UrlLauncher.java

@@ -20,12 +20,16 @@ import java.net.Socket;
 import java.net.SocketAddress;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.util.Arrays;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Locale;
 
 import net.i2p.I2PAppContext;
 import net.i2p.app.*;
 import static net.i2p.app.ClientAppState.*;
 import net.i2p.util.I2PAppThread;
+import net.i2p.util.Log;
 import net.i2p.util.ShellCommand;
 import net.i2p.util.SystemVersion;
 
@@ -46,6 +50,7 @@ public class UrlLauncher implements ClientApp {
     private final I2PAppContext _context;
     private final ClientAppManager _mgr;
     private final String[] _args;
+    private final Log _log;
 
     private static final int WAIT_TIME = 5*1000;
     private static final int MAX_WAIT_TIME = 5*60*1000;
@@ -69,6 +74,7 @@ public class UrlLauncher implements ClientApp {
             "defaultbrowser",  // puppy linux
             "opera -newpage",
             "firefox",
+            "chromium-browser",
             "mozilla",
             "netscape",
             "konqueror",
@@ -82,11 +88,14 @@ public class UrlLauncher implements ClientApp {
     /**
      *  ClientApp constructor used from clients.config
      *
+     *  @param mgr null OK
+     *  @param args URL in args[0] or null args for router console
      *  @since 0.9.18
      */
     public UrlLauncher(I2PAppContext context, ClientAppManager mgr, String[] args) {
         _state = UNINITIALIZED;
         _context = context;
+        _log = _context.logManager().getLog(UrlLauncher.class);
         _mgr = mgr;
         if (args == null || args.length <= 0)
             args = new String[] { context.portMapper().getConsoleURL() };
@@ -103,6 +112,7 @@ public class UrlLauncher implements ClientApp {
     public UrlLauncher() {
         _state = UNINITIALIZED;
         _context = I2PAppContext.getGlobalContext();
+        _log = _context.logManager().getLog(UrlLauncher.class);
         _mgr = null;
         _args = null;
         _shellCommand = new ShellCommand();
@@ -167,7 +177,8 @@ public class UrlLauncher implements ClientApp {
      * unsuccessful, an attempt is made to launch the URL using the most common
      * browsers.
      * 
-     * BLOCKING
+     * BLOCKING. This repeatedly probes the server port at the given url
+     * until it is apparently ready.
      * 
      * @param  url The URL to open.
      * @return     <code>true</code> if the operation was successful, otherwise
@@ -176,7 +187,9 @@ public class UrlLauncher implements ClientApp {
      * @throws IOException
      */ 
     public boolean openUrl(String url) throws IOException {
+        if (_log.shouldDebug()) _log.debug("Waiting for server");
         waitForServer(url);
+        if (_log.shouldDebug()) _log.debug("Done waiting for server");
         if (validateUrlFormat(url)) {
             String cbrowser = _context.getProperty(PROP_BROWSER);
             if (cbrowser != null) {
@@ -185,54 +198,72 @@ public class UrlLauncher implements ClientApp {
             if (SystemVersion.isMac()) {
                 String osName = System.getProperty("os.name");
                 if (osName.toLowerCase(Locale.US).startsWith("mac os x")) {
-
+                    String[] args = new String[] { "open", url };
-                    if (_shellCommand.executeSilentAndWaitTimed("open " + url, 5))
+                    if (_log.shouldDebug()) _log.debug("Execute: " + Arrays.toString(args));
+                    if (_shellCommand.executeSilentAndWaitTimed(args , 5))
                         return true;
-
                 } else {
                     return false;
                 }
-
+                String[] args = new String[] { "iexplore", url };
-                if (_shellCommand.executeSilentAndWaitTimed("iexplore " + url, 5))
+                if (_log.shouldDebug()) _log.debug("Execute: " + Arrays.toString(args));
+                if (_shellCommand.executeSilentAndWaitTimed(args , 5))
                     return true;
             } else if (SystemVersion.isWindows()) {
-                String         browserString  = "\"C:\\Program Files\\Internet Explorer\\iexplore.exe\" -nohome";
+                String[] browserString  = new String[] { "C:\\Program Files\\Internet Explorer\\iexplore.exe", "-nohome", url };
+                File foo = new File(_context.getTempDir(), "browser" + _context.random().nextLong() + ".reg");
+                String[] args = new String[] { "regedit", "/E", foo.getAbsolutePath(), "HKEY_CLASSES_ROOT\\http\\shell\\open\\command" };
+                if (_log.shouldDebug()) _log.debug("Execute: " + Arrays.toString(args));
+                boolean ok = _shellCommand.executeSilentAndWait(args);
+                if (ok) {
                     BufferedReader bufferedReader = null;
-
-                File foo = new File(_context.getTempDir(), "browser.reg");
-                _shellCommand.executeSilentAndWait("regedit /E \"" + foo.getAbsolutePath() + "\" \"HKEY_CLASSES_ROOT\\http\\shell\\open\\command\"");
-
                     try {
                         bufferedReader = new BufferedReader(new InputStreamReader(new FileInputStream(foo), "UTF-16"));
                         for (String line; (line = bufferedReader.readLine()) != null; ) {
+                            // @="\"C:\\Program Files (x86)\\Mozilla Firefox\\firefox.exe\" -osint -url \"%1\""
                             if (line.startsWith("@=")) {
-                            // we should really use the whole line and replace %1 with the url
+                                if (_log.shouldDebug()) _log.debug("From RegEdit: " + line);
-                            browserString = line.substring(3, line.toLowerCase(Locale.US).indexOf(".exe") + 4);
+                                line = line.substring(2).trim();
-                            if (browserString.startsWith("\\\""))
+                                if (line.startsWith("\"") && line.endsWith("\""))
-                                browserString = browserString.substring(2);
+                                    line = line.substring(1, line.length() - 1);
-                            browserString = "\"" + browserString + "\"";
+                                line = line.replace("\\\\", "\\");
+                                line = line.replace("\\\"", "\"");
+                                if (_log.shouldDebug()) _log.debug("Mod RegEdit: " + line);
+                                // "C:\Program Files (x86)\Mozilla Firefox\firefox.exe" -osint -url "%1"
+                                // use the whole line
+                                String[] aarg = parseArgs(line, url);
+                                if (aarg.length > 0) {
+                                    browserString = aarg;
+                                    break;
+                                }
                             }
                         }
-                    try {
-                        bufferedReader.close();
                     } catch (IOException e) {
-                        // No worries.
+                        if (_log.shouldWarn())
-                    }
+                            _log.warn("Reading regedit output", e);
-                    foo.delete();
-                } catch (IOException e) {
-                    // Defaults to IE.
                     } finally {
                         if (bufferedReader != null)
                             try { bufferedReader.close(); } catch (IOException ioe) {}
+                        foo.delete();
                     }
-                if (_shellCommand.executeSilentAndWaitTimed(browserString + ' ' + url, 5))
+                } else if (_log.shouldWarn()) {
+                    _log.warn("Regedit Failed: " + Arrays.toString(args));
+                }
+                if (_log.shouldDebug()) _log.debug("Execute: " + Arrays.toString(browserString));
+                if (_shellCommand.executeSilentAndWaitTimed(browserString, 5))
                     return true;
+                if (_log.shouldInfo()) _log.info("Failed: " + Arrays.toString(browserString));
             } else {
                 // fall through
             }
+            String[] args = new String[2];
+            args[1] = url;
             for (int i = 0; i < BROWSERS.length; i++) {
-                if (_shellCommand.executeSilentAndWaitTimed(BROWSERS[i] + ' ' + url, 5))
+                args[0] = BROWSERS[i];
+                if (_log.shouldDebug()) _log.debug("Execute: " + Arrays.toString(args));
+                if (_shellCommand.executeSilentAndWaitTimed(args, 5))
                     return true;
+                if (_log.shouldInfo()) _log.info("Failed: " + Arrays.toString(args));
             }
         }
         return false;
@@ -240,11 +271,17 @@ public class UrlLauncher implements ClientApp {
 
     /**
      * Opens the given URL with the given browser.
+     * As of 0.9.38, the browser parameter will be parsed into arguments
+     * separated by spaces or tabs.
+     * %1, if present, will be replaced with the url.
+     * Arguments may be surrounded by single or double quotes if
+     * they contain spaces or tabs.
+     * There is no mechanism to escape quotes or other chars with backslashes.
      * 
-     * BLOCKING
+     * BLOCKING. However, this does NOT probe the server port to see if it is ready.
      * 
      * @param  url     The URL to open.
-     * @param  browser The browser to use.
+     * @param  browser The browser to use. See above for quoting rules.
      * @return         <code>true</code> if the operation was successful,
      *                 otherwise <code>false</code>.
      * 
@@ -253,12 +290,87 @@ public class UrlLauncher implements ClientApp {
     public boolean openUrl(String url, String browser) throws IOException {
         waitForServer(url);
         if (validateUrlFormat(url)) {
-            if (_shellCommand.executeSilentAndWaitTimed(browser + " " + url, 5))
+            String[] args = parseArgs(browser, url);
+            if (args.length > 0) {
+                if (_log.shouldDebug()) _log.debug("Execute: " + Arrays.toString(args));
+                if (_shellCommand.executeSilentAndWaitTimed(args, 5))
                     return true;
             }
+        }
         return false;
     }
 
+    /**
+     *  Parse args into arguments
+     *  separated by spaces or tabs.
+     *  %1, if present, will be replaced with the url,
+     *  otherwise it will be added as the last argument.
+     *  Arguments may be surrounded by single or double quotes if
+     *  they contain spaces or tabs.
+     *  There is no mechanism to escape quotes or other chars with backslashes.
+     *  Adapted from i2ptunnel SSLHelper.
+     *
+     *  @return param args non-null
+     *  @return non-null
+     *  @since 0.9.38
+     */
+    private static String[] parseArgs(String args, String url) {
+        List<String> argList = new ArrayList<String>(4);
+        StringBuilder buf = new StringBuilder(32);
+        boolean isQuoted = false;
+        for (int j = 0; j < args.length(); j++) {
+            char c = args.charAt(j);
+            switch (c) {
+                case '\'':
+                case '"':
+                    if (isQuoted) {
+                        String str = buf.toString().trim();
+                        if (str.length() > 0)
+                            argList.add(str);
+                        buf.setLength(0);
+                    }
+                    isQuoted = !isQuoted;
+                    break;
+                case ' ':
+                case '\t':
+                    // whitespace - if we're in a quoted section, keep this as part of the quote,
+                    // otherwise use it as a delim
+                    if (isQuoted) {
+                        buf.append(c);
+                    } else {
+                        String str = buf.toString().trim();
+                        if (str.length() > 0)
+                            argList.add(str);
+                        buf.setLength(0);
+                    }
+                    break;
+                default:
+                    buf.append(c);
+                    break;
+            }
+        }
+        if (buf.length() > 0) {
+            String str = buf.toString().trim();
+            if (str.length() > 0)
+                argList.add(str);
+        }
+        if (argList.isEmpty())
+            return new String[] {};
+        boolean foundpct = false;
+        // replace %1 with the url
+        for (int i = 0; i < argList.size(); i++) {
+            String arg = argList.get(i);
+            if (arg.contains("%1")) {
+                argList.set(i, arg.replace("%1", url));
+                foundpct = true;
+            }
+        }
+        // add url if no %1
+        if (!foundpct)
+            argList.add(url);
+        return argList.toArray(new String[argList.size()]);
+    }
+
     private static boolean validateUrlFormat(String urlString) {
          try {
             // just to check validity

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

@@ -18,6 +18,8 @@ import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.util.Arrays;
 
+import net.i2p.I2PAppContext;
+
 /**
  * Passes a command to the OS shell for execution and manages the input and
  * output.
@@ -28,7 +30,6 @@ import java.util.Arrays;
  */
 public class ShellCommand {
 
-    private static final boolean DEBUG = false;
     private static final boolean CONSUME_OUTPUT    = true;
     private static final boolean NO_CONSUME_OUTPUT = false;
 
@@ -358,7 +359,8 @@ public class ShellCommand {
     private boolean executeSAWT(Object shellCommand, int seconds) {
         String name = null;
         long begin = 0;
-        if (DEBUG) {
+        Log log = I2PAppContext.getGlobalContext().logManager().getLog(ShellCommand.class);
+        if (log.shouldDebug()) {
             if (shellCommand instanceof String) {
                 name = (String) shellCommand;
             } else if (shellCommand instanceof String[]) {
@@ -374,16 +376,16 @@ public class ShellCommand {
             if (seconds > 0) {
                 commandThread.join(seconds * 1000);
                 if (commandThread.isAlive()) {
-                    if (DEBUG)
+                    if (log.shouldDebug())
-                        System.out.println("ShellCommand gave up waiting for \"" + name + "\" after " + seconds + " seconds");
+                        log.debug("ShellCommand gave up waiting for \"" + name + "\" after " + seconds + " seconds");
                     return true;
                 }
             }
         } catch (InterruptedException e) {
             // Wake up, time to die.
         }
-        if (DEBUG)
+        if (log.shouldDebug())
-            System.out.println("ShellCommand returning " + result.commandSuccessful + " for \"" + name + "\" after " + (System.currentTimeMillis() - begin) + " ms");
+            log.debug("ShellCommand returning " + result.commandSuccessful + " for \"" + name + "\" after " + (System.currentTimeMillis() - begin) + " ms");
         return result.commandSuccessful;
     }
 
@@ -426,18 +428,19 @@ public class ShellCommand {
     private boolean execute(Object shellCommand, boolean consumeOutput, boolean waitForExitStatus) {
         Process process;
         String name = null;  // for debugging only
+        Log log = I2PAppContext.getGlobalContext().logManager().getLog(ShellCommand.class);
         try {
             // easy way so we don't have to copy this whole method
             if (shellCommand instanceof String) {
                 name = (String) shellCommand;
-                if (DEBUG)
+                if (log.shouldDebug())
-                    System.out.println("ShellCommand exec \"" + name + "\" consume? " + consumeOutput + " wait? " + waitForExitStatus);
+                    log.debug("ShellCommand exec \"" + name + "\" consume? " + consumeOutput + " wait? " + waitForExitStatus);
                 process = Runtime.getRuntime().exec(name);
             } else if (shellCommand instanceof String[]) {
                 String[] arr = (String[]) shellCommand;
-                if (DEBUG) {
+                if (log.shouldDebug()) {
                     name = Arrays.toString(arr);
-                    System.out.println("ShellCommand exec \"" + name + "\" consume? " + consumeOutput + " wait? " + waitForExitStatus);
+                    log.debug("ShellCommand exec \"" + name + "\" consume? " + consumeOutput + " wait? " + waitForExitStatus);
                 }
                 process = Runtime.getRuntime().exec(arr);
             } else {
@@ -461,14 +464,13 @@ public class ShellCommand {
                 processStdoutReader.start();
             }
             if (waitForExitStatus) {
-                if (DEBUG)
+                if (log.shouldDebug())
-                    System.out.println("ShellCommand waiting for \"" + name + '\"');
+                    log.debug("ShellCommand waiting for \"" + name + '\"');
                 try {
                     process.waitFor();
                 } catch (InterruptedException e) {
-                    if (DEBUG) {
+                    if (log.shouldWarn()) {
-                        System.out.println("ShellCommand exception waiting for \"" + name + '\"');
+                        log.warn("ShellCommand exception waiting for \"" + name + '"', e);
-                        e.printStackTrace();
                     }
                     if (!consumeOutput)
                         killStreams();
@@ -478,16 +480,15 @@ public class ShellCommand {
                 if (!consumeOutput)
                     killStreams();
 
-                if (DEBUG)
+                if (log.shouldDebug())
-                    System.out.println("ShellCommand exit value is " + process.exitValue() + " for \"" + name + '\"');
+                    log.debug("ShellCommand exit value is " + process.exitValue() + " for \"" + name + '\"');
                 if (process.exitValue() > 0)
                     return false;
             }
         } catch (IOException e) {
             // probably IOException, file not found from exec()
-            if (DEBUG) {
+            if (log.shouldWarn()) {
-                System.out.println("ShellCommand execute exception for \"" + name + '\"');
+                log.warn("ShellCommand execute exception for \"" + name + '"', e);
-                e.printStackTrace();
             }
             return false;
         }

history.txt

@@ -1,3 +1,29 @@
+2018-12-12 zzz
+ * DTG: Use UrlLauncher to launch browser
+ * Installer: Drop unused systray.config
+ * UrlLauncher: Improvements and cleanups
+ * Util: Add another ShellCommand String[] method
+
+2018-12-11 zzz
+ * Crypto: HMAC-SHA256 cleanup
+ * Debian: Add conffiles list
+ * Utils: Enable TLSv1.3 for SSL sockets
+
+2018-12-08 zzz
+ * Console: Hide I2CP config if disabled
+ * NetDb: Allow longer expiration for Meta LS2
+ * Transport:
+   - Don't repeatedly publish RI if IPv6-only but
+     not configured IPv6-only
+   - Don't set status to disconnected if IPv6-only but
+     not configured IPv6-only
+
+2018-12-05 zzz
+ * I2CP:
+   - Propagate error from disconnect message to session listener
+   - Set offline keys in generated LS2
+   - Set and validate offline sig in SessionConfig
+
 2018-12-04 zzz
  * Data: Add preliminary PrivateKeyFile support for LS2 offline keys (proposal #123)
  * I2CP: Add preliminary support for LS2 offline keys (proposal #123)

router/java/src/net/i2p/router/RouterVersion.java

@@ -18,7 +18,7 @@ public class RouterVersion {
     /** deprecated */
     public final static String ID = "Monotone";
     public final static String VERSION = CoreVersion.VERSION;
-    public final static long BUILD = 8;
+    public final static long BUILD = 9;
 
     /** for example "-test" */
     public final static String EXTRA = "";