From 2fb8faa166fe39b47543a3610d0832d867946092 Mon Sep 17 00:00:00 2001
From: zzz <zzz@mail.i2p>
Date: Sat, 23 Apr 2016 15:53:02 +0000
Subject: [PATCH] Addressbook: - Enable parsing and handling of 'remove'
actions - Logging improvements BFNS: Limit max dests per host HostTxtEntry:
Test improvements
---
.../java/src/net/i2p/addressbook/Daemon.java | 133 +++++++++++-------
.../src/net/i2p/addressbook/HostTxtEntry.java | 81 ++++++-----
.../net/i2p/addressbook/HostTxtIterator.java | 7 +-
.../net/i2p/addressbook/HostTxtParser.java | 48 +++++--
.../client/naming/BlockfileNamingService.java | 8 +-
5 files changed, 173 insertions(+), 104 deletions(-)
diff --git a/apps/addressbook/java/src/net/i2p/addressbook/Daemon.java b/apps/addressbook/java/src/net/i2p/addressbook/Daemon.java
index d7df8fed33..099cdc6579 100644
--- a/apps/addressbook/java/src/net/i2p/addressbook/Daemon.java
+++ b/apps/addressbook/java/src/net/i2p/addressbook/Daemon.java
@@ -87,10 +87,9 @@ public class Daemon {
*/
public static void update(AddressBook master, AddressBook router,
File published, SubscriptionList subscriptions, Log log) {
- Iterator<AddressBook> iter = subscriptions.iterator();
- while (iter.hasNext()) {
+ for (AddressBook book : subscriptions) {
// yes, the EepGet fetch() is done in next()
- router.merge(iter.next(), false, log);
+ router.merge(book, false, log);
}
router.write();
if (published != null) {
@@ -147,6 +146,7 @@ public class Daemon {
int deleted = 0;
for (Map.Entry<String, HostTxtEntry> entry : addressbook) {
total++;
+ // may be null for 'remove' entries
String key = entry.getKey();
boolean isKnown;
// NOT set for text file NamingService
@@ -159,9 +159,9 @@ public class Daemon {
knownNames = router.getNames(opts);
}
oldDest = null;
- isKnown = knownNames.contains(key);
+ isKnown = key != null ? knownNames.contains(key) : null;
} else {
- oldDest = router.lookup(key);
+ oldDest = key != null ? router.lookup(key) : null;
isKnown = oldDest != null;
}
try {
@@ -169,19 +169,25 @@ public class Daemon {
Properties hprops = he.getProps();
boolean mustValidate = MUST_VALIDATE || hprops != null;
String action = hprops != null ? hprops.getProperty(HostTxtEntry.PROP_ACTION) : null;
- if (mustValidate && !he.hasValidSig()) {
+ if (key == null && !he.hasValidRemoveSig()) {
+ if (log != null) {
+ log.append("Bad signature of action " + action + " for key " +
+ hprops.getProperty(HostTxtEntry.PROP_NAME) +
+ ". From: " + addressbook.getLocation());
+ }
+ invalid++;
+ } else if (key != null && mustValidate && !he.hasValidSig()) {
if (log != null) {
- if (isKnown)
- log.append("Bad signature for old key " + key);
- else
- log.append("Bad signature for new key " + key);
+ log.append("Bad signature of action " + action + " for key " + key +
+ ". From: " + addressbook.getLocation());
}
invalid++;
} else if (action != null || !isKnown) {
- if (AddressBook.isValidKey(key)) {
+ if (key != null && AddressBook.isValidKey(key)) {
Destination dest = new Destination(he.getDest());
Properties props = new OrderedProperties();
props.setProperty("s", addressbook.getLocation());
+ boolean allowExistingKeyInPublished = false;
if (mustValidate) {
// sig checked above
props.setProperty("v", "true");
@@ -237,6 +243,7 @@ public class Daemon {
if (published != null) {
if (publishedNS == null)
publishedNS = new SingleFileNamingService(I2PAppContext.getGlobalContext(), published.getAbsolutePath());
+ // FIXME this fails, no support in SFNS
success = publishedNS.addDestination(key, dest, props);
if (log != null && !success)
log.append("Add to published address book " + published.getAbsolutePath() + " failed for " + key);
@@ -365,7 +372,7 @@ public class Daemon {
log.append("Replacing " + pod2.size() + " destinations for " + key +
". From: " + addressbook.getLocation());
}
- // TODO set flag to do non-putifabsent for published below
+ allowExistingKeyInPublished = true;
} else {
// mismatch, disallow
logMismatch(log, action, key, pod2, polddest, addressbook);
@@ -429,13 +436,57 @@ public class Daemon {
invalid++;
continue;
}
- } else if (action.equals(HostTxtEntry.ACTION_REMOVE)) {
- // FIXME can't get here, no key or dest
- // delete this entry
- if (!isKnown) {
- old++;
- continue;
+ } else if (action.equals(HostTxtEntry.ACTION_REMOVE) ||
+ action.equals(HostTxtEntry.ACTION_REMOVEALL)) {
+ // w/o name=dest handled below
+ if (log != null)
+ log.append("Action: " + action + " with name=dest invalid" +
+ ". From: " + addressbook.getLocation());
+ invalid++;
+ continue;
+ } else if (action.equals(HostTxtEntry.ACTION_UPDATE)) {
+ if (isKnown) {
+ allowExistingKeyInPublished = true;
}
+ } else {
+ if (log != null)
+ log.append("Action: " + action + " unrecognized" +
+ ". From: " + addressbook.getLocation());
+ invalid++;
+ continue;
+ }
+ } // action != null
+ boolean success = router.put(key, dest, props);
+ if (log != null) {
+ if (success)
+ log.append("New address " + key +
+ " added to address book. From: " + addressbook.getLocation());
+ else
+ log.append("Save to naming service " + router + " failed for new key " + key);
+ }
+ // now update the published addressbook
+ if (published != null) {
+ if (publishedNS == null)
+ publishedNS = new SingleFileNamingService(I2PAppContext.getGlobalContext(), published.getAbsolutePath());
+ if (allowExistingKeyInPublished)
+ success = publishedNS.put(key, dest, props);
+ else
+ success = publishedNS.putIfAbsent(key, dest, props);
+ if (log != null && !success) {
+ log.append("Save to published address book " + published.getAbsolutePath() + " failed for new key " + key);
+ }
+ }
+ if (isTextFile)
+ // keep track for later dup check
+ knownNames.add(key);
+ nnew++;
+ } else if (key == null) {
+ // 'remove' actions
+ // isKnown is false
+ if (action != null) {
+ // Process commands. hprops is non-null.
+ if (action.equals(HostTxtEntry.ACTION_REMOVE)) {
+ // delete this entry
String polddest = hprops.getProperty(HostTxtEntry.PROP_DEST);
String poldname = hprops.getProperty(HostTxtEntry.PROP_NAME);
if (polddest != null && poldname != null) {
@@ -469,6 +520,8 @@ public class Daemon {
// mismatch, disallow
logMismatch(log, action, key, pod2, polddest, addressbook);
invalid++;
+ } else {
+ old++;
}
} else {
if (log != null)
@@ -476,14 +529,8 @@ public class Daemon {
". From: " + addressbook.getLocation());
invalid++;
}
- continue;
} else if (action.equals(HostTxtEntry.ACTION_REMOVEALL)) {
- // FIXME can't get here, no key or dest
// delete all entries with this destination
- if (!isKnown) {
- old++;
- continue;
- }
String polddest = hprops.getProperty(HostTxtEntry.PROP_DEST);
// oldname is optional, but nice because not all books support reverse lookup
if (polddest != null) {
@@ -519,6 +566,8 @@ public class Daemon {
// mismatch, disallow
logMismatch(log, action, key, pod2, polddest, addressbook);
invalid++;
+ } else {
+ old++;
}
}
// reverse lookup, delete all
@@ -564,40 +613,20 @@ public class Daemon {
". From: " + addressbook.getLocation());
invalid++;
}
- continue;
- } else if (action.equals(HostTxtEntry.ACTION_UPDATE)) {
- if (isKnown) {
- // TODO set flag to do non-putifabsent for published below
- }
} else {
if (log != null)
- log.append("Action: " + action + " unrecognized" +
+ log.append("Action: " + action + " w/o name=dest unrecognized" +
". From: " + addressbook.getLocation());
invalid++;
- continue;
- }
- }
- boolean success = router.put(key, dest, props);
- if (log != null) {
- if (success)
- log.append("New address " + key +
- " added to address book. From: " + addressbook.getLocation());
- else
- log.append("Save to naming service " + router + " failed for new key " + key);
- }
- // now update the published addressbook
- if (published != null) {
- if (publishedNS == null)
- publishedNS = new SingleFileNamingService(I2PAppContext.getGlobalContext(), published.getAbsolutePath());
- success = publishedNS.putIfAbsent(key, dest, props);
- if (log != null && !success) {
- log.append("Save to published address book " + published.getAbsolutePath() + " failed for new key " + key);
}
+ continue;
+ } else {
+ if (log != null)
+ log.append("No action in command line" +
+ ". From: " + addressbook.getLocation());
+ invalid++;
+ continue;
}
- if (isTextFile)
- // keep track for later dup check
- knownNames.add(key);
- nnew++;
} else if (log != null) {
log.append("Bad hostname " + key + ". From: "
+ addressbook.getLocation());
diff --git a/apps/addressbook/java/src/net/i2p/addressbook/HostTxtEntry.java b/apps/addressbook/java/src/net/i2p/addressbook/HostTxtEntry.java
index d3706b6622..fe19277068 100644
--- a/apps/addressbook/java/src/net/i2p/addressbook/HostTxtEntry.java
+++ b/apps/addressbook/java/src/net/i2p/addressbook/HostTxtEntry.java
@@ -132,27 +132,34 @@ class HostTxtEntry {
* Includes newline.
*/
public void write(BufferedWriter out) throws IOException {
- out.write(name);
- out.write(KV_SEPARATOR);
- out.write(dest);
+ if (name != null && dest != null) {
+ out.write(name);
+ out.write(KV_SEPARATOR);
+ out.write(dest);
+ }
writeProps(out, false, false);
out.newLine();
}
/**
* Write as a "remove" line #!dest=dest#name=name#k1=v1#sig=sig...]
+ * This works whether constructed with name and dest, or just properties.
* Includes newline.
* Must have been constructed with non-null properties.
*/
public void writeRemove(BufferedWriter out) throws IOException {
if (props == null)
throw new IllegalStateException();
- props.setProperty(PROP_NAME, name);
- props.setProperty(PROP_DEST, dest);
+ if (name != null && dest != null) {
+ props.setProperty(PROP_NAME, name);
+ props.setProperty(PROP_DEST, dest);
+ }
writeProps(out, false, false);
out.newLine();
- props.remove(PROP_NAME);
- props.remove(PROP_DEST);
+ if (name != null && dest != null) {
+ props.remove(PROP_NAME);
+ props.remove(PROP_DEST);
+ }
}
/**
@@ -185,7 +192,7 @@ class HostTxtEntry {
* Verify with the dest public key using the "sig" property
*/
public boolean hasValidSig() {
- if (props == null)
+ if (props == null || name == null || dest == null)
return false;
if (!isValidated) {
isValidated = true;
@@ -230,7 +237,7 @@ class HostTxtEntry {
* Verify with the "olddest" property's public key using the "oldsig" property
*/
public boolean hasValidInnerSig() {
- if (props == null)
+ if (props == null || name == null || dest == null)
return false;
boolean rv = false;
// don't cache result
@@ -450,35 +457,35 @@ class HostTxtEntry {
//he.write(out);
//out.flush();
SigningPrivateKey priv = pkf.getSigningPrivKey();
- if (inner) {
- SigningPrivateKey priv2 = pkf2.getSigningPrivKey();
- he.signInner(priv2);
- //out.write("After signing inner:\n");
- //he.write(out);
- }
- he.sign(priv);
- //out.write("After signing:\n");
- he.write(out);
- out.flush();
- if (inner && !he.hasValidInnerSig())
- throw new IllegalStateException("Inner fail 1");
- if (!he.hasValidSig())
- throw new IllegalStateException("Outer fail 1");
- // now create 2nd, read in
StringWriter sw = new StringWriter(1024);
BufferedWriter buf = new BufferedWriter(sw);
- he.write(buf);
- buf.flush();
- String line = sw.toString();
- line = line.substring(line.indexOf(PROPS_SEPARATOR) + 2);
- HostTxtEntry he2 = new HostTxtEntry(host, pkf.getDestination().toBase64(), line);
- if (inner && !he2.hasValidInnerSig())
- throw new IllegalStateException("Inner fail 2");
- if (!he2.hasValidSig())
- throw new IllegalStateException("Outer fail 2");
-
- // 'remove' tests (corrupts earlier sigs)
- if (remove) {
+ if (!remove) {
+ if (inner) {
+ SigningPrivateKey priv2 = pkf2.getSigningPrivKey();
+ he.signInner(priv2);
+ //out.write("After signing inner:\n");
+ //he.write(out);
+ }
+ he.sign(priv);
+ //out.write("After signing:\n");
+ he.write(out);
+ out.flush();
+ if (inner && !he.hasValidInnerSig())
+ throw new IllegalStateException("Inner fail 1");
+ if (!he.hasValidSig())
+ throw new IllegalStateException("Outer fail 1");
+ // now create 2nd, read in
+ he.write(buf);
+ buf.flush();
+ String line = sw.toString();
+ line = line.substring(line.indexOf(PROPS_SEPARATOR) + 2);
+ HostTxtEntry he2 = new HostTxtEntry(host, pkf.getDestination().toBase64(), line);
+ if (inner && !he2.hasValidInnerSig())
+ throw new IllegalStateException("Inner fail 2");
+ if (!he2.hasValidSig())
+ throw new IllegalStateException("Outer fail 2");
+ } else {
+ // 'remove' tests (corrupts earlier sigs)
he.getProps().remove(PROP_SIG);
he.signRemove(priv);
//out.write("Remove entry:\n");
@@ -488,7 +495,7 @@ class HostTxtEntry {
buf.flush();
out.write(sw.toString());
out.flush();
- line = sw.toString().substring(2).trim();
+ String line = sw.toString().substring(2).trim();
HostTxtEntry he3 = new HostTxtEntry(line);
if (!he3.hasValidRemoveSig())
throw new IllegalStateException("Remove verify fail");
diff --git a/apps/addressbook/java/src/net/i2p/addressbook/HostTxtIterator.java b/apps/addressbook/java/src/net/i2p/addressbook/HostTxtIterator.java
index ad676a348a..ef1b99103c 100644
--- a/apps/addressbook/java/src/net/i2p/addressbook/HostTxtIterator.java
+++ b/apps/addressbook/java/src/net/i2p/addressbook/HostTxtIterator.java
@@ -73,7 +73,7 @@ class HostTxtIterator implements Iterator<Map.Entry<String, HostTxtEntry>>, Clos
try {
String inputLine;
while ((inputLine = input.readLine()) != null) {
- HostTxtEntry he = HostTxtParser.parse(inputLine);
+ HostTxtEntry he = HostTxtParser.parse(inputLine, true);
if (he == null)
continue;
next = new MapEntry(he.getName(), he);
@@ -86,6 +86,11 @@ class HostTxtIterator implements Iterator<Map.Entry<String, HostTxtEntry>>, Clos
return false;
}
+ /**
+ * 'remove' entries will be returned with a null key,
+ * and the value will contain a null name, null dest,
+ * and non-null props.
+ */
public Map.Entry<String, HostTxtEntry> next() {
if (!hasNext())
throw new NoSuchElementException();
diff --git a/apps/addressbook/java/src/net/i2p/addressbook/HostTxtParser.java b/apps/addressbook/java/src/net/i2p/addressbook/HostTxtParser.java
index 92bbce817a..cfcd7a6251 100644
--- a/apps/addressbook/java/src/net/i2p/addressbook/HostTxtParser.java
+++ b/apps/addressbook/java/src/net/i2p/addressbook/HostTxtParser.java
@@ -36,6 +36,8 @@ class HostTxtParser {
* are obviously not in the format key=value are also ignored.
* The key is converted to lower case.
*
+ * Returned map will not contain null ("remove") entries.
+ *
* @param input
* A BufferedReader with lines in key=value format to parse into
* a Map.
@@ -49,7 +51,7 @@ class HostTxtParser {
Map<String, HostTxtEntry> result = new HashMap<String, HostTxtEntry>();
String inputLine;
while ((inputLine = input.readLine()) != null) {
- HostTxtEntry he = parse(inputLine);
+ HostTxtEntry he = parse(inputLine, false);
if (he == null)
continue;
result.put(he.getName(), he);
@@ -64,32 +66,48 @@ class HostTxtParser {
* Return a HostTxtEntry from the contents of the inputLine.
*
* @param inputLine key=value[#!k1=v1#k2=v2...]
+ * @param allowCommandOnly if true, a line starting with #! will return
+ * a HostTxtEntry with a null name and dest and non-null props.
+ * If false, these lines will return null.
* @return null if no entry found or on error
*/
- public static HostTxtEntry parse(String inputLine) {
+ public static HostTxtEntry parse(String inputLine, boolean allowCommandOnly) {
if (inputLine.startsWith(";"))
return null;
int comment = inputLine.indexOf("#");
- if (comment == 0)
- return null;
String kv;
String sprops;
- if (comment > 0) {
+ if (comment >= 0) {
int shebang = inputLine.indexOf(HostTxtEntry.PROPS_SEPARATOR);
- if (shebang == comment && shebang + 2 < inputLine.length())
+ if (shebang == comment && shebang + 2 < inputLine.length()) {
+ if (comment == 0 && !allowCommandOnly)
+ return null;
sprops = inputLine.substring(shebang + 2);
- else
+ } else {
+ if (comment == 0)
+ return null;
sprops = null;
+ }
kv = inputLine.substring(0, comment);
} else {
sprops = null;
kv = inputLine;
}
- String[] splitLine = DataHelper.split(kv, "=", 2);
- if (splitLine.length < 2)
- return null;
- String name = splitLine[0].trim().toLowerCase(Locale.US);
- String dest = splitLine[1].trim();
+ String name, dest;
+ if (comment != 0) {
+ // we have a name=dest
+ String[] splitLine = DataHelper.split(kv, "=", 2);
+ if (splitLine.length < 2)
+ return null;
+ name = splitLine[0].trim().toLowerCase(Locale.US);
+ dest = splitLine[1].trim();
+ if (name.length() == 0 || dest.length() == 0)
+ return null;
+ } else {
+ // line starts with #!, rv will contain props only
+ name = null;
+ dest = null;
+ }
HostTxtEntry he;
if (sprops != null) {
try {
@@ -104,9 +122,11 @@ class HostTxtParser {
}
/**
- * Return a Map using the contents of the File file. See parseBufferedReader
+ * Return a Map using the contents of the File file. See parse(BufferedReader)
* for details of the input format.
*
+ * Returned map will not contain null ("remove") entries.
+ *
* @param file
* A File to parse.
* @return A Map containing the key, value pairs from file.
@@ -134,6 +154,8 @@ class HostTxtParser {
* Return a Map using the contents of the File file. If file cannot be read,
* use map instead, and write the result to where file should have been.
*
+ * Returned map will not contain null ("remove") entries.
+ *
* @param file
* A File to attempt to parse.
* @param map
diff --git a/core/java/src/net/i2p/client/naming/BlockfileNamingService.java b/core/java/src/net/i2p/client/naming/BlockfileNamingService.java
index 2ba0c7b233..3e7003d8db 100644
--- a/core/java/src/net/i2p/client/naming/BlockfileNamingService.java
+++ b/core/java/src/net/i2p/client/naming/BlockfileNamingService.java
@@ -137,6 +137,7 @@ public class BlockfileNamingService extends DummyNamingService {
private static final String DUMMY = "";
private static final int NEGATIVE_CACHE_SIZE = 32;
private static final int MAX_VALUE_LENGTH = 4096;
+ private static final int MAX_DESTS_PER_HOST = 8;
/**
* Opens the database at hostsdb.blockfile or creates a new
@@ -1486,6 +1487,8 @@ public class BlockfileNamingService extends DummyNamingService {
return put(hostname, d, options, false);
if (dests.contains(d))
return false;
+ if (dests.size() >= MAX_DESTS_PER_HOST)
+ return false;
List<Destination> newDests = new ArrayList<Destination>(dests.size() + 1);
newDests.addAll(dests);
// TODO better sort by sigtype preference.
@@ -1523,7 +1526,10 @@ public class BlockfileNamingService extends DummyNamingService {
}
List<Properties> storedOptions = new ArrayList<Properties>(4);
synchronized(_bf) {
- List<Destination> dests = lookupAll(hostname, options, storedOptions);
+ // We use lookupAll2(), not lookupAll(), because if hostname starts with www.,
+ // we do not want to read in from the
+ // non-www hostname and then copy it to a new www hostname.
+ List<Destination> dests = lookupAll2(hostname, options, storedOptions);
if (dests == null)
return false;
for (int i = 0; i < dests.size(); i++) {
--
GitLab