Upgrade to OkHttp 3.12.2#763
Conversation
Also includes Android Studio suggested Gradle updates.
Fix new repository entries otherwise the application will not build.
|
This generally looks great. However, the minimum API version is a significant change, which impacts device compatibility. Current stats suggest this will cut out about 10% of active Android devices across the ecosystem: https://developer.android.com/about/dashboards I could look at the stats specific to this app inside my own publisher dashboard, but if anything I'd expect it to be more severe. Could that part be removed from this PR, and the rest accepted? What's the rationale to move to 21 now? |
|
That's the minimum API level for OkHttp. There's still some Heartbeat
related SSL/TLS issues with lower Android API versions.
…On Sun, Mar 17, 2019 at 5:03 PM Eric Mill ***@***.***> wrote:
This generally looks great. However, the minimum API version is a
significant change, which impacts device compatibility. Current stats
suggest this will cut out about 10% of active Android devices across the
ecosystem: https://developer.android.com/about/dashboards I could look at
the stats specific to this app inside my own publisher dashboard, but if
anything I'd expect it to be more severe.
Could that part be removed from this PR, and the rest accepted? What's the
rationale to move to 21 now?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#763 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACYUhtSdfPmnWiN7xvf0APKgFqSQ2YLks5vXruUgaJpZM4b4Z5B>
.
|
|
Help me understand the impact here - is it required in order to enable TLS v1.2, or is it about mitigating weaknesses with arbitrary servers? The app has a defined set of services it can connect to, so it doesn't have to worry about connecting to known-weak services. |
|
AWS, Microsoft, and Google cloud services run at TLS v1.2 by default. All
major browsers will stop supporting anything before TLS v1.2 within the
next year. If ProPublica or congressional servers providing the images
(Picasso uses OkHttp) upgrade to servers using TLS v1.2 at a minimum, the
app will stop functioning on devices without TLS v1.2. Depending on how
the current versions of OkHttp in the application work, the app could stop
working on newer devices as well.
On the downside, going to a Lollipop minimum will drop some supported
devices on the new version of the app. That would exclude phones and
device produced before November 2014 and some after. Although, with the
SSL concerns around that time, most carriers and manufacturers upgraded
even earlier devices.
…On Mon, Mar 18, 2019 at 8:54 AM Eric Mill ***@***.***> wrote:
Help me understand the impact here - is it required in order to enable TLS
v1.2, or is it about mitigating weaknesses with arbitrary servers? The app
has a defined set of services it can connect to, so it doesn't have to
worry about connecting to known-weak services.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#763 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACYUhON_PZDQuKhR4sVZv5JgLUD5fozks5vX5qigaJpZM4b4Z5B>
.
|
|
Reading the OkHttp blog post on this, it seems like what we want for now is the 3.12 stable branch for which they'll backport fixes for a while. I'm all about improving our TLS security, but I don't see any reason to drop support for these devices until servers do. The app will still be preferring TLS v1.2 over 1.0 by moving to a recent 3.x version. Would you mind updating the PR to use 3.12, and to not bump the API level? The work you did in the PR to update the network classes and their use of the OkHTTP API is extremely helpful, and I'd love to merge that in to the app and move us to 3.x. |
|
OK, changes made to use in OkHttp 3.12.2 and minimum API lowered back to 16. |
|
Are there any concerns here that I can resolve? |
|
I'm waiting for this to be merged so I can start a light refactoring of the code, I think everything is good right now |
Android Studio.
…ngress-android into update_http_handling
|
Anything on this? The merging repo doesn't seem to exist anymore? :/ |
|
Yep, I deleted that repo. I'm going a different direction with my repo. Sitting for a year on a patch to merge is more than I really care to do. |
3 participants
Along with this package upgrade, it also requires a minimum Android SDK Version of 21 to get updated TLS security.
My changes for #762 are included as well.