Add cleartext traffic check #122
Conversation
scarlet-websocket-okhttp/src/main/java/com/tinder/scarlet/websocket/okhttp/OkHttpClientUtils.kt
Outdated
Show resolved
Hide resolved
scarlet-websocket-okhttp/src/main/java/com/tinder/scarlet/websocket/okhttp/OkHttpClientUtils.kt
Outdated
Show resolved
Hide resolved
scarlet-websocket-okhttp/src/main/java/com/tinder/scarlet/websocket/okhttp/OkHttpClientUtils.kt
Outdated
Show resolved
Hide resolved
|
thanks for the contribution @rogerthat94! Just a few nits |
|
Thank you for making this helpful library! I'm happy to give back with a small contribution. I hope this addresses the nits. |
It throws an exception when the URL starts with `ws:.
|
@rogerthat94 sorry it's been a while. if you can resolve conflicts, i'd be more than happy to accept this change. Thanks! |
|
|
|
@aaronweihe No problem! I have just resolved conflicts. |
It looks like the version of this cannot be specified separately OkHttp specifies this in a single string. See here: https://github.com/square/okhttp/blob/68241851c197a67cdcdd0e150e239f55d1e556f4/build.gradle#L28
|
ping @aaronweihe |
|
@aaronweihe Are you still interested in this change? I can rebase it on top of the latest version of the code if so. |
aaronweihe
left a comment
aaronweihe
left a comment
There was a problem hiding this comment.
looks good to me. Can you sign the CLA?
| fun OkHttpClient.newWebSocketFactory(url: String): WebSocket.Factory { | ||
| if (url.startsWith("ws:", ignoreCase = true)) { | ||
| try { | ||
| if ((Build.VERSION.SDK_INT > 23 && |
There was a problem hiding this comment.
I like the Platform#isCleartextTrafficPermitted abstraction. we could tackle it in the next PR
|
Thanks! I have signed the CLA. Is there anything else that you would like me to do? |
|
@aaronweihe Are you able to merge this? |
3 participants
When someone tries to open a Websocket connection without using TLS (ie "ws:" instead of "wss:") on Android 9 and newer, the connection will fail and Scarlet will not provide any information about this to the developer. This confused me as well as the author of this post.
Okhttp has a similar check here.
I check for a ClassNotFoundException, so it should not cause any issues for non-Android apps that use Scarlet.