-
Notifications
You must be signed in to change notification settings - Fork 21
Description
Default implementation for TryFromJavaValue looks like the following:
#[automatically_derived]
impl<
'env: 'borrow,
'borrow,
> ::robusta_jni::convert::TryFromJavaValue<'env, 'borrow>
for RubyModule<'env, 'borrow> {
type Source = ::robusta_jni::jni::objects::JObject<'env>;
fn try_from(
source: Self::Source,
env: &'borrow ::robusta_jni::jni::JNIEnv<'env>,
) -> ::robusta_jni::jni::errors::Result<Self> {
Ok(Self {
raw: ::robusta_jni::jni::objects::AutoLocal::new(
env,
source,
),
})
}
}it never fails and isn't actually safe. It allows to screw up with types and to write something like
let xml_module: jni::RubyModule = robusta_jni::convert::TryFromJavaValue::try_from(JObject::from(env.new_string("Xml").unwrap()), &env).unwrap();and it will compile and fail in runtime somewhere xml_module is used. If xml_module is used as a caller, java.lang.NoSuchMethodError will be - most likely - thrown, but if it's used as an argument, things become even worse, because Java doesn't check if arguments really match the signature provided, so it'll crash with some cryptic error - java.lang.NullPointerException: Cannot read the array length because "cache" is null - or even segfault.
So, maybe use is_instance_of/is_assignable_from check in TryFromJavaValue, at least in debug builds? Or mark try_from as unsafe?
(Yes, I see that things like JClass::from aren't safe and unsafe too, probably it should be changed too)