Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All APIs like env.new_*_array() should return a JObject wrapper (like JByteArray) allowing safe JValue conversion #396

Closed
EliseChouleur opened this issue Dec 9, 2022 · 2 comments · Fixed by #400
Milestone

Comments

@EliseChouleur
Copy link

Particulary to be able to call a methode with a byte array in parameter.
It was possible in JNI 0.19 thanks to into but the new conversion methode doesn't work with jbyteArray.

e.g:
jni-rs 0.19

    let buffer = env.new_byte_array(asset_size)?; 
    env.call_method(asset_stream, "read", "([B)I", &[buffer.into()])?;
@rib
Copy link
Contributor

rib commented Dec 9, 2022

Ah, right, I guess this was affected when addressing the soundness issues with allowing safe conversions from sys types: #197

The odd thing here is that jni-rs is directly returning a sys type from env.new_byte_array instead of a type like JObject that has an associated lifetime for the local reference.

We should also track this in the context of #392 where we're looking to ensure that local reference types are non-Copy.

Since the array is effectively just an object you can pass your array to method calls like this for now:

    let buffer = env.new_byte_array(asset_size)?;
    let buffer_obj = unsafe { JObject::from_raw(buffer) };
    env.call_method(stream, "read", "([B)I", &[buffer_obj.into()])?;

We probably shouldn't directly allow safe conversion between jbyteArray and JValue - similar to not allowing safe conversions of jobject to JObject otherwise safe code can can easily pass literal numbers as an invalid pointers.

What we probably need to do here is introduce a JByteArray<'env> type similar to JString or JClass that would be returned by env.new_byte_array() and then there should be a safe way to convert a JByteArray into a JValue (which should make your original code work again)

@rib rib changed the title Convert jbyteArray to JValue All APIs like env.new_*_array() should return a JObject wrapper (like JByteArray) allowing safe JValue conversion Dec 9, 2022
@rib rib added this to the 0.21 milestone Dec 9, 2022
@EliseChouleur
Copy link
Author

Hello
Thanks for the temporary fix !

rib added a commit to rib/jni-rs that referenced this issue Jan 18, 2023
rib added a commit to rib/jni-rs that referenced this issue Jan 18, 2023
Instead of all the array-based APIs being based on sys types like
`jbyteArray` (which can easily lead to the use of invalid local
references) this introduces types like `JByteArray<'local>` that
have a lifetime parameter to ensure the local reference can't
be read if it is deleted or goes out of scope.

`JByteArray` and `JIntArray` etc are based on a generic
`JPrimitiveArray<T>` which takes any of the primitive types that
may be put into arrays, like `jbyte` or `jint`.

`JObjectArray` is handled separately, considering that it can't be used
with `AutoArray` or `AutoPrimitiveArray`.

The implementations of `AutoArray` and `AutoPrimitiveArray` were updated
so they can be created based on a `JPrimitiveAarray`.

While updating the lifetime parameters to the `AutoArray` and
`AutoPrimitiveArray` APIs numerous implementation inconsistencies were
found, even though they should only really differ in how they acquire /
release the pointer to array elements. This patch also normalizes the
implementation of both of these APIs, so they are easily comparable. It
may even be possible to unify these at some point.

Closes; jni-rs#396
Closes: jni-rs#41
rib added a commit to rib/jni-rs that referenced this issue Jan 19, 2023
Instead of all the array-based APIs being based on sys types like
`jbyteArray` (which can easily lead to the use of invalid local
references) this introduces types like `JByteArray<'local>` that
have a lifetime parameter to ensure the local reference can't
be read if it is deleted or goes out of scope.

`JByteArray` and `JIntArray` etc are based on a generic
`JPrimitiveArray<T>` which takes any of the primitive types that
may be put into arrays, like `jbyte` or `jint`.

`JObjectArray` is handled separately, considering that it can't be used
with `AutoArray` or `AutoPrimitiveArray`.

The implementations of `AutoArray` and `AutoPrimitiveArray` were updated
so they can be created based on a `JPrimitiveAarray`.

While updating the lifetime parameters to the `AutoArray` and
`AutoPrimitiveArray` APIs numerous implementation inconsistencies were
found, even though they should only really differ in how they acquire /
release the pointer to array elements. This patch also normalizes the
implementation of both of these APIs, so they are easily comparable. It
may even be possible to unify these at some point.

Closes; jni-rs#396
Closes: jni-rs#41
rib added a commit to rib/jni-rs that referenced this issue Jan 19, 2023
Instead of all the array-based APIs being based on sys types like
`jbyteArray` (which can easily lead to the use of invalid local
references) this introduces types like `JByteArray<'local>` that
have a lifetime parameter to ensure the local reference can't
be read if it is deleted or goes out of scope.

`JByteArray` and `JIntArray` etc are based on a generic
`JPrimitiveArray<T>` which takes any of the primitive types that
may be put into arrays, like `jbyte` or `jint`.

Since types like `JByteArray` are simply aliases for `JPrimitiveArray<T>`
there's no need to keep separate `JNIEnv::get_<type>_array_elements`
methods since they can all be passed to `JNIEnv::get_array_elements`.

`JObjectArray` is handled separately, considering that it can't be used
with `AutoArray` or `AutoPrimitiveArray`.

The implementations of `AutoArray` and `AutoPrimitiveArray` were updated
so they can be created based on a `JPrimitiveAarray`.

While updating the lifetime parameters to the `AutoArray` and
`AutoPrimitiveArray` APIs numerous implementation inconsistencies were
found, even though they should only really differ in how they acquire /
release the pointer to array elements. This patch also normalizes the
implementation of both of these APIs, so they are easily comparable. It
may even be possible to unify these at some point.

For now the `test_get_array_elements` tests have been kept at least for
the sake of checking the data integrity from updating array elements of
different types/sizes, even though they now utilize the same
`get_array_elements` code.

Closes; jni-rs#396
Closes: jni-rs#41
rib added a commit to rib/jni-rs that referenced this issue Jan 20, 2023
Instead of all the array-based APIs being based on sys types like
`jbyteArray` (which can easily lead to the use of invalid local
references) this introduces types like `JByteArray<'local>` that
have a lifetime parameter to ensure the local reference can't
be read if it is deleted or goes out of scope.

`JByteArray` and `JIntArray` etc are based on a generic
`JPrimitiveArray<T>` which takes any of the primitive types that
may be put into arrays, like `jbyte` or `jint`.

Since types like `JByteArray` are simply aliases for `JPrimitiveArray<T>`
there's no need to keep separate `JNIEnv::get_<type>_array_elements`
methods since they can all be passed to `JNIEnv::get_array_elements`.

`JObjectArray` is handled separately, considering that it can't be used
with `AutoArray` or `AutoPrimitiveArray`.

The implementations of `AutoArray` and `AutoPrimitiveArray` were updated
so they can be created based on a `JPrimitiveAarray`.

While updating the lifetime parameters to the `AutoArray` and
`AutoPrimitiveArray` APIs numerous implementation inconsistencies were
found, even though they should only really differ in how they acquire /
release the pointer to array elements. This patch also normalizes the
implementation of both of these APIs, so they are easily comparable. It
may even be possible to unify these at some point.

For now the `test_get_array_elements` tests have been kept at least for
the sake of checking the data integrity from updating array elements of
different types/sizes, even though they now utilize the same
`get_array_elements` code.

Closes; jni-rs#396
Closes: jni-rs#41
rib added a commit to rib/jni-rs that referenced this issue Jan 22, 2023
Instead of all the array-based APIs being based on sys types like
`jbyteArray` (which can easily lead to the use of invalid local
references) this introduces types like `JByteArray<'local>` that
have a lifetime parameter to ensure the local reference can't
be read if it is deleted or goes out of scope.

`JByteArray` and `JIntArray` etc are based on a generic
`JPrimitiveArray<T>` which takes any of the primitive types that
may be put into arrays, like `jbyte` or `jint`.

Since types like `JByteArray` are simply aliases for `JPrimitiveArray<T>`
there's no need to keep separate `JNIEnv::get_<type>_array_elements`
methods since they can all be passed to `JNIEnv::get_array_elements`.

`JObjectArray` is handled separately, considering that it can't be used
with `AutoArray` or `AutoPrimitiveArray`.

The implementations of `AutoArray` and `AutoPrimitiveArray` were updated
so they can be created based on a `JPrimitiveAarray`.

While updating the lifetime parameters to the `AutoArray` and
`AutoPrimitiveArray` APIs numerous implementation inconsistencies were
found, even though they should only really differ in how they acquire /
release the pointer to array elements. This patch also normalizes the
implementation of both of these APIs, so they are easily comparable. It
may even be possible to unify these at some point.

For now the `test_get_array_elements` tests have been kept at least for
the sake of checking the data integrity from updating array elements of
different types/sizes, even though they now utilize the same
`get_array_elements` code.

Closes; jni-rs#396
Closes: jni-rs#41
rib added a commit to rib/jni-rs that referenced this issue Jan 23, 2023
Instead of all the array-based APIs being based on sys types like
`jbyteArray` (which can easily lead to the use of invalid local
references) this introduces types like `JByteArray<'local>` that
have a lifetime parameter to ensure the local reference can't
be read if it is deleted or goes out of scope.

`JByteArray` and `JIntArray` etc are based on a generic
`JPrimitiveArray<T>` which takes any of the primitive types that
may be put into arrays, like `jbyte` or `jint`.

Since types like `JByteArray` are simply aliases for `JPrimitiveArray<T>`
there's no need to keep separate `JNIEnv::get_<type>_array_elements`
methods since they can all be passed to `JNIEnv::get_array_elements`.

`JObjectArray` is handled separately, considering that it can't be used
with `AutoArray` or `AutoPrimitiveArray`.

The implementations of `AutoArray` and `AutoPrimitiveArray` were updated
so they can be created based on a `JPrimitiveAarray`.

While updating the lifetime parameters to the `AutoArray` and
`AutoPrimitiveArray` APIs numerous implementation inconsistencies were
found, even though they should only really differ in how they acquire /
release the pointer to array elements. This patch also normalizes the
implementation of both of these APIs, so they are easily comparable. It
may even be possible to unify these at some point.

For now the `test_get_array_elements` tests have been kept at least for
the sake of checking the data integrity from updating array elements of
different types/sizes, even though they now utilize the same
`get_array_elements` code.

Closes; jni-rs#396
Closes: jni-rs#41
rib added a commit to rib/jni-rs that referenced this issue Jan 24, 2023
Instead of all the array-based APIs being based on sys types like
`jbyteArray` (which can easily lead to the use of invalid local
references) this introduces types like `JByteArray<'local>` that
have a lifetime parameter to ensure the local reference can't
be read if it is deleted or goes out of scope.

`JByteArray` and `JIntArray` etc are based on a generic
`JPrimitiveArray<T>` which takes any of the primitive types that
may be put into arrays, like `jbyte` or `jint`.

Since types like `JByteArray` are simply aliases for `JPrimitiveArray<T>`
there's no need to keep separate `JNIEnv::get_<type>_array_elements`
methods since they can all be passed to `JNIEnv::get_array_elements`.

`JObjectArray` is handled separately, considering that it can't be used
with `AutoArray` or `AutoPrimitiveArray`.

The implementations of `AutoArray` and `AutoPrimitiveArray` were updated
so they can be created based on a `JPrimitiveAarray`.

While updating the lifetime parameters to the `AutoArray` and
`AutoPrimitiveArray` APIs numerous implementation inconsistencies were
found, even though they should only really differ in how they acquire /
release the pointer to array elements. This patch also normalizes the
implementation of both of these APIs, so they are easily comparable. It
may even be possible to unify these at some point.

For now the `test_get_array_elements` tests have been kept at least for
the sake of checking the data integrity from updating array elements of
different types/sizes, even though they now utilize the same
`get_array_elements` code.

Closes; jni-rs#396
Closes: jni-rs#41
rib added a commit to rib/jni-rs that referenced this issue Jan 31, 2023
Instead of all the array-based APIs being based on sys types like
`jbyteArray` (which can easily lead to the use of invalid local
references) this introduces types like `JByteArray<'local>` that
have a lifetime parameter to ensure the local reference can't
be read if it is deleted or goes out of scope.

`JByteArray` and `JIntArray` etc are based on a generic
`JPrimitiveArray<T>` which takes any of the primitive types that
may be put into arrays, like `jbyte` or `jint`.

Since types like `JByteArray` are simply aliases for `JPrimitiveArray<T>`
there's no need to keep separate `JNIEnv::get_<type>_array_elements`
methods since they can all be passed to `JNIEnv::get_array_elements`.

`JObjectArray` is handled separately, considering that it can't be used
with `AutoArray` or `AutoPrimitiveArray`.

The implementations of `AutoArray` and `AutoPrimitiveArray` were updated
so they can be created based on a `JPrimitiveAarray`.

While updating the lifetime parameters to the `AutoArray` and
`AutoPrimitiveArray` APIs numerous implementation inconsistencies were
found, even though they should only really differ in how they acquire /
release the pointer to array elements. This patch also normalizes the
implementation of both of these APIs, so they are easily comparable. It
may even be possible to unify these at some point.

For now the `test_get_array_elements` tests have been kept at least for
the sake of checking the data integrity from updating array elements of
different types/sizes, even though they now utilize the same
`get_array_elements` code.

Closes; jni-rs#396
Closes: jni-rs#41
rib added a commit to rib/jni-rs that referenced this issue Jan 31, 2023
Instead of all the array-based APIs being based on sys types like
`jbyteArray` (which can easily lead to the use of invalid local
references) this introduces types like `JByteArray<'local>` that
have a lifetime parameter to ensure the local reference can't
be read if it is deleted or goes out of scope.

`JByteArray` and `JIntArray` etc are based on a generic
`JPrimitiveArray<T>` which takes any of the primitive types that
may be put into arrays, like `jbyte` or `jint`.

Since types like `JByteArray` are simply aliases for `JPrimitiveArray<T>`
there's no need to keep separate `JNIEnv::get_<type>_array_elements`
methods since they can all be passed to `JNIEnv::get_array_elements`.

`JObjectArray` is handled separately, considering that it can't be used
with `AutoArray` or `AutoPrimitiveArray`.

The implementations of `AutoArray` and `AutoPrimitiveArray` were updated
so they can be created based on a `JPrimitiveAarray`.

While updating the lifetime parameters to the `AutoArray` and
`AutoPrimitiveArray` APIs numerous implementation inconsistencies were
found, even though they should only really differ in how they acquire /
release the pointer to array elements. This patch also normalizes the
implementation of both of these APIs, so they are easily comparable. It
may even be possible to unify these at some point.

For now the `test_get_array_elements` tests have been kept at least for
the sake of checking the data integrity from updating array elements of
different types/sizes, even though they now utilize the same
`get_array_elements` code.

Closes; jni-rs#396
Closes: jni-rs#41
rib added a commit to rib/jni-rs that referenced this issue Feb 1, 2023
Instead of all the array-based APIs being based on sys types like
`jbyteArray` (which can easily lead to the use of invalid local
references) this introduces types like `JByteArray<'local>` that
have a lifetime parameter to ensure the local reference can't
be read if it is deleted or goes out of scope.

`JByteArray` and `JIntArray` etc are based on a generic
`JPrimitiveArray<T>` which takes any of the primitive types that
may be put into arrays, like `jbyte` or `jint`.

Since types like `JByteArray` are simply aliases for `JPrimitiveArray<T>`
there's no need to keep separate `JNIEnv::get_<type>_array_elements`
methods since they can all be passed to `JNIEnv::get_array_elements`.

`JObjectArray` is handled separately, considering that it can't be used
with `AutoArray` or `AutoPrimitiveArray`.

The implementations of `AutoArray` and `AutoPrimitiveArray` were updated
so they can be created based on a `JPrimitiveAarray`.

While updating the lifetime parameters to the `AutoArray` and
`AutoPrimitiveArray` APIs numerous implementation inconsistencies were
found, even though they should only really differ in how they acquire /
release the pointer to array elements. This patch also normalizes the
implementation of both of these APIs, so they are easily comparable. It
may even be possible to unify these at some point.

For now the `test_get_array_elements` tests have been kept at least for
the sake of checking the data integrity from updating array elements of
different types/sizes, even though they now utilize the same
`get_array_elements` code.

Closes; jni-rs#396
Closes: jni-rs#41
rib added a commit to rib/jni-rs that referenced this issue Feb 1, 2023
Instead of all the array-based APIs being based on sys types like
`jbyteArray` (which can easily lead to the use of invalid local
references) this introduces types like `JByteArray<'local>` that
have a lifetime parameter to ensure the local reference can't
be read if it is deleted or goes out of scope.

`JByteArray` and `JIntArray` etc are based on a generic
`JPrimitiveArray<T>` which takes any of the primitive types that
may be put into arrays, like `jbyte` or `jint`.

Since types like `JByteArray` are simply aliases for `JPrimitiveArray<T>`
there's no need to keep separate `JNIEnv::get_<type>_array_elements`
methods since they can all be passed to `JNIEnv::get_array_elements`.

`JObjectArray` is handled separately, considering that it can't be used
with `AutoArray` or `AutoPrimitiveArray`.

The implementations of `AutoArray` and `AutoPrimitiveArray` were updated
so they can be created based on a `JPrimitiveAarray`.

While updating the lifetime parameters to the `AutoArray` and
`AutoPrimitiveArray` APIs numerous implementation inconsistencies were
found, even though they should only really differ in how they acquire /
release the pointer to array elements. This patch also normalizes the
implementation of both of these APIs, so they are easily comparable. It
may even be possible to unify these at some point.

For now the `test_get_array_elements` tests have been kept at least for
the sake of checking the data integrity from updating array elements of
different types/sizes, even though they now utilize the same
`get_array_elements` code.

Closes; jni-rs#396
Closes: jni-rs#41
rib added a commit to rib/jni-rs that referenced this issue Feb 1, 2023
Instead of all the array-based APIs being based on sys types like
`jbyteArray` (which can easily lead to the use of invalid local
references) this introduces types like `JByteArray<'local>` that
have a lifetime parameter to ensure the local reference can't
be read if it is deleted or goes out of scope.

`JByteArray` and `JIntArray` etc are based on a generic
`JPrimitiveArray<T>` which takes any of the primitive types that
may be put into arrays, like `jbyte` or `jint`.

Since types like `JByteArray` are simply aliases for `JPrimitiveArray<T>`
there's no need to keep separate `JNIEnv::get_<type>_array_elements`
methods since they can all be passed to `JNIEnv::get_array_elements`.

`JObjectArray` is handled separately, considering that it can't be used
with `AutoArray` or `AutoPrimitiveArray`.

The implementations of `AutoArray` and `AutoPrimitiveArray` were updated
so they can be created based on a `JPrimitiveAarray`.

While updating the lifetime parameters to the `AutoArray` and
`AutoPrimitiveArray` APIs numerous implementation inconsistencies were
found, even though they should only really differ in how they acquire /
release the pointer to array elements. This patch also normalizes the
implementation of both of these APIs, so they are easily comparable. It
may even be possible to unify these at some point.

For now the `test_get_array_elements` tests have been kept at least for
the sake of checking the data integrity from updating array elements of
different types/sizes, even though they now utilize the same
`get_array_elements` code.

Closes; jni-rs#396
Closes: jni-rs#41
@rib rib closed this as completed in #400 Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants