[Java][FlexBuffers] Deprecate typed vector strings due to design flaw (#5722)

* [Java][FlexBuffers] Deprecate typed vector strings due to design flaw

It will still be possible to read buffers with this type, but the
elements will be treated as FBT_KEY and will be read as null-terminated
string.

Trying to build a vector of strings as typed will throw an exception.

More information on https://github.com/google/flatbuffers/issues/5627

Also, fix another bug on strings, where long strings were not properly
aligned.

* [Java][FlexBuffers] Make FBT_VECTOR_STRING_DEPRECATED considered typed.

The logic for FlexBuffers.isVectorType() was changed
to not consider FBT_VECTOR_STRING_DEPRECATED a typed
vector, but that can lead to missinterpretation for
existing serialized data. So we are reverting.

java/com/google/flatbuffers/FlexBuffers.java

@@ -77,7 +77,9 @@ public class FlexBuffers {
     /** Represent a vector of keys type */
     public static final int FBT_VECTOR_KEY = 14;
     /** Represent a vector of strings type */
-    public static final int FBT_VECTOR_STRING = 15;
+    // DEPRECATED, use FBT_VECTOR or FBT_VECTOR_KEY instead.
+    // more info on thttps://github.com/google/flatbuffers/issues/5627.
+    public static final int FBT_VECTOR_STRING_DEPRECATED = 15;
 
     /// @cond FLATBUFFERS_INTERNAL
     public static final int FBT_VECTOR_INT2 = 16;  // Typed tuple  = no type table; no size field).
@@ -107,7 +109,7 @@ public class FlexBuffers {
      * @return true if typed vector
      */
     static boolean isTypedVector(int type) {
-        return (type >= FBT_VECTOR_INT && type <= FBT_VECTOR_STRING) || type == FBT_VECTOR_BOOL;
+        return (type >= FBT_VECTOR_INT && type <= FBT_VECTOR_STRING_DEPRECATED) || type == FBT_VECTOR_BOOL;
     }
 
     /**
@@ -145,7 +147,7 @@ public class FlexBuffers {
     }
 
     static boolean isTypedVectorElementType(int type) {
-        return (type >= FBT_INT && type <= FBT_STRING) || type == FBT_BOOL;
+        return (type >= FBT_INT && type <= FBT_KEY) || type == FBT_BOOL;
     }
 
     // return position of the element that the offset is pointing to
@@ -322,8 +324,7 @@ public class FlexBuffers {
          * @return true if a typed vector type
          */
         public boolean isTypedVector() {
-            return (type >= FBT_VECTOR_INT && type <= FBT_VECTOR_STRING) ||
+            return FlexBuffers.isTypedVector(type);
-                    type == FBT_VECTOR_BOOL;
         }
 
         /**
@@ -482,7 +483,7 @@ public class FlexBuffers {
         public String asString() {
             if (isString()) {
                 int start = indirect(bb, end, parentWidth);
-                int size = readInt(bb, start - byteWidth, byteWidth);
+                int size = (int) readUInt(bb, start - byteWidth, byteWidth);
                 return Utf8.getDefault().decodeUtf8(bb, start, size);
             }
             else if (isKey()){
@@ -516,6 +517,9 @@ public class FlexBuffers {
         public Vector asVector() {
             if (isVector()) {
                 return new Vector(bb, indirect(bb, end, parentWidth), byteWidth);
+            } else if(type == FlexBuffers.FBT_VECTOR_STRING_DEPRECATED) {
+                // deprecated. Should be treated as key vector
+                return new TypedVector(bb, indirect(bb, end, parentWidth), byteWidth, FlexBuffers.FBT_KEY);
             } else if (FlexBuffers.isTypedVector(type)) {
                 return new TypedVector(bb, indirect(bb, end, parentWidth), byteWidth, FlexBuffers.toTypedVectorElementType(type));
             } else {
@@ -590,7 +594,7 @@ public class FlexBuffers {
                 case FBT_VECTOR_UINT:
                 case FBT_VECTOR_FLOAT:
                 case FBT_VECTOR_KEY:
-                case FBT_VECTOR_STRING:
+                case FBT_VECTOR_STRING_DEPRECATED:
                 case FBT_VECTOR_BOOL:
                     return sb.append(asVector());
                 case FBT_VECTOR_INT2:

java/com/google/flatbuffers/FlexBuffersBuilder.java

@@ -358,24 +358,24 @@ public class FlexBuffersBuilder {
     }
 
     private Value writeString(int key, String s) {
-        return writeBlob(key, s.getBytes(StandardCharsets.UTF_8), FBT_STRING);
+        return writeBlob(key, s.getBytes(StandardCharsets.UTF_8), FBT_STRING, true);
     }
 
     // in bits to fit a unsigned int
-    private static int widthUInBits(long len) {
+    static int widthUInBits(long len) {
         if (len <= byteToUnsignedInt((byte)0xff)) return WIDTH_8;
         if (len <= shortToUnsignedInt((short)0xffff)) return WIDTH_16;
         if (len <= intToUnsignedLong(0xffff_ffff)) return WIDTH_32;
         return WIDTH_64;
     }
 
-    private Value writeBlob(int key, byte[] blob, int type) {
+    private Value writeBlob(int key, byte[] blob, int type, boolean trailing) {
         int bitWidth = widthUInBits(blob.length);
         int byteWidth = align(bitWidth);
         writeInt(blob.length, byteWidth);
         int sloc = bb.position();
         bb.put(blob);
-        if (type == FBT_STRING) {
+        if (trailing) {
             bb.put((byte) 0);
         }
         return Value.blob(key, sloc, type, bitWidth);
@@ -384,7 +384,7 @@ public class FlexBuffersBuilder {
     // Align to prepare for writing a scalar with a certain size.
     private int align(int alignment) {
         int byteWidth = 1 << alignment;
-        int padBytes = Value.paddingBytes(bb.capacity(), byteWidth);
+        int padBytes = Value.paddingBytes(bb.position(), byteWidth);
         while (padBytes-- != 0) {
             bb.put((byte) 0);
         }
@@ -417,7 +417,7 @@ public class FlexBuffersBuilder {
      */
     public int putBlob(String key, byte[] val) {
         int iKey = putKey(key);
-        Value value = writeBlob(iKey, val, FBT_BLOB);
+        Value value = writeBlob(iKey, val, FBT_BLOB, false);
         stack.add(value);
         return (int) value.iValue;
     }
@@ -504,6 +504,9 @@ public class FlexBuffersBuilder {
             if (typed) {
                 if (i == start) {
                     vectorType = stack.get(i).type;
+                    if (!FlexBuffers.isTypedVectorElementType(vectorType)) {
+                        throw new FlexBufferException("TypedVector does not support this element type");
+                    }
                 } else {
                     // If you get this assert, you are writing a typed vector with
                     // elements that are not all the same type.
@@ -659,7 +662,7 @@ public class FlexBuffersBuilder {
         }
 
         static Value blob(int key, int position, int type, int bitWidth) {
-            return new Value(key, type, WIDTH_8, position);
+            return new Value(key, type, bitWidth, position);
         }
 
         static Value int8(int key, int value) {

tests/JavaTest.java

@@ -12,6 +12,10 @@ import com.google.flatbuffers.FlexBuffers;
 import com.google.flatbuffers.FlexBuffersBuilder;
 import com.google.flatbuffers.StringVector;
 import com.google.flatbuffers.UnionVector;
+import com.google.flatbuffers.FlexBuffers.FlexBufferException;
+import com.google.flatbuffers.FlexBuffers.Reference;
+import com.google.flatbuffers.FlexBuffers.Vector;
+
 import java.io.*;
 import java.math.BigInteger;
 import java.nio.ByteBuffer;
@@ -694,6 +698,60 @@ class JavaTest {
         TestEq(mymap.get("blob").toString(), "\"AC\"");
     }
 
+    public static void testFlexBufferVectorStrings() {
+        FlexBuffersBuilder builder = new FlexBuffersBuilder(ByteBuffer.allocate(10000000));
+
+        int size = 3000;
+        StringBuilder sb = new StringBuilder();
+        for (int i=0; i< size; i++) {
+            sb.append("a");
+        }
+
+        String text = sb.toString();
+        TestEq(text.length(), size);
+
+        int pos = builder.startVector();
+
+        for (int i=0; i<size; i++) {
+            builder.putString(text);
+        }
+
+        try {
+            builder.endVector(null, pos, true, false);
+            // this should raise an exception as
+            // typed vector of string was deprecated
+            assert false;
+        } catch(FlexBufferException fb) {
+            // no op
+        }
+        // we finish the vector again as non-typed
+        builder.endVector(null, pos, false, false);
+
+        ByteBuffer b = builder.finish();
+        Vector v = FlexBuffers.getRoot(b).asVector();
+
+        TestEq(v.size(), size);
+        for (int i=0; i<size; i++) {
+            TestEq(v.get(i).asString().length(), size);
+            TestEq(v.get(i).asString(), text);
+        }
+    }
+
+    public static void testDeprecatedTypedVectorString() {
+        // tests whether we are able to support reading deprecated typed vector string
+        // data is equivalent to [ "abc", "abc", "abc", "abc"]
+        byte[] data = new byte[] {0x03, 0x61, 0x62, 0x63, 0x00, 0x03, 0x61, 0x62, 0x63, 0x00,
+            0x03, 0x61, 0x62, 0x63, 0x00, 0x03, 0x61, 0x62, 0x63, 0x00, 0x04, 0x14, 0x10,
+             0x0c, 0x08, 0x04, 0x3c, 0x01};
+        Reference ref = FlexBuffers.getRoot(ByteBuffer.wrap(data));
+        TestEq(ref.getType(), FlexBuffers.FBT_VECTOR_STRING_DEPRECATED);
+        TestEq(ref.isTypedVector(), true);
+        Vector vec = ref.asVector();
+        for (int i=0; i< vec.size(); i++) {
+            TestEq("abc", vec.get(i).asString());
+        }
+    }
+
     public static void testSingleElementBoolean() {
         FlexBuffersBuilder builder = new FlexBuffersBuilder(ByteBuffer.allocate(100));
         builder.putBoolean(true);
@@ -743,13 +801,32 @@ class JavaTest {
         TestEq(Double.compare(Double.MAX_VALUE, FlexBuffers.getRoot(b).asFloat()), 0);
     }
 
-    public static void testSingleElementString() {
+    public static void testSingleElementBigString() {
-        FlexBuffersBuilder builder = new FlexBuffersBuilder();
+        FlexBuffersBuilder builder = new FlexBuffersBuilder(ByteBuffer.allocate(10000));
-        builder.putString("wow");
+        StringBuilder sb = new StringBuilder();
+
+        for (int i=0; i< 3000; i++) {
+            sb.append("a");
+        }
+
+        builder.putString(sb.toString());
+        ByteBuffer b = builder.finish();
+
+        FlexBuffers.Reference r = FlexBuffers.getRoot(b);
+
+        TestEq(FlexBuffers.FBT_STRING, r.getType());
+        TestEq(sb.toString(), r.asString());
+    }
+
+    public static void testSingleElementSmallString() {
+        FlexBuffersBuilder builder = new FlexBuffersBuilder(ByteBuffer.allocate(10000));
+
+        builder.putString("aa");
         ByteBuffer b = builder.finish();
         FlexBuffers.Reference r = FlexBuffers.getRoot(b);
+
         TestEq(FlexBuffers.FBT_STRING, r.getType());
-        TestEq("wow", r.asString());
+        TestEq("aa", r.asString());
     }
 
     public static void testSingleElementBlob() {
@@ -813,12 +890,6 @@ class JavaTest {
         }
         builder.endVector("floats", vecPos, true, false);
 
-        vecPos = builder.startVector();
-        for (final String i : strings) {
-            builder.putString(i);
-        }
-        builder.endVector("strings", vecPos, true, false);
-
         vecPos = builder.startVector();
         for (final boolean i : booleans) {
             builder.putBoolean(i);
@@ -832,7 +903,6 @@ class JavaTest {
         FlexBuffers.Reference r = FlexBuffers.getRoot(b);
         assert(r.asMap().get("ints").isTypedVector());
         assert(r.asMap().get("floats").isTypedVector());
-        assert(r.asMap().get("strings").isTypedVector());
         assert(r.asMap().get("booleans").isTypedVector());
     }
 
@@ -944,7 +1014,8 @@ class JavaTest {
         testSingleElementLong();
         testSingleElementFloat();
         testSingleElementDouble();
-        testSingleElementString();
+        testSingleElementSmallString();
+        testSingleElementBigString();
         testSingleElementBlob();
         testSingleElementVector();
         testSingleFixedTypeVector();
@@ -955,6 +1026,8 @@ class JavaTest {
         testFlexBuffersTest();
         testHashMapToMap();
         testFlexBuferEmpty();
+        testFlexBufferVectorStrings();
+        testDeprecatedTypedVectorString();
     }
 
     static void TestVectorOfBytes() {