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

[Android] Use Android SDK instead of OpenSSL where possible #43740

Merged
merged 23 commits into from
Nov 12, 2020

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Oct 23, 2020

This PR redirects all RandomNumberGenerator, MD5, Sha1-512, HMAC and AES APIs to use Android SDK instead of OpenSSL.

@ghost
Copy link

ghost commented Oct 23, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@steveisok steveisok self-requested a review October 23, 2020 14:31
@jonpryor
Copy link
Member

Overall Aside: you cannot rely on "normal" JNI Local Reference semantics, in which JNI Local References are "automatically freed." You can't rely on this for two reasons:

  1. The "root stack frame" is not Java. The "auto release of JNI Local References" only happens when returning to Java. Here, you're (presumably) P/Invoking from C# into a C/C++ function which calls into Java. No "auto release" will occur.
  2. Android only allows ~512 JNI local references to exist at a time. If you exceed this count, the process will assert & exit.

Consequently, every time you invoke a JNI method which returns a JNI local reference -- which is most of them! -- you must ensure that you call JNIEnv::DeleteLocalRef() on the value. This includes jclass and jstring values, but does not include jmethodID values.

@EgorBo
Copy link
Member Author

EgorBo commented Oct 23, 2020

@jonpryor thank you so much, amazing feedback!

@EgorBo EgorBo marked this pull request as ready for review November 10, 2020 23:15
@EgorBo
Copy link
Member Author

EgorBo commented Nov 11, 2020

@jonpryor could you please re-review?

I've added error handling here and there but in general, the managed code on top of it is responsible for input validation like buffers' lengths, non-negative or out-of-bounds indices, etc.

@EgorBo EgorBo merged commit 5af16f2 into dotnet:master Nov 12, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants