Java byte array max size check before copying long-sized data from jni bridge

[azagrebin]

This issue partially addresses #2383.

When user merges a lot of data into one value, its size might grow indefinitely.
When user gets the value as byte[] in java client, its actual merged size might exceed the JVM limitation for returned java array size.

Currently size of java array is calculated just by explicit cast of underlying size_t of value:

rocksdb/java/rocksjni/portal.h

Line 4297 in 66c7aa3

const jsize jlen = static_cast<jsize>(bytes.size());

The problem is that if bytes.size() exceeds java 32b int, it leads to overflow of jlen which might be even negative. The result can be an unclear NegativeArraySizeException/ArrayIndexOutOfBoundsException at the user side.

The suggestion is to check the size overflow and throw more clear RocksDBException with problem description:

static jbyteArray createJavaByteArrayWithSizeCheck(JNIEnv* env, const char* bytes, const size_t size) {
      // Limitation for array size is vm specific
      // Current HotSpot VM limitation for array size is Integer.MAX_VALUE - 5 (2^31 - 1 - 5)
      // Integer.MAX_VALUE - 8 should be safe enough
      static const size_t MAX_JARRAY_SIZE = ((static_cast<size_t>(1)) << 31) - 9;
      if (size > MAX_JARRAY_SIZE) {
        rocksdb::RocksDBExceptionJni::ThrowNew(env, "Requested array size exceeds VM limit");
        return nullptr;
      }
      // return normal result
}

[adamretter]

I agree that the error should be as clear as possible, but I wonder if if is not more appropriate to throw a ArrayIndexOutOfBoundsException with your error message, rather than a RocksDBException? The problem is really a bounds issue IMHO.

[azagrebin]

thanks @adamretter, I can change PR to use ArrayIndexOutOfBoundsException. There are just some thoughts behind current PR I wanted to share.

My thinking was inspired by java OutOfMemoryError which could be also an option:

public class Foo {
   public static void main(String[] args) {
     byte[] array = new byte[Integer.MAX_VALUE];
   }
 }

$ javac Foo.java ; java Foo
Exception in thread "main" java.lang.OutOfMemoryError: Requested array size exceeds VM limit
	at Foo.main(Foo.java:3)

I used RocksDBException because the check and error come from RocksDB jni API but not from JVM.

[adamretter]

@azagrebin Okay I can understand your thinking. A couple of further comments:

1. I don't think that using OutOfMemoryError here would be appropriate. We have not in-fact run out of memory, quite the opposite, we are protecting against it.

2. After further consideration, I think I am inclined to agree that a RocksDBException is more appropriate. As the arguments provided to the various functions that use copyBytes are in fact themselves valid. Rather it is a limitation of RocksJava that we cannot access keys or values of a size more than 32bits.

[azagrebin]

@adamretter
I think last PR addressed the issue. I guess we can close the issue?

[adamretter]

Closed by #3850