Add bounds checking to our internal array get/set methods.
Change-Id: Ia570615e6b4ea81d796b6bf720444c62830a4f89
diff --git a/src/jni_internal.cc b/src/jni_internal.cc
index 81e2fdd..87decd7 100644
--- a/src/jni_internal.cc
+++ b/src/jni_internal.cc
@@ -1302,7 +1302,6 @@
// TODO: DecodeReference
ObjectArray<Object>* array = reinterpret_cast<ObjectArray<Object>*>(java_array);
Object* value = reinterpret_cast<Object*>(java_value);
- // TODO: who should throw? JNI or ObjectArray?
array->Set(index, value);
}
diff --git a/src/jni_internal_test.cc b/src/jni_internal_test.cc
index e5ba913..9394ff6 100644
--- a/src/jni_internal_test.cc
+++ b/src/jni_internal_test.cc
@@ -92,7 +92,8 @@
// TODO: check non-NULL initial elements.
- jclass c = env_->FindClass("[Ljava.lang.String;");
+ jclass c = env_->FindClass("[Ljava/lang/String;");
+ ASSERT_TRUE(c != NULL);
EXPECT_TRUE(env_->NewObjectArray(0, c, NULL) != NULL);
@@ -106,6 +107,18 @@
// TODO: check some non-ASCII strings.
}
+TEST_F(JniInternalTest, SetObjectArrayElement) {
+ jclass c = env_->FindClass("[Ljava/lang/Object;");
+ ASSERT_TRUE(c != NULL);
+
+ jobjectArray array = env_->NewObjectArray(1, c, NULL);
+ EXPECT_TRUE(array != NULL);
+ env_->SetObjectArrayElement(array, 0, c);
+ // TODO: check reading value back
+ // TODO: check IndexOutOfBoundsExceptions thrown for bad indexes.
+ // TODO: check ArrayStoreException thrown for bad types.
+}
+
bool EnsureInvokeStub(Method* method);
byte* AllocateCode(void* code, size_t length) {
diff --git a/src/object.h b/src/object.h
index 6fe2ecf..4f32cf9 100644
--- a/src/object.h
+++ b/src/object.h
@@ -625,9 +625,21 @@
length_ = length;
}
+ protected:
+ bool IsValidIndex(int32_t index) const {
+ if (index < 0 || index >= length_) {
+ // TODO: throw ArrayIndexOutOfBoundsException with the detail message
+ // "length=%d; index=%d", length_, index;
+ CHECK(false) << "ArrayIndexOutOfBoundsException: length="
+ << length_ << "; index=" << index;
+ return false;
+ }
+ return true;
+ }
+
private:
// The number of array elements.
- uint32_t length_;
+ int32_t length_;
// Padding to ensure the first member defined by a subclass begins on a 8-byte boundary
int32_t padding_;
@@ -651,13 +663,17 @@
}
T* Get(int32_t i) const {
- CHECK_LT(i, GetLength());
+ if (!IsValidIndex(i)) {
+ return NULL;
+ }
return GetData()[i];
}
void Set(int32_t i, T* object) {
- CHECK_LT(i, GetLength());
- GetData()[i] = object; // TODO: write barrier
+ if (IsValidIndex(i)) {
+ // TODO: ArrayStoreException
+ GetData()[i] = object; // TODO: write barrier
+ }
}
static void Copy(ObjectArray<T>* src, int src_pos,
@@ -1207,13 +1223,17 @@
}
T Get(int32_t i) const {
- CHECK_LT(i, GetLength());
+ if (!IsValidIndex(i)) {
+ return T();
+ }
return GetData()[i];
}
void Set(int32_t i, T value) {
- CHECK_LT(i, GetLength());
- GetData()[i] = value;
+ // TODO: ArrayStoreException
+ if (IsValidIndex(i)) {
+ GetData()[i] = value;
+ }
}
static void SetArrayClass(Class* array_class) {
@@ -1249,9 +1269,14 @@
return count_;
}
- uint16_t CharAt(uint32_t index) const {
- DCHECK_LE(index, GetLength());
- return GetCharArray()->Get(index + GetOffset());
+ uint16_t CharAt(int32_t index) const {
+ if (index < 0 || index >= count_) {
+ // TODO: throw new StringIndexOutOfBounds(this, index);
+ CHECK(false) << "StringIndexOutOfBounds: " << index;
+ return 0;
+ } else {
+ return GetCharArray()->Get(index + GetOffset());
+ }
}
static String* AllocFromUtf16(int32_t utf16_length,
@@ -1429,9 +1454,9 @@
uint32_t hash_code_;
- uint32_t offset_;
+ int32_t offset_;
- uint32_t count_;
+ int32_t count_;
static Class* GetJavaLangString() {
DCHECK(java_lang_String_ != NULL);