Skip to content

Conversation

@jonpryor
Copy link
Contributor

Context: dotnet/android#7285 (comment)

Java.Inteorp.JniEnvironment.Types.RegisterNatives() is overloaded:

namespace Java.Interop {
  partial class JniEnvironment {
    partial class Types {
      public static void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration[] methods);
      public static void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration[] methods, int numMethods);
    }
  }
}

If the int numMethods overload is used, then:

  1. it should be possible to pass an array which contains more than
    numMethods elements, and
  2. Passing such an array shouldn't throw an exception.

For example:

var methods = new JniNativeMethodRegistration [10];
JniEnvironment.Types.RegisterNatives (type, methods, 0);

Instead, when using a Debug configuration build of Java.Interop.dll,
a NullReferenceException would be thrown, because the foreach
loop would traverse every element, not just the first numMethods
elements, which could result in accessing
methods[i].Marshaler.GetType(), which could be null.

Fix the DEBUG && NETCOREAPP check so that only the first
numMethods elements are accessed, not all of them, and add a
null check around JniNativeMethodRegistration.Marshaler access.

This prevents the NullReferenceException.

@jonpryor jonpryor force-pushed the jonp-fix-registernatives-numMethods branch from bdc2053 to 6749463 Compare August 23, 2022 18:16
`Java.Inteorp.JniEnvironment.Types.RegisterNatives()` is overloaded:

	namespace Java.Interop {
	  partial class JniEnvironment {
	    partial class Types {
	      public static void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration[] methods);
	      public static void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration[] methods, int numMethods);
	    }
	  }
	}

If the `int numMethods` overload is used, then:

 1. it should be possible to pass an array which contains *more* than
    `numMethods` elements, and
 2. Passing such an array shouldn't throw an exception.

For example:

	var methods = new JniNativeMethodRegistration [10];
	JniEnvironment.Types.RegisterNatives (type, methods, 0);

Instead, when using a Debug configuration build of `Java.Interop.dll`,
a `NullReferenceException` would be thrown, because the `foreach`
loop would traverse *every element*, not just the first `numMethods`
elements, which could result in accessing
`methods[i].Marshaler.GetType()`, which could be `null`.

Fix the `DEBUG && NETCOREAPP` check so that only the first
`numMethods` elements are accessed, *not* all of them, and add a
`null` check around `JniNativeMethodRegistration.Marshaler` access.

This prevents the `NullReferenceException`.

Additionally, add some parameter validation and throw an
`ArgumentOutOfRangeException` if `numMethods` is negative or is
greater than `methods.Length`.
@jonpryor jonpryor force-pushed the jonp-fix-registernatives-numMethods branch from 6749463 to 42e652a Compare August 23, 2022 18:50
@jonpryor jonpryor merged commit e31d9c6 into dotnet:main Aug 23, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 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.

2 participants