BuildHandler: Improve handling of null 'from' value

when not IBGW (ticket #1738)

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

@@ -629,6 +629,12 @@ class BuildHandler implements Runnable {
         boolean isInGW = req.readIsInboundGateway();
         boolean isOutEnd = req.readIsOutboundEndpoint();
 
+        Hash from = state.fromHash;
+        if (from == null && state.from != null)
+            from = state.from.calculateHash();
+        // warning, from could be null, but it should only
+        // happen if we will be a IBGW and it came from us as a OBEP
+
         if (isInGW && isOutEnd) {
             _context.statManager().addRateData("tunnel.rejectHostile", 1);
             _log.error("Dropping build request, IBGW+OBEP");
@@ -645,21 +651,17 @@ class BuildHandler implements Runnable {
                 _log.warn("Dropping build request, we are the next hop");
             return;
         }
-        // previous test should be sufficient to keep it from getting here but maybe not?
         if (!isInGW) {
-            Hash from = state.fromHash;
+            // if from is null, it came via OutboundMessageDistributor.distribute(),
-            if (from == null && state.from != null)
+            // i.e. we were the OBEP, which is fine if we're going to be an IBGW
-                from = state.from.calculateHash();
+            // but if not, something is seriously wrong here.
-            if (_context.routerHash().equals(from)) {
+            if (from == null || _context.routerHash().equals(from)) {
                 _context.statManager().addRateData("tunnel.rejectHostile", 1);
                 _log.error("Dropping build request, we are the previous hop");
                 return;
             }
         }
         if ((!isOutEnd) && (!isInGW)) {
-            Hash from = state.fromHash;
-            if (from == null && state.from != null)
-                from = state.from.calculateHash();
             // Previous and next hop the same? Don't help somebody be evil. Drop it without a reply.
             // A-B-C-A is not preventable
             if (nextPeer.equals(from)) {
@@ -761,9 +763,6 @@ class BuildHandler implements Runnable {
         // This is at the end as it compares to a percentage of created tunnels.
         // We may need another counter above for requests.
         if (response == 0 && !isInGW) {
-            Hash from = state.fromHash;
-            if (from == null && state.from != null)
-                from = state.from.calculateHash();
             if (from != null && _throttler.shouldThrottle(from)) {
                 if (_log.shouldLog(Log.WARN))
                     _log.warn("Rejecting tunnel (hop throttle), previous hop: " + from);
@@ -784,8 +783,7 @@ class BuildHandler implements Runnable {
         if (_log.shouldLog(Log.DEBUG))
             _log.debug("Responding to " + state.msg.getUniqueId() + "/" + ourId
                        + " after " + recvDelay + " with " + response 
-                       + " from " + (state.fromHash != null ? state.fromHash : 
+                       + " from " + (from != null ? from : "tunnel"));
-                                     state.from != null ? state.from.calculateHash() : "tunnel"));
 
         HopConfig cfg = null;
         if (response == 0) {
@@ -798,10 +796,8 @@ class BuildHandler implements Runnable {
                 // default
                 //cfg.setReceiveFrom(null);
             } else {
-                if (state.fromHash != null) {
+                if (from != null) {
-                    cfg.setReceiveFrom(state.fromHash);
+                    cfg.setReceiveFrom(from);
-                } else if (state.from != null) {
-                    cfg.setReceiveFrom(state.from.calculateHash());
                 } else {
                     // b0rk
                     return;
@@ -838,21 +834,21 @@ class BuildHandler implements Runnable {
                     _log.warn("DUP ID failure " + state.msg.getUniqueId() + "/" + cfg.getReceiveTunnel() + " as " + (isOutEnd ? "outbound endpoint" : isInGW ? "inbound gw" : "participant"));
             }
         }
+
+        // determination of response is now complete
+
         if (response != 0) {
             _context.statManager().addRateData("tunnel.reject." + response, 1);
-            _context.messageHistory().tunnelRejected(state.fromHash, new TunnelId(ourId), nextPeer, 
+            _context.messageHistory().tunnelRejected(from, new TunnelId(ourId), nextPeer, 
                                                      "rejecting for " + response + ": " +
                                                      state.msg.getUniqueId() + "/" + ourId + "/" + req.readNextTunnelId() + " delay " +
                                                      recvDelay + " as " +
                                                      (isOutEnd ? "outbound endpoint" : isInGW ? "inbound gw" : "participant"));
-        }
-
             // Connection congestion control:
             // If we rejected the request, are near our conn limits, and aren't connected to the next hop,
             // just drop it.
             // 81% = between 75% control measures in Transports and 87% rejection above
-        if (response != 0 &&
+            if ((! _context.routerHash().equals(nextPeer)) &&
-            (! _context.routerHash().equals(nextPeer)) &&
                 (! _context.commSystem().haveOutboundCapacity(81)) &&
                 (! _context.commSystem().isEstablished(nextPeer))) {
                 _context.statManager().addRateData("tunnel.dropConnLimits", 1);
@@ -860,6 +856,7 @@ class BuildHandler implements Runnable {
                     _log.warn("Not sending rejection due to conn limits");
                 return;
             }
+        }
 
         EncryptedBuildRecord reply = BuildResponseRecord.create(_context, response, req.readReplyKey(), req.readReplyIV(), state.msg.getUniqueId());
         int records = state.msg.getRecordCount();
@@ -934,6 +931,11 @@ class BuildHandler implements Runnable {
      *  but could also be the reply where we are the IBEP.
      */
     private class TunnelBuildMessageHandlerJobBuilder implements HandlerJobBuilder {
+
+        /**
+         *  Either from or fromHash may be null, but both should be null only if
+         *  we're to be a IBGW and it came from us as a OBEP.
+         */
         public Job createJob(I2NPMessage receivedMessage, RouterIdentity from, Hash fromHash) {
             // need to figure out if this is a reply to an inbound tunnel request (where we are the
             // endpoint, receiving the request at the last hop)
@@ -1060,6 +1062,10 @@ class BuildHandler implements Runnable {
         final Hash fromHash;
         final long recvTime;
 
+        /**
+         *  Either f or h may be null, but both should be null only if
+         *  we're to be a IBGW and it came from us as a OBEP.
+         */
         public BuildMessageState(RouterContext ctx, I2NPMessage m, RouterIdentity f, Hash h) {
             _ctx = ctx;
             msg = (TunnelBuildMessage)m;