- Better serializer error handling and logging

- Automatic corruption repair in blockfile
- Automatic removal of bad entries in BFNS
- Use unsigned shorts to extend max lengths to 65535
- Check max length
- Throw IOE on negative ints
- Tweak fromProperties() exceptions
- Fix DataHelper encoding issues (ticket #436)
- CSS tweaks

apps/susidns/src/css.css

@@ -66,11 +66,11 @@ li {
 }
 
 tr.list1 {
-	background-color:#E0E0E0;
+	background-color:#E8E8EC;
 }
 
 tr.list0 {
-	background-color:white;
+	background-color:#F0F0F4;
 }
 
 p.messages {

apps/susidns/src/java/src/i2p/susi/dns/NamingServiceBean.java

@@ -183,7 +183,12 @@ public class NamingServiceBean extends AddressbookBean
 					}
 				}
 				String destination = entry.getValue().toBase64();
+				if (destination != null) {
 					list.addLast( new AddressBean( name, destination ) );
+				} else {
+					// delete it too?
+					System.err.println("Bad entry " + name + " in database " + service.getName());
+				}
 			}
 			AddressBean array[] = list.toArray(new AddressBean[list.size()]);
 			Arrays.sort( array, sorter );

core/java/src/net/i2p/client/naming/BlockfileNamingService.java

@@ -75,6 +75,7 @@ public class BlockfileNamingService extends DummyNamingService {
     private final BlockFile _bf;
     private final RandomAccessFile _raf;
     private final List<String> _lists;
+    private final List<InvalidEntry> _invalid;
     private volatile boolean _isClosed;
 
     private static final Serializer _infoSerializer = new PropertiesSerializer();
@@ -101,6 +102,7 @@ public class BlockfileNamingService extends DummyNamingService {
     public BlockfileNamingService(I2PAppContext context) {
         super(context);
         _lists = new ArrayList();
+        _invalid = new ArrayList();
         BlockFile bf = null;
         RandomAccessFile raf = null;
         File f = new File(_context.getRouterDir(), HOSTS_DB);
@@ -375,8 +377,10 @@ public class BlockfileNamingService extends DummyNamingService {
                 try {
                     DestEntry de = getEntry(list, key);
                     if (de != null) {
+                        if (!validate(key, de, listname))
+                            continue;
                         d = de.dest;
-                        if (storedOptions != null)
+                        if (storedOptions != null && de.props != null)
                             storedOptions.putAll(de.props);
                         break;
                     }
@@ -384,6 +388,7 @@ public class BlockfileNamingService extends DummyNamingService {
                     break;
                 }
             }
+            deleteInvalid();
         }
         if (d != null)
             putCache(hostname, d);
@@ -553,6 +558,7 @@ public class BlockfileNamingService extends DummyNamingService {
                     iter = sl.iterator();
                 Map<String, Destination> rv = new HashMap();
                 for (int i = 0; i < skip && iter.hasNext(); i++) {
+                    // don't bother validating here
                     iter.next();
                 }
                 for (int i = 0; i < limit && iter.hasNext(); ) {
@@ -566,6 +572,8 @@ public class BlockfileNamingService extends DummyNamingService {
                         }
                     }
                     DestEntry de = (DestEntry) iter.next();
+                    if (!validate(key, de, listname))
+                        continue;
                     if (search != null && key.indexOf(search) < 0)
                         continue;
                     rv.put(key, de.dest);
@@ -578,6 +586,8 @@ public class BlockfileNamingService extends DummyNamingService {
             } catch (RuntimeException re) {
                 _log.error("DB lookup error", re);
                 return Collections.EMPTY_MAP;
+            } finally {
+                deleteInvalid();
             }
         }
     }
@@ -619,6 +629,60 @@ public class BlockfileNamingService extends DummyNamingService {
 
     ////////// End NamingService API
 
+    /**
+     *  Continuously validate anything we read in.
+     *  Queue anything invalid to be removed at the end of the operation.
+     *  Caller must sync!
+     *  @return valid
+     */
+    private boolean validate(String key, DestEntry de, String listname) {
+        if (key == null)
+            return false;
+        // de.props may be null
+        // publickey check is a quick proxy to detect dest deserialization failure
+        boolean rv = key.length() > 0 &&
+                     de != null &&
+                     de.dest != null &&
+                     de.dest.getPublicKey() != null;
+        if (!rv)
+            _invalid.add(new InvalidEntry(key, listname));
+        return rv;
+    }
+
+    /**
+     *  Remove and log all invalid entries queued by validate()
+     *  while scanning in lookup() or getEntries().
+     *  We delete in the order detected, as an error may be corrupting later entries in the skiplist.
+     *  Caller must sync!
+     */
+    private void deleteInvalid() {
+        if (_invalid.isEmpty())
+            return;
+        _log.error("Removing " + _invalid.size() + " corrupt entries from database");
+        for (InvalidEntry ie : _invalid) {
+            String key = ie.key;
+            String list = ie.list;
+            try {
+                SkipList sl = _bf.getIndex(list, _stringSerializer, _destSerializer);
+                if (sl == null) {
+                    _log.error("No list found to remove corrupt \"" + key + "\" from database " + list);
+                    continue;
+                }
+                // this will often return null since it was corrupt
+                boolean success = removeEntry(sl, key) != null;
+                if (success)
+                    _log.error("Removed corrupt \"" + key + "\" from database " + list);
+                else
+                    _log.error("May have Failed to remove corrupt \"" + key + "\" from database " + list);
+            } catch (RuntimeException re) {
+                _log.error("Error while removing corrupt \"" + key + "\" from database " + list, re);
+            } catch (IOException ioe) {
+                _log.error("Error while removing corrput \"" + key + "\" from database " + list, ioe);
+            }
+        }
+        _invalid.clear();
+    }
+
     private void dumpDB() {
         synchronized(_bf) {
             if (_isClosed)
@@ -634,6 +698,8 @@ public class BlockfileNamingService extends DummyNamingService {
                     for (SkipIterator iter = sl.iterator(); iter.hasNext(); ) {
                          String key = (String) iter.nextKey();
                          DestEntry de = (DestEntry) iter.next();
+                         if (!validate(key, de, list))
+                             continue;
                          _log.error("DB " + list + " key " + key + " val " + de);
                          i++;
                     }
@@ -643,6 +709,7 @@ public class BlockfileNamingService extends DummyNamingService {
                     break;
                 }
             }
+            deleteInvalid();
         }
     }
 
@@ -661,6 +728,11 @@ public class BlockfileNamingService extends DummyNamingService {
         }
     }
 
+    /** for logging errors in the static serializers below */
+    private static void logError(String msg, Throwable t) {
+        I2PAppContext.getGlobalContext().logManager().getLog(BlockfileNamingService.class).error(msg, t);
+    }
+
     private class Shutdown implements Runnable {
         public void run() {
             close();
@@ -691,24 +763,33 @@ public class BlockfileNamingService extends DummyNamingService {
 
     /**
      *  Used for the values in the header skiplist
+     *  Take care not to throw on any error.
+     *  This means that some things will fail with no indication other than the log,
+     *  but if we threw a RuntimeException we would prevent access to entries later in
+     *  the SkipSpan.
      */
     private static class PropertiesSerializer implements Serializer {
+        /**
+         *  A format error on the properties is non-fatal (returns an empty properties)
+         */
         public byte[] getBytes(Object o) {
             Properties p = (Properties) o;
             try {
                 return DataHelper.toProperties(p);
             } catch (DataFormatException dfe) {
-                return null;
+                logError("DB Write Fail - properties too big?", dfe);
+                // null properties is a two-byte length of 0.
+                return new byte[2];
             }
         }
 
+        /** returns null on error */
         public Object construct(byte[] b) {
             Properties rv = new Properties();
             try {
                 DataHelper.fromProperties(b, 0, rv);
-            } catch (IOException ioe) {
-                return null;
             } catch (DataFormatException dfe) {
+                logError("DB Read Fail", dfe);
                 return null;
             }
             return rv;
@@ -734,22 +815,38 @@ public class BlockfileNamingService extends DummyNamingService {
 
     /**
      *  Used for the values in the addressbook skiplists
+     *  Take care not to throw on any error.
+     *  This means that some things will fail with no indication other than the log,
+     *  but if we threw a RuntimeException we would prevent access to entries later in
+     *  the SkipSpan.
      */
     private static class DestEntrySerializer implements Serializer {
+
+        /**
+         *  A format error on the properties is non-fatal (only the properties are lost)
+         *  A format error on the destination is fatal
+         */
         public byte[] getBytes(Object o) {
             DestEntry de = (DestEntry) o;
             ByteArrayOutputStream baos = new ByteArrayOutputStream(1024);
             try {
-                DataHelper.writeProperties(baos, de.props);
+                try {
+                    DataHelper.writeProperties(baos, de.props, true, false);  // UTF-8, unsorted
+                } catch (DataFormatException dfe) {
+                    logError("DB Write Fail - properties too big?", dfe);
+                    // null properties is a two-byte length of 0.
+                    baos.write(new byte[2]);
+		}
                 de.dest.writeBytes(baos);
             } catch (IOException ioe) {
-                return null;
+                logError("DB Write Fail", ioe);
             } catch (DataFormatException dfe) {
-                return null;
+                logError("DB Write Fail", dfe);
             }
             return baos.toByteArray();
         }
 
+        /** returns null on error */
         public Object construct(byte[] b) {
             DestEntry rv = new DestEntry();
             Destination dest = new Destination();
@@ -759,14 +856,29 @@ public class BlockfileNamingService extends DummyNamingService {
                 rv.props = DataHelper.readProperties(bais);
                 dest.readBytes(bais);
             } catch (IOException ioe) {
+                logError("DB Read Fail", ioe);
                 return null;
             } catch (DataFormatException dfe) {
+                logError("DB Read Fail", dfe);
                 return null;
             }
             return rv;
         }
     }
 
+    /**
+     *  Used to store entries that need deleting
+     */
+    private static class InvalidEntry {
+        public final String key;
+        public final String list;
+
+        public InvalidEntry(String k, String l) {
+            key = k;
+            list = l;
+        }
+    }
+
     public static void main(String[] args) {
         BlockfileNamingService bns = new BlockfileNamingService(I2PAppContext.getGlobalContext());
         List<String> names = null;

core/java/src/net/i2p/data/DataHelper.java

@@ -54,8 +54,16 @@ import net.i2p.util.Translate;
  * @author jrandom
  */
 public class DataHelper {
-    private final static byte EQUAL_BYTES[] = "=".getBytes(); // in UTF-8
+    private static final byte EQUAL_BYTES[];
-    private final static byte SEMICOLON_BYTES[] = ";".getBytes(); // in UTF-8
+    private static final byte SEMICOLON_BYTES[];
+    static {
+        try {
+            EQUAL_BYTES = "=".getBytes("UTF-8");
+            SEMICOLON_BYTES = ";".getBytes("UTF-8");
+        } catch (UnsupportedEncodingException uee) {
+            throw new RuntimeException("no utf8!?");
+        }
+    }
 
     /** Read a mapping from the stream, as defined by the I2P data structure spec,
      * and store it into a Properties object.
@@ -80,7 +88,7 @@ public class DataHelper {
         long size = readLong(rawStream, 2);
         byte data[] = new byte[(int) size];
         int read = read(rawStream, data);
-        if (read != size) throw new DataFormatException("Not enough data to read the properties");
+        if (read != size) throw new DataFormatException("Not enough data to read the properties, expected " + size + " but got " + read);
         ByteArrayInputStream in = new ByteArrayInputStream(data);
         byte eqBuf[] = new byte[EQUAL_BYTES.length];
         byte semiBuf[] = new byte[SEMICOLON_BYTES.length];
@@ -251,23 +259,33 @@ public class DataHelper {
      * @param target returned Properties
      * @return new offset
      */
-    public static int fromProperties(byte source[], int offset, Properties target) throws DataFormatException, IOException {
+    public static int fromProperties(byte source[], int offset, Properties target) throws DataFormatException {
         int size = (int)fromLong(source, offset, 2);
         offset += 2;
         ByteArrayInputStream in = new ByteArrayInputStream(source, offset, size);
         byte eqBuf[] = new byte[EQUAL_BYTES.length];
         byte semiBuf[] = new byte[SEMICOLON_BYTES.length];
         while (in.available() > 0) {
-            String key = readString(in);
+            String key;
+            try {
+                key = readString(in);
                 int read = read(in, eqBuf);
                 if ((read != eqBuf.length) || (!eq(eqBuf, EQUAL_BYTES))) {
                     throw new DataFormatException("Bad key");
                 }
-            String val = readString(in);
+            } catch (IOException ioe) {
-            read = read(in, semiBuf);
+                throw new DataFormatException("Bad key", ioe);
+            }
+            String val;
+            try {
+                val = readString(in);
+                int read = read(in, semiBuf);
                 if ((read != semiBuf.length) || (!eq(semiBuf, SEMICOLON_BYTES))) {
                     throw new DataFormatException("Bad value");
                 }
+            } catch (IOException ioe) {
+                throw new DataFormatException("Bad value", ioe);
+            }
             target.put(key, val);
         }
         return offset + size;

core/java/src/net/metanotion/io/RAIFile.java

@@ -82,6 +82,17 @@ public class RAIFile implements RandomAccessInterface, DataInput, DataOutput {
 	public int readUnsignedByte()		throws IOException { return delegate.readUnsignedByte(); }
 	public int readUnsignedShort()		throws IOException { return delegate.readUnsignedShort(); }
 
+	/**
+	 *  I2P
+	 *  @throws IOException if the read value is negative
+	 */
+	public int readUnsignedInt()  throws IOException {
+		int rv = readInt();
+		if (rv < 0)
+			throw new IOException("Negative value for unsigned int: " + rv);
+		return rv;
+	}
+
 	/** Read a UTF encoded string
 	 	I would delegate here. But Java's read/writeUTF combo suck.
 	 	A signed 2 byte length is not enough.

core/java/src/net/metanotion/io/RandomAccessInterface.java

@@ -56,6 +56,8 @@ public interface RandomAccessInterface {
 	public short readShort() throws IOException;
 	public int readUnsignedByte() throws IOException;
 	public int readUnsignedShort() throws IOException;
+	// I2P
+	public int readUnsignedInt() throws IOException;
 	public String readUTF() throws IOException;
 	public int skipBytes(int n) throws IOException;
 

core/java/src/net/metanotion/io/block/BlockFile.java

@@ -42,10 +42,12 @@ import net.metanotion.io.data.IntBytes;
 import net.metanotion.io.data.LongBytes;
 import net.metanotion.io.data.NullBytes;
 import net.metanotion.io.data.StringBytes;
-
 import net.metanotion.io.block.index.BSkipList;
 import net.metanotion.util.skiplist.SkipList;
 
+import net.i2p.I2PAppContext;
+import net.i2p.util.Log;
+
 class CorruptFileException extends IOException { }
 class BadFileFormatException extends IOException { }
 class BadVersionException extends IOException { }
@@ -53,14 +55,15 @@ class BadVersionException extends IOException { }
 public class BlockFile {
 	public static final long PAGESIZE = 1024;
 	public static final long OFFSET_MOUNTED = 20;
+	public static final Log log = I2PAppContext.getGlobalContext().logManager().getLog(BlockFile.class);
 
 	public RandomAccessInterface file;
 
 	private long magicBytes = 0x3141deadbeef0100L;
 	private long fileLen = PAGESIZE * 2;
 	private int freeListStart = 0;
-	private short mounted = 0;
+	private int mounted = 0;
-	public short spanSize = 16;
+	public int spanSize = 16;
 
 	private BSkipList metaIndex = null;
 	private HashMap openIndices = new HashMap();
@@ -84,9 +87,9 @@ public class BlockFile {
 		file.seek(0);
 		magicBytes		= file.readLong();
 		fileLen			= file.readLong();
-		freeListStart	= file.readInt();
+		freeListStart	= file.readUnsignedInt();
-		mounted			= file.readShort();
+		mounted			= file.readUnsignedShort();
-		spanSize		= file.readShort();
+		spanSize		= file.readUnsignedShort();
 	}
 
 	public static void main(String args[]) {
@@ -125,7 +128,7 @@ public class BlockFile {
 				}
 				BlockFile.pageSeek(this.file, curNextPage);
 				curPage = curNextPage;
-				curNextPage = this.file.readInt();
+				curNextPage = this.file.readUnsignedInt();
 				pageCounter = 4;
 				len = ((int) BlockFile.PAGESIZE) - pageCounter;
 			}
@@ -149,7 +152,7 @@ public class BlockFile {
 			if(len <= 0) {
 				BlockFile.pageSeek(this.file, curNextPage);
 				curPage = curNextPage;
-				curNextPage = this.file.readInt();
+				curNextPage = this.file.readUnsignedInt();
 				pageCounter = 4;
 				len = ((int) BlockFile.PAGESIZE) - pageCounter;
 			}

core/java/src/net/metanotion/io/block/FreeListBlock.java

@@ -43,12 +43,12 @@ public class FreeListBlock {
 		this.file = file;
 		this.page = startPage;
 		BlockFile.pageSeek(file, startPage);
-		nextPage = file.readInt();
+		nextPage = file.readUnsignedInt();
-		len = file.readInt();
+		len = file.readUnsignedInt();
 		if(len > 0) {
 			branches = new int[len];
 			for(int i=0;i<len;i++) {
-				branches[i] = file.readInt();
+				branches[i] = file.readUnsignedInt();
 			}
 		}
 	}

core/java/src/net/metanotion/io/block/index/BSkipLevels.java

@@ -50,15 +50,15 @@ public class BSkipLevels extends SkipLevels {
 
 		bsl.levelHash.put(new Integer(this.levelPage), this);
 
-		int maxLen = bf.file.readShort();
+		int maxLen = bf.file.readUnsignedShort();
-		int nonNull = bf.file.readShort();
+		int nonNull = bf.file.readUnsignedShort();
-		spanPage = bf.file.readInt();
+		spanPage = bf.file.readUnsignedInt();
 		bottom = (BSkipSpan) bsl.spanHash.get(new Integer(spanPage));
 
 		this.levels = new BSkipLevels[maxLen];
 		int lp;
 		for(int i=0;i<nonNull;i++) {
-			lp = bf.file.readInt();
+			lp = bf.file.readUnsignedInt();
 			if(lp != 0) {
 				levels[i] = (BSkipLevels) bsl.levelHash.get(new Integer(lp));
 				if(levels[i] == null) {

core/java/src/net/metanotion/io/block/index/BSkipList.java

@@ -59,10 +59,10 @@ public class BSkipList extends SkipList {
 		this.bf = bf;
 
 		BlockFile.pageSeek(bf.file, skipPage);
-		firstSpanPage = bf.file.readInt();
+		firstSpanPage = bf.file.readUnsignedInt();
-		firstLevelPage = bf.file.readInt();
+		firstLevelPage = bf.file.readUnsignedInt();
-		size = bf.file.readInt();
+		size = bf.file.readUnsignedInt();
-		spans = bf.file.readInt();
+		spans = bf.file.readUnsignedInt();
 		//System.out.println(size + " " + spans); 
 
 		this.fileOnly = fileOnly;

core/java/src/net/metanotion/io/block/index/BSkipSpan.java

@@ -74,7 +74,7 @@ public class BSkipSpan extends SkipSpan {
 			int next;
 			while(curPage != 0) {
 				BlockFile.pageSeek(bf.file, curPage);
-				next = bf.file.readInt();
+				next = bf.file.readUnsignedInt();
 				bf.freePage(curPage);
 				curPage = next;
 			}
@@ -83,6 +83,13 @@ public class BSkipSpan extends SkipSpan {
 	}
 
 	public void flush() {
+		fflush();
+	}
+
+	/**
+	 * I2P - avoid super.flush()
+	 */
+	private void fflush() {
 		try {
 			BlockFile.pageSeek(bf.file, page);
 			bf.file.writeInt(overflowPage);
@@ -111,11 +118,27 @@ public class BSkipSpan extends SkipSpan {
 					}
 					BlockFile.pageSeek(bf.file, curNextPage[0]);
 					curPage = curNextPage[0];
-					curNextPage[0] = bf.file.readInt();
+					curNextPage[0] = bf.file.readUnsignedInt();
 					pageCounter[0] = 4;
 				}
+				// Drop bad entry without throwing exception
+				if (keys[i] == null || vals[i] == null) {
+					BlockFile.log.error("Dropping null data in entry " + i + " page " + curPage +
+					                    " key=" + this.keys[i] + " val=" + this.vals[i]);
+					nKeys--;
+					i--;
+					continue;
+				}
 				keyData = this.keySer.getBytes(keys[i]);
 				valData = this.valSer.getBytes(vals[i]);
+				// Drop bad entry without throwing exception
+				if (keyData.length > 65535 || valData.length > 65535) {
+					BlockFile.log.error("Dropping huge data in entry " + i + " page " + curPage +
+					                    " keylen=" + keyData.length + " vallen=" + valData.length);
+					nKeys--;
+					i--;
+					continue;
+				}
 				pageCounter[0] += 4;
 				bf.file.writeShort(keyData.length);
 				bf.file.writeShort(valData.length);
@@ -123,8 +146,11 @@ public class BSkipSpan extends SkipSpan {
 				curPage = bf.writeMultiPageData(valData, curPage, pageCounter, curNextPage);
 			}
 			BlockFile.pageSeek(bf.file, this.page);
-			this.overflowPage = bf.file.readInt();
+			this.overflowPage = bf.file.readUnsignedInt();
 		} catch (IOException ioe) { throw new RuntimeException("Error writing to database", ioe); }
+		// FIXME can't get there from here
+		//bsl.size -= fail;
+		//bsl.flush();
 	}
 
 	private static void load(BSkipSpan bss, BlockFile bf, BSkipList bsl, int spanPage, Serializer key, Serializer val) throws IOException {
@@ -146,11 +172,11 @@ public class BSkipSpan extends SkipSpan {
 
 		BlockFile.pageSeek(bf.file, spanPage);
 
-		bss.overflowPage = bf.file.readInt();
+		bss.overflowPage = bf.file.readUnsignedInt();
-		bss.prevPage = bf.file.readInt();
+		bss.prevPage = bf.file.readUnsignedInt();
-		bss.nextPage = bf.file.readInt();
+		bss.nextPage = bf.file.readUnsignedInt();
-		bss.spanSize = bf.file.readShort();
+		bss.spanSize = bf.file.readUnsignedShort();
-		bss.nKeys = bf.file.readShort();
+		bss.nKeys = bf.file.readUnsignedShort();
 	}
 
 	/**
@@ -158,6 +184,15 @@ public class BSkipSpan extends SkipSpan {
 	 * Load the whole span's keys and values into memory
 	 */
 	protected void loadData() throws IOException {
+		loadData(true);
+	}
+
+	/**
+	 * I2P - second half of load()
+	 * Load the whole span's keys and values into memory
+	 * @param flushOnError set to false if you are going to flush anyway
+	 */
+	protected void loadData(boolean flushOnError) throws IOException {
 		this.keys = new Comparable[this.spanSize];
 		this.vals = new Object[this.spanSize];
 
@@ -168,15 +203,16 @@ public class BSkipSpan extends SkipSpan {
 		int[] pageCounter = new int[1];
 		pageCounter[0] = 16;
 //		System.out.println("Span Load " + sz + " nKeys " + nKeys + " page " + curPage);
+		int fail = 0;
 		for(int i=0;i<this.nKeys;i++) {
 			if((pageCounter[0] + 4) > BlockFile.PAGESIZE) {
 				BlockFile.pageSeek(this.bf.file, curNextPage[0]);
 				curPage = curNextPage[0];
-				curNextPage[0] = this.bf.file.readInt();
+				curNextPage[0] = this.bf.file.readUnsignedInt();
 				pageCounter[0] = 4;
 			}
-			ksz = this.bf.file.readShort();
+			ksz = this.bf.file.readUnsignedShort();
-			vsz = this.bf.file.readShort();
+			vsz = this.bf.file.readUnsignedShort();
 			pageCounter[0] +=4;
 			byte[] k = new byte[ksz];
 			byte[] v = new byte[vsz];
@@ -185,8 +221,24 @@ public class BSkipSpan extends SkipSpan {
 //			System.out.println("i=" + i + ", Page " + curPage + ", offset " + pageCounter[0] + " ksz " + ksz + " vsz " + vsz);
 			this.keys[i] = (Comparable) this.keySer.construct(k);
 			this.vals[i] = this.valSer.construct(v);
+			// Drop bad entry without throwing exception
+			if (this.keys[i] == null || this.vals[i] == null) {
+				BlockFile.log.error("Null deserialized data in entry " + i + " page " + curPage +
+				                    " key=" + this.keys[i] + " val=" + this.vals[i]);
+				fail++;
+				nKeys--;
+				i--;
+				continue;
+			}
+		}
+		if (fail > 0) {
+			BlockFile.log.error("Repairing corruption of " + fail + " entries");
+			if (flushOnError)
+				fflush();
+			// FIXME can't get there from here
+			//bsl.size -= fail;
+			//bsl.flush();
 		}
-
 	}
 
 	protected BSkipSpan() { }

core/java/src/net/metanotion/io/block/index/IBSkipSpan.java

@@ -114,12 +114,16 @@ public class IBSkipSpan extends BSkipSpan {
 		curNextPage[0] = this.overflowPage;
 		int[] pageCounter = new int[1];
 		pageCounter[0] = 16;
-		ksz = this.bf.file.readShort();
+		ksz = this.bf.file.readUnsignedShort();
 		this.bf.file.skipBytes(2);  //vsz
 		pageCounter[0] +=4;
 		byte[] k = new byte[ksz];
 		curPage = this.bf.readMultiPageData(k, curPage, pageCounter, curNextPage);
 		this.firstKey = (Comparable) this.keySer.construct(k);
+		if (this.firstKey == null) {
+			BlockFile.log.error("Null deserialized first key in page " + curPage);
+			repair(1);
+		}
 		if (DEBUG)
 			System.err.println("Loaded header for page " + this.page + " containing " + this.nKeys + '/' + this.spanSize + " first key: " + this.firstKey);
 	}
@@ -153,16 +157,17 @@ public class IBSkipSpan extends BSkipSpan {
 		curNextPage[0] = this.overflowPage;
 		int[] pageCounter = new int[1];
 		pageCounter[0] = 16;
+		int fail = 0;
 		//System.out.println("Span Load " + sz + " nKeys " + nKeys + " page " + curPage);
 		for(int i=0;i<this.nKeys;i++) {
 			if((pageCounter[0] + 4) > BlockFile.PAGESIZE) {
 				BlockFile.pageSeek(this.bf.file, curNextPage[0]);
 				curPage = curNextPage[0];
-				curNextPage[0] = this.bf.file.readInt();
+				curNextPage[0] = this.bf.file.readUnsignedInt();
 				pageCounter[0] = 4;
 			}
-			ksz = this.bf.file.readShort();
+			ksz = this.bf.file.readUnsignedShort();
-			vsz = this.bf.file.readShort();
+			vsz = this.bf.file.readUnsignedShort();
 			pageCounter[0] +=4;
 			byte[] k = new byte[ksz];
 			byte[] v = new byte[vsz];
@@ -170,20 +175,49 @@ public class IBSkipSpan extends BSkipSpan {
 			curPage = this.bf.readMultiPageData(v, curPage, pageCounter, curNextPage);
 			//System.out.println("i=" + i + ", Page " + curPage + ", offset " + pageCounter[0] + " ksz " + ksz + " vsz " + vsz);
 			Comparable ckey = (Comparable) this.keySer.construct(k);
+			if (ckey == null) {
+				BlockFile.log.error("Null deserialized key in entry " + i + " page " + curPage);
+				fail++;
+				continue;
+			}
 			int diff = ckey.compareTo(key);
 			if (diff == 0) {
 				//System.err.println("Found " + key + " at " + i + " (first: " + this.firstKey + ')');
-				return this.valSer.construct(v);
+				Object rv = this.valSer.construct(v);
+				if (rv == null) {
+					BlockFile.log.error("Null deserialized value in entry " + i + " page " + curPage +
+					                    " key=" + ckey);
+					fail++;
+				}
+				if (fail > 0)
+					repair(fail);
+				return rv;
 			}
 			if (diff > 0) {
 				//System.err.println("NOT Found " + key + " at " + i + " (first: " + this.firstKey + " current: " + ckey + ')');
+				if (fail > 0)
+					repair(fail);
 				return null;
 			}
 		}
 		//System.err.println("NOT Found " + key + " at end (first: " + this.firstKey + ')');
+		if (fail > 0)
+			repair(fail);
 		return null;
 	}
 
+        private void repair(int fail) {
+		try {
+			loadData(false);
+			if (this.nKeys > 0)
+				this.firstKey = this.keys[0];
+			flush();
+			BlockFile.log.error("Repaired corruption of " + fail + " entries");
+		} catch (IOException ioe) {
+			BlockFile.log.error("Failed to repair corruption of " + fail + " entries", ioe);
+		}
+	}
+
 	protected IBSkipSpan() { }
 
 	public IBSkipSpan(BlockFile bf, BSkipList bsl, int spanPage, Serializer key, Serializer val) throws IOException {