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

Fix Conscrypt NPE workaround #7219

Merged
merged 1 commit into from Apr 9, 2022

Conversation

PhilGlass
Copy link
Contributor

After turning on HTTPS with MockWebServer 4.9.2 we started seeing the transient ssl == null failure described in #5587 on older emulators.

#5590 attempted to fix the problem, but I don't think the catch it introduced will ever be hit because the problematic NPE is wrapped in an InvocationTargetException:

11-05 11:53:36.557  5607 23163 E AndroidRuntime: FATAL EXCEPTION: MockWebServer
11-05 11:53:36.557  5607 23163 E AndroidRuntime: Process: com.xxx.xxx.xxx, PID: 5607
11-05 11:53:36.557  5607 23163 E AndroidRuntime: java.lang.AssertionError: java.lang.reflect.InvocationTargetException
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	at okhttp3.internal.platform.android.AndroidSocketAdapter.a(SourceFile:86)
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	at okhttp3.internal.platform.AndroidPlatform.b(SourceFile:86)
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	at okhttp3.mockwebserver.MockWebServer$SocketHandler.handle(MockWebServer.kt:516)
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	at okhttp3.mockwebserver.MockWebServer$serveConnection$$inlined$execute$1.run(Util.kt:580)
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	at java.lang.Thread.run(Thread.java:764)
11-05 11:53:36.557  5607 23163 E AndroidRuntime: Caused by: java.lang.reflect.InvocationTargetException
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	at java.lang.reflect.Method.invoke(Native Method)
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	at okhttp3.internal.platform.android.AndroidSocketAdapter.a(SourceFile:81)
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	... 6 more
11-05 11:53:36.557  5607 23163 E AndroidRuntime: Caused by: java.lang.NullPointerException: ssl == null
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	at com.android.org.conscrypt.NativeCrypto.getApplicationProtocol(Native Method)
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	at com.android.org.conscrypt.NativeSsl.getApplicationProtocol(NativeSsl.java:568)
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	at com.android.org.conscrypt.ConscryptFileDescriptorSocket.getApplicationProtocol(ConscryptFileDescriptorSocket.java:1085)
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	at com.android.org.conscrypt.OpenSSLSocketImpl.getAlpnSelectedProtocol(OpenSSLSocketImpl.java:134)
11-05 11:53:36.557  5607 23163 E AndroidRuntime: 	... 8 more

@yschimke
Copy link
Collaborator

yschimke commented Apr 8, 2022

@PhilGlass ughhhh.... and thanks. The folly of trying to fix an exception you haven't been able to reproduce.

Have you completed the CLA https://github.com/square/okhttp/blob/master/CONTRIBUTING.md

@yschimke
Copy link
Collaborator

yschimke commented Apr 8, 2022

Would a fix on 4.9.4 be required (our live release branch)? How urgent is this for you?

@PhilGlass
Copy link
Contributor Author

PhilGlass commented Apr 8, 2022

I've filled in the CLA.

For our use case it's not super urgent, because we can work around it by forcing HTTP 1.1 (with protocolNegotiationEnabled = false) which bypasses that code path. But if there's going to be a 4.9.4 I don't see any harm in including it!

@swankjesse swankjesse merged commit 6ba23dc into square:master Apr 9, 2022
@swankjesse
Copy link
Member

Thanks!

yschimke pushed a commit to yschimke/okhttp that referenced this pull request Apr 9, 2022
yschimke added a commit that referenced this pull request Apr 9, 2022
(cherry picked from commit 6ba23dc)

Co-authored-by: Phil Glass <phil@phil.glass>
yschimke added a commit to yschimke/okhttp that referenced this pull request Apr 10, 2022
swankjesse pushed a commit that referenced this pull request Apr 10, 2022
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.

None yet

3 participants