fix: resolve http client builder when runtime not same && security re…

…inforcement for HTTP requests
aliyun · Jul 18, 2024 · 74fc3a7 · 74fc3a7
1 parent 561793a
commit 74fc3a7
Show file tree

Hide file tree

Showing 11 changed files with 84 additions and 102 deletions.
diff --git a/src/main/java/com/aliyun/tea/Tea.java b/src/main/java/com/aliyun/tea/Tea.java
@@ -19,7 +19,7 @@ public class Tea {
     private static String composeUrl(TeaRequest request) {
         Map<String, String> queries = request.query;
         String host = request.headers.get("host");
-        String protocol = null == request.protocol ? "http" : request.protocol;
+        String protocol = null == request.protocol ? "https" : request.protocol;
         StringBuilder urlBuilder = new StringBuilder();
         urlBuilder.append(protocol);
         urlBuilder.append("://").append(host);
@@ -29,7 +29,7 @@ private static String composeUrl(TeaRequest request) {
         if (null != request.pathname) {
             urlBuilder.append(request.pathname);
         }
-        if (queries != null && queries.size() > 0) {
+        if (queries != null && !queries.isEmpty()) {
             if (urlBuilder.indexOf("?") >= 1) {
                 urlBuilder.append("&");
             } else {
@@ -88,24 +88,6 @@ public static TeaResponse doAction(TeaRequest request, Map<String, Object> runti
         return context.teaResponse();
     }
 
-    private static Map<String, String> setProxyAuthorization(Map<String, String> header, Object httpsProxy) {
-        try {
-            if (!StringUtils.isEmpty(httpsProxy)) {
-                URL proxyUrl = new URL(String.valueOf(httpsProxy));
-                String userInfo = proxyUrl.getUserInfo();
-                if (null != userInfo) {
-                    String[] userMessage = userInfo.split(":");
-                    String credential = Credentials.basic(userMessage[0], userMessage[1]);
-                    header.put("Proxy-Authorization", credential);
-                }
-            }
-            return header;
-        } catch (Exception e) {
-            throw new TeaException(e.getMessage(), e);
-        }
-
-    }
-
     public static boolean allowRetry(Map<String, ?> map, int retryTimes, long now) {
         if (0 == retryTimes) {
             return true;

diff --git a/src/main/java/com/aliyun/tea/TeaModel.java b/src/main/java/com/aliyun/tea/TeaModel.java
@@ -29,7 +29,7 @@ public static Map<String, Object> toMap(Object object) {
 
     private static Map<String, Object> toMap(Object object, Boolean exceptStream) {
         Map<String, Object> map = new HashMap<String, Object>();
-        if (null != object && object instanceof Map) {
+        if (object instanceof Map) {
             return (Map<String, Object>) object;
         }
         if (null == object || !TeaModel.class.isAssignableFrom(object.getClass())) {

diff --git a/src/main/java/com/aliyun/tea/TeaUnretryableException.java b/src/main/java/com/aliyun/tea/TeaUnretryableException.java
@@ -2,9 +2,6 @@
 
 public class TeaUnretryableException extends RuntimeException {
 
-    /**
-     *
-     */
     private static final long serialVersionUID = -7006694712718176751L;
 
     private TeaRequest lastRequest = null;

diff --git a/src/main/java/com/aliyun/tea/interceptor/InterceptorChain.java b/src/main/java/com/aliyun/tea/interceptor/InterceptorChain.java
@@ -10,9 +10,9 @@
 import java.util.Map;
 
 public class InterceptorChain implements AutoCloseable {
-    private List<RuntimeOptionsInterceptor> runtimeOptionsInterceptors = new ArrayList<>();
-    private List<RequestInterceptor> requestInterceptors = new ArrayList<>();
-    private List<ResponseInterceptor> responseInterceptors = new ArrayList<>();
+    private final List<RuntimeOptionsInterceptor> runtimeOptionsInterceptors = new ArrayList<>();
+    private final List<RequestInterceptor> requestInterceptors = new ArrayList<>();
+    private final List<ResponseInterceptor> responseInterceptors = new ArrayList<>();
 
     private InterceptorChain() {
     }
@@ -41,29 +41,26 @@ public void close() {
     }
 
     public InterceptorContext modifyRuntimeOptions(InterceptorContext context, AttributeMap attributes) {
-        InterceptorContext result = context;
         for (RuntimeOptionsInterceptor interceptor : runtimeOptionsInterceptors) {
-            Map<String, Object> interceptorResult = interceptor.modifyRuntimeOptions(result, attributes);
-            result.setRuntimeOptions(interceptorResult);
+            Map<String, Object> interceptorResult = interceptor.modifyRuntimeOptions(context, attributes);
+            context.setRuntimeOptions(interceptorResult);
         }
-        return result;
+        return context;
     }
 
     public InterceptorContext modifyRequest(InterceptorContext context, AttributeMap attributes) {
-        InterceptorContext result = context;
         for (RequestInterceptor interceptor : requestInterceptors) {
-            TeaRequest interceptorResult = interceptor.modifyRequest(result, attributes);
-            result.setTeaRequest(interceptorResult);
+            TeaRequest interceptorResult = interceptor.modifyRequest(context, attributes);
+            context.setTeaRequest(interceptorResult);
         }
-        return result;
+        return context;
     }
 
     public InterceptorContext modifyResponse(InterceptorContext context, AttributeMap attributes) {
-        InterceptorContext result = context;
         for (ResponseInterceptor interceptor : responseInterceptors) {
-            TeaResponse interceptorResult = interceptor.modifyResponse(result, attributes);
-            result.setTeaResponse(interceptorResult);
+            TeaResponse interceptorResult = interceptor.modifyResponse(context, attributes);
+            context.setTeaResponse(interceptorResult);
         }
-        return result;
+        return context;
     }
 }
diff --git a/src/main/java/com/aliyun/tea/okhttp/ClientHelper.java b/src/main/java/com/aliyun/tea/okhttp/ClientHelper.java
@@ -1,33 +1,15 @@
 package com.aliyun.tea.okhttp;
 
-import com.aliyun.tea.utils.StringUtils;
 import okhttp3.OkHttpClient;
 
-import java.net.URI;
-import java.net.URL;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
 public class ClientHelper {
     public static final ConcurrentHashMap<String, OkHttpClient> clients = new ConcurrentHashMap<String, OkHttpClient>();
 
     public static OkHttpClient getOkHttpClient(String host, int port, Map<String, Object> map) throws Exception {
-        String key;
-        if (null != map.get("httpProxy") || null != map.get("httpsProxy")) {
-            Object urlString = null == map.get("httpProxy") ? map.get("httpsProxy") : map.get("httpProxy");
-            URL url = new URL(String.valueOf(urlString));
-            key = StringUtils.isEmpty(url.getUserInfo()) ?
-                    getClientKey(url.getHost(), url.getPort()) :
-                    getClientKey(url.getHost(), url.getPort(), url.getUserInfo());
-        } else if (null != map.get("socks5Proxy")) {
-            Object urlString = map.get("socks5Proxy");
-            URI url = new URI(String.valueOf(urlString));
-            key = StringUtils.isEmpty(url.getUserInfo()) ?
-                    getClientKey(url.getHost(), url.getPort()) :
-                    getClientKey(url.getHost(), url.getPort(), url.getUserInfo());
-        } else {
-            key = getClientKey(host, port);
-        }
+        String key = getClientKey(host, port, map);
         OkHttpClient client = clients.get(key);
         if (null == client) {
             client = creatClient(map);
@@ -36,18 +18,27 @@ public static OkHttpClient getOkHttpClient(String host, int port, Map<String, Ob
         return client;
     }
 
-    public static OkHttpClient creatClient(Map<String, Object> map) {
+    private static OkHttpClient creatClient(Map<String, Object> map) {
         OkHttpClientBuilder builder = new OkHttpClientBuilder();
         builder = builder.protocols(map).connectTimeout(map).readTimeout(map).connectionPool(map).certificate(map).proxy(map).proxyAuthenticator(map);
-        OkHttpClient client = builder.buildOkHttpClient();
-        return client;
+        return builder.buildOkHttpClient();
     }
 
-    public static String getClientKey(String host, int port) {
+    private static String getClientKey(String host, int port) {
         return String.format("%s:%d", host, port);
     }
 
-    public static String getClientKey(String host, int port, String userInfo) {
+    private static String getClientKey(String host, int port, String userInfo) {
         return String.format("%s@%s:%d", userInfo, host, port);
     }
+
+    private static String getClientKey(String host, int port, Map<String, Object> map) {
+        return String.format("%s:%d", host, port) +
+                (map.containsKey("httpProxy") && null != map.get("httpProxy") ? ":" + map.get("httpProxy") : "") +
+                (map.containsKey("httpsProxy") && null != map.get("httpsProxy") ? ":" + map.get("httpsProxy") : "") +
+                (map.containsKey("socks5Proxy") && null != map.get("socks5Proxy") ? ":" + map.get("socks5Proxy") : "") +
+                (map.containsKey("connectTimeout") && null != map.get("connectTimeout") ? ":" + map.get("connectTimeout") : "") +
+                (map.containsKey("readTimeout") && null != map.get("readTimeout") ? ":" + map.get("readTimeout") : "") +
+                (map.containsKey("ignoreSSL") && null != map.get("ignoreSSL") ? ":" + map.get("ignoreSSL") : "");
+    }
 }
diff --git a/src/main/java/com/aliyun/tea/okhttp/OkHttpClientBuilder.java b/src/main/java/com/aliyun/tea/okhttp/OkHttpClientBuilder.java
@@ -22,14 +22,14 @@
 
 public class OkHttpClientBuilder {
     private static final String charset = "UTF-8";
-    private OkHttpClient.Builder builder;
+    private final OkHttpClient.Builder builder;
 
     public OkHttpClientBuilder() {
         builder = new OkHttpClient().newBuilder();
     }
 
     public OkHttpClientBuilder protocols(Map<String, Object> map) {
-        if (map.containsKey("disableHttp2") && null != map.get("disableHttp2")) {
+        if (null != map.get("disableHttp2")) {
             Object object = map.get("disableHttp2");
             boolean disableHttp2 = false;
             try {
@@ -86,17 +86,16 @@ public OkHttpClientBuilder connectionPool(Map<String, Object> map) {
 
     public OkHttpClientBuilder certificate(Map<String, Object> map) {
         try {
-            if (Boolean.parseBoolean(String.valueOf(map.get("ignoreSSL")))) {
+            if (null != map.get("ignoreSSL") && Boolean.parseBoolean(String.valueOf(map.get("ignoreSSL")))) {
                 X509TrustManager compositeX509TrustManager = new X509TrustManagerImp();
                 SSLContext sslContext = SSLContext.getInstance("TLS");
                 sslContext.init(null, new TrustManager[]{compositeX509TrustManager}, new java.security.SecureRandom());
                 this.builder.sslSocketFactory(sslContext.getSocketFactory(), compositeX509TrustManager).
                         hostnameVerifier(new TrueHostnameVerifier());
-            } else if (map.containsKey("ca") && !StringUtils.isEmpty(map.get("ca"))) {
+            } else if (!StringUtils.isEmpty(map.get("ca"))) {
                 SSLContext sslContext = SSLContext.getInstance("TLS");
                 KeyManagerFactory keyManagerFactory = null;
-                if (map.containsKey("key") && !StringUtils.isEmpty(map.get("key"))
-                        && map.containsKey("cert") && !StringUtils.isEmpty(map.get("cert"))) {
+                if (!StringUtils.isEmpty(map.get("key")) && !StringUtils.isEmpty(map.get("cert"))) {
                     KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
                     String cert = String.valueOf(map.get("cert"));
                     try (InputStream is = new ByteArrayInputStream(cert.getBytes(charset))) {

diff --git a/src/main/java/com/aliyun/tea/okhttp/OkRequestBody.java b/src/main/java/com/aliyun/tea/okhttp/OkRequestBody.java
@@ -11,8 +11,8 @@
 
 public class OkRequestBody extends RequestBody {
 
-    private InputStream inputStream;
-    private String contentType;
+    private final InputStream inputStream;
+    private final String contentType;
 
     public OkRequestBody(TeaRequest teaRequest) {
         this.inputStream = teaRequest.body;

diff --git a/src/main/java/com/aliyun/tea/okhttp/OkRequestBuilder.java b/src/main/java/com/aliyun/tea/okhttp/OkRequestBuilder.java
@@ -8,7 +8,7 @@
 
 
 public class OkRequestBuilder {
-    private Request.Builder builder;
+    private final Request.Builder builder;
 
     public OkRequestBuilder(Request.Builder builder) {
         this.builder = builder;

diff --git a/src/main/java/com/aliyun/tea/okhttp/interceptors/SocksProxyAuthInterceptor.java b/src/main/java/com/aliyun/tea/okhttp/interceptors/SocksProxyAuthInterceptor.java
@@ -5,8 +5,8 @@
 import java.io.IOException;
 
 public class SocksProxyAuthInterceptor implements Interceptor {
-    private String user;
-    private String password;
+    private final String user;
+    private final String password;
 
     public SocksProxyAuthInterceptor(String user, String password) {
         this.user = user;

diff --git a/src/test/java/com/aliyun/tea/TeaTest.java b/src/test/java/com/aliyun/tea/TeaTest.java
@@ -7,6 +7,7 @@
 import okhttp3.Protocol;
 import org.junit.Assert;
 import org.junit.Test;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.lang.reflect.InvocationTargetException;
@@ -34,17 +35,17 @@ public void composeUrlTest() throws NoSuchMethodException, InvocationTargetExcep
         request.protocol = null;
         request.query = map;
         String str = (String) composeUrl.invoke(Tea.class, request);
-        Assert.assertEquals("http://test/test?host=test", str);
+        Assert.assertEquals("https://test/test?host=test", str);
 
         request.query = new HashMap<>();
         request.pathname = null;
         str = (String) composeUrl.invoke(Tea.class, request);
-        Assert.assertEquals("http://test", str);
+        Assert.assertEquals("https://test", str);
 
         request.query = null;
         request.pathname = null;
         str = (String) composeUrl.invoke(Tea.class, request);
-        Assert.assertEquals("http://test", str);
+        Assert.assertEquals("https://test", str);
 
         request.query = new HashMap<>();
         request.query.put("test", "and");
@@ -254,21 +255,4 @@ public void toReadableTest() throws IOException {
         String result = new String(bytes, 0, index);
         Assert.assertTrue(str.equals(result));
     }
-
-    @Test
-    public void setProxyAuthorizationTest() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
-        Method setProxyAuthorization = Tea.class.getDeclaredMethod("setProxyAuthorization", Map.class, Object.class);
-        setProxyAuthorization.setAccessible(true);
-        Object httpsProxy = "http://user:password@127.0.0.1:8080";
-        Map<String, String> result = (Map<String, String>) setProxyAuthorization.invoke(new Tea(), new HashMap<String, String>(), httpsProxy);
-        assert "Basic dXNlcjpwYXNzd29yZA==".equals(result.get("Proxy-Authorization"));
-
-        httpsProxy = "http://127.0.0.1:8080";
-        result = (Map<String, String>) setProxyAuthorization.invoke(new Tea(), new HashMap<String, String>(), httpsProxy);
-        assert null == result.get("Proxy-Authorization");
-
-        httpsProxy = null;
-        result = (Map<String, String>) setProxyAuthorization.invoke(new Tea(), new HashMap<String, String>(), httpsProxy);
-        assert null == result.get("Proxy-Authorization");
-    }
 }
diff --git a/src/test/java/com/aliyun/tea/okhttp/ClientHelperTest.java b/src/test/java/com/aliyun/tea/okhttp/ClientHelperTest.java
@@ -4,16 +4,47 @@
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.Map;
 
 import static com.aliyun.tea.okhttp.ClientHelper.clients;
 
 public class ClientHelperTest {
     @Test
-    public void getClientKeyTest() {
+    public void getClientKeyTest() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
         new ClientHelper();
-        Assert.assertTrue("0:0:0:0:0:0:0:1:0".equals(ClientHelper.getClientKey("0:0:0:0:0:0:0:1", 0)));
+        Method getClientKey = ClientHelper.class.getDeclaredMethod("getClientKey", String.class, int.class);
+        getClientKey.setAccessible(true);
+        String str = (String) getClientKey.invoke(ClientHelper.class, "0:0:0:0:0:0:0:1", 0);
+        Assert.assertEquals("0:0:0:0:0:0:0:1:0", str);
+        str = (String) getClientKey.invoke(ClientHelper.class, "0:0:0:0:0:0:0:1", 0);
+        Assert.assertEquals("0:0:0:0:0:0:0:1:0", str);
+
+        getClientKey = ClientHelper.class.getDeclaredMethod("getClientKey", String.class, int.class, String.class);
+        getClientKey.setAccessible(true);
+        str = (String) getClientKey.invoke(ClientHelper.class, "0:0:0:0:0:0:0:1", 0, "user:passwd");
+        Assert.assertEquals("user:passwd@0:0:0:0:0:0:0:1:0", str);
+        str = (String) getClientKey.invoke(ClientHelper.class, "0:0:0:0:0:0:0:1", 0, "user:passwd");
+        Assert.assertEquals("user:passwd@0:0:0:0:0:0:0:1:0", str);
+
+        getClientKey = ClientHelper.class.getDeclaredMethod("getClientKey", String.class, int.class, Map.class);
+        getClientKey.setAccessible(true);
+        str = (String) getClientKey.invoke(ClientHelper.class, "0:0:0:0:0:0:0:1", 0, new HashMap<String, Object>());
+        Assert.assertEquals("0:0:0:0:0:0:0:1:0", str);
+        str = (String) getClientKey.invoke(ClientHelper.class, "0:0:0:0:0:0:0:1", 0, new HashMap<String, Object>());
+        Assert.assertEquals("0:0:0:0:0:0:0:1:0", str);
+
+        Map<String, Object> map = new HashMap<>();
+        map.put("httpProxy", "http://127.0.0.1:80");
+        map.put("httpsProxy", "https://127.0.0.1:80");
+        map.put("socks5Proxy", "socks5://user:password@127.0.0.1:1080");
+        map.put("connectTimeout", 1000);
+        map.put("readTimeout", 2000);
+        map.put("ignoreSSL", false);
+        str = (String) getClientKey.invoke(ClientHelper.class, "0:0:0:0:0:0:0:1", 0, map);
+        Assert.assertEquals("0:0:0:0:0:0:0:1:0:http://127.0.0.1:80:https://127.0.0.1:80:socks5://user:password@127.0.0.1:1080:1000:2000:false", str);
     }
 
     @Test
@@ -27,26 +58,27 @@ public void getOkHttpClientTest() throws Exception {
         map.put("httpsProxy", "https://127.0.0.1:80");
         client = ClientHelper.getOkHttpClient(null, 0, map);
         Assert.assertNotNull(client);
-        Assert.assertNotNull(clients.get("127.0.0.1:80"));
+        Assert.assertNotNull(clients.get("null:0:http://127.0.0.1:80"));
+        Assert.assertNotNull(clients.get("null:0:https://127.0.0.1:80"));
 
         map.put("httpsProxy", "https://user:password@127.0.0.1:80");
         client = ClientHelper.getOkHttpClient(null, 0, map);
         Assert.assertNotNull(client);
-        Assert.assertNotNull(clients.get("user:password@127.0.0.1:80"));
-        Assert.assertNotSame(clients.get("user:password@127.0.0.1:80"), clients.get("127.0.0.1:80"));
+        Assert.assertNotNull(clients.get("null:0:https://user:password@127.0.0.1:80"));
+        Assert.assertNotSame(clients.get("null:0:http://127.0.0.1:80"), clients.get("null:0:https://127.0.0.1:80"));
+        Assert.assertNotSame(clients.get("null:0:https://user:password@127.0.0.1:80"), clients.get("null:0:https://127.0.0.1:80"));
 
 
         map.put("httpsProxy", null);
         map.put("socks5Proxy", "socks5://user:password@127.0.0.1:1080");
         client = ClientHelper.getOkHttpClient(null, 0, map);
         Assert.assertNotNull(client);
 
-        Assert.assertNull(clients.get("127.0.0.1:1080"));
-        Assert.assertNotNull(clients.get("user:password@127.0.0.1:1080"));
+        Assert.assertNotNull(clients.get("null:0:socks5://user:password@127.0.0.1:1080"));
 
         map.put("socks5Proxy", "socks5://user:passwd@127.0.0.1:1080");
         client = ClientHelper.getOkHttpClient(null, 0, map);
         Assert.assertNotNull(client);
-        Assert.assertNotSame(clients.get("user:password@127.0.0.1:1080"), clients.get("user:passwd@127.0.0.1:1080"));
+        Assert.assertNotSame(clients.get("null:0:socks5://user:password@127.0.0.1:1080"), clients.get("null:0:socks5://user:passwd@127.0.0.1:1080"));
     }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -2,9 +2,6 @@ @@
     public class TeaUnretryableException extends RuntimeException {
-        /**
-         *
-         */
         private static final long serialVersionUID = -7006694712718176751L;
         private TeaRequest lastRequest = null;
@@ Expand Down @@