Skip to content

Guard primitive array value-manager routing - #12114

Open
simonrozsival wants to merge 4 commits into
mainfrom
dev/simonrozsival/fix-valuemanager-array-routing
Open

Guard primitive array value-manager routing#12114
simonrozsival wants to merge 4 commits into
mainfrom
dev/simonrozsival/fix-valuemanager-array-routing

Conversation

@simonrozsival

@simonrozsival simonrozsival commented Jul 15, 2026

Copy link
Copy Markdown
Member

Summary

  • require the JNI runtime type to match the selected primitive-array marshaler before invoking array JNI APIs
  • route non-array IList<T> handles through JavaConvert only when JNI confirms the object implements java.util.List
  • apply the equivalent exact JNI signature and List-assignability guards to the trimmable PrimitiveArrayInfo path
  • honor JniObjectReferenceOptions before all new invalid-cast failures

CheckJNI evidence and safety boundary

The reproducer passed a java.util.ArrayList handle while requesting IList<bool>. The reflection value manager selected JavaBooleanArray.ArrayMarshaler from the managed target type alone, leading CheckJNI to abort the process with jarray argument has non-array type: java.util.ArrayList. This change establishes the safety boundary before any array JNI call: only the matching primitive array signature (for example, [Z) may enter that marshaler. A non-array reference may enter collection conversion only when JNI IsInstanceOf confirms java.util.List; other objects and mismatched arrays fail with InvalidCastException. Failure paths dispose and invalidate transferred references according to the requested ownership options.

Retained behavior

A genuine Java primitive array still marshals to IList<T>, and genuine Java List implementations still use JavaConvert. The guard compares the actual JNI runtime type rather than rejecting IList<T> generally.

Tests

Added on-device coverage in JavaConvertTest for:

  • java.util.ArrayList<Boolean> handle -> IList<bool>
  • Java boolean[] handle -> IList<bool>
  • Java String handle -> IList<bool> safely throws, preserving Copy and consuming CopyAndDispose
  • Java int[] handle -> IList<bool> safely throws and consumes CopyAndDispose

Validation:

  • dotnet build external/Java.Interop/src/Java.Interop/Java.Interop.csproj ... passed with 0 warnings/errors after the review fixes.
  • On-device tests were not run locally because this worktree has no prepared local SDK (make prepare is required).
  • A direct Mono.Android.csproj build reaches the repository prerequisite failure because bin/BuildDebug/net10.0/xa-prep-tasks.dll is absent.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0e043be9-9070-43bc-aea4-a48438690bdf
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0e043be9-9070-43bc-aea4-a48438690bdf
@simonrozsival

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot AI commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved the merge conflict with origin/main in commit 3b58d52b. The resolution keeps the primitive-array ValueManager regression tests and the new upstream FromJniHandle collection tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival
simonrozsival marked this pull request as ready for review August 21, 2026 10:31
Copilot AI lite review requested due to automatic review settings August 21, 2026 10:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds defensive JNI type guards around primitive-array marshaling so IList<T> conversions don’t accidentally invoke array JNI APIs on non-array objects (avoiding CheckJNI aborts), while still allowing java.util.List objects to be converted via JavaConvert when appropriate.

Changes:

  • In ReflectionJniValueManager, gate primitive-array marshalers on the actual JNI runtime type signature (e.g. only [Z may enter the boolean-array marshaler), and route non-array IList<T> conversions via a new CreateNonArrayListValue hook only when JNI confirms java.util.List.
  • Apply equivalent guards to the trimmable typemap path (PrimitiveArrayInfo).
  • Add on-device regression tests covering valid conversions and invalid-cast/ownership behavior for IList<bool> scenarios.
Show a summary per file
File Description
tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaConvertTest.cs Adds on-device regression tests for list/primitive-array IList<bool> conversions and failure/ownership behaviors.
src/Mono.Android/Microsoft.Android.Runtime/PrimitiveArrayInfo.cs Adds JNI runtime-type guard for primitive array wrappers and allows java.util.List to fall back to JavaConvert for IList<T>.
src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalValueManager.cs Overrides new CreateNonArrayListValue hook to route guarded non-array list conversions through JavaConvert.
src/Mono.Android/Android.Runtime/AndroidRuntime.cs Same override as above for the runtime value manager used by AndroidRuntime.
external/Java.Interop/src/Java.Interop/PublicAPI.Unshipped.txt Declares the newly added virtual ReflectionJniValueManager.CreateNonArrayListValue(...) API.
external/Java.Interop/src/Java.Interop/Java.Interop/JniRuntime.ReflectionJniValueManager.cs Implements primitive-array signature checks, java.util.List assignability gate, and new CreateNonArrayListValue extensibility point.

Review details

  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment on lines +187 to +190
var reference = new JniObjectReference (JNIEnv.NewArray (new [] { true, false }), JniObjectReferenceType.Local);
var converted = JniEnvironment.Runtime.ValueManager.GetValue<IList<bool>> (ref reference, JniObjectReferenceOptions.CopyAndDispose);

CollectionAssert.AreEqual (new [] { true, false }, converted);
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 this pull request may close these issues.

3 participants