Skip to content

Conversation

jonpryor
Copy link
Contributor

@jonpryor jonpryor commented Jun 29, 2024

Fixes: #9038

Changes: dotnet/java-interop@ccafbe6...7a058c0

dotnet/java-interop@7a058c0e adds a new IJavaPeerable.JavaAs<T>()
extension method, to perform type casts which return null when
the cast will not succeed instead of throwing an exception.

Update AndroidValueManager.CreatePeer() to check for Java-side
type compatibility (by way of TypeManager.CreateInstance()).
Previously, this would not throw:

var instance        = …
var r               = instance.PeerReference;
var wrap_instance   = JniRuntime.CurrentRuntime.ValueManager.CreatePeer (
        reference: ref r,
        options: JniObjectReferenceOptions.Copy,
        targetType: typeof (SomeInterfaceThatInstanceDoesNotImplement));
// `wrap_instance` is not null, `.CreatePeer()` does not throw

wrap_instance.SomeMethod();
// will throw, due to a type mismatch.

AndroidValueManager.CreatePeer() will now return null if the
Java instance referenced by the reference: parameter is not
convertible to targetType, as per JNIEnv::IsAssignableFrom().

jonpryor added 4 commits July 8, 2024 13:49
Fascinating "corner case" discovered:
`AndroidValueManager.CreatePeer()` didn't do a Java-side type check,
and thus would allow anything to be "cast to" anything.
(The `.JavaCast<T>()` extension method *did* check things, so the
most likely codepath worked properly, but not this one.)

Fix `AndroidValueManager.CreatePeer()`, bump Java.Interop.

Hopefully *now* all unit tests pass.
@jonpryor jonpryor force-pushed the dev/jonp/jonp-try-ji-1234 branch from 599d75e to a69782d Compare July 8, 2024 17:51
@jonpryor
Copy link
Contributor Author

jonpryor commented Jul 8, 2024

Draft commit message:

Bump to dotnet/java-interop/main@7a058c0e

Fixes: https://github.com/dotnet/android/issues/9038

Changes: https://github.com/dotnet/java-interop/compare/ccafbe6a102a85efe84ceb210b3b8b93e49edbcb...7a058c0ee50d39dc5783d2b2770ea5e0f1001a56

  * dotnet/java-interop@7a058c0e: [Java.Interop] Add `IJavaPeerable.JavaAs()` extension method (dotnet/java-interop#1234)
  * dotnet/java-interop@6f9defa5: Bump to dotnet/android-tools/main@3debf8e0 (dotnet/java-interop#1236)
  * dotnet/java-interop@6923fb89: [ci] Add dependabot branches to build triggers (dotnet/java-interop#1233)
  * dotnet/java-interop@573028f3: [ci] Use macOS 13 Ventura build agents (dotnet/java-interop#1235)

dotnet/java-interop@7a058c0e adds a new `IJavaPeerable.JavaAs<T>()`
extension method, to perform type casts which return `null` when
the cast will not succeed instead of throwing an exception.

Update `AndroidValueManager.CreatePeer()` to check for Java-side
type compatibility (by way of `TypeManager.CreateInstance()`).
Previously, this would not throw:

	var instance        = …
	var r               = instance.PeerReference;
	var wrap_instance   = JniRuntime.CurrentRuntime.ValueManager.CreatePeer (
	        reference: ref r,
	        options: JniObjectReferenceOptions.Copy,
	        targetType: typeof (SomeInterfaceThatInstanceDoesNotImplement));
	// `wrap_instance` is not null, `.CreatePeer()` does not throw

	wrap_instance.SomeMethod();
	// will throw, due to a type mismatch.

`AndroidValueManager.CreatePeer()` will now return `null` if the
Java instance referenced by the `reference:` parameter is not
convertible to `targetType`, as per [`JNIEnv::IsAssignableFrom()`][0].

[0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#IsAssignableFrom

@jonpryor jonpryor marked this pull request as ready for review July 8, 2024 20:19
@jonpryor jonpryor changed the title Try dotnet/java-interop#1234 Bump to dotnet/java-interop/main@7a058c0e Jul 8, 2024
@jonpryor jonpryor merged commit 35f41dc into main Jul 8, 2024
@jonpryor jonpryor deleted the dev/jonp/jonp-try-ji-1234 branch July 8, 2024 22:31
grendello added a commit that referenced this pull request Jul 23, 2024
* main: (23 commits)
  Localized file check-in by OneLocBuild Task (#9129)
  [ci] Disable CodeQL on CI/PR pipelines (#9128)
  Refine 16k page alignment support (#9075)
  [build] fix `ConfigureLocalWorkload` target (#9124)
  Bump to NDK r27 (#9020)
  [ci] Use drop service for SDK insertion artifacts  (#9116)
  Fix up all mapping paths (#9121)
  [ci] Fix maestro publishing for stable packages (#9118)
  Bump to dotnet/sdk@2f14fea98b 9.0.100-preview.7.24367.21 (#9108)
  Missing androidx.window.[extensions|sidecar] warnings (#9085)
  [ci] Use sign-artifacts template for macOS signing (#9091)
  [ci] Use DotNetCoreCLI to sign macOS files (#9102)
  [ci] Disable CodeQL on macOS, Linux, non-main jobs (#9111)
  [tests] re-enable `JavaAbstractMethodTest` (#9097)
  [Microsoft.Android.Sdk.ILLink] preserve types with `IJniNameProviderAttribute` (#9099)
  [Mono.Android] Data sharing and Close() overrides (#9103)
  [AndroidManifest] Add `Android.App.PropertyAttribute` (#9016)
  [Mono.Android] Add support for AndroidMessageHandler ClientCertificates (#8961)
  [Mono.Android] Bind and enumify API-35 (#9043)
  Bump to dotnet/java-interop@7a058c0e (#9066)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Aug 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing exception-free JavaCast overload

1 participant