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: support datafile download and events upload for API-16 #373

Merged
merged 20 commits into from
Jun 2, 2021

Conversation

jaeopt
Copy link
Contributor

@jaeopt jaeopt commented May 5, 2021

Summary

Android devices with API-19 and lower is set by default to use SSL lower than the our server requirements (TLS1.2+).
With this fix, those low-API devices will be set to use TLS1.2 for both datafile download and event upload.

Test plan

  • Unit tests for emulators with API-16 and API-19

Issues

@jaeopt jaeopt requested a review from a team as a code owner May 5, 2021 01:07
@jaeopt jaeopt requested a review from mnoman09 May 5, 2021 01:08
@jaeopt jaeopt removed their assignment May 5, 2021
@jaeopt jaeopt requested a review from msohailhussain May 5, 2021 01:09
Copy link
Contributor

@mnoman09 mnoman09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall looks good to me. Just one comment and all headers needs to get updated.

}

private Socket enableTLSOnSocket(Socket socket) {
if(socket != null && (socket instanceof SSLSocket)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

socket != null is not required before instanceof check. So I think we can remove it

@@ -0,0 +1,72 @@
package com.optimizely.ab.android.shared;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add header

@jaeopt jaeopt requested a review from mnoman09 June 2, 2021 17:13
Copy link
Contributor

@mnoman09 mnoman09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jaeopt jaeopt merged commit 25f472b into master Jun 2, 2021
@jaeopt jaeopt deleted the jae/ssl-api16 branch June 2, 2021 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants