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

isCleartextTrafficPermitted() fails on OpenJDK 8 + Robolectric #2533

Closed
kungfoo opened this issue May 4, 2016 · 39 comments
Closed

isCleartextTrafficPermitted() fails on OpenJDK 8 + Robolectric #2533

kungfoo opened this issue May 4, 2016 · 39 comments
Labels
bug Bug in existing code needs info More information needed from reporter
Milestone

Comments

@kungfoo
Copy link

kungfoo commented May 4, 2016

How to reproduce:

  • Install OpenJDK 8
  • Use current 3.3.0-SNAPSHOT version
  • new OkHttpClient(): The code fails in the static initializer, when trying to figure out the platform.

The code that breaks was introduced here: #2513

Either the detection of the runtime is broken, or the code is wrong there. I'll try to figure out an automatic test for this.

@kungfoo
Copy link
Author

kungfoo commented May 4, 2016

This is when running tests with Roboelectric, BTW. So maybe the code determines, the Platform is android, when it's actually not.

@kungfoo
Copy link
Author

kungfoo commented May 4, 2016

I tried to write up a simple testcase that reproduces this, but apparently it truly is the combination of the following things that make the code fail:

  • OpenJDK 8
  • Current Snapshot
  • Roboelectric (needs to be an android project for this)

@kungfoo
Copy link
Author

kungfoo commented May 4, 2016

Okay, so because the setup is kinda contrived: Here is a sample that reproduces this...

https://github.com/kungfoo/okhttp-on-jdk-with-robolectric

@swankjesse
Copy link
Collaborator

Does it throw or return the wrong result?

@kungfoo
Copy link
Author

kungfoo commented May 4, 2016

It crashes/throws in the static initializer which results in a
ClassNotFoundException.
On May 4, 2016 14:54, "Jesse Wilson" notifications@github.com wrote:

Does it throw or return the wrong result?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#2533 (comment)

@swankjesse
Copy link
Collaborator

@kungfoo could you paste the stacktrace?

@kungfoo
Copy link
Author

kungfoo commented May 4, 2016

Will do later when I get my laptop back online...
On May 4, 2016 16:56, "Jesse Wilson" notifications@github.com wrote:

@kungfoo https://github.com/kungfoo could you paste the stacktrace?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#2533 (comment)

@kungfoo
Copy link
Author

kungfoo commented May 4, 2016

If you want to debug the code under the described circumstances, you can simply use gradle to run the sample project I created for this.

Here's the stacktrace:

at okhttp3.internal.Platform$Android.isCleartextTrafficPermitted(Platform.java:324)
    at okhttp3.OkHttpClient.<clinit>(OkHttpClient.java:73)
    at ch.ergon.sample.CreateOkHttpClientTest.canCreateAClient(CreateOkHttpClientTest.java:17)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:483)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.robolectric.RobolectricTestRunner$2.evaluate(RobolectricTestRunner.java:251)
    at org.robolectric.RobolectricTestRunner.runChild(RobolectricTestRunner.java:188)
    at org.robolectric.RobolectricTestRunner.runChild(RobolectricTestRunner.java:54)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.robolectric.RobolectricTestRunner$1.evaluate(RobolectricTestRunner.java:152)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.runTestClass(JUnitTestClassExecuter.java:112)
    at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.execute(JUnitTestClassExecuter.java:56)
    at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassProcessor.processTestClass(JUnitTestClassProcessor.java:66)
    at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:51)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:483)

@swankjesse swankjesse changed the title isCleartextTrafficPermitted() fails on OpenJDK 8 (1.8.0_92-b14) isCleartextTrafficPermitted() fails on OpenJDK 8 + Robolectric May 5, 2016
@swankjesse
Copy link
Collaborator

Wanna submit a fix?

@kungfoo
Copy link
Author

kungfoo commented May 5, 2016

Might be able to do that, when I'm back out of the woods... :)
On May 5, 2016 06:09, "Jesse Wilson" notifications@github.com wrote:

Wanna submit a fix?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#2533 (comment)

@swankjesse swankjesse added the needs info More information needed from reporter label May 8, 2016
@swankjesse swankjesse added this to the 3.4 milestone May 8, 2016
@swankjesse swankjesse added the bug Bug in existing code label May 8, 2016
@eygraber
Copy link

Here's the message for the target Exception of the InvocationTargetException that gets thrown when calling the offending Method.invoke

Method getInstance in android.security.NetworkSecurityPolicy not mocked. See http://g.co/androidstudio/not-mocked for details.

@JakeWharton
Copy link
Collaborator

Sounds like you're not using Robolectric since that message comes from the
mockable jar.

On Thu, May 26, 2016 at 12:56 AM Eliezer Graber notifications@github.com
wrote:

Here's the message for the target Exception of the
InvocationTargetException that gets thrown when calling the offending
Method.invoke
e3cd9b9#diff-5803d99d45dfa6a23e51dd60d44f2de9R314

Method getInstance in android.security.NetworkSecurityPolicy not mocked.
See http://g.co/androidstudio/not-mocked for details.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#2533 (comment)

@eygraber
Copy link

I'm definitely using Robolectric (verified on the call stack), which is why I was confused. Could it be a classloader issue where Class.forname is using the platform class?

@JakeWharton
Copy link
Collaborator

Maybe, but that would be a robolectric bug in its ParallelUniverse then. If
Class.forName didn't work then things like LayoutInflater wouldn't work
either.

On Thu, May 26, 2016 at 1:09 AM Eliezer Graber notifications@github.com
wrote:

I'm definitely using Robolectric (verified on the call stack), which is
why I was confused. Could it be a classloader issue where Class.forname
is using the platform class?


You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#2533 (comment)

@eygraber
Copy link

Could it be happening because I'm on Robolectric 2.4 (which I believe only supports up to API 18), and NetworkSecurityPolicy wasn't added until API 23?

@JakeWharton
Copy link
Collaborator

Possibly. Try the latest.

On Thu, May 26, 2016 at 1:21 AM Eliezer Graber notifications@github.com
wrote:

Could it be happening because I'm on Robolectric 2.4 (which I believe only
supports up to API 18), and NetworkSecurityPolicy wasn't added until API
23?


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#2533 (comment)

@eygraber
Copy link

eygraber commented May 26, 2016

We're pretty stuck on 2.4, so I used the project referenced above and changed the robolectric properties to use API 23. I was greeted by this when running the test:

java.lang.UnsupportedOperationException: Robolectric does not support API level 23.

@JakeWharton
Copy link
Collaborator

Seems like a clear deficiency on their side then. Not sure what action
there would be for us to take.

On Thu, May 26, 2016 at 1:34 AM Eliezer Graber notifications@github.com
wrote:

We're pretty stuck on 2.4, so I used the project referenced above
#2533 (comment) and
change the robolectric properties to use API 23. I was greeted by this when
running the test:

java.lang.UnsupportedOperationException: Robolectric does not support API
level 23.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#2533 (comment)

@eygraber
Copy link

Would wrapping it in a version check help?

@JakeWharton
Copy link
Collaborator

It's already behind one. You're compiling with API 23 (or newer) and thus
OkHttp attempts to use APIs from that version.

On Thu, May 26, 2016 at 2:11 AM Eliezer Graber notifications@github.com
wrote:

Would wrapping it in a version check help?


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#2533 (comment)

@eygraber
Copy link

Would that affect a runtime check? Something like:

Class c = Class.forName("android.os.Build$VERSION");
Field f = c.getDeclaredField("SDK_INT");
int apiLevel = f.getInt(null);

When I try that running in Robolectric it returns 21.

@meoyawn
Copy link

meoyawn commented May 29, 2016

I'm getting

java.lang.AssertionError
    at okhttp3.internal.AndroidPlatform.isCleartextTrafficPermitted(AndroidPlatform.java:143)
    at okhttp3.OkHttpClient.<clinit>(OkHttpClient.java:73)
    at okhttp3.OkHttpClient$Builder.<init>(OkHttpClient.java:381)

with okhttp 3.3.1 and both robolectric 3.0 and 3.1-rc1
all good with okhttp 3.2.+

@dave-r12
Copy link
Collaborator

I was able to get the test green. Here's what I did:

  1. Change to using OkHttp-3.3.1. The project was using a SNAPSHOT version, but 3.3 has since been released.
  2. Upgrade to robolectric 3.1-rc1.
  3. Added config annotation with SDK specified to the test class:
@RunWith(RobolectricGradleTestRunner.class)
@Config(sdk = 23)
public class CreateOkHttpClientTest {

As for why it works, I don't know. It appears the default SDK is 21. The bit I don't understand is why it isn't throwing a ClassNotFoundException given this class is only in SDK 23. The test project itself is depending on API 23, which may have something to do with it.

@dave-r12
Copy link
Collaborator

I have a vague idea of why this works now, and I opened up an issue on robolectric project. Let's see what they say, robolectric/robolectric#2478

@felipecsl
Copy link
Contributor

Robolectric 3.1-rc1 is pretty much unusable right now, as running tests take ~10x longer. Any other workarounds?

@felipecsl
Copy link
Contributor

Oh, It works if you add a String argument to the isCleartetTrafficPermitted shadow method. Here's the updated (hideous) workaround:

@Implements(NetworkSecurityPolicy.class)
public class NetworkSecurityPolicyWorkaround {
    @Implementation
    public static NetworkSecurityPolicy getInstance() {
        //noinspection OverlyBroadCatchBlock
        try {
            Class<?> shadow = Class.forName("android.security.NetworkSecurityPolicy");
            return (NetworkSecurityPolicy) shadow.newInstance();
        } catch (Exception e) {
            throw new AssertionError();
        }
    }

    @Implementation
    public boolean isCleartextTrafficPermitted(String hostname) {
        return true;
    }
}

ghost pushed a commit to facebook/react-native that referenced this issue Jul 20, 2016
Summary:
- 3.3.1 wasn't compatible with Robolectric 3.0, however 3.4.0 [fixes](square/okhttp#2533 (comment)) this issue.
- Few other fixes to OkHttp mentioned in the [CHANGELOG.md](https://github.com/square/okhttp/blob/master/CHANGELOG.md)

> Fixes: #7743
>
- Android apps can recover from a `REFUSED_STREAM` in HTTP/2.
- A few other fixes mentioned in the [CHANGELOG.md](https://github.com/square/okhttp/blob/master/CHANGELOG.md)

- CircleCi
- Test with `/Examples` ✅
Closes #8672

Reviewed By: alsutton

Differential Revision: D3541293

Pulled By: bestander

fbshipit-source-id: 76429861b4f4df15cb9c18ab0f177daee3e1459d
fadils pushed a commit to fadils/react-native that referenced this issue Jul 21, 2016
Summary:
- 3.3.1 wasn't compatible with Robolectric 3.0, however 3.4.0 [fixes](square/okhttp#2533 (comment)) this issue.
- Few other fixes to OkHttp mentioned in the [CHANGELOG.md](https://github.com/square/okhttp/blob/master/CHANGELOG.md)

> Fixes: facebook#7743
>
- Android apps can recover from a `REFUSED_STREAM` in HTTP/2.
- A few other fixes mentioned in the [CHANGELOG.md](https://github.com/square/okhttp/blob/master/CHANGELOG.md)

- CircleCi
- Test with `/Examples` ✅
Closes facebook#8672

Reviewed By: alsutton

Differential Revision: D3541293

Pulled By: bestander

fbshipit-source-id: 76429861b4f4df15cb9c18ab0f177daee3e1459d
bubblesunyum pushed a commit to iodine/react-native that referenced this issue Aug 23, 2016
Summary:
- 3.3.1 wasn't compatible with Robolectric 3.0, however 3.4.0 [fixes](square/okhttp#2533 (comment)) this issue.
- Few other fixes to OkHttp mentioned in the [CHANGELOG.md](https://github.com/square/okhttp/blob/master/CHANGELOG.md)

> Fixes: facebook#7743
>
- Android apps can recover from a `REFUSED_STREAM` in HTTP/2.
- A few other fixes mentioned in the [CHANGELOG.md](https://github.com/square/okhttp/blob/master/CHANGELOG.md)

- CircleCi
- Test with `/Examples` ✅
Closes facebook#8672

Reviewed By: alsutton

Differential Revision: D3541293

Pulled By: bestander

fbshipit-source-id: 76429861b4f4df15cb9c18ab0f177daee3e1459d
mpretty-cyro pushed a commit to HomePass/react-native that referenced this issue Aug 25, 2016
Summary:
- 3.3.1 wasn't compatible with Robolectric 3.0, however 3.4.0 [fixes](square/okhttp#2533 (comment)) this issue.
- Few other fixes to OkHttp mentioned in the [CHANGELOG.md](https://github.com/square/okhttp/blob/master/CHANGELOG.md)

> Fixes: facebook#7743
>
- Android apps can recover from a `REFUSED_STREAM` in HTTP/2.
- A few other fixes mentioned in the [CHANGELOG.md](https://github.com/square/okhttp/blob/master/CHANGELOG.md)

- CircleCi
- Test with `/Examples` ✅
Closes facebook#8672

Reviewed By: alsutton

Differential Revision: D3541293

Pulled By: bestander

fbshipit-source-id: 76429861b4f4df15cb9c18ab0f177daee3e1459d
@thalescm
Copy link

thalescm commented Sep 6, 2016

I'm not using Roboeletric, but i'm in pointing towards API 24, with okhttp version 3.4.1:
My problem is that I'm getting a NullPointerException (when running UnitTests) in okhttp3.internal.platform.AndroidPlatform when trying to run method:

@Override public boolean isCleartextTrafficPermitted(String hostname) {
    try {
      Class<?> networkPolicyClass = Class.forName("android.security.NetworkSecurityPolicy");
      Method getInstanceMethod = networkPolicyClass.getMethod("getInstance");
      Object networkSecurityPolicy = getInstanceMethod.invoke(null);
      Method isCleartextTrafficPermittedMethod = networkPolicyClass
          .getMethod("isCleartextTrafficPermitted", String.class);
      return (boolean) isCleartextTrafficPermittedMethod.invoke(networkSecurityPolicy, hostname);
    } catch (ClassNotFoundException | NoSuchMethodException e) {
      return super.isCleartextTrafficPermitted(hostname);
    } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
      throw new AssertionError();
    }
  }

which comes from RealConnection::connect.
Debugging it, it happens on the actual invoke isCleartextTrafficPermittedMethod.invoke(networkSecurityPolicy, hostname).
This problem didn't happen to me at version 3.2.0...
Can anyone help me?

@thalescm
Copy link

thalescm commented Sep 6, 2016

@felipecsl solution (for those who can't upgrade to Robolectric 3+) works in this case too. But I really wouldn't like to use that :(

@swankjesse swankjesse modified the milestones: 3.6, 3.5 Nov 20, 2016
aafa added a commit to aafa/bitrader that referenced this issue Dec 11, 2016
@PaulWoitaschek
Copy link
Contributor

Happens with OkHttp 3.5.0 and robolectric 3.1.4

@vRallev
Copy link

vRallev commented Jan 16, 2017

We're running into the same issue with OkHttp 3.5.0. Instead of using Robolectric directly we use unmock (which relies on Robolectric's android-all jar).

@swankjesse swankjesse modified the milestones: 3.6, 3.7 Jan 29, 2017
@teburd
Copy link

teburd commented Feb 1, 2017

I'm so glad I found this ticket, was going crazy trying to understand why okhttp suddenly had an exception when running with robolectric

@xstherrera1987
Copy link

I am also still seeing this, but have resolved it with same fix as @eygraber.
note that I'm not on OpenJDK as OP was

  • OkHttp 3.6.0
  • Robolectric 3.3 only for @Config(sdk = _) < 23
  • Java HotSpot(TM) 64-Bit Server VM (build 25.121-b13, mixed mode) on OSX

add to unit-tests source-set only, eg: project-dir/app/src/test/java/android/security by default

package android.security;
// https://github.com/square/okhttp/issues/2533#issuecomment-223093100
public class NetworkSecurityPolicy {
    private static final NetworkSecurityPolicy INSTANCE = new NetworkSecurityPolicy();
    @SuppressLint("NewApi") public static NetworkSecurityPolicy getInstance() { return INSTANCE; }
    public boolean isCleartextTrafficPermitted() { return true; }
}

lastly note that this may break with OkHttp 3.7.x

@swankjesse swankjesse modified the milestones: 3.7, 3.8 Apr 28, 2017
@swankjesse
Copy link
Collaborator

Is there anything to do in OkHttp for this?

@swankjesse swankjesse modified the milestones: 3.10, 3.9 Aug 30, 2017
@yschimke
Copy link
Collaborator

Reading this, seems like it is fixed in Robolectric 3.3

tungdo194 pushed a commit to tungdo194/rn-test that referenced this issue Apr 28, 2024
Summary:
- 3.3.1 wasn't compatible with Robolectric 3.0, however 3.4.0 [fixes](square/okhttp#2533 (comment)) this issue.
- Few other fixes to OkHttp mentioned in the [CHANGELOG.md](https://github.com/square/okhttp/blob/master/CHANGELOG.md)

> Fixes: #7743
>
- Android apps can recover from a `REFUSED_STREAM` in HTTP/2.
- A few other fixes mentioned in the [CHANGELOG.md](https://github.com/square/okhttp/blob/master/CHANGELOG.md)

- CircleCi
- Test with `/Examples` ✅
Closes facebook/react-native#8672

Reviewed By: alsutton

Differential Revision: D3541293

Pulled By: bestander

fbshipit-source-id: 76429861b4f4df15cb9c18ab0f177daee3e1459d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug in existing code needs info More information needed from reporter
Projects
None yet
Development

No branches or pull requests