From 8bdc773f1fe4f734da980e730cbe46e8ecb1a8d1 Mon Sep 17 00:00:00 2001 From: Kaartic Sivaraam Date: Sun, 9 Feb 2020 00:45:54 +0530 Subject: [PATCH 1/2] Revert stopgaps related to beta server cert issue The upstream issue with Commons beta server has been fixed now[1]. So, there's no point in stopgapping the issue anymore. So, revert the related changes. This reverts fa87eb566128c777dde3afc79ba2b68ce70efd55 and df426f7c427057fe97dc79186287260c02d3de25 which correspond to PRs #3350 and #3349 respectively. [1]: https://phabricator.wikimedia.org/T243881#5861983 --- .../free/nrw/commons/CommonsApplication.java | 20 +- .../nrw/commons/CustomNetworkFetcher.java | 206 ------------------ .../nrw/commons/OkHttpConnectionFactory.java | 12 +- .../free/nrw/commons/di/NetworkingModule.java | 19 +- .../java/fr/free/nrw/commons/di/SslUtils.kt | 47 ---- .../nrw/commons/TestCommonsApplication.kt | 6 +- 6 files changed, 15 insertions(+), 295 deletions(-) delete mode 100644 app/src/main/java/fr/free/nrw/commons/CustomNetworkFetcher.java delete mode 100644 app/src/main/java/fr/free/nrw/commons/di/SslUtils.kt diff --git a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java index c0bc11ba92..1fb0d02bc7 100644 --- a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java +++ b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java @@ -15,10 +15,6 @@ import com.facebook.drawee.backends.pipeline.Fresco; import com.facebook.imagepipeline.core.ImagePipeline; import com.facebook.imagepipeline.core.ImagePipelineConfig; -import com.facebook.imagepipeline.producers.Consumer; -import com.facebook.imagepipeline.producers.FetchState; -import com.facebook.imagepipeline.producers.NetworkFetcher; -import com.facebook.imagepipeline.producers.ProducerContext; import com.mapbox.mapboxsdk.Mapbox; import com.squareup.leakcanary.LeakCanary; import com.squareup.leakcanary.RefWatcher; @@ -57,7 +53,6 @@ import io.reactivex.internal.functions.Functions; import io.reactivex.plugins.RxJavaPlugins; import io.reactivex.schedulers.Schedulers; -import okhttp3.OkHttpClient; import timber.log.Timber; import static org.acra.ReportField.ANDROID_VERSION; @@ -92,9 +87,6 @@ public class CommonsApplication extends Application { @Inject @Named("default_preferences") JsonKvStore defaultPrefs; - @Inject - OkHttpClient okHttpClient; - /** * Constants begin */ @@ -157,15 +149,9 @@ public void onCreate() { } // Set DownsampleEnabled to True to downsample the image in case it's heavy - ImagePipelineConfig.Builder imagePipelineConfigBuilder = ImagePipelineConfig.newBuilder(this) - .setDownsampleEnabled(true); - - if(ConfigUtils.isBetaFlavour()){ - NetworkFetcher networkFetcher=new CustomNetworkFetcher(okHttpClient); - imagePipelineConfigBuilder.setNetworkFetcher(networkFetcher); - } - - ImagePipelineConfig config = imagePipelineConfigBuilder.build(); + ImagePipelineConfig config = ImagePipelineConfig.newBuilder(this) + .setDownsampleEnabled(true) + .build(); try { Fresco.initialize(this, config); } catch (Exception e) { diff --git a/app/src/main/java/fr/free/nrw/commons/CustomNetworkFetcher.java b/app/src/main/java/fr/free/nrw/commons/CustomNetworkFetcher.java deleted file mode 100644 index 4879b143be..0000000000 --- a/app/src/main/java/fr/free/nrw/commons/CustomNetworkFetcher.java +++ /dev/null @@ -1,206 +0,0 @@ -package fr.free.nrw.commons; - -import android.net.Uri; -import android.os.Looper; -import android.os.SystemClock; -import com.facebook.imagepipeline.common.BytesRange; -import com.facebook.imagepipeline.image.EncodedImage; -import com.facebook.imagepipeline.producers.BaseNetworkFetcher; -import com.facebook.imagepipeline.producers.BaseProducerContextCallbacks; -import com.facebook.imagepipeline.producers.Consumer; -import com.facebook.imagepipeline.producers.FetchState; -import com.facebook.imagepipeline.producers.NetworkFetcher; -import com.facebook.imagepipeline.producers.ProducerContext; -import java.io.IOException; -import java.util.HashMap; -import java.util.Map; -import java.util.concurrent.Executor; -import okhttp3.CacheControl; -import okhttp3.Call; -import okhttp3.OkHttpClient; -import okhttp3.Request; -import okhttp3.Response; -import okhttp3.ResponseBody; - -/** Network fetcher that uses OkHttp 3 as a backend. */ -public class CustomNetworkFetcher - extends BaseNetworkFetcher { - - public static class OkHttpNetworkFetchState extends FetchState { - - public long submitTime; - public long responseTime; - public long fetchCompleteTime; - - public OkHttpNetworkFetchState( - Consumer consumer, ProducerContext producerContext) { - super(consumer, producerContext); - } - } - - private static final String QUEUE_TIME = "queue_time"; - private static final String FETCH_TIME = "fetch_time"; - private static final String TOTAL_TIME = "total_time"; - private static final String IMAGE_SIZE = "image_size"; - - private final Call.Factory mCallFactory; - private final CacheControl mCacheControl; - - private Executor mCancellationExecutor; - - /** @param okHttpClient client to use */ - public CustomNetworkFetcher(OkHttpClient okHttpClient) { - this(okHttpClient, okHttpClient.dispatcher().executorService()); - } - - /** - * @param callFactory custom {@link Call.Factory} for fetching image from the network - * @param cancellationExecutor executor on which fetching cancellation is performed if - * cancellation is requested from the UI Thread - */ - public CustomNetworkFetcher(Call.Factory callFactory, Executor cancellationExecutor) { - this(callFactory, cancellationExecutor, true); - } - - /** - * @param callFactory custom {@link Call.Factory} for fetching image from the network - * @param cancellationExecutor executor on which fetching cancellation is performed if - * cancellation is requested from the UI Thread - * @param disableOkHttpCache true if network requests should not be cached by OkHttp - */ - public CustomNetworkFetcher( - Call.Factory callFactory, Executor cancellationExecutor, boolean disableOkHttpCache) { - mCallFactory = callFactory; - mCancellationExecutor = cancellationExecutor; - mCacheControl = disableOkHttpCache ? new CacheControl.Builder().noStore().build() : null; - } - - @Override - public OkHttpNetworkFetchState createFetchState( - Consumer consumer, ProducerContext context) { - return new OkHttpNetworkFetchState(consumer, context); - } - - @Override - public void fetch( - final OkHttpNetworkFetchState fetchState, final NetworkFetcher.Callback callback) { - fetchState.submitTime = SystemClock.elapsedRealtime(); - final Uri uri = fetchState.getUri(); - - try { - final Request.Builder requestBuilder = new Request.Builder().url(uri.toString()).get(); - - if (mCacheControl != null) { - requestBuilder.cacheControl(mCacheControl); - } - - final BytesRange bytesRange = fetchState.getContext().getImageRequest().getBytesRange(); - if (bytesRange != null) { - requestBuilder.addHeader("Range", bytesRange.toHttpRangeHeaderValue()); - } - - fetchWithRequest(fetchState, callback, requestBuilder.build()); - } catch (Exception e) { - // handle error while creating the request - callback.onFailure(e); - } - } - - @Override - public void onFetchCompletion(OkHttpNetworkFetchState fetchState, int byteSize) { - fetchState.fetchCompleteTime = SystemClock.elapsedRealtime(); - } - - @Override - public Map getExtraMap(OkHttpNetworkFetchState fetchState, int byteSize) { - Map extraMap = new HashMap<>(4); - extraMap.put(QUEUE_TIME, Long.toString(fetchState.responseTime - fetchState.submitTime)); - extraMap.put(FETCH_TIME, Long.toString(fetchState.fetchCompleteTime - fetchState.responseTime)); - extraMap.put(TOTAL_TIME, Long.toString(fetchState.fetchCompleteTime - fetchState.submitTime)); - extraMap.put(IMAGE_SIZE, Integer.toString(byteSize)); - return extraMap; - } - - protected void fetchWithRequest( - final OkHttpNetworkFetchState fetchState, - final NetworkFetcher.Callback callback, - final Request request) { - final Call call = mCallFactory.newCall(request); - - fetchState - .getContext() - .addCallbacks( - new BaseProducerContextCallbacks() { - @Override - public void onCancellationRequested() { - if (Looper.myLooper() != Looper.getMainLooper()) { - call.cancel(); - } else { - mCancellationExecutor.execute( - new Runnable() { - @Override - public void run() { - call.cancel(); - } - }); - } - } - }); - - call.enqueue( - new okhttp3.Callback() { - @Override - public void onResponse(Call call, Response response) throws IOException { - fetchState.responseTime = SystemClock.elapsedRealtime(); - final ResponseBody body = response.body(); - try { - if (!response.isSuccessful()) { - handleException( - call, new IOException("Unexpected HTTP code " + response), callback); - return; - } - - BytesRange responseRange = - BytesRange.fromContentRangeHeader(response.header("Content-Range")); - if (responseRange != null - && !(responseRange.from == 0 - && responseRange.to == BytesRange.TO_END_OF_CONTENT)) { - // Only treat as a partial image if the range is not all of the content - fetchState.setResponseBytesRange(responseRange); - fetchState.setOnNewResultStatusFlags(Consumer.IS_PARTIAL_RESULT); - } - - long contentLength = body.contentLength(); - if (contentLength < 0) { - contentLength = 0; - } - callback.onResponse(body.byteStream(), (int) contentLength); - } catch (Exception e) { - handleException(call, e, callback); - } finally { - body.close(); - } - } - - @Override - public void onFailure(Call call, IOException e) { - handleException(call, e, callback); - } - }); - } - - /** - * Handles exceptions. - * - *

OkHttp notifies callers of cancellations via an IOException. If IOException is caught after - * request cancellation, then the exception is interpreted as successful cancellation and - * onCancellation is called. Otherwise onFailure is called. - */ - private void handleException(final Call call, final Exception e, final Callback callback) { - if (call.isCanceled()) { - callback.onCancellation(); - } else { - callback.onFailure(e); - } - } -} \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/OkHttpConnectionFactory.java b/app/src/main/java/fr/free/nrw/commons/OkHttpConnectionFactory.java index 8b12ee5f0f..6fe317c0a4 100644 --- a/app/src/main/java/fr/free/nrw/commons/OkHttpConnectionFactory.java +++ b/app/src/main/java/fr/free/nrw/commons/OkHttpConnectionFactory.java @@ -8,8 +8,6 @@ import java.io.File; import java.io.IOException; -import fr.free.nrw.commons.di.SslUtils; -import fr.free.nrw.commons.utils.ConfigUtils; import okhttp3.Cache; import okhttp3.Interceptor; import okhttp3.OkHttpClient; @@ -31,17 +29,13 @@ public final class OkHttpConnectionFactory { @NonNull private static OkHttpClient createClient() { - OkHttpClient.Builder builder = new OkHttpClient.Builder() + return new OkHttpClient.Builder() .cookieJar(SharedPreferenceCookieManager.getInstance()) .cache(NET_CACHE) .addInterceptor(getLoggingInterceptor()) .addInterceptor(new UnsuccessfulResponseInterceptor()) - .addInterceptor(new CommonHeaderRequestInterceptor()); - - if(ConfigUtils.isBetaFlavour()){ - builder.sslSocketFactory(SslUtils.INSTANCE.getTrustAllHostsSSLSocketFactory()); - } - return builder.build(); + .addInterceptor(new CommonHeaderRequestInterceptor()) + .build(); } private static HttpLoggingInterceptor getLoggingInterceptor() { diff --git a/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java b/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java index 830b142024..c6e6033f58 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java +++ b/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java @@ -18,8 +18,6 @@ import javax.inject.Named; import javax.inject.Singleton; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLSocketFactory; import dagger.Module; import dagger.Provides; @@ -33,7 +31,6 @@ import fr.free.nrw.commons.mwapi.UserInterface; import fr.free.nrw.commons.review.ReviewInterface; import fr.free.nrw.commons.upload.UploadInterface; -import fr.free.nrw.commons.utils.ConfigUtils; import fr.free.nrw.commons.wikidata.WikidataInterface; import okhttp3.Cache; import okhttp3.HttpUrl; @@ -61,20 +58,14 @@ public class NetworkingModule { public OkHttpClient provideOkHttpClient(Context context, HttpLoggingInterceptor httpLoggingInterceptor) { File dir = new File(context.getCacheDir(), "okHttpCache"); - OkHttpClient.Builder builder = new OkHttpClient.Builder().connectTimeout(60, TimeUnit.SECONDS) - .writeTimeout(60, TimeUnit.SECONDS) + return new OkHttpClient.Builder().connectTimeout(60, TimeUnit.SECONDS) + .writeTimeout(60, TimeUnit.SECONDS) .addInterceptor(httpLoggingInterceptor) - .readTimeout(60, TimeUnit.SECONDS) - .cache(new Cache(dir, OK_HTTP_CACHE_SIZE)); - - if(ConfigUtils.isBetaFlavour()){ - builder.sslSocketFactory(SslUtils.INSTANCE.getTrustAllHostsSSLSocketFactory()); - } - return builder.build(); + .readTimeout(60, TimeUnit.SECONDS) + .cache(new Cache(dir, OK_HTTP_CACHE_SIZE)) + .build(); } - - @Provides @Singleton public HttpLoggingInterceptor provideHttpLoggingInterceptor() { diff --git a/app/src/main/java/fr/free/nrw/commons/di/SslUtils.kt b/app/src/main/java/fr/free/nrw/commons/di/SslUtils.kt deleted file mode 100644 index 043cdbc609..0000000000 --- a/app/src/main/java/fr/free/nrw/commons/di/SslUtils.kt +++ /dev/null @@ -1,47 +0,0 @@ -package fr.free.nrw.commons.di - -import java.security.KeyManagementException -import java.security.NoSuchAlgorithmException -import java.security.cert.CertificateException -import java.security.cert.X509Certificate -import javax.net.ssl.SSLContext -import javax.net.ssl.SSLSocketFactory -import javax.net.ssl.TrustManager -import javax.net.ssl.X509TrustManager - -object SslUtils { - - fun getTrustAllHostsSSLSocketFactory(): SSLSocketFactory? { - try { - // Create a trust manager that does not validate certificate chains - val trustAllCerts = arrayOf(object : X509TrustManager { - - override fun getAcceptedIssuers(): Array { - return arrayOf() - } - - @Throws(CertificateException::class) - override fun checkClientTrusted(chain: Array, authType: String) { - } - - @Throws(CertificateException::class) - override fun checkServerTrusted(chain: Array, authType: String) { - } - }) - - // Install the all-trusting trust manager - val sslContext = SSLContext.getInstance("SSL") - sslContext.init(null, trustAllCerts, java.security.SecureRandom()) - // Create an ssl socket factory with our all-trusting manager - - return sslContext.socketFactory - } catch (e: KeyManagementException) { - e.printStackTrace() - return null - } catch (e: NoSuchAlgorithmException) { - e.printStackTrace() - return null - } - - } -} \ No newline at end of file diff --git a/app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt b/app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt index a59748f1d8..5368a34ad0 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt @@ -1,6 +1,5 @@ package fr.free.nrw.commons -import android.app.Application import android.content.ContentProviderClient import android.content.Context import androidx.collection.LruCache @@ -15,7 +14,7 @@ import fr.free.nrw.commons.di.DaggerCommonsApplicationComponent import fr.free.nrw.commons.kvstore.JsonKvStore import fr.free.nrw.commons.location.LocationServiceManager -class TestCommonsApplication : Application() { +class TestCommonsApplication : CommonsApplication() { private var mockApplicationComponent: CommonsApplicationComponent? = null override fun onCreate() { @@ -26,6 +25,9 @@ class TestCommonsApplication : Application() { } super.onCreate() } + + // No leakcanary in unit tests. + override fun setupLeakCanary(): RefWatcher = RefWatcher.DISABLED } @Suppress("MemberVisibilityCanBePrivate") From 4612df55749fc45663b9632c510e0bfea6268930 Mon Sep 17 00:00:00 2001 From: Kaartic Sivaraam Date: Thu, 13 Feb 2020 10:44:51 +0530 Subject: [PATCH 2/2] Test-fix: fix the failing CI test --- .../kotlin/fr/free/nrw/commons/TestCommonsApplication.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt b/app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt index 5368a34ad0..a59748f1d8 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt @@ -1,5 +1,6 @@ package fr.free.nrw.commons +import android.app.Application import android.content.ContentProviderClient import android.content.Context import androidx.collection.LruCache @@ -14,7 +15,7 @@ import fr.free.nrw.commons.di.DaggerCommonsApplicationComponent import fr.free.nrw.commons.kvstore.JsonKvStore import fr.free.nrw.commons.location.LocationServiceManager -class TestCommonsApplication : CommonsApplication() { +class TestCommonsApplication : Application() { private var mockApplicationComponent: CommonsApplicationComponent? = null override fun onCreate() { @@ -25,9 +26,6 @@ class TestCommonsApplication : CommonsApplication() { } super.onCreate() } - - // No leakcanary in unit tests. - override fun setupLeakCanary(): RefWatcher = RefWatcher.DISABLED } @Suppress("MemberVisibilityCanBePrivate")