2008-01-08 zzz

* addressbook: Limit size of subscribed hosts.txt,
        don't save old etag or last-modified data
    * EepGet: Add some logging,
        enforce size limits even when size not in returned header,
        don't return old etag or last-modified data,
        don't call transferFailed listener more than once

apps/addressbook/java/src/addressbook/AddressBook.java

@@ -66,6 +66,7 @@ public class AddressBook {
      *            where key is a human readable name, and value is a base64 i2p
      *            destination.
      */
+/* unused
     public AddressBook(String url, String proxyHost, int proxyPort) {
         this.location = url;
         EepGet get = new EepGet(I2PAppContext.getGlobalContext(), true,
@@ -79,23 +80,26 @@ public class AddressBook {
         }
         new File("addressbook.tmp").delete();
     }
-
+*/
     /**
      * Construct an AddressBook from the Subscription subscription. If the
      * address book at subscription has not changed since the last time it was
      * read or cannot be read, return an empty AddressBook.
+     * Set a maximum size of the remote book to make it a little harder for a malicious book-sender.
      * 
      * @param subscription
      *            A Subscription instance pointing at a remote address book.
      */
+    static final long MAX_SUB_SIZE = 3 * 1024 * 1024l; //about 5,000 hosts
     public AddressBook(Subscription subscription, String proxyHost, int proxyPort) {
         this.location = subscription.getLocation();
         EepGet get = new EepGet(I2PAppContext.getGlobalContext(), true,
-                proxyHost, proxyPort, 0, "addressbook.tmp", 
+                proxyHost, proxyPort, 0, -1l, MAX_SUB_SIZE, "addressbook.tmp", null,
-                subscription.getLocation(), true, subscription.getEtag(), subscription.getLastModified());
+                subscription.getLocation(), true, subscription.getEtag(), subscription.getLastModified(), null);
-        get.fetch();
+        if (get.fetch()) {
-        subscription.setEtag(get.getETag());
+            subscription.setEtag(get.getETag());
-        subscription.setLastModified(get.getLastModified());
+            subscription.setLastModified(get.getLastModified());
+        }
         try {            
             this.addresses = ConfigParser.parse(new File("addressbook.tmp"));
         } catch (IOException exp) {

core/java/src/net/i2p/util/EepGet.java

@@ -392,6 +392,8 @@ public class EepGet {
                 timeout.cancel();
                 for (int i = 0; i < _listeners.size(); i++) 
                     ((StatusListener)_listeners.get(i)).attemptFailed(_url, _bytesTransferred, _bytesRemaining, _currentAttempt, _numRetries, ioe);
+                if (_log.shouldLog(Log.WARN))
+                    _log.warn("ERR: doFetch failed " +  ioe);
             } finally {
                 if (_out != null) {
                     try {
@@ -418,6 +420,8 @@ public class EepGet {
 
         for (int i = 0; i < _listeners.size(); i++) 
             ((StatusListener)_listeners.get(i)).transferFailed(_url, _bytesTransferred, _bytesRemaining, _currentAttempt);
+        if (_log.shouldLog(Log.WARN))
+            _log.warn("All attempts failed for " + _url);
         return false;
     }
 
@@ -478,7 +482,7 @@ public class EepGet {
         boolean strictSize = (_bytesRemaining >= 0);
 
         // If minimum or maximum size defined, ensure they aren't exceeded
-        if ((_minSize > -1) && (_bytesRemaining < _minSize))
+        if ((_minSize > 0) && (_bytesRemaining < _minSize))
             throw new IOException("HTTP response size " + _bytesRemaining + " violates minimum of " + _minSize + " bytes");
         if ((_maxSize > -1) && (_bytesRemaining > _maxSize))
             throw new IOException("HTTP response size " + _bytesRemaining + " violates maximum of " + _maxSize + " bytes");
@@ -500,6 +504,8 @@ public class EepGet {
             // Hopefully this won't break compatibility with existing status listeners
             // (cause them to behave weird, or show weird numbers).
             _alreadyTransferred += read;
+            if ((_maxSize > -1) && (_alreadyTransferred > _maxSize)) // could transfer a little over maxSize
+                throw new IOException("Bytes transferred " + _alreadyTransferred + " violates maximum of " + _maxSize + " bytes");
             remaining -= read;
             if (remaining==0 && _encodingChunked) {
                 int char1 = _proxyIn.read();
@@ -544,10 +550,13 @@ public class EepGet {
         if (_log.shouldLog(Log.DEBUG))
             _log.debug("Done transferring " + _bytesTransferred + " (ok? " + !_transferFailed + ")");
 
+
         if (_transferFailed) {
-            // 404, etc
+            // 404, etc - transferFailed is called after all attempts fail, by fetch() above
             for (int i = 0; i < _listeners.size(); i++) 
-                ((StatusListener)_listeners.get(i)).transferFailed(_url, _bytesTransferred, _bytesRemaining, _currentAttempt);
+                ((StatusListener)_listeners.get(i)).attemptFailed(_url, _bytesTransferred, _bytesRemaining, _currentAttempt, _numRetries, new Exception("Attempt failed"));
+        } else if ((_minSize > 0) && (_alreadyTransferred < _minSize)) {
+            throw new IOException("Bytes transferred " + _alreadyTransferred + " violates minimum of " + _minSize + " bytes");
         } else if ( (_bytesRemaining == -1) || (remaining == 0) ) {
             for (int i = 0; i < _listeners.size(); i++) 
                 ((StatusListener)_listeners.get(i)).transferComplete(
@@ -615,6 +624,11 @@ public class EepGet {
                 rcOk = false;
                 _transferFailed = true;
         }
+
+        // clear out the arguments, as we use the same variables for return values
+        _etag = null;
+        _lastModified = null;
+
         buf.setLength(0);
         byte lookahead[] = new byte[3];
         while (true) {

history.txt

@@ -1,4 +1,12 @@
-$Id: history.txt,v 1.611 2008-01-03 21:37:27 zzz Exp $
+$Id: history.txt,v 1.612 2008-01-07 03:09:44 zzz Exp $
+
+2008-01-08 zzz
+    * addressbook: Limit size of subscribed hosts.txt,
+        don't save old etag or last-modified data
+    * EepGet: Add some logging,
+        enforce size limits even when size not in returned header,
+        don't return old etag or last-modified data,
+        don't call transferFailed listener more than once
 
 2008-01-07 zzz
     * profiles.jsp formatting cleanup

router/java/src/net/i2p/router/RouterVersion.java

@@ -15,9 +15,9 @@ import net.i2p.CoreVersion;
  *
  */
 public class RouterVersion {
-    public final static String ID = "$Revision: 1.546 $ $Date: 2008-01-03 21:37:24 $";
+    public final static String ID = "$Revision: 1.547 $ $Date: 2008-01-07 03:09:43 $";
     public final static String VERSION = "0.6.1.30";
-    public final static long BUILD = 18;
+    public final static long BUILD = 19;
     public static void main(String args[]) {
         System.out.println("I2P Router version: " + VERSION + "-" + BUILD);
         System.out.println("Router ID: " + RouterVersion.ID);