Fixed bugs in packing/unpacking bytearrays: we must be aware of 'U' format;

when length is provided in format, use it; do padding.

Fixed signed checking for getInt/Long/Short.

Fixed bugs in packing strings: embedded nulls; use length when provided.
Logic copied from packing.py.

Added special case encoding for 8256 (to match Python and C).

3 files changed, 78 insertions, 24 deletions

diff --git a/lang/java/src/com/wiredtiger/db/PackFormatInputStream.java b/lang/java/src/com/wiredtiger/db/PackFormatInputStream.java
index c53938d0a58..0d0ce42d375 100644
--- a/lang/java/src/com/wiredtiger/db/PackFormatInputStream.java
+++ b/lang/java/src/com/wiredtiger/db/PackFormatInputStream.java
@@ -180,5 +180,12 @@ public class PackFormatInputStream {
         }
         return valueLen;
     }
-}
 
+    /**
+     * Return whether there is an explicit length indicated in the format
+     * string.
+     */
+    protected boolean hasLength() {
+        return (getIntFromFormat(false) > 0);
+    }
+}
diff --git a/lang/java/src/com/wiredtiger/db/PackInputStream.java b/lang/java/src/com/wiredtiger/db/PackInputStream.java
index a49b2e01f17..77eb19d1de1 100644
--- a/lang/java/src/com/wiredtiger/db/PackInputStream.java
+++ b/lang/java/src/com/wiredtiger/db/PackInputStream.java
@@ -119,7 +119,6 @@ public class PackInputStream {
     throws WiredTigerPackingException {
         format.checkType('U', false);
         getByteArrayInternal(getByteArrayLength(), dest, off, len);
-
     }
 
     /**
@@ -128,6 +127,7 @@ public class PackInputStream {
      */
     public byte[] getByteArray()
     throws WiredTigerPackingException {
+        format.checkType('U', false);
         int itemLen = getByteArrayLength();
         byte[] unpacked = new byte[itemLen];
         getByteArrayInternal(itemLen, unpacked, 0, itemLen);
@@ -141,8 +141,17 @@ public class PackInputStream {
     private int getByteArrayLength()
     throws WiredTigerPackingException {
         int itemLen = 0;
-        /* The rest of the buffer is a byte array. */
-        if (format.available() == 1) {
+
+        if (format.hasLength()) {
+            // If the format has a length, it's always used.
+            itemLen = format.getLengthFromFormat(true);
+        } else if (format.getType() == 'U') {
+            // The 'U' format is used internally, and may be exposed to us.
+            // It indicates that the size is always stored unless there
+            // is a size in the format.
+            itemLen = unpackInt(false);
+        } else if (format.available() == 1) {
+            // The rest of the buffer is a byte array.
             itemLen = valueLen - valueOff;
         } else {
             itemLen = unpackInt(false);
@@ -156,7 +165,6 @@ public class PackInputStream {
     private void getByteArrayInternal(
         int itemLen, byte[] dest, int off, int destLen)
     throws WiredTigerPackingException {
-        /* TODO: padding. */
         int copyLen = itemLen;
         if (itemLen > destLen) {
             copyLen = destLen;
@@ -171,11 +179,11 @@ public class PackInputStream {
      */
     public int getInt()
     throws WiredTigerPackingException {
-        boolean signed = false;
+        boolean signed = true;
         format.checkType('i', false);
         if (format.getType() == 'I' ||
                 format.getType() == 'L') {
-            signed = true;
+            signed = false;
         }
         format.consume();
         return unpackInt(signed);
@@ -186,10 +194,10 @@ public class PackInputStream {
      */
     public long getLong()
     throws WiredTigerPackingException {
-        boolean signed = false;
+        boolean signed = true;
         format.checkType('q', false);
         if (format.getType() == 'Q') {
-            signed = true;
+            signed = false;
         }
         format.consume();
         return unpackLong(signed);
@@ -210,10 +218,10 @@ public class PackInputStream {
      */
     public short getShort()
     throws WiredTigerPackingException {
-        boolean signed = false;
+        boolean signed = true;
         format.checkType('h', false);
         if (format.getType() == 'H') {
-            signed = true;
+            signed = false;
         }
         format.consume();
         return unpackShort(signed);
diff --git a/lang/java/src/com/wiredtiger/db/PackOutputStream.java b/lang/java/src/com/wiredtiger/db/PackOutputStream.java
index e79b4c63498..cc8b93e3457 100644
--- a/lang/java/src/com/wiredtiger/db/PackOutputStream.java
+++ b/lang/java/src/com/wiredtiger/db/PackOutputStream.java
@@ -109,14 +109,31 @@ public class PackOutputStream {
      */
     public void addByteArray(byte[] value, int off, int len)
     throws WiredTigerPackingException {
-        format.checkType('U', true);
-        // If this is not the last item, store the size.
-        if (format.available() > 0) {
-            packLong(len, false);
+        int padBytes = 0;
+
+        format.checkType('U', false);
+        boolean havesize = format.hasLength();
+        char type = format.getType();
+        if (havesize) {
+            int size = format.getLengthFromFormat(true);
+            if (len > size) {
+                len = size;
+            } else if (size > len) {
+                padBytes = size - len;
+            }
         }
+        // We're done pulling information from the field now.
+        format.consume();
 
+        // If this is not the last item and the format does not have the
+        // size, or we're using the internal 'U' format, store the size.
+        if (!havesize && (format.available() > 0 || type == 'U')) {
+            packLong(len, false);
+        }
         packed.write(value, off, len);
-        /* TODO: padding. */
+        while(padBytes-- > 0) {
+            packed.write(0);
+        }
     }
 
     /**
@@ -178,17 +195,33 @@ public class PackOutputStream {
         // Strings have two possible encodings. A lower case 's' is not null
         // terminated, and has a length define in the format (default 1). An
         // upper case 'S' is variable length and has a null terminator.
-        if (fieldFormat == 's') {
-            stringLen = format.getLengthFromFormat(true);
-            valLen = value.length();
-            if (stringLen > valLen) {
-                padBytes = stringLen - valLen;
-                stringLen = valLen;
-            }
+
+        // Logic from python packing.py:
+        boolean havesize = format.hasLength();
+        int nullpos = value.indexOf('\0');
+        int size = 0;
+
+        if (fieldFormat == 'S' && nullpos >= 0) {
+            stringLen = nullpos;
         } else {
             stringLen = value.length();
-            padBytes = 1; // Null terminator
         }
+        if (havesize) {
+            size = format.getLengthFromFormat(true);
+            if (stringLen > size) {
+                stringLen = size;
+            }
+        } else if (fieldFormat == 's') {
+            havesize = true;
+            size = 1;
+        }
+
+        if (fieldFormat == 'S' && !havesize) {
+            padBytes = 1;
+        } else if (size > stringLen) {
+            padBytes = size - stringLen;
+        }
+
         // We're done pulling information from the field now.
         format.consume();
 
@@ -249,6 +282,12 @@ public class PackOutputStream {
             intBuf[offset++] =
                 (byte)(PackUtil.POS_2BYTE_MARKER | PackUtil.GET_BITS(x, 13, 8));
             intBuf[offset++] = PackUtil.GET_BITS(x, 8, 0);
+        } else if (x == PackUtil.POS_2BYTE_MAX + 1) {
+            // This is a special case where we could store the value with
+            // just a single byte, but we append a zero byte so that the
+            // encoding doesn't get shorter for this one value.
+            intBuf[offset++] = (byte)(PackUtil.POS_MULTI_MARKER | 0x01);
+            intBuf[offset++] = 0;
         } else {
             x -= PackUtil.POS_2BYTE_MAX + 1;
             intBuf[offset] = PackUtil.POS_MULTI_MARKER;