Skip to content
This repository was archived by the owner on Mar 19, 2024. It is now read-only.

Commit 4de236b

Browse files
abelgardepabelgardep
authored
Merge pull request #364 from owncloud/fix/okhttp_singleton
remove okhttp singleton
2 parents 06e361a + f6eb631 commit 4de236b

31 files changed

+186
-501
lines changed

‎owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/ConnectionValidator.kt

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,27 @@
1+
/* ownCloud Android Library is available under MIT license
2+
* Copyright (C) 2016 ownCloud GmbH.
3+
*
4+
* Permission is hereby granted, free of charge, to any person obtaining a copy
5+
* of this software and associated documentation files (the "Software"), to deal
6+
* in the Software without restriction, including without limitation the rights
7+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
8+
* copies of the Software, and to permit persons to whom the Software is
9+
* furnished to do so, subject to the following conditions:
10+
*
11+
* The above copyright notice and this permission notice shall be included in
12+
* all copies or substantial portions of the Software.
13+
*
14+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
15+
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
17+
* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
18+
* BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
19+
* ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
20+
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
21+
* THE SOFTWARE.
22+
*
23+
*/
24+
125
package com.owncloud.android.lib.common
226

327
import android.accounts.AccountManager
@@ -15,14 +39,19 @@ import timber.log.Timber
1539
import java.io.IOException
1640
import java.lang.Exception
1741

42+
/**
43+
* ConnectionValidator
44+
*
45+
* @author Christian Schabesberger
46+
*/
1847
class ConnectionValidator(
1948
val context: Context,
2049
val clearCookiesOnValidation: Boolean
2150
) {
22-
fun validate(baseClient: OwnCloudClient, singleSessionManager: SingleSessionManager): Boolean {
51+
fun validate(baseClient: OwnCloudClient, singleSessionManager: SingleSessionManager, context: Context): Boolean {
2352
try {
2453
var validationRetryCount = 0
25-
val client = OwnCloudClient(baseClient.baseUri, null, false, singleSessionManager)
54+
val client = OwnCloudClient(baseClient.baseUri, null, false, singleSessionManager, context)
2655
if (clearCookiesOnValidation) {
2756
client.clearCookies()
2857
} else {

‎owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/OwnCloudClient.java

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
package com.owncloud.android.lib.common;
2727

28+
import android.content.Context;
2829
import android.net.Uri;
2930

3031
import com.owncloud.android.lib.common.accounts.AccountUtils;
@@ -76,7 +77,10 @@ public class OwnCloudClient extends HttpClient {
7677
public OwnCloudClient(Uri baseUri,
7778
ConnectionValidator connectionValidator,
7879
boolean synchronizeRequests,
79-
SingleSessionManager singleSessionManager) {
80+
SingleSessionManager singleSessionManager,
81+
Context context) {
82+
super(context);
83+
8084
if (baseUri == null) {
8185
throw new IllegalArgumentException("Parameter 'baseUri' cannot be NULL");
8286
}
@@ -99,7 +103,7 @@ public void clearCredentials() {
99103
}
100104

101105
public int executeHttpMethod(HttpBaseMethod method) throws Exception {
102-
if(mSynchronizeRequests) {
106+
if (mSynchronizeRequests) {
103107
synchronized (mRequestMutex) {
104108
return saveExecuteHttpMethod(method);
105109
}
@@ -112,7 +116,7 @@ private int saveExecuteHttpMethod(HttpBaseMethod method) throws Exception {
112116
int repeatCounter = 0;
113117
int status;
114118

115-
if(mFollowRedirects) {
119+
if (mFollowRedirects) {
116120
method.setFollowRedirects(true);
117121
}
118122

@@ -131,11 +135,11 @@ private int saveExecuteHttpMethod(HttpBaseMethod method) throws Exception {
131135
method.setRequestHeader(AUTHORIZATION_HEADER, mCredentials.getHeaderAuth());
132136
}
133137

134-
status = method.execute();
138+
status = method.execute(this);
135139

136140
if (shouldConnectionValidatorBeCalled(method, status)) {
137-
retry = mConnectionValidator.validate(this, mSingleSessionManager); // retry on success fail on no success
138-
} else if(method.getFollowPermanentRedirects() && status == HTTP_MOVED_PERMANENTLY) {
141+
retry = mConnectionValidator.validate(this, mSingleSessionManager, getContext()); // retry on success fail on no success
142+
} else if (method.getFollowPermanentRedirects() && status == HTTP_MOVED_PERMANENTLY) {
139143
retry = true;
140144
method.setFollowRedirects(true);
141145
}
@@ -146,12 +150,15 @@ private int saveExecuteHttpMethod(HttpBaseMethod method) throws Exception {
146150
}
147151

148152
private boolean shouldConnectionValidatorBeCalled(HttpBaseMethod method, int status) {
149-
return !mFollowRedirects &&
150-
!method.getFollowRedirects() &&
151-
mConnectionValidator != null &&
152-
(status == HttpConstants.HTTP_MOVED_TEMPORARILY ||
153-
(!(mCredentials instanceof OwnCloudAnonymousCredentials) &&
154-
status == HttpConstants.HTTP_UNAUTHORIZED));
153+
154+
return mConnectionValidator != null && (
155+
(!(mCredentials instanceof OwnCloudAnonymousCredentials) &&
156+
status == HttpConstants.HTTP_UNAUTHORIZED
157+
) || (!mFollowRedirects &&
158+
!method.getFollowRedirects() &&
159+
status == HttpConstants.HTTP_MOVED_TEMPORARILY
160+
)
161+
);
155162
}
156163

157164
/**

‎owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/SingleSessionManager.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,11 @@ public static void setUserAgent(String userAgent) {
7777
sUserAgent = userAgent;
7878
}
7979

80-
private static OwnCloudClient createOwnCloudClient(Uri uri, Context context, ConnectionValidator connectionValidator, SingleSessionManager singleSessionManager) {
81-
OwnCloudClient client = new OwnCloudClient(uri, connectionValidator, true, singleSessionManager);
82-
HttpClient.setContext(context);
83-
80+
private static OwnCloudClient createOwnCloudClient(Uri uri,
81+
Context context,
82+
ConnectionValidator connectionValidator,
83+
SingleSessionManager singleSessionManager) {
84+
OwnCloudClient client = new OwnCloudClient(uri, connectionValidator, true, singleSessionManager, context);
8485
return client;
8586
}
8687

@@ -130,17 +131,16 @@ public OwnCloudClient getClientFor(OwnCloudAccount account,
130131
// no client to reuse - create a new one
131132
client = createOwnCloudClient(
132133
account.getBaseUri(),
133-
context.getApplicationContext(),
134+
context,
134135
connectionValidator,
135-
this);
136+
this); // TODO remove dependency on OwnCloudClientFactory
136137

137138
//the next two lines are a hack because okHttpclient is used as a singleton instead of being an
138139
//injected instance that can be deleted when required
139140
client.clearCookies();
140141
client.clearCredentials();
141142

142143
client.setAccount(account);
143-
HttpClient.setContext(context);
144144

145145
account.loadCredentials(context);
146146
client.setCredentials(account.getCredentials());

‎owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/http/HttpClient.java

Lines changed: 40 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import okhttp3.Cookie;
3232
import okhttp3.CookieJar;
3333
import okhttp3.HttpUrl;
34-
import okhttp3.Interceptor;
3534
import okhttp3.OkHttpClient;
3635
import okhttp3.Protocol;
3736
import okhttp3.TlsVersion;
@@ -41,7 +40,6 @@
4140
import javax.net.ssl.SSLSocketFactory;
4241
import javax.net.ssl.TrustManager;
4342
import javax.net.ssl.X509TrustManager;
44-
import java.security.KeyManagementException;
4543
import java.security.NoSuchAlgorithmException;
4644
import java.util.Collections;
4745
import java.util.HashMap;
@@ -55,33 +53,46 @@
5553
*/
5654

5755
public class HttpClient {
58-
private static OkHttpClient sOkHttpClient;
59-
private static Context sContext;
60-
private static HashMap<String, List<Cookie>> sCookieStore = new HashMap<>();
61-
private static LogInterceptor sLogInterceptor;
62-
private static Interceptor sDebugInterceptor;
63-
64-
public static OkHttpClient getOkHttpClient() {
65-
if (sOkHttpClient == null) {
56+
private Context mContext;
57+
private HashMap<String, List<Cookie>> mCookieStore = new HashMap<>();
58+
private LogInterceptor mLogInterceptor = new LogInterceptor();
59+
60+
private OkHttpClient mOkHttpClient = null;
61+
62+
protected HttpClient(Context context) {
63+
if (context == null) {
64+
Timber.e("Context may not be NULL!");
65+
throw new NullPointerException("Context may not be NULL!");
66+
}
67+
mContext = context;
68+
}
69+
70+
public OkHttpClient getOkHttpClient() {
71+
if (mOkHttpClient == null) {
6672
try {
6773
final X509TrustManager trustManager = new AdvancedX509TrustManager(
68-
NetworkUtils.getKnownServersStore(sContext));
69-
final SSLSocketFactory sslSocketFactory = getNewSslSocketFactory(trustManager);
70-
// Automatic cookie handling, NOT PERSISTENT
71-
final CookieJar cookieJar = new CookieJarImpl(sCookieStore);
74+
NetworkUtils.getKnownServersStore(mContext));
7275

73-
// TODO: Not verifying the hostname against certificate. ask owncloud security human if this is ok.
74-
//.hostnameVerifier(new BrowserCompatHostnameVerifier());
75-
sOkHttpClient = buildNewOkHttpClient(sslSocketFactory, trustManager, cookieJar);
76+
final SSLContext sslContext = buildSSLContext();
77+
sslContext.init(null, new TrustManager[]{trustManager}, null);
78+
final SSLSocketFactory sslSocketFactory = sslContext.getSocketFactory();
7679

80+
// Automatic cookie handling, NOT PERSISTENT
81+
final CookieJar cookieJar = new CookieJarImpl(mCookieStore);
82+
mOkHttpClient = buildNewOkHttpClient(sslSocketFactory, trustManager, cookieJar);
83+
84+
} catch (NoSuchAlgorithmException nsae) {
85+
Timber.e(nsae, "Could not setup SSL system.");
86+
throw new RuntimeException("Could not setup okHttp client.", nsae);
7787
} catch (Exception e) {
78-
Timber.e(e, "Could not setup SSL system.");
88+
Timber.e(e, "Could not setup okHttp client.");
89+
throw new RuntimeException("Could not setup okHttp client.", e);
7990
}
8091
}
81-
return sOkHttpClient;
92+
return mOkHttpClient;
8293
}
8394

84-
private static SSLContext getSslContext() throws NoSuchAlgorithmException {
95+
private SSLContext buildSSLContext() throws NoSuchAlgorithmException {
8596
try {
8697
return SSLContext.getInstance(TlsVersion.TLS_1_3.javaName());
8798
} catch (NoSuchAlgorithmException tlsv13Exception) {
@@ -102,15 +113,8 @@ private static SSLContext getSslContext() throws NoSuchAlgorithmException {
102113
}
103114
}
104115

105-
private static SSLSocketFactory getNewSslSocketFactory(X509TrustManager trustManager)
106-
throws NoSuchAlgorithmException, KeyManagementException {
107-
final SSLContext sslContext = getSslContext();
108-
sslContext.init(null, new TrustManager[]{trustManager}, null);
109-
return sslContext.getSocketFactory();
110-
}
111-
112-
private static OkHttpClient buildNewOkHttpClient(SSLSocketFactory sslSocketFactory, X509TrustManager trustManager,
113-
CookieJar cookieJar) {
116+
private OkHttpClient buildNewOkHttpClient(SSLSocketFactory sslSocketFactory, X509TrustManager trustManager,
117+
CookieJar cookieJar) {
114118
return new OkHttpClient.Builder()
115119
.addNetworkInterceptor(getLogInterceptor())
116120
.addNetworkInterceptor(DebugInterceptorFactory.INSTANCE.getInterceptor())
@@ -125,22 +129,19 @@ private static OkHttpClient buildNewOkHttpClient(SSLSocketFactory sslSocketFacto
125129
.build();
126130
}
127131

128-
public static LogInterceptor getLogInterceptor() {
129-
if (sLogInterceptor == null) {
130-
sLogInterceptor = new LogInterceptor();
131-
}
132-
return sLogInterceptor;
132+
public Context getContext() {
133+
return mContext;
133134
}
134135

135-
public Context getContext() {
136-
return sContext;
136+
public LogInterceptor getLogInterceptor() {
137+
return mLogInterceptor;
137138
}
138139

139-
public static void setContext(Context context) {
140-
sContext = context;
140+
public List<Cookie> getCookiesFromUrl(HttpUrl httpUrl) {
141+
return mCookieStore.get(httpUrl.host());
141142
}
142143

143144
public void clearCookies() {
144-
sCookieStore.clear();
145+
mCookieStore.clear();
145146
}
146147
}

‎owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/http/methods/HttpBaseMethod.kt

Lines changed: 29 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -38,24 +38,41 @@ import java.net.URL
3838
import java.util.concurrent.TimeUnit
3939

4040
abstract class HttpBaseMethod constructor(url: URL) {
41-
var okHttpClient: OkHttpClient
4241
var httpUrl: HttpUrl = url.toHttpUrlOrNull() ?: throw MalformedURLException()
4342
var request: Request
44-
private var _followPermanentRedirects = false
43+
var followPermanentRedirects = false
4544
abstract var response: Response
46-
4745
var call: Call? = null
4846

47+
var followRedirects: Boolean = true
48+
var retryOnConnectionFailure: Boolean = false
49+
var connectionTimeoutVal: Long? = null
50+
var connectionTimeoutUnit: TimeUnit? = null
51+
var readTimeoutVal: Long? = null
52+
private set
53+
var readTimeoutUnit: TimeUnit? = null
54+
private set
55+
4956
init {
50-
okHttpClient = HttpClient.getOkHttpClient()
5157
request = Request.Builder()
5258
.url(httpUrl)
5359
.build()
5460
}
5561

5662
@Throws(Exception::class)
57-
open fun execute(): Int {
58-
return onExecute()
63+
open fun execute(httpClient: HttpClient): Int {
64+
val okHttpClient = httpClient.okHttpClient.newBuilder().apply {
65+
retryOnConnectionFailure.let { retryOnConnectionFailure(it) }
66+
followRedirects.let { followRedirects(it) }
67+
readTimeoutUnit?.let { unit ->
68+
readTimeoutVal?.let { readTimeout(it, unit) }
69+
}
70+
connectionTimeoutUnit?.let { unit ->
71+
connectionTimeoutVal?.let { connectTimeout(it, unit) }
72+
}
73+
}.build()
74+
75+
return onExecute(okHttpClient)
5976
}
6077

6178
open fun setUrl(url: HttpUrl) {
@@ -137,42 +154,21 @@ abstract class HttpBaseMethod constructor(url: URL) {
137154
// Setter
138155
//////////////////////////////
139156
// Connection parameters
140-
open fun setRetryOnConnectionFailure(retryOnConnectionFailure: Boolean) {
141-
okHttpClient = okHttpClient.newBuilder()
142-
.retryOnConnectionFailure(retryOnConnectionFailure)
143-
.build()
144-
}
157+
145158

146159
open fun setReadTimeout(readTimeout: Long, timeUnit: TimeUnit) {
147-
okHttpClient = okHttpClient.newBuilder()
148-
.readTimeout(readTimeout, timeUnit)
149-
.build()
160+
readTimeoutVal = readTimeout
161+
readTimeoutUnit = timeUnit
150162
}
151163

152164
open fun setConnectionTimeout(
153165
connectionTimeout: Long,
154166
timeUnit: TimeUnit
155167
) {
156-
okHttpClient = okHttpClient.newBuilder()
157-
.readTimeout(connectionTimeout, timeUnit)
158-
.build()
168+
connectionTimeoutVal = connectionTimeout
169+
connectionTimeoutUnit = timeUnit
159170
}
160171

161-
open fun setFollowRedirects(followRedirects: Boolean) {
162-
okHttpClient = okHttpClient.newBuilder()
163-
.followRedirects(followRedirects)
164-
.build()
165-
}
166-
167-
open fun getFollowRedirects() = okHttpClient.followRedirects
168-
169-
open fun setFollowPermanentRedirects(followRedirects: Boolean) {
170-
_followPermanentRedirects = followRedirects
171-
}
172-
173-
open fun getFollowPermanentRedirects() = _followPermanentRedirects
174-
175-
176172
/************
177173
*** Call ***
178174
************/
@@ -187,5 +183,5 @@ abstract class HttpBaseMethod constructor(url: URL) {
187183
// For override
188184
//////////////////////////////
189185
@Throws(Exception::class)
190-
protected abstract fun onExecute(): Int
186+
protected abstract fun onExecute(okHttpClient: OkHttpClient): Int
191187
}

0 commit comments

Comments
 (0)