From 591557c99a88388c4ea46d1ca7ba32b8ebb5afe0 Mon Sep 17 00:00:00 2001 From: Van Tuan Vo Date: Mon, 1 Mar 2021 17:20:08 +0100 Subject: [PATCH 1/3] Introduce ConnectionInterceptor for individual connection processing Currently, it is only possible to set static http header values once. It is not possible to add header values which change with each network request. This introduces an interceptor interface to process each connection, i.e. add custom header which change with each request. --- .../android/sdk/ConnectionProcessorTests.java | 85 +++++++++++++++++++ .../android/sdk/ConnectionInterceptor.java | 18 ++++ .../android/sdk/ConnectionProcessor.java | 30 ++++++- .../ly/count/android/sdk/ConnectionQueue.java | 9 +- .../java/ly/count/android/sdk/Countly.java | 10 +++ 5 files changed, 148 insertions(+), 4 deletions(-) create mode 100644 sdk/src/main/java/ly/count/android/sdk/ConnectionInterceptor.java diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/ConnectionProcessorTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ConnectionProcessorTests.java index 183e7183d..3d4533f70 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ConnectionProcessorTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ConnectionProcessorTests.java @@ -23,6 +23,7 @@ of this software and associated documentation files (the "Software"), to deal import androidx.test.ext.junit.runners.AndroidJUnit4; import java.io.ByteArrayInputStream; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.UnsupportedEncodingException; @@ -32,12 +33,17 @@ of this software and associated documentation files (the "Software"), to deal import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; import static ly.count.android.sdk.UtilsNetworking.sha256Hash; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.isNull; @@ -271,6 +277,85 @@ public void testRun_storeHasTwoConnections() throws IOException { verify(mockURLConnection, times(2)).disconnect(); } + @Test + public void testUrlConnectionUsesInterceptor() throws IOException { + final String eventData = "blahblahblah"; + ConnectionInterceptor interceptor = mock(ConnectionInterceptor.class); + when(interceptor.intercept(any(HttpURLConnection.class), nullable(byte[].class))).thenAnswer(new Answer() { + @Override public HttpURLConnection answer(InvocationOnMock invocation) throws Throwable { + return invocation.getArgument(0, HttpURLConnection.class); + } + }); + connectionProcessor.setConnectionInterceptor(interceptor); + final URLConnection urlConnection = connectionProcessor.urlConnectionForServerRequest(eventData, null); + verify(interceptor).intercept(any(HttpURLConnection.class), nullable(byte[].class)); + assertEquals(30000, urlConnection.getConnectTimeout()); + assertEquals(30000, urlConnection.getReadTimeout()); + assertFalse(urlConnection.getUseCaches()); + assertTrue(urlConnection.getDoInput()); + assertFalse(urlConnection.getDoOutput()); + assertEquals(new URL(connectionProcessor.getServerURL() + "/i?" + eventData + "&checksum256=" + sha256Hash(eventData + null)), urlConnection.getURL()); + } + + @Test + public void testUrlConnectionInterceptorCanSetRequestPropertiesOnGet() throws IOException { + final String eventData = "blahblahblah"; + ConnectionInterceptor interceptor = new ConnectionInterceptor() { + @Override public HttpURLConnection intercept(HttpURLConnection connection, byte[] body) { + connection.setRequestProperty("Prop", "SomeDynamicHeaderValue"); + return connection; + } + }; + connectionProcessor.setConnectionInterceptor(interceptor); + URLConnection conn = connectionProcessor.urlConnectionForServerRequest(eventData, null); + assertEquals("SomeDynamicHeaderValue", conn.getRequestProperty("Prop")); + } + + @Test + public void connectionInterceptorCanSetRequestPropertiesOnPost() throws IOException { + // Crash data uses http post + final String eventData = "blahblahblah&crash=lol"; + connectionProcessor = new ConnectionProcessor("https://count.ly/", mockStore, mockDeviceId, null, null, moduleLog); + ConnectionInterceptor interceptor = new ConnectionInterceptor() { + @Override public HttpURLConnection intercept(HttpURLConnection connection, byte[] body) { + connection.setRequestProperty("Prop", "SomeDynamicHeaderValue"); + return connection; + } + }; + connectionProcessor.setConnectionInterceptor(interceptor); + URLConnection conn = connectionProcessor.urlConnectionForServerRequest(eventData, null); + assertEquals("SomeDynamicHeaderValue", conn.getRequestProperty("Prop")); + } + + @Test + public void testConnectionInterceptorCanSetRequestPropertiesOnPostPicturePath() throws IOException { + File picture = File.createTempFile("IconicFinance", ".png"); + final String eventData = "picturePath="+picture.getPath(); + connectionProcessor = new ConnectionProcessor("https://count.ly/", mockStore, mockDeviceId, null, null, moduleLog); + ConnectionInterceptor interceptor = new ConnectionInterceptor() { + @Override public HttpURLConnection intercept(HttpURLConnection connection, byte[] body) { + connection.setRequestProperty("Prop", "SomeDynamicHeaderValue"); + return connection; + } + }; + connectionProcessor.setConnectionInterceptor(interceptor); + URLConnection conn = connectionProcessor.urlConnectionForServerRequest(eventData, null); + assertEquals("SomeDynamicHeaderValue", conn.getRequestProperty("Prop")); + } + + @Test + public void testUrlConnectionDoesNotUseInterceptorWhenNotAvailable() throws IOException { + final String eventData = "blahblahblah"; + final URLConnection urlConnection = connectionProcessor.urlConnectionForServerRequest(eventData, null); + assertNull(connectionProcessor.getConnectionInterceptor()); + assertEquals(30000, urlConnection.getConnectTimeout()); + assertEquals(30000, urlConnection.getReadTimeout()); + assertFalse(urlConnection.getUseCaches()); + assertTrue(urlConnection.getDoInput()); + assertFalse(urlConnection.getDoOutput()); + assertEquals(new URL(connectionProcessor.getServerURL() + "/i?" + eventData + "&checksum256=" + sha256Hash(eventData + null)), urlConnection.getURL()); + } + private static class TestInputStream2 extends InputStream { boolean closed = false; diff --git a/sdk/src/main/java/ly/count/android/sdk/ConnectionInterceptor.java b/sdk/src/main/java/ly/count/android/sdk/ConnectionInterceptor.java new file mode 100644 index 000000000..f20a23e23 --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/ConnectionInterceptor.java @@ -0,0 +1,18 @@ +package ly.count.android.sdk; + +import java.net.HttpURLConnection; + +/** + * Interface to intercept Countly requests + */ +public interface ConnectionInterceptor { + + /** + * This is called for each request which is send by Countly + * + * @param connection The connection which is about to be send + * @param body Body of the connection, null for GET requests + * @return HttpURLConnection which is used for connection + */ + HttpURLConnection intercept(HttpURLConnection connection, byte[] body); +} \ No newline at end of file diff --git a/sdk/src/main/java/ly/count/android/sdk/ConnectionProcessor.java b/sdk/src/main/java/ly/count/android/sdk/ConnectionProcessor.java index 217ce0ed3..90c769347 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ConnectionProcessor.java +++ b/sdk/src/main/java/ly/count/android/sdk/ConnectionProcessor.java @@ -21,8 +21,8 @@ of this software and associated documentation files (the "Software"), to deal */ package ly.count.android.sdk; -import android.util.Log; import java.io.BufferedWriter; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileInputStream; import java.io.IOException; @@ -67,6 +67,8 @@ private enum RequestResult { REMOVE // bad request, remove } + private ConnectionInterceptor connectionInterceptor; + ConnectionProcessor(final String serverURL, final CountlyStore store, final DeviceId deviceId, final SSLContext sslContext, final Map requestHeaderCustomValues, ModuleLog logModule) { serverURL_ = serverURL; store_ = store; @@ -92,7 +94,7 @@ synchronized public URLConnection urlConnectionForServerRequest(String requestDa urlStr += "&checksum256=" + UtilsNetworking.sha256Hash(requestData + salt); } final URL url = new URL(urlStr); - final HttpURLConnection conn; + HttpURLConnection conn; if (Countly.publicKeyPinCertificates == null && Countly.certificatePinCertificates == null) { conn = (HttpURLConnection) url.openConnection(); } else { @@ -134,7 +136,7 @@ synchronized public URLConnection urlConnectionForServerRequest(String requestDa String CRLF = "\r\n"; String charset = "UTF-8"; conn.setRequestProperty("Content-Type", "multipart/form-data; boundary=" + boundary); - OutputStream output = conn.getOutputStream(); + ByteArrayOutputStream output = new ByteArrayOutputStream(); PrintWriter writer = new PrintWriter(new OutputStreamWriter(output, charset), true); // Send binary file. writer.append("--").append(boundary).append(CRLF); @@ -158,10 +160,21 @@ synchronized public URLConnection urlConnectionForServerRequest(String requestDa // End of multipart/form-data. writer.append("--").append(boundary).append("--").append(CRLF).flush(); + if (connectionInterceptor != null) { + conn = connectionInterceptor.intercept(conn, output.toByteArray()); + } + OutputStream connectionOutput = conn.getOutputStream(); + connectionOutput.write(output.toByteArray()); + connectionOutput.flush(); + output.close(); + connectionOutput.close(); } else { if (usingHttpPost) { conn.setDoOutput(true); conn.setRequestMethod("POST"); + if (connectionInterceptor != null) { + conn = connectionInterceptor.intercept(conn, requestData.getBytes()); + } OutputStream os = conn.getOutputStream(); BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(os, "UTF-8")); writer.write(requestData); @@ -169,6 +182,9 @@ synchronized public URLConnection urlConnectionForServerRequest(String requestDa writer.close(); os.close(); } else { + if (connectionInterceptor != null) { + conn = connectionInterceptor.intercept(conn, null); + } L.v("[Connection Processor] Using HTTP GET"); conn.setDoOutput(false); } @@ -394,4 +410,12 @@ CountlyStore getCountlyStore() { DeviceId getDeviceId() { return deviceId_; } + + public ConnectionInterceptor getConnectionInterceptor() { + return connectionInterceptor; + } + + public void setConnectionInterceptor(ConnectionInterceptor connectionInterceptor) { + this.connectionInterceptor = connectionInterceptor; + } } diff --git a/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java b/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java index 1b28d2bed..346094ea1 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java +++ b/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java @@ -51,6 +51,7 @@ public class ConnectionQueue { private Future connectionProcessorFuture_; private DeviceId deviceId_; private SSLContext sslContext_; + private ConnectionInterceptor connectionInterceptor_; private Map requestHeaderCustomValues; Map metricOverride = null; @@ -652,8 +653,14 @@ void tick() { } } + public void setConnectionInterceptor(ConnectionInterceptor interceptor) { + this.connectionInterceptor_ = interceptor; + } + public ConnectionProcessor createConnectionProcessor() { - return new ConnectionProcessor(getServerURL(), store_, deviceId_, sslContext_, requestHeaderCustomValues, L); + ConnectionProcessor processor = new ConnectionProcessor(getServerURL(), store_, deviceId_, sslContext_, requestHeaderCustomValues, L); + processor.setConnectionInterceptor(connectionInterceptor_); + return processor; } public boolean queueContainsTemporaryIdItems() { diff --git a/sdk/src/main/java/ly/count/android/sdk/Countly.java b/sdk/src/main/java/ly/count/android/sdk/Countly.java index 8922323bf..b1eb2f5c0 100644 --- a/sdk/src/main/java/ly/count/android/sdk/Countly.java +++ b/sdk/src/main/java/ly/count/android/sdk/Countly.java @@ -883,6 +883,16 @@ public void onRegistrationId(String registrationId, CountlyMessagingMode mode, C connectionQueue_.tokenSession(registrationId, mode, provider); } + /** + * Sets an interceptor which can be used to run custom connection processing for each network requests. + * This is useful to add dynamic headers for each request. + * + * @param interceptor Gets an HttpURLConnection and returns a new HttpURLConnection + */ + public void setConnectionInterceptor(ConnectionInterceptor interceptor) { + connectionQueue_.setConnectionInterceptor(interceptor); + } + /** * Changes current device id type to the one specified in parameter. Closes current session and * reopens new one with new id. Doesn't merge user profiles on the server From 031608a1e4420f62f8abb4d1e717be31fe641ca8 Mon Sep 17 00:00:00 2001 From: Van Tuan Vo Date: Mon, 1 Mar 2021 23:52:51 +0100 Subject: [PATCH 2/3] Set ConnectionInterceptor in CountlyConfig --- .../ly/count/android/sdk/CountlyConfigTests.java | 10 ++++++++++ .../java/ly/count/android/sdk/ConnectionQueue.java | 12 ++++++++---- sdk/src/main/java/ly/count/android/sdk/Countly.java | 11 +---------- .../java/ly/count/android/sdk/CountlyConfig.java | 13 +++++++++++++ 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/CountlyConfigTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/CountlyConfigTests.java index bf6f7527e..7282d3acc 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/CountlyConfigTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/CountlyConfigTests.java @@ -4,6 +4,7 @@ import android.app.Application; import android.content.Context; import androidx.test.ext.junit.runners.AndroidJUnit4; +import java.net.HttpURLConnection; import java.util.HashMap; import java.util.Map; import org.junit.Assert; @@ -93,6 +94,12 @@ public boolean filterCrash(String crash) { Application app = new Application(); + ConnectionInterceptor interceptor = new ConnectionInterceptor() { + @Override public HttpURLConnection intercept(HttpURLConnection connection, byte[] body) { + return null; + } + }; + assertDefaultValues(config, true); config.setServerURL(s[0]); @@ -142,6 +149,7 @@ public boolean filterCrash(String crash) { config.setDisableLocation(); config.setLocation("CC", "city", "loc", "ip"); config.setMetricOverride(metricOverride); + config.setConnectionInterceptor(interceptor); Assert.assertEquals(s[0], config.serverURL); Assert.assertEquals(c, config.context); @@ -194,6 +202,7 @@ public boolean filterCrash(String crash) { Assert.assertEquals("loc", config.locationLocation); Assert.assertEquals("ip", config.locationIpAddress); Assert.assertEquals(metricOverride, config.metricOverride); + Assert.assertEquals(interceptor, config.interceptor); config.setLocation("CC", "city", "loc", "ip"); } @@ -265,5 +274,6 @@ void assertDefaultValues(CountlyConfig config, boolean includeConstructorValues) Assert.assertNull(config.locationLocation); Assert.assertNull(config.locationIpAddress); Assert.assertNull(config.metricOverride); + Assert.assertNull(config.interceptor); } } diff --git a/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java b/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java index 346094ea1..ae5831e92 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java +++ b/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java @@ -111,6 +111,14 @@ public void setDeviceId(DeviceId deviceId) { this.deviceId_ = deviceId; } + public ConnectionInterceptor getConnectionInterceptor() { + return connectionInterceptor_; + } + + public void setConnectionInterceptor(ConnectionInterceptor connectionInterceptor_) { + this.connectionInterceptor_ = connectionInterceptor_; + } + protected void setRequestHeaderCustomValues(Map headerCustomValues) { requestHeaderCustomValues = headerCustomValues; } @@ -653,10 +661,6 @@ void tick() { } } - public void setConnectionInterceptor(ConnectionInterceptor interceptor) { - this.connectionInterceptor_ = interceptor; - } - public ConnectionProcessor createConnectionProcessor() { ConnectionProcessor processor = new ConnectionProcessor(getServerURL(), store_, deviceId_, sslContext_, requestHeaderCustomValues, L); processor.setConnectionInterceptor(connectionInterceptor_); diff --git a/sdk/src/main/java/ly/count/android/sdk/Countly.java b/sdk/src/main/java/ly/count/android/sdk/Countly.java index b1eb2f5c0..2ea379e01 100644 --- a/sdk/src/main/java/ly/count/android/sdk/Countly.java +++ b/sdk/src/main/java/ly/count/android/sdk/Countly.java @@ -599,6 +599,7 @@ public synchronized Countly init(CountlyConfig config) { connectionQueue_.setDeviceId(config.deviceIdInstance); connectionQueue_.setRequestHeaderCustomValues(requestHeaderCustomValues); connectionQueue_.setMetricOverride(config.metricOverride); + connectionQueue_.setConnectionInterceptor(config.interceptor); connectionQueue_.setContext(context_); eventQueue_ = new EventQueue(countlyStore); @@ -883,16 +884,6 @@ public void onRegistrationId(String registrationId, CountlyMessagingMode mode, C connectionQueue_.tokenSession(registrationId, mode, provider); } - /** - * Sets an interceptor which can be used to run custom connection processing for each network requests. - * This is useful to add dynamic headers for each request. - * - * @param interceptor Gets an HttpURLConnection and returns a new HttpURLConnection - */ - public void setConnectionInterceptor(ConnectionInterceptor interceptor) { - connectionQueue_.setConnectionInterceptor(interceptor); - } - /** * Changes current device id type to the one specified in parameter. Closes current session and * reopens new one with new id. Doesn't merge user profiles on the server diff --git a/sdk/src/main/java/ly/count/android/sdk/CountlyConfig.java b/sdk/src/main/java/ly/count/android/sdk/CountlyConfig.java index f79ac53b4..39a85e113 100644 --- a/sdk/src/main/java/ly/count/android/sdk/CountlyConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/CountlyConfig.java @@ -134,6 +134,8 @@ public class CountlyConfig { protected boolean recordAppStartTime = false; + protected ConnectionInterceptor interceptor = null; + boolean disableLocation = false; String locationCountyCode = null; @@ -583,4 +585,15 @@ public synchronized CountlyConfig setLogListener(ModuleLog.LogCallback logCallba providedLogCallback = logCallback; return this; } + + /** + * Sets an interceptor which can be used to run custom connection processing for each network requests. + * This is useful to add dynamic headers for each request. + * + * @param interceptor Gets an HttpURLConnection and returns a new HttpURLConnection + */ + public synchronized CountlyConfig setConnectionInterceptor(ConnectionInterceptor interceptor) { + this.interceptor = interceptor; + return this; + } } From 98c6b6c5ac756893867294fce2801835791b822e Mon Sep 17 00:00:00 2001 From: Van Tuan Vo Date: Tue, 2 Mar 2021 01:04:36 +0100 Subject: [PATCH 3/3] Fix Multipart sending when interceptor is used --- .../android/sdk/ConnectionProcessor.java | 67 ++++++++++--------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/sdk/src/main/java/ly/count/android/sdk/ConnectionProcessor.java b/sdk/src/main/java/ly/count/android/sdk/ConnectionProcessor.java index 90c769347..2ec99983e 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ConnectionProcessor.java +++ b/sdk/src/main/java/ly/count/android/sdk/ConnectionProcessor.java @@ -78,6 +78,38 @@ private enum RequestResult { L = logModule; } + private void writeMultipartDataToOutput(File binaryFile, String boundary, OutputStream output) throws IOException { + // Line separator required by multipart/form-data. + String CRLF = "\r\n"; + String charset = "UTF-8"; + PrintWriter writer = new PrintWriter(new OutputStreamWriter(output, charset), true); + // Send binary file. + writer.append("--").append(boundary).append(CRLF); + writer.append("Content-Disposition: form-data; name=\"binaryFile\"; filename=\"").append(binaryFile.getName()).append("\"").append(CRLF); + writer.append("Content-Type: ").append(URLConnection.guessContentTypeFromName(binaryFile.getName())).append(CRLF); + writer.append("Content-Transfer-Encoding: binary").append(CRLF); + writer.append(CRLF).flush(); + FileInputStream fileInputStream = new FileInputStream(binaryFile); + byte[] buffer = new byte[1024]; + int len; + try { + while ((len = fileInputStream.read(buffer)) != -1) { + output.write(buffer, 0, len); + } + } catch (IOException ex) { + ex.printStackTrace(); + } + output.flush(); // Important before continuing with writer! + writer.append(CRLF).flush(); // CRLF is important! It indicates end of boundary. + fileInputStream.close(); + + // End of multipart/form-data. + writer.append("--").append(boundary).append("--").append(CRLF).flush(); + writer.close(); + output.flush(); + output.close(); + } + synchronized public URLConnection urlConnectionForServerRequest(String requestData, final String customEndpoint) throws IOException { String urlEndpoint = "/i"; if (customEndpoint != null) { @@ -132,42 +164,13 @@ synchronized public URLConnection urlConnectionForServerRequest(String requestDa conn.setDoOutput(true); // Just generate some unique random value. String boundary = Long.toHexString(System.currentTimeMillis()); - // Line separator required by multipart/form-data. - String CRLF = "\r\n"; - String charset = "UTF-8"; conn.setRequestProperty("Content-Type", "multipart/form-data; boundary=" + boundary); - ByteArrayOutputStream output = new ByteArrayOutputStream(); - PrintWriter writer = new PrintWriter(new OutputStreamWriter(output, charset), true); - // Send binary file. - writer.append("--").append(boundary).append(CRLF); - writer.append("Content-Disposition: form-data; name=\"binaryFile\"; filename=\"").append(binaryFile.getName()).append("\"").append(CRLF); - writer.append("Content-Type: ").append(URLConnection.guessContentTypeFromName(binaryFile.getName())).append(CRLF); - writer.append("Content-Transfer-Encoding: binary").append(CRLF); - writer.append(CRLF).flush(); - FileInputStream fileInputStream = new FileInputStream(binaryFile); - byte[] buffer = new byte[1024]; - int len; - try { - while ((len = fileInputStream.read(buffer)) != -1) { - output.write(buffer, 0, len); - } - } catch (IOException ex) { - ex.printStackTrace(); - } - output.flush(); // Important before continuing with writer! - writer.append(CRLF).flush(); // CRLF is important! It indicates end of boundary. - fileInputStream.close(); - - // End of multipart/form-data. - writer.append("--").append(boundary).append("--").append(CRLF).flush(); if (connectionInterceptor != null) { + ByteArrayOutputStream output = new ByteArrayOutputStream(); + writeMultipartDataToOutput(binaryFile, boundary, output); conn = connectionInterceptor.intercept(conn, output.toByteArray()); } - OutputStream connectionOutput = conn.getOutputStream(); - connectionOutput.write(output.toByteArray()); - connectionOutput.flush(); - output.close(); - connectionOutput.close(); + writeMultipartDataToOutput(binaryFile, boundary, conn.getOutputStream()); } else { if (usingHttpPost) { conn.setDoOutput(true);