Addressbook:

- Enable parsing and handling of 'remove' actions
- Logging improvements
BFNS: Limit max dests per host
HostTxtEntry: Test improvements

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();
+        for (AddressBook book : subscriptions) {
-        while (iter.hasNext()) {
             // 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) {
-                            if (isKnown)
+                            log.append("Bad signature of action " + action + " for key " +
-                                log.append("Bad signature for old key " + key);
+                                       hprops.getProperty(HostTxtEntry.PROP_NAME) +
-                            else
+                                       ". From: " + addressbook.getLocation());
-                                log.append("Bad signature for new key " + key);
+                        }
+                        invalid++;
+                    } else if (key != null && mustValidate && !he.hasValidSig()) {
+                        if (log != null) {
+                            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)) {
+                                } else if (action.equals(HostTxtEntry.ACTION_REMOVE) ||
-                                    // FIXME can't get here, no key or dest
+                                           action.equals(HostTxtEntry.ACTION_REMOVEALL)) {
-                                    // delete this entry
+                                    // w/o name=dest handled below
-                                    if (!isKnown) {
+                                    if (log != null)
-                                        old++;
+                                        log.append("Action: " + action + " with name=dest invalid" +
-                                        continue;
+                                                   ". 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;
                                 }
+                                continue;
+                            } else {
+                                if (log != null)
+                                    log.append("No action in command line" +
+                                               ". 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);
-                                }
-                            }
-                            if (isTextFile)
-                                // keep track for later dup check
-                                knownNames.add(key);
-                            nnew++;
                         } else if (log != null) {
                             log.append("Bad hostname " + key + ". From: "
                                    + addressbook.getLocation());

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);
+        if (name != null && dest != null) {
-        out.write(KV_SEPARATOR);
+            out.write(name);
-        out.write(dest);
+            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);
+        if (name != null && dest != null) {
-        props.setProperty(PROP_DEST, dest);
+            props.setProperty(PROP_NAME, name);
+            props.setProperty(PROP_DEST, dest);
+        }
         writeProps(out, false, false);
         out.newLine();
-        props.remove(PROP_NAME);
+        if (name != null && dest != null) {
-        props.remove(PROP_DEST);
+            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);
+        if (!remove) {
-        buf.flush();
+            if (inner) {
-        String line = sw.toString();
+                SigningPrivateKey priv2 = pkf2.getSigningPrivKey();
-        line = line.substring(line.indexOf(PROPS_SEPARATOR) + 2);
+                he.signInner(priv2);
-        HostTxtEntry he2 = new HostTxtEntry(host, pkf.getDestination().toBase64(), line);
+                //out.write("After signing inner:\n");
-        if (inner && !he2.hasValidInnerSig())
+                //he.write(out);
-            throw new IllegalStateException("Inner fail 2");
+            }
-        if (!he2.hasValidSig())
+            he.sign(priv);
-            throw new IllegalStateException("Outer fail 2");
+            //out.write("After signing:\n");
-
+            he.write(out);
-        // 'remove' tests (corrupts earlier sigs)
+            out.flush();
-        if (remove) {
+            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");

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();

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);
+        String name, dest;
-        if (splitLine.length < 2)
+        if (comment != 0) {
-            return null;
+            // we have a name=dest
-        String name = splitLine[0].trim().toLowerCase(Locale.US);
+            String[] splitLine = DataHelper.split(kv, "=", 2);
-        String dest = splitLine[1].trim();
+            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

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++) {