From 36a3edf61267305412d9c2cc4c863825917e52ca Mon Sep 17 00:00:00 2001
From: zzz <zzz@mail.i2p>
Date: Thu, 27 Feb 2014 13:37:11 +0000
Subject: [PATCH]  - DatabaseStoreMessage: Don't instantiate an ArrayList
 unless we need it  - Overrides in GarlicConfig/PGC for efficiency and clarity
  - Check for valid ID in DeliveryStatusMessage  - Misc. log tweaks, javadocs,
 cleanups

---
 .../i2p/data/i2np/DatabaseLookupMessage.java  |  4 +-
 .../i2p/data/i2np/DeliveryStatusMessage.java  | 10 ++++-
 router/java/src/net/i2p/router/Blocklist.java | 14 +++----
 .../src/net/i2p/router/InNetMessagePool.java  | 35 ++++++++--------
 router/java/src/net/i2p/router/ReplyJob.java  |  6 +++
 .../net/i2p/router/message/GarlicConfig.java  | 19 +++++++--
 .../router/message/PayloadGarlicConfig.java   | 40 +++++++++++++++++--
 .../kademlia/FloodOnlyLookupSelector.java     |  5 +++
 .../kademlia/IterativeLookupSelector.java     |  5 +++
 .../kademlia/PersistentDataStore.java         |  3 +-
 .../tunnel/InboundMessageDistributor.java     |  4 +-
 11 files changed, 109 insertions(+), 36 deletions(-)

diff --git a/router/java/src/net/i2p/data/i2np/DatabaseLookupMessage.java b/router/java/src/net/i2p/data/i2np/DatabaseLookupMessage.java
index 06d9968574..1df3ae647f 100644
--- a/router/java/src/net/i2p/data/i2np/DatabaseLookupMessage.java
+++ b/router/java/src/net/i2p/data/i2np/DatabaseLookupMessage.java
@@ -297,7 +297,7 @@ public class DatabaseLookupMessage extends FastI2NPMessageImpl {
         
         if ( (numPeers < 0) || (numPeers > MAX_NUM_PEERS) )
             throw new I2NPMessageException("Invalid number of peers - " + numPeers);
-        List<Hash> peers = new ArrayList<Hash>(numPeers);
+        List<Hash> peers = numPeers > 0 ? new ArrayList<Hash>(numPeers) : null;
         for (int i = 0; i < numPeers; i++) {
             //byte peer[] = new byte[Hash.HASH_LENGTH];
             //System.arraycopy(data, curIndex, peer, 0, Hash.HASH_LENGTH);
@@ -417,7 +417,7 @@ public class DatabaseLookupMessage extends FastI2NPMessageImpl {
             buf.append("\n\tReply Key: ").append(_replyKey);
         if (_replyTag != null)
             buf.append("\n\tReply Tag: ").append(_replyTag);
-        buf.append("\n\tDont Include Peers: ");
+        buf.append("\n\tDon't Include Peers: ");
         if (_dontIncludePeers != null)
             buf.append(_dontIncludePeers.size());
         buf.append("]");
diff --git a/router/java/src/net/i2p/data/i2np/DeliveryStatusMessage.java b/router/java/src/net/i2p/data/i2np/DeliveryStatusMessage.java
index 6831d8fcde..28c07b51c0 100644
--- a/router/java/src/net/i2p/data/i2np/DeliveryStatusMessage.java
+++ b/router/java/src/net/i2p/data/i2np/DeliveryStatusMessage.java
@@ -22,6 +22,9 @@ public class DeliveryStatusMessage extends FastI2NPMessageImpl {
     private long _id;
     private long _arrival;
     
+    /** 4 bytes unsigned */
+    private static final long MAX_MSG_ID = (1L << 32) - 1;
+
     public DeliveryStatusMessage(I2PAppContext context) {
         super(context);
         _id = -1;
@@ -31,11 +34,15 @@ public class DeliveryStatusMessage extends FastI2NPMessageImpl {
     public long getMessageId() { return _id; }
 
     /**
+     *  @param id 0 to (2**32) - 1
      *  @throws IllegalStateException if id previously set, to protect saved checksum
+     *  @throws IllegalArgumentException
      */
     public void setMessageId(long id) {
         if (_id >= 0)
             throw new IllegalStateException();
+        if (id < 0 || id > MAX_MSG_ID)
+            throw new IllegalArgumentException();
         _id = id;
     }
     
@@ -70,6 +77,7 @@ public class DeliveryStatusMessage extends FastI2NPMessageImpl {
     protected int calculateWrittenLength() { 
         return 4 + DataHelper.DATE_LENGTH; // id + arrival
     }
+
     /** write the message body to the output array, starting at the given index */
     protected int writeMessageBody(byte out[], int curIndex) throws I2NPMessageException {
         if ( (_id < 0) || (_arrival <= 0) ) throw new I2NPMessageException("Not enough data to write out");
@@ -85,7 +93,7 @@ public class DeliveryStatusMessage extends FastI2NPMessageImpl {
     
     @Override
     public int hashCode() {
-        return (int)getMessageId() + (int)getArrival();
+        return (int)getMessageId() ^ (int)getArrival();
     }
     
     @Override
diff --git a/router/java/src/net/i2p/router/Blocklist.java b/router/java/src/net/i2p/router/Blocklist.java
index 9e711016da..093a6fdc5a 100644
--- a/router/java/src/net/i2p/router/Blocklist.java
+++ b/router/java/src/net/i2p/router/Blocklist.java
@@ -272,13 +272,13 @@ public class Blocklist {
             return;
         }
         _blocklistSize = count - removed;
-        if (_log.shouldLog(Log.WARN)) {
-            _log.warn("Removed " + badcount + " bad entries and comment lines");
-            _log.warn("Read " + count + " valid entries from the blocklist " + BLFile);
-            _log.warn("Merged " + removed + " overlapping entries");
-            _log.warn("Result is " + _blocklistSize + " entries");
-            _log.warn("Blocking " + ipcount + " IPs and " + peercount + " hashes");
-            _log.warn("Blocklist processing finished, time: " + (_context.clock().now() - start));
+        if (_log.shouldLog(Log.INFO)) {
+            _log.info("Removed " + badcount + " bad entries and comment lines");
+            _log.info("Read " + count + " valid entries from the blocklist " + BLFile);
+            _log.info("Merged " + removed + " overlapping entries");
+            _log.info("Result is " + _blocklistSize + " entries");
+            _log.info("Blocking " + ipcount + " IPs and " + peercount + " hashes");
+            _log.info("Blocklist processing finished, time: " + (_context.clock().now() - start));
         }
     }
 
diff --git a/router/java/src/net/i2p/router/InNetMessagePool.java b/router/java/src/net/i2p/router/InNetMessagePool.java
index 1c3f34b0bd..9e7ad2e9ce 100644
--- a/router/java/src/net/i2p/router/InNetMessagePool.java
+++ b/router/java/src/net/i2p/router/InNetMessagePool.java
@@ -10,6 +10,7 @@ package net.i2p.router;
 
 import java.io.Writer;
 import java.util.ArrayList;
+import java.util.Date;
 import java.util.List;
 
 import net.i2p.data.Hash;
@@ -127,18 +128,19 @@ public class InNetMessagePool implements Service {
         long exp = messageBody.getMessageExpiration();
         
         if (_log.shouldLog(Log.INFO))
-                _log.info("Received inbound " 
-                          + " with id " + messageBody.getUniqueId()
-                          + " expiring on " + exp
-                          + " of type " + messageBody.getClass().getSimpleName());
+                _log.info("Rcvd" 
+                          + " ID " + messageBody.getUniqueId()
+                          + " exp. " + new Date(exp)
+                          + " type " + messageBody.getClass().getSimpleName());
         
         //if (messageBody instanceof DataMessage) {
         //    _context.statManager().getStatLog().addData(fromRouterHash.toBase64().substring(0,6), "udp.floodDataReceived", 1, 0);
         //    return 0;
         //}
         
+        int type = messageBody.getType();
         String invalidReason = null;
-        if (messageBody instanceof TunnelDataMessage) {
+        if (type == TunnelDataMessage.MESSAGE_TYPE) {
             // the IV validator is sufficient for dup detection on tunnel messages, so
             // just validate the expiration
             invalidReason = _context.messageValidator().validateMessage(exp);
@@ -169,7 +171,6 @@ public class InNetMessagePool implements Service {
         }
 
         boolean jobFound = false;
-        int type = messageBody.getType();
         boolean allowMatches = true;
         
         if (type == TunnelGatewayMessage.MESSAGE_TYPE) {
@@ -184,8 +185,8 @@ public class InNetMessagePool implements Service {
                 HandlerJobBuilder builder = _handlerJobBuilders[type];
 
                 if (_log.shouldLog(Log.DEBUG))
-                    _log.debug("Add message to the inNetMessage pool - builder: " + builder 
-                               + " message class: " + messageBody.getClass().getSimpleName());
+                    _log.debug("Add msg to the pool - builder: " + builder 
+                               + " type: " + messageBody.getClass().getSimpleName());
 
                 if (builder != null) {
                     Job job = builder.createJob(messageBody, fromRouter, 
@@ -258,15 +259,16 @@ public class InNetMessagePool implements Service {
     
     public int handleReplies(I2NPMessage messageBody) {
         List<OutNetMessage> origMessages = _context.messageRegistry().getOriginalMessages(messageBody);
-        if (_log.shouldLog(Log.DEBUG))
-            _log.debug("Original messages for inbound message: " + origMessages.size());
-        if (origMessages.size() > 1) {
-            if (_log.shouldLog(Log.DEBUG))
-                _log.debug("Orig: " + origMessages + " \nthe above are replies for: " + messageBody, 
-                           new Exception("Multiple matches"));
+        int sz = origMessages.size();
+        if (sz <= 0)
+            return 0;
+        if (_log.shouldLog(Log.DEBUG)) {
+            _log.debug("Original messages for inbound message: " + sz);
+            if (sz > 1)
+                _log.debug("Orig: " + origMessages + " \nthe above are replies for: " + messageBody);
         }
 
-        for (int i = 0; i < origMessages.size(); i++) {
+        for (int i = 0; i < sz; i++) {
             OutNetMessage omsg = origMessages.get(i);
             ReplyJob job = omsg.getOnReplyJob();
             if (_log.shouldLog(Log.DEBUG))
@@ -278,7 +280,7 @@ public class InNetMessagePool implements Service {
                 _context.jobQueue().addJob(job);
             }
         }
-        return origMessages.size();
+        return sz;
     }
     
     // the following short circuits the tunnel dispatching - i'm not sure whether
@@ -297,6 +299,7 @@ public class InNetMessagePool implements Service {
                 _context.jobQueue().addJob(_shortCircuitGatewayJob);
         }
     }
+
     private void doShortCircuitTunnelGateway(I2NPMessage messageBody) {
         if (_log.shouldLog(Log.DEBUG))
             _log.debug("Shortcut dispatch tunnelGateway message " + messageBody);
diff --git a/router/java/src/net/i2p/router/ReplyJob.java b/router/java/src/net/i2p/router/ReplyJob.java
index 4b33eb198b..5b2dcff4c3 100644
--- a/router/java/src/net/i2p/router/ReplyJob.java
+++ b/router/java/src/net/i2p/router/ReplyJob.java
@@ -15,5 +15,11 @@ import net.i2p.data.i2np.I2NPMessage;
  *
  */
 public interface ReplyJob extends Job {
+
+    /**
+     *  Called by InNetMessagePool when an I2NPMessage
+     *  matching a MessageSelector registered with the OutboundMessageRegistry
+     *  is received
+     */
     public void setMessage(I2NPMessage message);
 }
diff --git a/router/java/src/net/i2p/router/message/GarlicConfig.java b/router/java/src/net/i2p/router/message/GarlicConfig.java
index 101c8c3289..9fe299e0c3 100644
--- a/router/java/src/net/i2p/router/message/GarlicConfig.java
+++ b/router/java/src/net/i2p/router/message/GarlicConfig.java
@@ -18,8 +18,11 @@ import net.i2p.data.RouterInfo;
 import net.i2p.data.i2np.DeliveryInstructions;
 
 /**
- * Define the contents of a garlic chunk that contains 1 or more sub garlics
+ * Define the contents of a garlic chunk that contains 1 or more sub garlics.
  *
+ * This is the top-level config for a Garlic Message that contains cloves.
+ * For cloves themselves, see PayloadGarlicConfig.
+ * Note that this is somewhat misnamed as it contains the actual cloves, not just the config.
  */
 class GarlicConfig {
     private RouterInfo _recipient;
@@ -39,9 +42,13 @@ class GarlicConfig {
     //private long _replyBlockExpiration;
     
     public GarlicConfig() {
+	this(new ArrayList<GarlicConfig>(4));
+    }
+
+    protected GarlicConfig(List<GarlicConfig> cloveConfigs) {
 	_id = -1;
 	_expiration = -1;
-	_cloveConfigs = new ArrayList<GarlicConfig>(4);
+        _cloveConfigs = cloveConfigs;
 	//_replyBlockMessageId = -1;
 	//_replyBlockExpiration = -1;
     }
@@ -52,6 +59,8 @@ class GarlicConfig {
      * forward it as an I2NPMessage to a router, forward it as an I2NPMessage to a Destination,
      * or forward it as an I2NPMessage to a tunnel.
      *
+     * Used only if recipient public key is not set.
+     *
      */
     public void setRecipient(RouterInfo info) { _recipient = info; }
     public RouterInfo getRecipient() { return _recipient; }
@@ -151,13 +160,17 @@ class GarlicConfig {
 	    _cloveConfigs.add(config);
 	}
     }
+
     public int getCloveCount() { return _cloveConfigs.size(); }
+
     public GarlicConfig getClove(int index) { return _cloveConfigs.get(index); }
+
     public void clearCloves() { _cloveConfigs.clear(); }
     
-    
     protected String getSubData() { return ""; }
+
     private final static String NL = System.getProperty("line.separator");
+
     @Override
     public String toString() {
 	StringBuilder buf = new StringBuilder();
diff --git a/router/java/src/net/i2p/router/message/PayloadGarlicConfig.java b/router/java/src/net/i2p/router/message/PayloadGarlicConfig.java
index fb92898c81..7d697c6f7d 100644
--- a/router/java/src/net/i2p/router/message/PayloadGarlicConfig.java
+++ b/router/java/src/net/i2p/router/message/PayloadGarlicConfig.java
@@ -11,14 +11,16 @@ package net.i2p.router.message;
 import net.i2p.data.i2np.I2NPMessage;
 
 /**
- * Garlic config containing an I2NP message
+ *  Garlic config for a single clove, containing an I2NP message and no sub-cloves.
  *
+ *  It is used for individual cloves in a Garlic Message, and as the configuration
+ *  for a single garlic-wrapped message by netdb MessageWrapper and tunnel TestJob.
  */
 public class PayloadGarlicConfig extends GarlicConfig {
     private I2NPMessage _payload;
 
     public PayloadGarlicConfig() {
-	super();
+	super(null);
     }
     
     /**
@@ -27,9 +29,8 @@ public class PayloadGarlicConfig extends GarlicConfig {
      */
     public void setPayload(I2NPMessage message) { 
 	_payload = message; 
-	if (message != null)
-	    clearCloves();
     }
+
     public I2NPMessage getPayload() { return _payload; }
  
     @Override
@@ -38,4 +39,35 @@ public class PayloadGarlicConfig extends GarlicConfig {
 	buf.append("<payloadMessage>").append(_payload).append("</payloadMessage>");
 	return buf.toString(); 
     }
+
+    /**
+     *  @since 0.9.12
+     *  @throws UnsupportedOperationException always
+     */
+    @Override
+    public void addClove(GarlicConfig config) {
+        throw new UnsupportedOperationException();
+    }
+
+    /**
+     *  @return zero
+     *  @since 0.9.12
+     */
+    @Override
+    public int getCloveCount() { return 0; }
+
+    /**
+     *  @since 0.9.12
+     *  @throws UnsupportedOperationException always
+     */
+    @Override
+    public GarlicConfig getClove(int index) {
+        throw new UnsupportedOperationException();
+    }
+
+    /**
+     *  @since 0.9.12
+     */
+    @Override
+    public void clearCloves() { }
 }
diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/FloodOnlyLookupSelector.java b/router/java/src/net/i2p/router/networkdb/kademlia/FloodOnlyLookupSelector.java
index cf1f58695d..f428bbafba 100644
--- a/router/java/src/net/i2p/router/networkdb/kademlia/FloodOnlyLookupSelector.java
+++ b/router/java/src/net/i2p/router/networkdb/kademlia/FloodOnlyLookupSelector.java
@@ -71,4 +71,9 @@ class FloodOnlyLookupSelector implements MessageSelector {
         }
         return false;
     }   
+
+    /** @since 0.9.12 */
+    public String toString() {
+        return "FOL Selector for " + _search.getKey();
+    }
 }
diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/IterativeLookupSelector.java b/router/java/src/net/i2p/router/networkdb/kademlia/IterativeLookupSelector.java
index c066b36feb..fc216f859a 100644
--- a/router/java/src/net/i2p/router/networkdb/kademlia/IterativeLookupSelector.java
+++ b/router/java/src/net/i2p/router/networkdb/kademlia/IterativeLookupSelector.java
@@ -62,4 +62,9 @@ class IterativeLookupSelector implements MessageSelector {
         }
         return false;
     }   
+
+    /** @since 0.9.12 */
+    public String toString() {
+        return "IL Selector for " + _search.getKey();
+    }
 }
diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/PersistentDataStore.java b/router/java/src/net/i2p/router/networkdb/kademlia/PersistentDataStore.java
index e187d274f0..27435d5b56 100644
--- a/router/java/src/net/i2p/router/networkdb/kademlia/PersistentDataStore.java
+++ b/router/java/src/net/i2p/router/networkdb/kademlia/PersistentDataStore.java
@@ -496,7 +496,8 @@ class PersistentDataStore extends TransientDataStore {
                         // prevent injection from reseeding
                         // this is checked in KNDF.validate() but catch it sooner and log as error.
                         corrupt = true;
-                        _log.error(ri.getIdentity().calculateHash() + " does not match " + _key + " from " + _routerFile);
+                        if (_log.shouldLog(Log.WARN))
+                            _log.warn(ri.getIdentity().calculateHash() + " does not match " + _key + " from " + _routerFile);
                     } else if (ri.getPublished() <= _knownDate) {
                         // Don't store but don't delete
                         if (_log.shouldLog(Log.WARN))
diff --git a/router/java/src/net/i2p/router/tunnel/InboundMessageDistributor.java b/router/java/src/net/i2p/router/tunnel/InboundMessageDistributor.java
index 062d5f1dee..3d35493a83 100644
--- a/router/java/src/net/i2p/router/tunnel/InboundMessageDistributor.java
+++ b/router/java/src/net/i2p/router/tunnel/InboundMessageDistributor.java
@@ -170,7 +170,7 @@ class InboundMessageDistributor implements GarlicMessageReceiver.CloveReceiver {
         switch (instructions.getDeliveryMode()) {
             case DeliveryInstructions.DELIVERY_MODE_LOCAL:
                 if (_log.shouldLog(Log.DEBUG))
-                    _log.debug("local delivery instructions for clove: " + data.getClass().getName());
+                    _log.debug("local delivery instructions for clove: " + data.getClass().getSimpleName());
                 int type = data.getType();
                 if (type == GarlicMessage.MESSAGE_TYPE) {
                     _receiver.receive((GarlicMessage)data);
@@ -261,7 +261,7 @@ class InboundMessageDistributor implements GarlicMessageReceiver.CloveReceiver {
                 // Can we route UnknownI2NPMessages to a destination too?
                 if (!(data instanceof DataMessage)) {
                     if (_log.shouldLog(Log.ERROR))
-                        _log.error("cant send a " + data.getClass().getName() + " to a destination");
+                        _log.error("cant send a " + data.getClass().getSimpleName() + " to a destination");
                 } else if ( (_client != null) && (_client.equals(instructions.getDestination())) ) {
                     if (_log.shouldLog(Log.DEBUG))
                         _log.debug("data message came down a tunnel for " 
-- 
GitLab