-
Notifications
You must be signed in to change notification settings - Fork 84
Test for Brotli decompressor #2066
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f6a6245
d7108fd
b9d9c10
c14bd73
db45ff3
1dde551
81128d8
dc3331f
3bbdc9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| import static io.envoyproxy.envoymobile.engine.EnvoyConfiguration.TrustChainVerification.VERIFY_TRUST_CHAIN; | ||
|
|
||
| import android.content.Context; | ||
| import androidx.annotation.VisibleForTesting; | ||
| import io.envoyproxy.envoymobile.engine.AndroidEngineImpl; | ||
| import io.envoyproxy.envoymobile.engine.AndroidJniLibrary; | ||
| import io.envoyproxy.envoymobile.engine.AndroidNetworkMonitor; | ||
|
|
@@ -15,6 +16,7 @@ | |
| import io.envoyproxy.envoymobile.engine.types.EnvoyLogger; | ||
| import io.envoyproxy.envoymobile.engine.types.EnvoyOnEngineRunning; | ||
| import io.envoyproxy.envoymobile.engine.types.EnvoyStringAccessor; | ||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
@@ -26,6 +28,24 @@ | |
| */ | ||
| public class NativeCronetEngineBuilderImpl extends CronetEngineBuilderImpl { | ||
|
|
||
| private static final String BROTLI_CONFIG = | ||
| "\n" | ||
| + | ||
| " \"@type\": type.googleapis.com/envoy.extensions.filters.http.decompressor.v3.Decompressor\n" | ||
| + " decompressor_library:\n" | ||
| + " name: text_optimized\n" | ||
| + " typed_config:\n" | ||
| + | ||
| " \"@type\": type.googleapis.com/envoy.extensions.compression.brotli.decompressor.v3.Brotli\n" | ||
| + " request_direction_config:\n" | ||
| + " common_config:\n" | ||
| + " enabled:\n" | ||
| + " default_value: false\n" | ||
| + " runtime_key: request_decompressor_enabled\n" | ||
| + " response_direction_config:\n" | ||
| + " common_config:\n" | ||
| + " ignore_no_transform_header: true\n"; | ||
|
|
||
| private final EnvoyLogger mEnvoyLogger = null; | ||
| private final EnvoyEventTracker mEnvoyEventTracker = null; | ||
| private boolean mAdminInterfaceEnabled = false; | ||
|
|
@@ -51,9 +71,6 @@ public class NativeCronetEngineBuilderImpl extends CronetEngineBuilderImpl { | |
| private String mAppId = "unspecified"; | ||
| private TrustChainVerification mTrustChainVerification = VERIFY_TRUST_CHAIN; | ||
| private String mVirtualClusters = "[]"; | ||
| private List<EnvoyHTTPFilterFactory> mPlatformFilterChain = Collections.emptyList(); | ||
| private List<EnvoyNativeFilterConfig> mNativeFilterChain = Collections.emptyList(); | ||
| private Map<String, EnvoyStringAccessor> mStringAccessors = Collections.emptyMap(); | ||
|
|
||
| /** | ||
| * Builder for Native Cronet Engine. Default config enables SPDY, disables QUIC and HTTP cache. | ||
|
|
@@ -62,6 +79,17 @@ public class NativeCronetEngineBuilderImpl extends CronetEngineBuilderImpl { | |
| */ | ||
| public NativeCronetEngineBuilderImpl(Context context) { super(context); } | ||
|
|
||
| /** | ||
| * Indicates to skip the TLS certificate verification. | ||
| * | ||
| * @return the builder to facilitate chaining. | ||
| */ | ||
| @VisibleForTesting | ||
| public CronetEngineBuilderImpl setMockCertVerifierForTesting() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this logic is (roughly) moving from CronetEngineBuilder to NativeCronetEngineBuilderImpl. Out of curiosity, what's that motivation?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I moved this here because this is an EnvoyMobile enum that is being use, and this is the only class bridging EM and Cronet for the Configuration. The second weaker reason is that CronetEngineBuilderImpl (where that method was before this PR) is also used by the Java implementation of Cronet, where it makes no sense. |
||
| mTrustChainVerification = TrustChainVerification.ACCEPT_UNTRUSTED; | ||
| return this; | ||
| } | ||
|
|
||
| @Override | ||
| public ExperimentalCronetEngine build() { | ||
| if (getUserAgent() == null) { | ||
|
|
@@ -80,14 +108,21 @@ EnvoyEngine createEngine(EnvoyOnEngineRunning onEngineRunning) { | |
| } | ||
|
|
||
| private EnvoyConfiguration createEnvoyConfiguration() { | ||
| List<EnvoyHTTPFilterFactory> platformFilterChain = Collections.emptyList(); | ||
| List<EnvoyNativeFilterConfig> nativeFilterChain = new ArrayList<>(); | ||
| Map<String, EnvoyStringAccessor> stringAccessors = Collections.emptyMap(); | ||
| if (brotliEnabled()) { | ||
| nativeFilterChain.add( | ||
| new EnvoyNativeFilterConfig("envoy.filters.http.decompressor", BROTLI_CONFIG)); | ||
| } | ||
| return new EnvoyConfiguration( | ||
| mAdminInterfaceEnabled, mGrpcStatsDomain, mStatsDPort, mConnectTimeoutSeconds, | ||
| mDnsRefreshSeconds, mDnsFailureRefreshSecondsBase, mDnsFailureRefreshSecondsMax, | ||
| mDnsQueryTimeoutSeconds, mDnsPreresolveHostnames, mDnsFallbackNameservers, | ||
| mEnableDnsFilterUnroutableFamilies, mEnableHappyEyeballs, mEnableInterfaceBinding, | ||
| mH2ConnectionKeepaliveIdleIntervalMilliseconds, mH2ConnectionKeepaliveTimeoutSeconds, | ||
| mH2RawDomains, mStatsFlushSeconds, mStreamIdleTimeoutSeconds, mPerTryIdleTimeoutSeconds, | ||
| mAppVersion, mAppId, mTrustChainVerification, mVirtualClusters, mNativeFilterChain, | ||
| mPlatformFilterChain, mStringAccessors); | ||
| mAppVersion, mAppId, mTrustChainVerification, mVirtualClusters, nativeFilterChain, | ||
| platformFilterChain, stringAccessors); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| package org.chromium.net; | ||
|
|
||
| import static org.chromium.net.testing.CronetTestRule.SERVER_CERT_PEM; | ||
| import static org.chromium.net.testing.CronetTestRule.SERVER_KEY_PKCS8_PEM; | ||
| import static org.chromium.net.testing.CronetTestRule.getContext; | ||
| import static org.junit.Assert.assertEquals; | ||
| import static org.junit.Assert.assertFalse; | ||
| import static org.junit.Assert.assertTrue; | ||
|
|
||
| import androidx.test.filters.SmallTest; | ||
| import org.chromium.net.testing.CronetTestRule; | ||
| import org.chromium.net.testing.CronetTestRule.OnlyRunNativeCronet; | ||
| import org.chromium.net.testing.CronetTestUtil; | ||
| import org.chromium.net.testing.Feature; | ||
| import org.chromium.net.testing.Http2TestServer; | ||
| import org.chromium.net.testing.TestFilesInstaller; | ||
| import org.chromium.net.testing.TestUrlRequestCallback; | ||
| import org.junit.After; | ||
| import org.junit.Before; | ||
| import org.junit.Rule; | ||
| import org.junit.Test; | ||
| import org.junit.runner.RunWith; | ||
| import org.robolectric.RobolectricTestRunner; | ||
|
|
||
| /** | ||
| * Simple test for Brotli support. | ||
| */ | ||
| @RunWith(RobolectricTestRunner.class) | ||
| public class BrotliTest { | ||
| @Rule public final CronetTestRule mTestRule = new CronetTestRule(); | ||
|
|
||
| private CronetEngine mCronetEngine; | ||
|
|
||
| @Before | ||
| public void setUp() throws Exception { | ||
| TestFilesInstaller.installIfNeeded(getContext()); | ||
| assertTrue( | ||
| Http2TestServer.startHttp2TestServer(getContext(), SERVER_CERT_PEM, SERVER_KEY_PKCS8_PEM)); | ||
| } | ||
|
|
||
| @After | ||
| public void tearDown() throws Exception { | ||
| assertTrue(Http2TestServer.shutdownHttp2TestServer()); | ||
| if (mCronetEngine != null) { | ||
| mCronetEngine.shutdown(); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| @SmallTest | ||
| @Feature({"Cronet"}) | ||
| @OnlyRunNativeCronet | ||
| public void testBrotliAdvertised() throws Exception { | ||
| ExperimentalCronetEngine.Builder builder = new ExperimentalCronetEngine.Builder(getContext()); | ||
| builder.enableBrotli(true); | ||
| CronetTestUtil.setMockCertVerifierForTesting(builder); | ||
| mCronetEngine = builder.build(); | ||
| String url = Http2TestServer.getEchoAllHeadersUrl(); | ||
| TestUrlRequestCallback callback = startAndWaitForComplete(url); | ||
| assertEquals(200, callback.mResponseInfo.getHttpStatusCode()); | ||
| // TODO(carloseltuerto): also support "deflate" decompressor - Cronet does. | ||
| assertTrue(callback.mResponseAsString.contains("accept-encoding: br,gzip")); | ||
| } | ||
|
|
||
| @Test | ||
| @SmallTest | ||
| @Feature({"Cronet"}) | ||
| @OnlyRunNativeCronet | ||
| public void testBrotliNotAdvertised() throws Exception { | ||
| ExperimentalCronetEngine.Builder builder = new ExperimentalCronetEngine.Builder(getContext()); | ||
| CronetTestUtil.setMockCertVerifierForTesting(builder); | ||
| mCronetEngine = builder.build(); | ||
| String url = Http2TestServer.getEchoAllHeadersUrl(); | ||
| TestUrlRequestCallback callback = startAndWaitForComplete(url); | ||
| assertEquals(200, callback.mResponseInfo.getHttpStatusCode()); | ||
| assertFalse(callback.mResponseAsString.contains("br")); | ||
| } | ||
|
|
||
| @Test | ||
| @SmallTest | ||
| @Feature({"Cronet"}) | ||
| @OnlyRunNativeCronet | ||
| public void testBrotliDecoded() throws Exception { | ||
| ExperimentalCronetEngine.Builder builder = new ExperimentalCronetEngine.Builder(getContext()); | ||
| builder.enableBrotli(true); | ||
| CronetTestUtil.setMockCertVerifierForTesting(builder); | ||
| mCronetEngine = builder.build(); | ||
| String url = Http2TestServer.getServeSimpleBrotliResponse(); | ||
| TestUrlRequestCallback callback = startAndWaitForComplete(url); | ||
| assertEquals(200, callback.mResponseInfo.getHttpStatusCode()); | ||
| String expectedResponse = "The quick brown fox jumps over the lazy dog"; | ||
| assertEquals(expectedResponse, callback.mResponseAsString); | ||
| // TODO(https://github.com/envoyproxy/envoy-mobile/issues/2086): uncomment this line. | ||
| // assertEquals(callback.mResponseInfo.getAllHeaders().get("content-encoding").get(0),"br"); | ||
| } | ||
|
|
||
| private TestUrlRequestCallback startAndWaitForComplete(String url) { | ||
| TestUrlRequestCallback callback = new TestUrlRequestCallback(); | ||
| UrlRequest.Builder builder = | ||
| mCronetEngine.newUrlRequestBuilder(url, callback, callback.getExecutor()); | ||
| builder.build().start(); | ||
| callback.blockForDone(); | ||
| return callback; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package org.chromium.net.testing; | ||
|
|
||
| import org.chromium.net.ExperimentalCronetEngine; | ||
| import org.chromium.net.impl.NativeCronetEngineBuilderImpl; | ||
|
|
||
| /** | ||
| * Utilities for Cronet testing | ||
| */ | ||
| public final class CronetTestUtil { | ||
|
|
||
| public static void setMockCertVerifierForTesting(ExperimentalCronetEngine.Builder builder) { | ||
| getCronetEngineBuilderImpl(builder).setMockCertVerifierForTesting(); | ||
| } | ||
|
|
||
| public static NativeCronetEngineBuilderImpl | ||
| getCronetEngineBuilderImpl(ExperimentalCronetEngine.Builder builder) { | ||
| return (NativeCronetEngineBuilderImpl)builder.getBuilderDelegate(); | ||
| } | ||
|
|
||
| private CronetTestUtil() {} | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup.