- i2np.ntcp.autoip=true redefined to enable inbound only if

SSU reachability is OK. i2np.ntcp.autoip=always for the old behavior.
  autoip default is now "true".
  i2np.ntcp.hostname=xxx now trumps i2np.tcp.autoip.
- SSU always tells NTCP when status changes.

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

@@ -27,9 +27,8 @@ public class ConfigNetHandler extends FormHandler {
     private String _ntcpPort;
     private String _tcpPort;
     private String _udpPort;
-    private boolean _ntcpAutoIP;
+    private String _ntcpAutoIP;
     private boolean _ntcpAutoPort;
-    private boolean _ntcpInboundDisabled;
     private boolean _upnp;
     private String _inboundRate;
     private String _inboundBurstRate;
@@ -60,9 +59,7 @@ public class ConfigNetHandler extends FormHandler {
     public void setDynamicKeys(String moo) { _dynamicKeys = true; }
     public void setEnableloadtesting(String moo) { _enableLoadTesting = true; }
     public void setNtcpAutoIP(String mode) {
-        _ntcpAutoIP = mode.equals("2");
+        _ntcpAutoIP = mode;
-        if (mode.equals("0"))
-            _ntcpInboundDisabled = true;
     }
     public void setNtcpAutoPort(String mode) {
         _ntcpAutoPort = mode.equals("2");
@@ -125,31 +122,24 @@ public class ConfigNetHandler extends FormHandler {
             if (oldNHost == null) oldNHost = "";
             String oldNPort = _context.router().getConfigSetting(ConfigNetHelper.PROP_I2NP_NTCP_PORT);
             if (oldNPort == null) oldNPort = "";
-            String sAutoHost = _context.router().getConfigSetting(ConfigNetHelper.PROP_I2NP_NTCP_AUTO_IP);
+            String oldAutoHost = _context.router().getConfigSetting(ConfigNetHelper.PROP_I2NP_NTCP_AUTO_IP);
             String sAutoPort = _context.router().getConfigSetting(ConfigNetHelper.PROP_I2NP_NTCP_AUTO_PORT);
-            boolean oldAutoHost = "true".equalsIgnoreCase(sAutoHost);
             boolean oldAutoPort = "true".equalsIgnoreCase(sAutoPort);
-            if (_ntcpHostname == null || _ntcpInboundDisabled) _ntcpHostname = "";
+            if (_ntcpHostname == null) _ntcpHostname = "";
-            if (_ntcpPort == null || _ntcpInboundDisabled) _ntcpPort = "";
+            if (_ntcpPort == null) _ntcpPort = "";
-            if (_ntcpInboundDisabled) {
-                _ntcpAutoIP = false;
-                _ntcpAutoPort = false;
-            }
 
             if (oldAutoHost != _ntcpAutoIP || ! oldNHost.equalsIgnoreCase(_ntcpHostname)) {
-                if (_ntcpAutoIP) {
+                if ("false".equals(_ntcpAutoIP) && _ntcpHostname.length() > 0) {
-                    _context.router().setConfigSetting(ConfigNetHelper.PROP_I2NP_NTCP_AUTO_IP, "true");
-                    _context.router().removeConfigSetting(ConfigNetHelper.PROP_I2NP_NTCP_HOSTNAME);
-                    addFormNotice("Updating inbound TCP address to auto");
-                } else if (_ntcpHostname.length() > 0) {
                     _context.router().setConfigSetting(ConfigNetHelper.PROP_I2NP_NTCP_HOSTNAME, _ntcpHostname);
-                    _context.router().removeConfigSetting(ConfigNetHelper.PROP_I2NP_NTCP_AUTO_IP);
                     addFormNotice("Updating inbound TCP address to " + _ntcpHostname);
                 } else {
                     _context.router().removeConfigSetting(ConfigNetHelper.PROP_I2NP_NTCP_HOSTNAME);
-                    _context.router().removeConfigSetting(ConfigNetHelper.PROP_I2NP_NTCP_AUTO_IP);
+                    if ("false".equals(_ntcpAutoIP))
                         addFormNotice("Disabling inbound TCP");
+                    else
+                        addFormNotice("Updating inbound TCP address to auto");
                 }
+                _context.router().setConfigSetting(ConfigNetHelper.PROP_I2NP_NTCP_AUTO_IP, _ntcpAutoIP);
                 restartRequired = true;
             }
             if (oldAutoPort != _ntcpAutoPort || ! oldNPort.equals(_ntcpPort)) {

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

@@ -114,10 +114,13 @@ public class ConfigNetHelper extends HelperBase {
             return DISABLED;
         String hostname = _context.getProperty(PROP_I2NP_NTCP_HOSTNAME); 
         boolean specified = hostname != null && hostname.length() > 0;
-        boolean auto = Boolean.valueOf(_context.getProperty(PROP_I2NP_NTCP_AUTO_IP)).booleanValue();
+        String auto = _context.getProperty(PROP_I2NP_NTCP_AUTO_IP);
-        if ((mode == 0 && (!specified) && !auto) ||
+        if (auto == null)
-            (mode == 1 && specified && !auto) ||
+            auto = "false";
-            (mode == 2 && auto))
+        if ((mode == 0 && (!specified) && auto.equals("false")) ||
+            (mode == 1 && specified && auto.equals("false")) ||
+            (mode == 2 && auto.equals("true")) ||
+            (mode == 3 && auto.equals("always")))
             return CHECKED;
         return "";
     }

apps/routerconsole/jsp/config.jsp

@@ -114,12 +114,14 @@
  <p>
  <b>Inbound TCP connection configuration:</b><br />
  Externally reachable hostname or IP address:<br />
-    <input type="radio" name="ntcpAutoIP" value="0" <%=nethelper.getTcpAutoIPChecked(0) %> />
+    <input type="radio" name="ntcpAutoIP" value="false" <%=nethelper.getTcpAutoIPChecked(0) %> />
-    Disable<br />
+    Disable (Firewalled)<br />
-    <input type="radio" name="ntcpAutoIP" value="2" <%=nethelper.getTcpAutoIPChecked(2) %> />
+    <input type="radio" name="ntcpAutoIP" value="always" <%=nethelper.getTcpAutoIPChecked(3) %> />
-    Use IP address detected by SSU
+    Use IP address detected by SSU (Not firewalled)
     <i>(currently <jsp:getProperty name="nethelper" property="udpIP" />)</i><br />
-    <input type="radio" name="ntcpAutoIP" value="1" <%=nethelper.getTcpAutoIPChecked(1) %> />
+    <input type="radio" name="ntcpAutoIP" value="true" <%=nethelper.getTcpAutoIPChecked(2) %> />
+    Use IP address detected by SSU, only if we do not appear to be firewalled<br />
+    <input type="radio" name="ntcpAutoIP" value="false" <%=nethelper.getTcpAutoIPChecked(1) %> />
     Specify hostname or IP:
     <input name ="ntcphost" type="text" size="16" value="<jsp:getProperty name="nethelper" property="ntcphostname" />" />
     <i>(dyndns and the like are fine)</i><br />

router/java/src/net/i2p/router/transport/CommSystemFacadeImpl.java

@@ -269,6 +269,8 @@ public class CommSystemFacadeImpl extends CommSystemFacade {
         }
 
         boolean changed = false;
+
+        // Auto Port Setting
         // old behavior (<= 0.7.3): auto-port defaults to false, and true trumps explicit setting
         // new behavior (>= 0.7.4): auto-port defaults to true, but explicit setting trumps auto
         String oport = newProps.getProperty(NTCPAddress.PROP_PORT);
@@ -288,9 +290,26 @@ public class CommSystemFacadeImpl extends CommSystemFacade {
             changed = true;
         }
 
+        // Auto IP Setting
+        // old behavior (<= 0.7.3): auto-ip defaults to false, and trumps configured hostname,
+        //                          and ignores reachability status - leading to
+        //                          "firewalled with inbound TCP enabled" warnings.
+        // new behavior (>= 0.7.4): auto-ip defaults to true, and explicit setting trumps auto,
+        //                          and only takes effect if reachability is OK.
+        //                          And new "always" setting ignores reachability status, like
+        //                          "true" was in 0.7.3
         String ohost = newProps.getProperty(NTCPAddress.PROP_HOST);
-        if (Boolean.valueOf(_context.getProperty(PROP_I2NP_NTCP_AUTO_IP)).booleanValue()) {
+        String enabled = _context.getProperty(PROP_I2NP_NTCP_AUTO_IP, "true");
+        String name = _context.getProperty(PROP_I2NP_NTCP_HOSTNAME);
+        if (name != null && name.length() > 0)
+            enabled = "false";
+        if (_log.shouldLog(Log.INFO))
+            _log.info("old: " + ohost + " config: " + name + " auto: " + enabled + " status: " + getReachabilityStatus());
+        if (enabled.equalsIgnoreCase("always") ||
+            (enabled.equalsIgnoreCase("true") && getReachabilityStatus() == CommSystemFacade.STATUS_OK)) {
             String nhost = UDPProps.getProperty(UDPAddress.PROP_HOST);
+            if (_log.shouldLog(Log.INFO))
+                _log.info("old: " + ohost + " config: " + name + " new: " + nhost);
             if (nhost == null || nhost.length() <= 0)
                 return;
             if (ohost == null || ! ohost.equalsIgnoreCase(nhost)) {

router/java/src/net/i2p/router/transport/udp/UDPTransport.java

@@ -2102,7 +2102,10 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
         if ( (status != old) && (status != CommSystemFacade.STATUS_UNKNOWN) ) {
             if (_log.shouldLog(Log.INFO))
                 _log.info("Old status: " + old + " New status: " + status + " from: ", new Exception("traceback"));
-            if (needsRebuild())
+            // Always rebuild when the status changes, even if our address hasn't changed,
+            // as rebuildExternalAddress() calls replaceAddress() which calls CSFI.notifyReplaceAddress()
+            // which will start up NTCP inbound when we transition to OK.
+            // if (needsRebuild())
                 rebuildExternalAddress();
         }
     }