Attempt at fixing https://github.com/robolectric/robolectric/issues/8242

[alwa]

Overview

#8242

Proposed Changes

Add extra interceptor for Cipher to handle getCurrentSpi()

[utzcoz]

@alixwar please squash to one commit.

[alwa]

Done @utzcoz

[utzcoz]

@alixwar There are some code formatting errors. Please check https://github.com/robolectric/robolectric/wiki/Robolectric's-code-style how to fix it.

[utzcoz]

And I prefer to write a more reasonable commit message like "Intercept Cipher#getCurrentSpi to avoid running error".

[utzcoz]

@alwa Do you test it with local snapshot building for your project? Can it fix your issue?

[utzcoz]

@alwa This PR still has formatting issues. I recommend you to run it locally to ensure code formatting job will happy to reduce feedback time. See the abvoe link I posted.

[utzcoz]

Please fix code formatting for AndroidInterceptors.java.

[alwa]

These instructions are not Windows-friendly, so I can't really follow them: https://github.com/robolectric/robolectric/wiki/Running-google-java-format

Since this is a trivial change I'm sure I can fix the formatting manually if I can just understand what's wrong with my changes.

Update: OK, I think I got it now. I removed some indentation that caused the check to fail. This project does not play well with my own formatting rules in the IDE.

I have run gradlew spotlessCheck (not sure it covers what I changed though...):

[utzcoz]

@alwa I think only curl command needs to be changed on Windows. But you can download script and jar files directly, and then running it with git + python.

[utzcoz]

--- sandbox/src/main/java/org/robolectric/interceptors/AndroidInterceptors.java	(before formatting)
+++ sandbox/src/main/java/org/robolectric/interceptors/AndroidInterceptors.java	(after formatting)
@@ -[26](https://github.com/robolectric/robolectric/actions/runs/5144102417/jobs/9261458810?pr=8250#step:6:27),7 +26,6 @@
 import javax.annotation.Nullable;
 import javax.crypto.Cipher;
 import javax.crypto.CipherSpi;
-
 import org.robolectric.fakes.CleanerCompat;
 import org.robolectric.internal.bytecode.Interceptor;
 import org.robolectric.internal.bytecode.MethodRef;
@@ -492,7 +491,7 @@
 
     @Override
     public MethodHandle getMethodHandle(String methodName, MethodType type)
-            throws NoSuchMethodException, IllegalAccessException {
+        throws NoSuchMethodException, IllegalAccessException {
       return lookup.findStatic(
           getClass(), "getCurrentSpi", methodType(CipherSpi.class, void.class));
     }

They are left formatting errors. I still recommend you to run it locally.

[alwa]

--- sandbox/src/main/java/org/robolectric/interceptors/AndroidInterceptors.java	(before formatting)
+++ sandbox/src/main/java/org/robolectric/interceptors/AndroidInterceptors.java	(after formatting)
@@ -[26](https://github.com/robolectric/robolectric/actions/runs/5144102417/jobs/9261458810?pr=8250#step:6:27),7 +26,6 @@
 import javax.annotation.Nullable;
 import javax.crypto.Cipher;
 import javax.crypto.CipherSpi;
-
 import org.robolectric.fakes.CleanerCompat;
 import org.robolectric.internal.bytecode.Interceptor;
 import org.robolectric.internal.bytecode.MethodRef;
@@ -492,7 +491,7 @@
 
     @Override
     public MethodHandle getMethodHandle(String methodName, MethodType type)
-            throws NoSuchMethodException, IllegalAccessException {
+        throws NoSuchMethodException, IllegalAccessException {
       return lookup.findStatic(
           getClass(), "getCurrentSpi", methodType(CipherSpi.class, void.class));
     }

They are left formatting errors. I still recommend you to run it locally.

It wasn't easy but I managed to run it locally now

[alwa]

@utzcoz

graphics_tests
Started 4h 25m 1s ago

Is it stuck? Can it be restarted?

[utzcoz]

@utzcoz

graphics_tests
Started 4h 25m 1s ago

Is it stuck? Can it be restarted?

Never mind. It hangs because of M1 GitHub Action configuration changed, and the self-hosted M1 machine doesn't update. I have another question, do you test it with local snapshot for your demo?

[utzcoz]

LGTM.

[utzcoz]

@alwa Thanks for your contribution. Please test it with official robolectric 11 snapshot, and give feedback here. Thanks.

[alwa]

I have issues building the project locally:

gradlew clean assemble

Missing file org/robolectric/Shadows.html in C:\Users\Alix\StudioProjects\robolectric\shadows\framework\build\libs\shadows-framework-4.11-SNAPSHOT-javadoc.jar

I tried with both JDK 8 and 11.

In the build log I see many errors related to javadoc. Example error from the stack trace:

C:\Users\Alix\StudioProjects\robolectric\shadows\framework\src\main\java\org\robolectric\shadows\MediaCodecInfoBuilder.java:223: error: unexpected text
     * @throws {@link NullPointerException} if profileLevels is null.
       ^
error: An internal exception has occurred.
        (java.lang.IllegalStateException: ERRONEOUS)
Please file a bug against the javadoc tool via the Java bug reporting page
(https://bugreport.java.com) after checking the Bug Database (https://bugs.java.com)
for duplicates. Include error messages and the following diagnostic in your report. Thank you.
java.lang.IllegalStateException: ERRONEOUS
        at jdk.javadoc/jdk.javadoc.internal.doclets.formats.html.HtmlDocletWriter.seeTagToContent(HtmlDocletWriter.java:1019)
        at jdk.javadoc/jdk.javadoc.internal.doclets.formats.html.TagletWriterImpl.seeTagOutput(TagletWriterImpl.java:318)    

[utzcoz]

@alwa You need to use JDK 9+, and JDK 11 is very recommended. These errors are related to JavaDoc, you can omit them if you don't want to help improve JavaDoc.

[alwa]

Thanks. Building with JDK11 works if I skip the javadoc step. I'm not sure what needs to be fixed locally for this to also work but in the context of this bug fix it is enough if I can deploy the main jar locally.

[alwa]

This is disheartening, it looks as if I can still reproduce the issue:

'javax.crypto.CipherSpi javax.crypto.Cipher.getCurrentSpi()'
java.lang.NoSuchMethodError: 'javax.crypto.CipherSpi javax.crypto.Cipher.getCurrentSpi()'
	at android.security.keystore.AndroidKeyStoreProvider.getKeyStoreOperationHandle(AndroidKeyStoreProvider.java:176)
	at android.hardware.biometrics.CryptoObject.getOpId(CryptoObject.java:95)

What I did:

1. In robolectric project, locally commented the javadocJar artifact and:

gradlew clean assemble
gradlew publishToMavenLocal

2. In my project with the failing tests, changed robolectric dependency version to 4.11-SNAPSHOT and added mavenLocal() as repository

In my IDE I can see that the robolectric dependencies are listed correctly as belonging to version 4.11-SNAPSHOT.

But I'll make some more checks to really confirm this. We have some shadows and rules that may interfere...

[utzcoz]

@alwa You can use online 4.11-snapshot now. See the README.md to learn how to use it.

[alwa]

Same issue I'm afraid. Full stacktrace (my company class names obfuscated):

'javax.crypto.CipherSpi javax.crypto.Cipher.getCurrentSpi()'
java.lang.NoSuchMethodError: 'javax.crypto.CipherSpi javax.crypto.Cipher.getCurrentSpi()'
	at android.security.keystore.AndroidKeyStoreProvider.getKeyStoreOperationHandle(AndroidKeyStoreProvider.java:176)
	at android.hardware.biometrics.CryptoObject.getOpId(CryptoObject.java:95)
	at android.hardware.biometrics.BiometricPrompt.authenticateInternal(BiometricPrompt.java:937)
	at android.hardware.biometrics.BiometricPrompt.authenticate(BiometricPrompt.java:865)
	at androidx.biometric.BiometricFragment$Api28Impl.authenticate(BiometricFragment.java:1287)
	at androidx.biometric.BiometricFragment.authenticateWithBiometricPrompt(BiometricFragment.java:579)
	at androidx.biometric.BiometricFragment.showBiometricPromptForAuthentication(BiometricFragment.java:522)
	at androidx.biometric.BiometricFragment.showPromptForAuthentication(BiometricFragment.java:433)
	at androidx.biometric.BiometricFragment.authenticate(BiometricFragment.java:409)
	at androidx.biometric.BiometricPrompt.authenticateInternal(BiometricPrompt.java:1033)
	at androidx.biometric.BiometricPrompt.authenticate(BiometricPrompt.java:996)
	at mycompany.myapp.biometrics.BiometricsDataManager.retrieveData$suspendImpl(BiometricsDataManager.kt:93)
	at mycompany.myapp.biometrics.BiometricsDataManager.retrieveData(BiometricsDataManager.kt)
	at mycompany.myapp.biometrics.BiometricsDataManagerTest$retrieveData - when device is secure and stored data - shows fragment$1.invokeSuspend(BiometricsDataManagerTest.kt:208)
	at mycompany.myapp.biometrics.BiometricsDataManagerTest$retrieveData - when device is secure and stored data - shows fragment$1.invoke(BiometricsDataManagerTest.kt)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt$runTestCoroutine$2.invokeSuspend(TestBuilders.kt:212)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt$runTestCoroutine$2.invoke(TestBuilders.kt)
	at kotlinx.coroutines.intrinsics.UndispatchedKt.startCoroutineUndispatched(Undispatched.kt:55)
	at kotlinx.coroutines.CoroutineStart.invoke(CoroutineStart.kt:112)
	at kotlinx.coroutines.AbstractCoroutine.start(AbstractCoroutine.kt:126)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTestCoroutine(TestBuilders.kt:211)
	at kotlinx.coroutines.test.TestBuildersKt.runTestCoroutine(Unknown Source)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt$runTest$1$1.invokeSuspend(TestBuilders.kt:167)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt$runTest$1$1.invoke(TestBuilders.kt)
	at kotlinx.coroutines.test.TestBuildersJvmKt$createTestResult$1.invokeSuspend(TestBuildersJvm.kt:13)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
	at kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:284)
	at kotlinx.coroutines.BlockingCoroutine.joinBlocking(Builders.kt:85)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt:59)
	at kotlinx.coroutines.BuildersKt.runBlocking(Unknown Source)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking$default(Builders.kt:38)
	at kotlinx.coroutines.BuildersKt.runBlocking$default(Unknown Source)
	at kotlinx.coroutines.test.TestBuildersJvmKt.createTestResult(TestBuildersJvm.kt:12)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest(TestBuilders.kt:166)
	at kotlinx.coroutines.test.TestBuildersKt.runTest(Unknown Source)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest$default(TestBuilders.kt:161)
	at kotlinx.coroutines.test.TestBuildersKt.runTest$default(Unknown Source)
	at mycompany.myapp.biometrics.BiometricsDataManagerTest.retrieveData - when device is secure and stored data - shows fragment(BiometricsDataManagerTest.kt:193)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at mycompany.myapp.ApplicationTestUtilKtTestRule$ApplicationTestUtilBeforeAfter.evaluate(ApplicationTestUtilKtTestRule.kt:19)
	at mycompany.myapp.CoroutineScopeTestRule$CoroutineScopeTestRuleBeforeAfter.evaluate(CoroutineScopeTestRule.kt:37)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:593)
	at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$2(SandboxTestRunner.java:290)
	at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:99)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)

[utzcoz]

@alwa I think it is related to the following code:

    @Override
    public MethodHandle getMethodHandle(String methodName, MethodType type)
        throws NoSuchMethodException, IllegalAccessException {
      return lookup.findStatic(
          getClass(), "getCurrentSpi", methodType(CipherSpi.class, void.class));
    }
  }

Maybe you need to change getClass() to Cypher.class.

[alwa]

I'll create a new PR

[utzcoz]

@alwa You can test it locally before you pushing. I can't confirm the above solution can fix this issue.

[hoisie]

To get an end to end test working you either need to regenerate preinstrumented jars or disable preinstrumented jars. To release a version with this change we also have a bump the preinstrumented jar level. This may have been why you didn't see it working.

[utzcoz]

@hoisie Instead of intercepting, adding shadow for upper call method is might be better.

[hoisie]

It is better to intercept these in case they are called from multiple places in framework code. It's not a big deal to bump the preinstrumented level. We can bump it on -master and @alwa can use SNAPSHOTS.

[alwa]

It is better to intercept these in case they are called from multiple places in framework code. It's not a big deal to bump the preinstrumented level. We can bump it on -master and @alwa can use SNAPSHOTS.

The initial fix was reverted though. Let me know when to try

[hoisie]

I have been thinking a bit about the best way to write an end-to-end unit test for this.

The current https://github.com/robolectric/robolectric/blob/master/robolectric/src/test/java/org/robolectric/interceptors/AndroidInterceptorsIntegrationTest.java is not great because it doesn't actually test the instrumentation part.

Ideally it would be something like using ASM to generate a class that has a method that calls Cipher.getCurrentSpi, ensure that class gets instrumented, and the method using reflection.

[utzcoz]

@hoisie Okay. Maybe you can help to upgrade preinstrumented jar version after @alwa changes checked in again. But I hope @alwa can write tests for it and disable preinstrumented jar for CI if it not. IIRC, we have faced this issue before?