From b3055feff6248bb0f70eedfed7f505ef0e03a446 Mon Sep 17 00:00:00 2001
From: zzz <zzz@i2pmail.org>
Date: Sun, 20 Mar 2022 07:45:33 -0400
Subject: [PATCH] NetDB: Remove duplicate store in FloodOnlyLookupMatchJob
which bypassed all the checks in HandleFloodfillDatabaseStoreMessageJob
Don't store an entry which is older
---
.../kademlia/FloodOnlyLookupMatchJob.java | 59 ++++++++-----------
.../KademliaNetworkDatabaseFacade.java | 14 +++--
2 files changed, 34 insertions(+), 39 deletions(-)
diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/FloodOnlyLookupMatchJob.java b/router/java/src/net/i2p/router/networkdb/kademlia/FloodOnlyLookupMatchJob.java
index 085305cd01..5515bc3e36 100644
--- a/router/java/src/net/i2p/router/networkdb/kademlia/FloodOnlyLookupMatchJob.java
+++ b/router/java/src/net/i2p/router/networkdb/kademlia/FloodOnlyLookupMatchJob.java
@@ -5,7 +5,6 @@ import net.i2p.data.LeaseSet;
import net.i2p.data.i2np.DatabaseSearchReplyMessage;
import net.i2p.data.i2np.DatabaseStoreMessage;
import net.i2p.data.i2np.I2NPMessage;
-import net.i2p.data.router.RouterInfo;
import net.i2p.router.JobImpl;
import net.i2p.router.ReplyJob;
import net.i2p.router.RouterContext;
@@ -14,6 +13,7 @@ import net.i2p.util.Log;
class FloodOnlyLookupMatchJob extends JobImpl implements ReplyJob {
private final Log _log;
private final FloodSearchJob _search;
+ private volatile boolean _success;
public FloodOnlyLookupMatchJob(RouterContext ctx, FloodSearchJob job) {
super(ctx);
@@ -22,13 +22,15 @@ class FloodOnlyLookupMatchJob extends JobImpl implements ReplyJob {
}
public void runJob() {
- if (getContext().netDb().lookupLocally(_search.getKey()) != null) {
+ if (_success) {
if (_log.shouldLog(Log.INFO))
- _log.info(_search.getJobId() + ": search match and found locally");
+ _log.info(_search.getJobId() + ": search match for " + _search.getKey());
_search.success();
} else {
// In practice, we always have zero remaining when this is called,
// because the selector only returns true when there is zero remaining
+ if (_log.shouldLog(Log.INFO))
+ _log.info(_search.getJobId() + ": search failed for " + _search.getKey());
_search.failed();
}
}
@@ -36,44 +38,33 @@ class FloodOnlyLookupMatchJob extends JobImpl implements ReplyJob {
public String getName() { return "NetDb flood search match"; }
public void setMessage(I2NPMessage message) {
- if (message.getType() == DatabaseSearchReplyMessage.MESSAGE_TYPE) {
+ int mtype = message.getType();
+ if (mtype == DatabaseSearchReplyMessage.MESSAGE_TYPE) {
// DSRM processing now in FloodOnlyLookupSelector instead of here,
// a dsrm is only passed in when there are no more lookups remaining
// so that all DSRM's are processed, not just the last one.
_search.failed();
return;
}
- try {
- DatabaseStoreMessage dsm = (DatabaseStoreMessage)message;
- if (_log.shouldLog(Log.INFO))
- _log.info(_search.getJobId() + ": got a DSM for "
- + dsm.getKey().toBase64());
- // This store will be duplicated by HFDSMJ
- // We do it here first to make sure it is in the DB before
- // runJob() and search.success() is called???
- // Should we just pass the DataStructure directly back to somebody?
- DatabaseEntry entry = dsm.getEntry();
- int type = entry.getType();
- if (DatabaseEntry.isLeaseSet(type)) {
- // Since HFDSMJ wants to setReceivedAsPublished(), we have to
- // set a flag saying this was really the result of a query,
- // so don't do that.
- LeaseSet ls = (LeaseSet) dsm.getEntry();
- ls.setReceivedAsReply();
- getContext().netDb().store(dsm.getKey(), ls);
- } else if (type == DatabaseEntry.KEY_TYPE_ROUTERINFO) {
- getContext().netDb().store(dsm.getKey(), (RouterInfo) dsm.getEntry());
- } else {
- if (_log.shouldWarn())
- _log.warn(_search.getJobId() + ": got a DSM of unknown type " + type
- + " for " + dsm.getKey().toBase64());
- }
- } catch (UnsupportedCryptoException uce) {
- _search.failed();
+ if (mtype != DatabaseStoreMessage.MESSAGE_TYPE)
return;
- } catch (IllegalArgumentException iae) {
- if (_log.shouldLog(Log.WARN))
- _log.warn(_search.getJobId() + ": Received an invalid store reply", iae);
+
+ DatabaseStoreMessage dsm = (DatabaseStoreMessage)message;
+ if (_log.shouldLog(Log.INFO))
+ _log.info(_search.getJobId() + ": got a DSM for "
+ + dsm.getKey().toBase64());
+ // This store will handled by HFDSMJ.
+ // Just note success here.
+ if (dsm.getKey().equals(_search.getKey()))
+ _success = true;
+ DatabaseEntry entry = dsm.getEntry();
+ int type = entry.getType();
+ if (DatabaseEntry.isLeaseSet(type)) {
+ // Since HFDSMJ wants to setReceivedAsPublished(), we have to
+ // set a flag saying this was really the result of a query,
+ // so don't do that.
+ LeaseSet ls = (LeaseSet) dsm.getEntry();
+ ls.setReceivedAsReply();
}
}
}
diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java b/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java
index b04b3ddded..357761bd6e 100644
--- a/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java
+++ b/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java
@@ -973,10 +973,12 @@ public abstract class KademliaNetworkDatabaseFacade extends NetworkDatabaseFacad
public LeaseSet store(Hash key, LeaseSet leaseSet) throws IllegalArgumentException {
if (!_initialized) return null;
- LeaseSet rv = null;
+ LeaseSet rv;
try {
rv = (LeaseSet)_ds.get(key);
- if ( (rv != null) && (rv.equals(leaseSet)) ) {
+ if (rv != null && rv.getEarliestLeaseDate() >= leaseSet.getEarliestLeaseDate()) {
+ if (_log.shouldDebug())
+ _log.debug("Not storing older " + key);
// if it hasn't changed, no need to do anything
// except copy over the flags
Hash to = leaseSet.getReceivedBy();
@@ -1239,11 +1241,13 @@ public abstract class KademliaNetworkDatabaseFacade extends NetworkDatabaseFacad
RouterInfo store(Hash key, RouterInfo routerInfo, boolean persist) throws IllegalArgumentException {
if (!_initialized) return null;
- RouterInfo rv = null;
+ RouterInfo rv;
try {
rv = (RouterInfo)_ds.get(key, persist);
- if ( (rv != null) && (rv.equals(routerInfo)) ) {
- // no need to validate
+ if (rv != null && rv.getPublished() >= routerInfo.getPublished()) {
+ if (_log.shouldDebug())
+ _log.debug("Not storing older " + key);
+ // quick check without calling validate()
return rv;
}
} catch (ClassCastException cce) {
--
GitLab