Skip to content

Commit ceb2dd0

Browse files
AugustyniakAugustyniak
authored
android proxy: fix initial proxy settings state (#2709)
Description: Upon investigation of multiple sessions with active proxy settings it seems to be highly likely that our EM proxy logic did not engage in all of the aforementioned sessions. That appears to be a direct result of the way our logic for registering to `PROXY_CHANGE` intent works. The implementation assumes that upon subscription to the intent we always receive an initial intent with the state of proxy settings. That does not seem to be guaranteed by Android as `PROXY_CHANGE` intent is not documented to be sticky (a term used by Android) and whether it's sticky or not may depend on manufacturer / type of the proxy update. Risk Level: Low, hidden behind a feature flag and disabled by default. Testing: Integration tests Docs Changes: N/A Release Notes: N/A Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
1 parent 6eafceb commit ceb2dd0

8 files changed

Lines changed: 278 additions & 8 deletions

‎library/java/io/envoyproxy/envoymobile/engine/AndroidProxyMonitor.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@ private AndroidProxyMonitor(Context context, EnvoyEngine envoyEngine) {
3535
this.envoyEngine = envoyEngine;
3636
this.connectivityManager =
3737
(ConnectivityManager)context.getSystemService(Context.CONNECTIVITY_SERVICE);
38+
// PROXY_INFO is not guaranteed to be a sticky intent so we need to trigger
39+
// a manual poll of proxy settings. Proxy settings received as a result of PROXY_INFO
40+
// intent updates should take precedence/override locally polled settings,
41+
// hence we trigger a manual proxy settings poll before we subscribe to PROXY_INFO intent
42+
// updates.
43+
handleProxyChange(null);
3844
registerReceiver(context);
3945
}
4046

@@ -76,6 +82,11 @@ private ProxyInfo extractProxyInfo(final Intent intent) {
7682
//
7783
// See https://github.com/envoyproxy/envoy-mobile/issues/2531 for more details.
7884
if (info.getPacFileUrl() != null && info.getPacFileUrl() != Uri.EMPTY) {
85+
if (intent == null) {
86+
// PAC proxies are supported only when Intent is present
87+
return null;
88+
}
89+
7990
Bundle extras = intent.getExtras();
8091
if (extras == null) {
8192
return null;

‎test/kotlin/integration/proxying/BUILD‎

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ envoy_mobile_kt_library(
1212
)
1313

1414
envoy_mobile_android_test(
15-
name = "perform_http_request_using_proxy_test",
15+
name = "proxy_info_intent_perform_http_request_using_proxy_test",
1616
srcs = [
17-
"PerformHTTPRequestUsingProxyTest.kt",
17+
"ProxyInfoIntentPerformHTTPRequestUsingProxyTest.kt",
1818
],
1919
exec_properties = {
2020
# TODO(lfpino): Remove this once the sandboxNetwork=off works for ipv4 localhost addresses.
@@ -33,9 +33,9 @@ envoy_mobile_android_test(
3333
)
3434

3535
envoy_mobile_android_test(
36-
name = "perform_https_request_using_proxy_test",
36+
name = "proxy_info_intent_perform_https_request_using_proxy_test",
3737
srcs = [
38-
"PerformHTTPSRequestUsingProxyTest.kt",
38+
"ProxyInfoIntentPerformHTTPSRequestUsingProxyTest.kt",
3939
],
4040
exec_properties = {
4141
# TODO(lfpino): Remove this once the sandboxNetwork=off works for ipv4 localhost addresses.
@@ -54,9 +54,9 @@ envoy_mobile_android_test(
5454
)
5555

5656
envoy_mobile_android_test(
57-
name = "perform_https_request_using_async_proxy_test",
57+
name = "proxy_info_intent_perform_https_request_using_async_proxy_test",
5858
srcs = [
59-
"PerformHTTPSRequestUsingAsyncProxyTest.kt",
59+
"ProxyInfoIntentPerformHTTPSRequestUsingAsyncProxyTest.kt",
6060
],
6161
exec_properties = {
6262
# TODO(lfpino): Remove this once the sandboxNetwork=off works for ipv4 localhost addresses.
@@ -75,9 +75,51 @@ envoy_mobile_android_test(
7575
)
7676

7777
envoy_mobile_android_test(
78-
name = "perform_https_request_bad_hostname",
78+
name = "proxy_info_intent_perform_https_request_bad_hostname",
7979
srcs = [
80-
"PerformHTTPSRequestBadHostname.kt",
80+
"ProxyInfoIntentPerformHTTPSRequestBadHostnameTest.kt",
81+
],
82+
exec_properties = {
83+
# TODO(lfpino): Remove this once the sandboxNetwork=off works for ipv4 localhost addresses.
84+
"sandboxNetwork": "standard",
85+
"dockerNetwork": "standard",
86+
},
87+
native_deps = [
88+
"//library/common/jni:libndk_envoy_jni.so",
89+
"//library/common/jni:libndk_envoy_jni.jnilib",
90+
],
91+
deps = [
92+
"//library/kotlin/io/envoyproxy/envoymobile:envoy_interfaces_lib",
93+
"//library/kotlin/io/envoyproxy/envoymobile:envoy_lib",
94+
"//test/kotlin/integration/proxying:proxy_lib",
95+
],
96+
)
97+
98+
envoy_mobile_android_test(
99+
name = "proxy_poll_perform_http_request_using_proxy",
100+
srcs = [
101+
"ProxyPollPerformHTTPRequestUsingProxyTest.kt",
102+
],
103+
exec_properties = {
104+
# TODO(lfpino): Remove this once the sandboxNetwork=off works for ipv4 localhost addresses.
105+
"sandboxNetwork": "standard",
106+
"dockerNetwork": "standard",
107+
},
108+
native_deps = [
109+
"//library/common/jni:libndk_envoy_jni.so",
110+
"//library/common/jni:libndk_envoy_jni.jnilib",
111+
],
112+
deps = [
113+
"//library/kotlin/io/envoyproxy/envoymobile:envoy_interfaces_lib",
114+
"//library/kotlin/io/envoyproxy/envoymobile:envoy_lib",
115+
"//test/kotlin/integration/proxying:proxy_lib",
116+
],
117+
)
118+
119+
envoy_mobile_android_test(
120+
name = "proxy_poll_perform_http_request_without_using_pac_proxy",
121+
srcs = [
122+
"ProxyPollPerformHTTPRequestWithoutUsingPACProxyTest.kt",
81123
],
82124
exec_properties = {
83125
# TODO(lfpino): Remove this once the sandboxNetwork=off works for ipv4 localhost addresses.

test/kotlin/integration/proxying/PerformHTTPRequestUsingProxyTest.kt renamed to test/kotlin/integration/proxying/ProxyInfoIntentPerformHTTPRequestUsingProxyTest.kt

File renamed without changes.

test/kotlin/integration/proxying/PerformHTTPSRequestBadHostname.kt renamed to test/kotlin/integration/proxying/ProxyInfoIntentPerformHTTPSRequestBadHostnameTest.kt

File renamed without changes.

test/kotlin/integration/proxying/PerformHTTPSRequestUsingAsyncProxyTest.kt renamed to test/kotlin/integration/proxying/ProxyInfoIntentPerformHTTPSRequestUsingAsyncProxyTest.kt

File renamed without changes.

test/kotlin/integration/proxying/PerformHTTPSRequestUsingProxyTest.kt renamed to test/kotlin/integration/proxying/ProxyInfoIntentPerformHTTPSRequestUsingProxyTest.kt

File renamed without changes.

‎test/kotlin/integration/proxying/ProxyPollPerformHTTPRequestUsingProxyTest.kt‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
package test.kotlin.integration.proxying
2+
3+
import android.content.Intent
4+
import android.content.Context
5+
import android.net.ConnectivityManager
6+
import android.net.Proxy
7+
import android.net.ProxyInfo
8+
import androidx.test.core.app.ApplicationProvider
9+
10+
import io.envoyproxy.envoymobile.AndroidEngineBuilder
11+
import io.envoyproxy.envoymobile.Custom
12+
import io.envoyproxy.envoymobile.Engine
13+
import io.envoyproxy.envoymobile.engine.JniLibrary
14+
import io.envoyproxy.envoymobile.LogLevel
15+
import io.envoyproxy.envoymobile.RequestHeadersBuilder
16+
import io.envoyproxy.envoymobile.RequestMethod
17+
import io.envoyproxy.envoymobile.ResponseHeaders
18+
import io.envoyproxy.envoymobile.StreamIntel
19+
20+
import java.util.concurrent.CountDownLatch
21+
import java.util.concurrent.Executors
22+
import java.util.concurrent.TimeUnit
23+
24+
import org.assertj.core.api.Assertions.assertThat
25+
import org.junit.Test
26+
import org.junit.runner.RunWith
27+
import org.mockito.Mock
28+
import org.mockito.Mockito
29+
import org.robolectric.RobolectricTestRunner
30+
31+
// ┌──────────────────┐
32+
// │ Proxy Engine │
33+
// │ ┌──────────────┐ │
34+
// ┌────────────────────────┐ ┌─┼─►listener_proxy│ │
35+
// │http://api.lyft.com/ping│ ┌──────────────┬┘ │ └──────┬───────┘ │ ┌────────────┐
36+
// │ Request ├──►Android Engine│ │ │ │ │api.lyft.com│
37+
// └────────────────────────┘ └──────────────┘ │ ┌──────▼──────┐ │ └──────▲─────┘
38+
// │ │cluster_proxy│ │ │
39+
// │ └─────────────┴──┼────────┘
40+
// │ │
41+
// └──────────────────┘
42+
@RunWith(RobolectricTestRunner::class)
43+
class PerformHTTPRequestUsingProxy {
44+
init {
45+
JniLibrary.loadTestLibrary()
46+
}
47+
48+
@Test
49+
fun `performs an HTTP request through a proxy`() {
50+
val port = (10001..11000).random()
51+
52+
val context = Mockito.spy(ApplicationProvider.getApplicationContext<Context>())
53+
val connectivityManager: ConnectivityManager = Mockito.mock(ConnectivityManager::class.java)
54+
Mockito.doReturn(connectivityManager).`when`(context).getSystemService(Context.CONNECTIVITY_SERVICE)
55+
Mockito.`when`(connectivityManager.getDefaultProxy()).thenReturn(ProxyInfo.buildDirectProxy("127.0.0.1", port))
56+
57+
val onProxyEngineRunningLatch = CountDownLatch(1)
58+
val onEngineRunningLatch = CountDownLatch(1)
59+
val onRespondeHeadersLatch = CountDownLatch(1)
60+
61+
val proxyEngineBuilder = Proxy(context, port)
62+
.http()
63+
val proxyEngine = proxyEngineBuilder
64+
.addLogLevel(LogLevel.DEBUG)
65+
.setOnEngineRunning { onProxyEngineRunningLatch.countDown() }
66+
.build()
67+
68+
onProxyEngineRunningLatch.await(10, TimeUnit.SECONDS)
69+
assertThat(onProxyEngineRunningLatch.count).isEqualTo(0)
70+
71+
val builder = AndroidEngineBuilder(context)
72+
val engine = builder
73+
.addLogLevel(LogLevel.DEBUG)
74+
.enableProxying(true)
75+
.setOnEngineRunning { onEngineRunningLatch.countDown() }
76+
.build()
77+
78+
onEngineRunningLatch.await(10, TimeUnit.SECONDS)
79+
assertThat(onEngineRunningLatch.count).isEqualTo(0)
80+
81+
val requestHeaders = RequestHeadersBuilder(
82+
method = RequestMethod.GET,
83+
scheme = "http",
84+
authority = "api.lyft.com",
85+
path = "/ping"
86+
)
87+
.build()
88+
89+
engine
90+
.streamClient()
91+
.newStreamPrototype()
92+
.setOnResponseHeaders { responseHeaders, _, _ ->
93+
val status = responseHeaders.httpStatus ?: 0L
94+
assertThat(status).isEqualTo(301)
95+
assertThat(responseHeaders.value("x-proxy-response")).isEqualTo(listOf("true"))
96+
onRespondeHeadersLatch.countDown()
97+
}
98+
.start(Executors.newSingleThreadExecutor())
99+
.sendHeaders(requestHeaders, true)
100+
101+
onRespondeHeadersLatch.await(15, TimeUnit.SECONDS)
102+
assertThat(onRespondeHeadersLatch.count).isEqualTo(0)
103+
104+
engine.terminate()
105+
proxyEngine.terminate()
106+
}
107+
}

‎test/kotlin/integration/proxying/ProxyPollPerformHTTPRequestWithoutUsingPACProxyTest.kt‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
package test.kotlin.integration.proxying
2+
3+
import android.content.Intent
4+
import android.content.Context
5+
import android.net.ConnectivityManager
6+
import android.net.Proxy
7+
import android.net.ProxyInfo
8+
import android.net.Uri
9+
import androidx.test.core.app.ApplicationProvider
10+
11+
import io.envoyproxy.envoymobile.AndroidEngineBuilder
12+
import io.envoyproxy.envoymobile.Custom
13+
import io.envoyproxy.envoymobile.Engine
14+
import io.envoyproxy.envoymobile.engine.JniLibrary
15+
import io.envoyproxy.envoymobile.LogLevel
16+
import io.envoyproxy.envoymobile.RequestHeadersBuilder
17+
import io.envoyproxy.envoymobile.RequestMethod
18+
import io.envoyproxy.envoymobile.ResponseHeaders
19+
import io.envoyproxy.envoymobile.StreamIntel
20+
21+
import java.util.concurrent.CountDownLatch
22+
import java.util.concurrent.Executors
23+
import java.util.concurrent.TimeUnit
24+
25+
import org.assertj.core.api.Assertions.assertThat
26+
import org.junit.Test
27+
import org.junit.runner.RunWith
28+
import org.mockito.Mock
29+
import org.mockito.Mockito
30+
import org.robolectric.RobolectricTestRunner
31+
32+
// ┌──────────────────┐
33+
// │ Proxy Engine │
34+
// │ ┌──────────────┐ │
35+
// ┌─────────────────────────┐ │ │listener_proxy│ │
36+
// │https://api.lyft.com/ping│ ┌──────────────┐ │ └──────┬───────┘ │ ┌────────────┐
37+
// │ HTTP Request ├──►Android Engine│ │ │ │ │api.lyft.com│
38+
// └─────────────────────────┘ └───────┬──────┘ │ ┌──────▼──────┐ │ └──────▲─────┘
39+
// │ │ │cluster_proxy│ │ │
40+
// │ │ └─────────────┘ │ │
41+
// │ │ │ │
42+
// │ └──────────────────┘ │
43+
// │ │
44+
// └─────────────────────────────────────┘
45+
@RunWith(RobolectricTestRunner::class)
46+
class PerformHTTPRequestUsingProxy {
47+
init {
48+
JniLibrary.loadTestLibrary()
49+
}
50+
51+
@Test
52+
fun `performs an HTTP request through a proxy`() {
53+
val port = (10001..11000).random()
54+
55+
val context = Mockito.spy(ApplicationProvider.getApplicationContext<Context>())
56+
val connectivityManager: ConnectivityManager = Mockito.mock(ConnectivityManager::class.java)
57+
Mockito.doReturn(connectivityManager).`when`(context).getSystemService(Context.CONNECTIVITY_SERVICE)
58+
Mockito.`when`(connectivityManager.getDefaultProxy()).thenReturn(ProxyInfo.buildPacProxy(Uri.parse("https://example.com")))
59+
60+
val onProxyEngineRunningLatch = CountDownLatch(1)
61+
val onEngineRunningLatch = CountDownLatch(1)
62+
val onRespondeHeadersLatch = CountDownLatch(1)
63+
64+
val proxyEngineBuilder = Proxy(context, port)
65+
.http()
66+
val proxyEngine = proxyEngineBuilder
67+
.addLogLevel(LogLevel.DEBUG)
68+
.setOnEngineRunning { onProxyEngineRunningLatch.countDown() }
69+
.build()
70+
71+
onProxyEngineRunningLatch.await(10, TimeUnit.SECONDS)
72+
assertThat(onProxyEngineRunningLatch.count).isEqualTo(0)
73+
74+
val builder = AndroidEngineBuilder(context)
75+
val engine = builder
76+
.addLogLevel(LogLevel.DEBUG)
77+
.enableProxying(true)
78+
.setOnEngineRunning { onEngineRunningLatch.countDown() }
79+
.build()
80+
81+
onEngineRunningLatch.await(10, TimeUnit.SECONDS)
82+
assertThat(onEngineRunningLatch.count).isEqualTo(0)
83+
84+
val requestHeaders = RequestHeadersBuilder(
85+
method = RequestMethod.GET,
86+
scheme = "http",
87+
authority = "api.lyft.com",
88+
path = "/ping"
89+
)
90+
.build()
91+
92+
engine
93+
.streamClient()
94+
.newStreamPrototype()
95+
.setOnResponseHeaders { responseHeaders, _, _ ->
96+
val status = responseHeaders.httpStatus ?: 0L
97+
assertThat(status).isEqualTo(301)
98+
assertThat(responseHeaders.value("x-proxy-response")).isNull();
99+
onRespondeHeadersLatch.countDown()
100+
}
101+
.start(Executors.newSingleThreadExecutor())
102+
.sendHeaders(requestHeaders, true)
103+
104+
onRespondeHeadersLatch.await(15, TimeUnit.SECONDS)
105+
assertThat(onRespondeHeadersLatch.count).isEqualTo(0)
106+
107+
engine.terminate()
108+
proxyEngine.terminate()
109+
}
110+
}

0 commit comments

Comments
 (0)