Tunnels: Fix and consolidate allow-zero-hop logic,

prevent zero-hop client tunnels even when no active peers
Remove buildRequestZeroHopTime stat

history.txt

@@ -1,3 +1,7 @@
+2018-04-27 zzz
+  * Tunnels: Fix and consolidate allow-zero-hop logic,
+    prevent zero-hop client tunnels when no active peers
+
 2018-04-24 meeh
   * Mac OS X launcher is still WIP, but:
     - Building the I2P.app bundle is mostly done
@@ -6,6 +10,10 @@
   * Added new entries to mtn-ignore so we avoid any PEBCAK with commiting build directories
   * Added an SBT AutoPlugin named IconHelper to generate valid ICNS images for Mac OS X
 
+2018-04-23 zzz
+ * Clock: Fix early NPE via DoH
+ * EepGet: Handle HTTP response line with no status text
+
 2018-04-23 meeh
   * Added launchers for Browser Bundle and Mac OS X
 

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 = 6;
+    public final static long BUILD = 7;
 
     /** for example "-test" */
     public final static String EXTRA = "";

router/java/src/net/i2p/router/TunnelPoolSettings.java

@@ -79,7 +79,7 @@ public class TunnelPoolSettings {
     private static final int    DEFAULT_OB_EXPL_LENGTH_VARIANCE = 0;
     //private static final int    DEFAULT_OB_EXPL_LENGTH_VARIANCE = isSlow ? 0 : 1;
 
-    public static final boolean DEFAULT_ALLOW_ZERO_HOP = true;
+    public static final boolean DEFAULT_ALLOW_ZERO_HOP = false;
     public static final int     DEFAULT_IP_RESTRICTION = 2;    // class B (/16)
     private static final int MIN_PRIORITY = -25;
     private static final int MAX_PRIORITY = 25;
@@ -161,16 +161,29 @@ public class TunnelPoolSettings {
     
     /**
      * If there are no tunnels to build with, will this pool allow 0 hop tunnels?
+     *
      * Always true for exploratory.
-     * Generally true for client, but should probably be ignored...
+     * Prior to 0.9.35, generally true for client.
-     * use getLength() + getLengthVariance() > 0 instead.
+     * As of 0.9.35, generally false for client, but true if
+     * getLength() + Math.min(getLengthVariance(), 0) <= 0,
+     * OR if getLengthOverride() == 0
+     * OR if setAllowZeroHop(true) was called or set in properties.
      */
-    public boolean getAllowZeroHop() { return _allowZeroHop; }
+    public boolean getAllowZeroHop() {
+        return _allowZeroHop ||
+               _length + Math.min(_lengthVariance, 0) <= 0 ||
+               _lengthOverride == 0;
+    }
 
     /**
      * If there are no tunnels to build with, will this pool allow 0 hop tunnels?
      * No effect on exploratory (always true)
+     *
+     * @param ok if true, getAllowZeroHop() will always return true
+     *           if false, getAllowZeroHop will return as documented.
+     * @deprecated unused
      */
+    @Deprecated
     public void setAllowZeroHop(boolean ok) {
         if (!_isExploratory)
             _allowZeroHop = ok;

router/java/src/net/i2p/router/tunnel/pool/BuildExecutor.java

@@ -66,7 +66,7 @@ class BuildExecutor implements Runnable {
         _context.statManager().createRequiredRateStat("tunnel.buildClientReject", "Response time for rejection (ms)", "Tunnels", new long[] { 10*60*1000, 60*60*1000 });
         _context.statManager().createRequiredRateStat("tunnel.buildRequestTime", "Time to build a tunnel request (ms)", "Tunnels", new long[] { 60*1000, 10*60*1000 });
         _context.statManager().createRateStat("tunnel.buildConfigTime", "Time to build a tunnel request (ms)", "Tunnels", new long[] { 60*1000, 10*60*1000 });
-        _context.statManager().createRateStat("tunnel.buildRequestZeroHopTime", "How long it takes to build a zero hop tunnel", "Tunnels", new long[] { 60*1000, 10*60*1000 });
+        //_context.statManager().createRateStat("tunnel.buildRequestZeroHopTime", "How long it takes to build a zero hop tunnel", "Tunnels", new long[] { 60*1000, 10*60*1000 });
         //_context.statManager().createRateStat("tunnel.pendingRemaining", "How many inbound requests are pending after a pass (period is how long the pass takes)?", "Tunnels", new long[] { 60*1000, 10*60*1000 });
         _context.statManager().createRateStat("tunnel.buildFailFirstHop", "How often we fail to build a OB tunnel because we can't contact the first hop", "Tunnels", new long[] { 60*1000, 10*60*1000 });
         _context.statManager().createRateStat("tunnel.buildReplySlow", "Build reply late, but not too late", "Tunnels", new long[] { 10*60*1000 });
@@ -535,11 +535,10 @@ class BuildExecutor implements Runnable {
         boolean ok = BuildRequestor.request(_context, pool, cfg, this);
         if (!ok)
             return;
-        long buildTime = System.currentTimeMillis() - beforeBuild;
+        if (cfg.getLength() > 1) {
-        if (cfg.getLength() <= 1)
+            long buildTime = System.currentTimeMillis() - beforeBuild;
-            _context.statManager().addRateData("tunnel.buildRequestZeroHopTime", buildTime, 0);
-        else
             _context.statManager().addRateData("tunnel.buildRequestTime", buildTime, 0);
+        }
         long id = cfg.getReplyMessageId();
         if (id > 0) {
             synchronized (_recentBuildIds) { 

router/java/src/net/i2p/router/tunnel/pool/TunnelPool.java

@@ -176,8 +176,7 @@ public class TunnelPool {
     TunnelInfo selectTunnel() { return selectTunnel(true); }
 
     private TunnelInfo selectTunnel(boolean allowRecurseOnFail) {
-        boolean avoidZeroHop = getSettings().getLength() > 0 &&
+        boolean avoidZeroHop = !_settings.getAllowZeroHop();
-                               getSettings().getLength() + getSettings().getLengthVariance() > 0;
         
         long period = curPeriod();
         synchronized (_tunnels) {
@@ -243,7 +242,7 @@ public class TunnelPool {
             }
         }
         
-        if (_alive && _settings.getAllowZeroHop())
+        if (_alive && !avoidZeroHop)
             buildFallback();
         if (allowRecurseOnFail)
             return selectTunnel(false); 
@@ -264,8 +263,7 @@ public class TunnelPool {
      * @since 0.8.10
      */
     TunnelInfo selectTunnel(Hash closestTo) {
-        boolean avoidZeroHop = getSettings().getLength() > 0 &&
+        boolean avoidZeroHop = !_settings.getAllowZeroHop();
-                               getSettings().getLength() + getSettings().getLengthVariance() > 0;
         TunnelInfo rv = null;
         synchronized (_tunnels) {
             if (!_tunnels.isEmpty()) {
@@ -659,13 +657,6 @@ public class TunnelPool {
             return false;
 
         if (_settings.getAllowZeroHop()) {
-            if ( (_settings.getLength() + _settings.getLengthVariance() > 0) && 
-                 (!_settings.isExploratory()) &&
-                 (_context.profileOrganizer().countActivePeers() > 0) ) {
-                // if it is a client tunnel pool and our variance doesn't allow 0 hop, prefer failure to
-                // 0 hop operation (unless our router is offline)
-                return false;
-            }
             if (_log.shouldLog(Log.INFO))
                 _log.info(toString() + ": building a fallback tunnel (usable: " + usable + " needed: " + quantity + ")");
             
@@ -847,7 +838,7 @@ public class TunnelPool {
         }
         int wanted = getAdjustedTotalQuantity();
         
-        boolean allowZeroHop = ((getSettings().getLength() + getSettings().getLengthVariance()) <= 0);
+        boolean allowZeroHop = _settings.getAllowZeroHop();
           
         /**
          * This algorithm builds based on the previous average length of time it takes