[RocksJava] CF Name shall handle bytes correctly

Summary:
Bytes are currently misinterpreted by the Java if the
byte array contains zero bytes within its content. For Strings
thats usually not useful. As the Java API allows every kind
of byte array values it might be the case that zero padding might
happen.

Test Plan:
make rocksdbjava
make jtest

Reviewers: adamretter, yhchiang, ankgup87

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D33165

java/rocksjni/rocksjni.cc

@@ -77,8 +77,6 @@ jobject
   const char* db_path = env->GetStringUTFChars(jdb_path, 0);
 
   std::vector<jbyte*> cfnames_to_free;
-  // the zero-terminated version of cfnames_to_free.
-  std::vector<char*> c_cfnames_to_free;
   std::vector<jbyteArray> jcfnames_for_free;
 
   std::vector<rocksdb::ColumnFamilyDescriptor> column_families;
@@ -106,17 +104,13 @@ jobject
           rocksdb::ColumnFamilyOptionsJni::getHandle(env, jcf_opt_obj);
 
       jbyte* cfname = env->GetByteArrayElements(byteArray, 0);
-      const int len = env->GetArrayLength(byteArray) + 1;
+      const int len = env->GetArrayLength(byteArray);
-      char* c_cfname = new char[len];
-      memcpy(c_cfname, cfname, len - 1);
-      c_cfname[len - 1] = 0;
 
       // free allocated cfnames after call to open
       cfnames_to_free.push_back(cfname);
-      c_cfnames_to_free.push_back(c_cfname);
       jcfnames_for_free.push_back(byteArray);
       column_families.push_back(rocksdb::ColumnFamilyDescriptor(
-          c_cfname, *cfOptions));
+          std::string(reinterpret_cast<char *>(cfname), len), *cfOptions));
   }
 
   rocksdb::Status s = rocksdb::DB::OpenForReadOnly(*opt,
@@ -127,8 +121,6 @@ jobject
       i != cfnames_to_free.size(); i++) {
     // free  cfnames
     env->ReleaseByteArrayElements(jcfnames_for_free[i], cfnames_to_free[i], 0);
-    // free c_cfnames
-    delete[] c_cfnames_to_free[i];
   }
 
   // check if open operation was successful
@@ -170,8 +162,6 @@ jobject Java_org_rocksdb_RocksDB_open__JLjava_lang_String_2Ljava_util_List_2I(
   const char* db_path = env->GetStringUTFChars(jdb_path, 0);
 
   std::vector<jbyte*> cfnames_to_free;
-  // the zero-terminated version of cfnames_to_free.
-  std::vector<char*> c_cfnames_to_free;
   std::vector<jbyteArray> jcfnames_for_free;
 
   std::vector<rocksdb::ColumnFamilyDescriptor> column_families;
@@ -199,17 +189,13 @@ jobject Java_org_rocksdb_RocksDB_open__JLjava_lang_String_2Ljava_util_List_2I(
           rocksdb::ColumnFamilyOptionsJni::getHandle(env, jcf_opt_obj);
 
       jbyte* cfname = env->GetByteArrayElements(byteArray, 0);
-      const int len = env->GetArrayLength(byteArray) + 1;
+      const int len = env->GetArrayLength(byteArray);
-      char* c_cfname = new char[len];
-      memcpy(c_cfname, cfname, len - 1);
-      c_cfname[len - 1] = 0;
 
       // free allocated cfnames after call to open
       cfnames_to_free.push_back(cfname);
-      c_cfnames_to_free.push_back(c_cfname);
       jcfnames_for_free.push_back(byteArray);
       column_families.push_back(rocksdb::ColumnFamilyDescriptor(
-          c_cfname, *cfOptions));
+          std::string(reinterpret_cast<char *>(cfname), len), *cfOptions));
   }
 
   rocksdb::Status s = rocksdb::DB::Open(*opt, db_path, column_families,
@@ -220,8 +206,6 @@ jobject Java_org_rocksdb_RocksDB_open__JLjava_lang_String_2Ljava_util_List_2I(
       i != cfnames_to_free.size(); i++) {
     // free  cfnames
     env->ReleaseByteArrayElements(jcfnames_for_free[i], cfnames_to_free[i], 0);
-    // free c_cfnames
-    delete[] c_cfnames_to_free[i];
   }
 
   // check if open operation was successful
@@ -281,7 +265,7 @@ jobject Java_org_rocksdb_RocksDB_listColumnFamilies(
           env->NewByteArray(static_cast<jsize>(column_family_names[i].size()));
       env->SetByteArrayRegion(
           jcf_value, 0, static_cast<jsize>(column_family_names[i].size()),
-          reinterpret_cast<const jbyte*>(column_family_names[i].c_str()));
+          reinterpret_cast<const jbyte*>(column_family_names[i].data()));
       env->CallBooleanMethod(jvalue_list,
           rocksdb::ListJni::getListAddMethodId(env), jcf_value);
     }
@@ -1294,15 +1278,11 @@ jlong Java_org_rocksdb_RocksDB_createColumnFamily(
       rocksdb::ColumnFamilyOptionsJni::getHandle(env, jcf_opt_obj);
 
   jbyte* cfname = env->GetByteArrayElements(byteArray, 0);
-  const int len = env->GetArrayLength(byteArray) + 1;
+  const int len = env->GetArrayLength(byteArray);
-  char* c_cfname = new char[len];
-  memcpy(c_cfname, cfname, len - 1);
-  c_cfname[len - 1] = 0;
 
   rocksdb::Status s = db_handle->CreateColumnFamily(
-      *cfOptions, c_cfname, &handle);
+      *cfOptions, std::string(reinterpret_cast<char *>(cfname), len), &handle);
   env->ReleaseByteArrayElements(byteArray, cfname, 0);
-  delete[] c_cfname;
 
   if (s.ok()) {
     return reinterpret_cast<jlong>(handle);

java/rocksjni/ttl.cc

@@ -57,8 +57,6 @@ jobject
   const char* db_path = env->GetStringUTFChars(jdb_path, 0);
 
   std::vector<jbyte*> cfnames_to_free;
-  // the zero-terminated version of cfnames_to_free.
-  std::vector<char*> c_cfnames_to_free;
   std::vector<jbyteArray> jcfnames_for_free;
 
   std::vector<rocksdb::ColumnFamilyDescriptor> column_families;
@@ -87,17 +85,13 @@ jobject
           rocksdb::ColumnFamilyOptionsJni::getHandle(env, jcf_opt_obj);
 
       jbyte* cfname = env->GetByteArrayElements(byteArray, 0);
-      const int len = env->GetArrayLength(byteArray) + 1;
+      const int len = env->GetArrayLength(byteArray);
-      char* c_cfname = new char[len];
-      memcpy(c_cfname, cfname, len - 1);
-      c_cfname[len - 1] = 0;
 
       // free allocated cfnames after call to open
       cfnames_to_free.push_back(cfname);
-      c_cfnames_to_free.push_back(c_cfname);
       jcfnames_for_free.push_back(byteArray);
       column_families.push_back(rocksdb::ColumnFamilyDescriptor(
-          c_cfname, *cfOptions));
+          std::string(reinterpret_cast<char *>(cfname), len), *cfOptions));
   }
   // get iterator for TTL values
   iteratorObj = env->CallObjectMethod(
@@ -122,8 +116,6 @@ jobject
       i != cfnames_to_free.size(); i++) {
     // free  cfnames
     env->ReleaseByteArrayElements(jcfnames_for_free[i], cfnames_to_free[i], 0);
-    // free c_cfnames
-    delete[] c_cfnames_to_free[i];
   }
 
   // check if open operation was successful
@@ -176,15 +168,12 @@ jlong Java_org_rocksdb_TtlDB_createColumnFamilyWithTtl(
       rocksdb::ColumnFamilyOptionsJni::getHandle(env, jcf_opt_obj);
 
   jbyte* cfname = env->GetByteArrayElements(byteArray, 0);
-  const int len = env->GetArrayLength(byteArray) + 1;
+  const int len = env->GetArrayLength(byteArray);
-  char* c_cfname = new char[len];
-  memcpy(c_cfname, cfname, len - 1);
-  c_cfname[len - 1] = 0;
 
   rocksdb::Status s = db_handle->CreateColumnFamilyWithTtl(
-      *cfOptions, c_cfname, &handle, jttl);
+      *cfOptions, std::string(reinterpret_cast<char *>(cfname),
+          len), &handle, jttl);
   env->ReleaseByteArrayElements(byteArray, cfname, 0);
-  delete[] c_cfname;
 
   if (s.ok()) {
     return reinterpret_cast<jlong>(handle);

java/src/test/java/org/rocksdb/ColumnFamilyTest.java

@@ -5,10 +5,7 @@
 
 package org.rocksdb;
 
-import java.util.HashMap;
+import java.util.*;
-import java.util.List;
-import java.util.ArrayList;
-import java.util.Map;
 
 import org.junit.ClassRule;
 import org.junit.Rule;
@@ -604,4 +601,77 @@ public class ColumnFamilyTest {
     }
   }
 
+  @Test
+  public void testByteCreateFolumnFamily() throws RocksDBException {
+    RocksDB db = null;
+    Options options = null;
+    try {
+      options = new Options().setCreateIfMissing(true);
+      db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath());
+
+      byte[] b0 = new byte[] { (byte)0x00 };
+      byte[] b1 = new byte[] { (byte)0x01 };
+      byte[] b2 = new byte[] { (byte)0x02 };
+      db.createColumnFamily(new ColumnFamilyDescriptor(b0));
+      db.createColumnFamily(new ColumnFamilyDescriptor(b1));
+      List<byte[]> families = RocksDB.listColumnFamilies(options, dbFolder.getRoot().getAbsolutePath());
+      assertThat(families).contains("default".getBytes(), b0, b1);
+      db.createColumnFamily(new ColumnFamilyDescriptor(b2));
+    } finally {
+      if (db != null) {
+        db.close();
+      }
+      if (options != null) {
+        options.dispose();
+      }
+    }
+  }
+
+  @Test
+  public void testCFNamesWithZeroBytes() throws RocksDBException {
+    RocksDB db = null;
+    Options options = null;
+    try {
+      options = new Options().setCreateIfMissing(true);
+      db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath());
+
+      byte[] b0 = new byte[] { 0, 0 };
+      byte[] b1 = new byte[] { 0, 1 };
+      db.createColumnFamily(new ColumnFamilyDescriptor(b0));
+      db.createColumnFamily(new ColumnFamilyDescriptor(b1));
+      List<byte[]> families = RocksDB.listColumnFamilies(options, dbFolder.getRoot().getAbsolutePath());
+      assertThat(families).contains("default".getBytes(), b0, b1);
+    } finally {
+      if (db != null) {
+        db.close();
+      }
+      if (options != null) {
+        options.dispose();
+      }
+    }
+  }
+
+  @Test
+  public void testCFNameSimplifiedChinese() throws RocksDBException {
+    RocksDB db = null;
+    Options options = null;
+    try {
+      options = new Options().setCreateIfMissing(true);
+      db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath());
+      final String simplifiedChinese = "简体字";
+      db.createColumnFamily(new ColumnFamilyDescriptor(simplifiedChinese.getBytes()));
+
+      List<byte[]> families = RocksDB.listColumnFamilies(options, dbFolder.getRoot().getAbsolutePath());
+      assertThat(families).contains("default".getBytes(), simplifiedChinese.getBytes());
+    } finally {
+      if (db != null) {
+        db.close();
+      }
+      if (options != null) {
+        options.dispose();
+      }
+    }
+
+
+  }
 }