Attempt to avoid gzip response compression between backend services

dialogue-core/src/main/java/com/palantir/dialogue/core/ContentDecodingChannel.java

@@ -55,19 +55,34 @@ final class ContentDecodingChannel implements EndpointChannel {
     private static final String GZIP = "gzip";
 
     private final EndpointChannel delegate;
+    private final boolean sendAcceptGzip;
 
-    ContentDecodingChannel(EndpointChannel delegate) {
+    private ContentDecodingChannel(EndpointChannel delegate, boolean sendAcceptGzip) {
         this.delegate = Preconditions.checkNotNull(delegate, "Channel is required");
+        this.sendAcceptGzip = sendAcceptGzip;
+    }
+
+    static EndpointChannel create(Config cf, EndpointChannel delegate) {
+        boolean sendAcceptGzip = shouldSendAcceptGzip(cf);
+        return new ContentDecodingChannel(delegate, sendAcceptGzip);
+    }
+
+    private static boolean shouldSendAcceptGzip(Config cf) {
+        // In mesh mode or environments which appear to be within an environment,
+        // prefer not to request compressed responses. This heuristic assumes response
+        // compression should not be used in a service mesh, nor when load balancing
+        // is handled by the client.
+        return cf.mesh() == MeshMode.DEFAULT_NO_MESH && cf.clientConf().uris().size() == 1;
     }
 
     @Override
     public ListenableFuture<Response> execute(Request request) {
-        Request augmentedRequest = acceptEncoding(request);
+        Request augmentedRequest = acceptEncoding(request, sendAcceptGzip);
         return DialogueFutures.transform(delegate.execute(augmentedRequest), ContentDecodingChannel::decompress);
     }
 
-    private static Request acceptEncoding(Request request) {
-        if (request.headerParams().containsKey(ACCEPT_ENCODING)) {
+    private static Request acceptEncoding(Request request, boolean sendAcceptGzip) {
+        if (!sendAcceptGzip || request.headerParams().containsKey(ACCEPT_ENCODING)) {
             // Do not replace existing accept-encoding values
             return request;
         }

dialogue-core/src/main/java/com/palantir/dialogue/core/DialogueChannel.java

@@ -181,7 +181,7 @@ public DialogueChannel build() {
                 channel = UserAgentEndpointChannel.create(
                         channel, endpoint, cf.clientConf().userAgent().get());
                 channel = DeprecationWarningChannel.create(cf, channel, endpoint);
-                channel = new ContentDecodingChannel(channel);
+                channel = ContentDecodingChannel.create(cf, channel);
                 channel = new RangeAcceptsIdentityEncodingChannel(channel);
                 channel = ContentEncodingChannel.of(channel, endpoint);
                 channel = TracedChannel.create(cf, channel, endpoint);

dialogue-core/src/test/java/com/palantir/dialogue/core/ContentDecodingChannelTest.java

@@ -20,20 +20,45 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.guava.api.Assertions.assertThat;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.io.ByteStreams;
 import com.google.common.util.concurrent.Futures;
+import com.palantir.conjure.java.client.config.ClientConfiguration;
+import com.palantir.conjure.java.client.config.ClientConfigurations;
 import com.palantir.dialogue.Request;
 import com.palantir.dialogue.Response;
 import com.palantir.dialogue.TestResponse;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.util.zip.GZIPOutputStream;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.X509TrustManager;
 import org.assertj.core.data.MapEntry;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
 
 public final class ContentDecodingChannelTest {
 
+    private Config standard;
+    private Config mesh;
+
+    @BeforeEach
+    void before() throws Exception {
+        X509TrustManager tm = Mockito.mock(X509TrustManager.class);
+        ClientConfiguration clientConfig = ClientConfigurations.of(
+                ImmutableList.of("https://localhost:8123"),
+                SSLContext.getDefault().getSocketFactory(),
+                tm);
+        standard = Mockito.mock(Config.class);
+        Mockito.when(standard.mesh()).thenReturn(MeshMode.DEFAULT_NO_MESH);
+        Mockito.when(standard.clientConf()).thenReturn(clientConfig);
+        mesh = Mockito.mock(Config.class);
+        Mockito.when(mesh.mesh()).thenReturn(MeshMode.USE_EXTERNAL_MESH);
+        Mockito.when(mesh.clientConf()).thenReturn(clientConfig);
+    }
+
     @Test
     public void testDecoding() throws Exception {
         byte[] expected = new byte[] {1, 2, 3, 4};
@@ -43,7 +68,30 @@ public void testDecoding() throws Exception {
         } catch (IOException e) {
             throw new IllegalStateException(e);
         }
-        Response response = new ContentDecodingChannel(
+        Response response = ContentDecodingChannel.create(
+                        standard,
+                        _request -> Futures.immediateFuture(new TestResponse(out.toByteArray())
+                                .withHeader("content-encoding", "gzip")
+                                .withHeader("content-length", Integer.toString(out.size()))))
+                .execute(Request.builder().build())
+                .get();
+        assertThat(response.headers().get("content-encoding")).isEmpty();
+        assertThat(ByteStreams.toByteArray(response.body())).containsExactly(expected);
+    }
+
+    // In mesh mode, decoding should continue to work, but the accept-encoding header will not be sent
+    // in order to hint that we don't want the server to encode.
+    @Test
+    public void testDecoding_mesh() throws Exception {
+        byte[] expected = new byte[] {1, 2, 3, 4};
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        try (GZIPOutputStream compressor = new GZIPOutputStream(out)) {
+            compressor.write(expected);
+        } catch (IOException e) {
+            throw new IllegalStateException(e);
+        }
+        Response response = ContentDecodingChannel.create(
+                        mesh,
                         _request -> Futures.immediateFuture(new TestResponse(out.toByteArray())
                                 .withHeader("content-encoding", "gzip")
                                 .withHeader("content-length", Integer.toString(out.size()))))
@@ -55,9 +103,12 @@ public void testDecoding() throws Exception {
 
     @Test
     public void testDecoding_delayedFailure() throws Exception {
-        Response response = new ContentDecodingChannel(_request -> Futures.immediateFuture(
-                        // Will fail because it's not valid gzip content
-                        new TestResponse(new byte[] {1, 2, 3, 4}).withHeader("content-encoding", "gzip")))
+        // Will fail because it's not valid gzip content
+        Response response = ContentDecodingChannel.create(
+                        standard,
+                        _request -> Futures.immediateFuture(
+                                // Will fail because it's not valid gzip content
+                                new TestResponse(new byte[] {1, 2, 3, 4}).withHeader("content-encoding", "gzip")))
                 .execute(Request.builder().build())
                 .get();
         assertThat(response.headers().get("content-encoding")).isEmpty();
@@ -67,8 +118,10 @@ public void testDecoding_delayedFailure() throws Exception {
     @Test
     public void testOnlyDecodesGzip() throws Exception {
         byte[] content = new byte[] {1, 2, 3, 4};
-        Response response = new ContentDecodingChannel(_request ->
-                        Futures.immediateFuture(new TestResponse(content).withHeader("content-encoding", "unknown")))
+        Response response = ContentDecodingChannel.create(
+                        standard,
+                        _request -> Futures.immediateFuture(
+                                new TestResponse(content).withHeader("content-encoding", "unknown")))
                 .execute(Request.builder().build())
                 .get();
         assertThat(response.headers()).containsAllEntriesOf(ImmutableListMultimap.of("content-encoding", "unknown"));
@@ -77,17 +130,27 @@ public void testOnlyDecodesGzip() throws Exception {
 
     @Test
     public void testRequestHeader() throws Exception {
-        new ContentDecodingChannel(request -> {
+        ContentDecodingChannel.create(standard, request -> {
                     assertThat(request.headerParams()).contains(MapEntry.entry("accept-encoding", "gzip"));
                     return Futures.immediateFuture(new TestResponse());
                 })
                 .execute(Request.builder().build())
                 .get();
     }
 
+    @Test
+    public void testRequestHeader_mesh() throws Exception {
+        ContentDecodingChannel.create(mesh, request -> {
+                    assertThat(request.headerParams().keySet()).doesNotContain("accept-encoding");
+                    return Futures.immediateFuture(new TestResponse());
+                })
+                .execute(Request.builder().build())
+                .get();
+    }
+
     @Test
     public void testRequestHeader_existingIsNotReplaced() throws Exception {
-        new ContentDecodingChannel(request -> {
+        ContentDecodingChannel.create(standard, request -> {
                     assertThat(request.headerParams())
                             .as("The requested 'identity' encoding should not be replaced")
                             .contains(MapEntry.entry("accept-encoding", "identity"));
@@ -101,7 +164,7 @@ public void testRequestHeader_existingIsNotReplaced() throws Exception {
 
     @Test
     public void testResponseHeaders() throws Exception {
-        new ContentDecodingChannel(request -> {
+        ContentDecodingChannel.create(standard, request -> {
                     assertThat(request.headerParams())
                             .as("The requested 'identity' encoding should not be replaced")
                             .contains(MapEntry.entry("accept-encoding", "identity"));