Allow configuring the maximum header size of origin responses (issue #…

…598) (#599)

Add option to configure max origin bound header size (issue #598)

codequality/checkstyle_suppressions.xml

@@ -35,7 +35,7 @@
   <suppress checks="FileLength" files="HttpRequest.java"/>
   <suppress checks="MethodLength" files="ConnectionPoolFactory.java"/>
 
-  <suppress checks="ParameterNumber" files=".*StartupConfig.java|.*ProxyServiceSupplier.java"/>
+  <suppress checks="ParameterNumber" files=".*StartupConfig.java|.*ProxyServiceSupplier.java|.*HostProxy.java"/>
 
   <suppress checks="RegexpSingleline" files="LOGBackConfigurer.java"/>
 

components/api/src/main/java/com/hotels/styx/api/extension/service/BackendService.java

@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2013-2018 Expedia Inc.
+  Copyright (C) 2013-2020 Expedia Inc.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -44,6 +44,7 @@
  */
 public final class BackendService implements Identifiable {
     public static final int DEFAULT_RESPONSE_TIMEOUT_MILLIS = 1000;
+    public static final int USE_DEFAULT_MAX_HEADER_SIZE = 0;
 
     /**
      * A protocol used for the backend service. This can be either HTTP or HTTPS.
@@ -61,6 +62,7 @@ public enum Protocol {
     private final StickySessionConfig stickySessionConfig;
     private final List<RewriteConfig> rewrites;
     private final int responseTimeoutMillis;
+    private final int maxHeaderSize;
     private final TlsSettings tlsSettings;
 
     /**
@@ -94,6 +96,7 @@ private BackendService(Builder builder) {
                 ? DEFAULT_RESPONSE_TIMEOUT_MILLIS
                 : builder.responseTimeoutMillis;
         this.tlsSettings = builder.tlsSettings;
+        this.maxHeaderSize = builder.maxHeaderSize;
 
         checkThatOriginsAreDistinct(origins);
         checkArgument(responseTimeoutMillis >= 0, "Request timeout must be greater than or equal to zero");
@@ -142,6 +145,10 @@ public int responseTimeoutMillis() {
         return this.responseTimeoutMillis;
     }
 
+    public int maxHeaderSize() {
+        return this.maxHeaderSize;
+    }
+
     public Optional<TlsSettings> tlsSettings() {
         return Optional.ofNullable(this.tlsSettings);
     }
@@ -161,7 +168,8 @@ public Protocol protocol() {
     @Override
     public int hashCode() {
         return Objects.hash(id, path, connectionPoolSettings, origins,
-                healthCheckConfig, stickySessionConfig, rewrites, responseTimeoutMillis);
+                healthCheckConfig, stickySessionConfig, rewrites,
+                responseTimeoutMillis, maxHeaderSize);
     }
 
     @Override
@@ -181,7 +189,8 @@ public boolean equals(Object obj) {
                 && Objects.equals(this.stickySessionConfig, other.stickySessionConfig)
                 && Objects.equals(this.rewrites, other.rewrites)
                 && Objects.equals(this.tlsSettings, other.tlsSettings)
-                && Objects.equals(this.responseTimeoutMillis, other.responseTimeoutMillis);
+                && Objects.equals(this.responseTimeoutMillis, other.responseTimeoutMillis)
+                && Objects.equals(this.maxHeaderSize, other.maxHeaderSize);
     }
 
     @Override
@@ -213,7 +222,8 @@ public static final class Builder {
         private StickySessionConfig stickySessionConfig = stickySessionDisabled();
         private HealthCheckConfig healthCheckConfig;
         private List<RewriteConfig> rewrites = emptyList();
-        public int responseTimeoutMillis = DEFAULT_RESPONSE_TIMEOUT_MILLIS;
+        private int responseTimeoutMillis = DEFAULT_RESPONSE_TIMEOUT_MILLIS;
+        private int maxHeaderSize = USE_DEFAULT_MAX_HEADER_SIZE;
         private TlsSettings tlsSettings;
 
         public Builder() {
@@ -228,6 +238,7 @@ private Builder(BackendService backendService) {
             this.healthCheckConfig = backendService.healthCheckConfig;
             this.rewrites = backendService.rewrites;
             this.responseTimeoutMillis = backendService.responseTimeoutMillis;
+            this.maxHeaderSize = backendService.maxHeaderSize;
             this.tlsSettings = backendService.tlsSettings().orElse(null);
         }
 
@@ -284,6 +295,18 @@ public Builder responseTimeoutMillis(int timeout) {
             return this;
         }
 
+        /**
+         * Sets the response max header size in bytes.
+         * 0 means use the default.
+         *
+         * @param maxHeaderSize
+         * @return
+         */
+        public Builder maxHeaderSize(int maxHeaderSize) {
+            this.maxHeaderSize = maxHeaderSize;
+            return this;
+        }
+
         /**
          * Sets hosts.
          *

components/client/src/main/java/com/hotels/styx/client/HttpConfig.java

@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2013-2019 Expedia Inc.
+  Copyright (C) 2013-2020 Expedia Inc.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -15,37 +15,31 @@
  */
 package com.hotels.styx.client;
 
-import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonProperty;
-
-import static com.google.common.base.MoreObjects.firstNonNull;
 import static java.util.Arrays.asList;
 import static java.util.Collections.emptyList;
 
 /**
  * HTTP configuration settings, used to connect to origins.
  */
 public final class HttpConfig {
+
+    private static final int USE_DEFAULT_MAX_HEADER_SIZE = 0;
+    private static final int DEFAULT_MAX_HEADER_SIZE = 8192;
+
     private boolean compress;
     private final int maxInitialLength;
     private final int maxHeadersSize;
     private final int maxChunkSize;
-    private int maxContentLength = 65536;
-    private Iterable<ChannelOptionSetting> settings = emptyList();
-
-    @JsonCreator
-    HttpConfig(@JsonProperty("maxChunkSize") Integer maxChunkSize,
-               @JsonProperty("maxHeaderSize") Integer maxHeadersSize,
-               @JsonProperty("maxInitialLength") Integer maxInitialLength) {
-        this.maxChunkSize = firstNonNull(maxChunkSize, 8192);
-        this.maxHeadersSize = firstNonNull(maxHeadersSize, 8192);
-        this.maxInitialLength = firstNonNull(maxInitialLength, 4096);
-    }
+    private int maxContentLength;
+    private Iterable<ChannelOptionSetting> settings;
+
 
     private HttpConfig(Builder builder) {
         this.compress = builder.compress;
         this.maxInitialLength = builder.maxInitialLength;
-        this.maxHeadersSize = builder.maxHeadersSize;
+        this.maxHeadersSize = builder.maxHeadersSize == USE_DEFAULT_MAX_HEADER_SIZE
+                ? DEFAULT_MAX_HEADER_SIZE
+                : builder.maxHeadersSize;
         this.maxChunkSize = builder.maxChunkSize;
         this.maxContentLength = builder.maxContentLength;
         this.settings = builder.settings;
@@ -129,7 +123,7 @@ public static HttpConfig defaultHttpConfig() {
     public static final class Builder {
         private boolean compress;
         private int maxInitialLength = 4096;
-        private int maxHeadersSize = 8192;
+        private int maxHeadersSize = DEFAULT_MAX_HEADER_SIZE;
         private int maxChunkSize = 8192;
         private int maxContentLength = 65536;
         private Iterable<ChannelOptionSetting> settings = emptyList();
@@ -173,7 +167,7 @@ public Builder setMaxInitialLength(int maxInitialLength) {
         /**
          * Set the maximum combined size of the HTTP headers in bytes.
          *
-         * @param maxHeaderSize maximum combined size of the HTTP headers
+         * @param maxHeaderSize maximum combined size of the HTTP headers. 0 means use the default value.
          * @return this builder
          */
         public Builder setMaxHeadersSize(int maxHeaderSize) {

components/client/src/main/java/com/hotels/styx/client/connectionpool/ConnectionPools.java

@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2013-2018 Expedia Inc.
+  Copyright (C) 2013-2020 Expedia Inc.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -23,6 +23,7 @@
 import com.hotels.styx.client.netty.connectionpool.NettyConnectionFactory;
 
 import static com.hotels.styx.api.extension.service.ConnectionPoolSettings.defaultConnectionPoolSettings;
+import static com.hotels.styx.client.HttpConfig.newHttpConfigBuilder;
 import static com.hotels.styx.client.HttpRequestOperationFactory.Builder.httpRequestOperationFactoryBuilder;
 import static java.lang.String.format;
 
@@ -90,6 +91,7 @@ public static ConnectionPool.Factory simplePoolFactory(MetricRegistry metricRegi
 
     public static ConnectionPool.Factory simplePoolFactory(BackendService backendService, MetricRegistry metricRegistry) {
         NettyConnectionFactory connectionFactory = new NettyConnectionFactory.Builder()
+                .httpConfig(newHttpConfigBuilder().setMaxHeadersSize(backendService.maxHeaderSize()).build())
                 .httpRequestOperationFactory(
                         httpRequestOperationFactoryBuilder()
                                 .responseTimeoutMillis(backendService.responseTimeoutMillis())

components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/NettyConnectionFactoryTest.java

@@ -23,7 +23,9 @@
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.SimpleChannelInboundHandler;
+import io.netty.handler.codec.TooLongFrameException;
 import io.netty.handler.codec.http.DefaultFullHttpRequest;
+import io.netty.handler.codec.http.DefaultHttpResponse;
 import io.netty.handler.codec.http.FullHttpRequest;
 import io.netty.handler.codec.http.HttpObject;
 import io.netty.handler.codec.http.HttpResponse;
@@ -42,19 +44,22 @@
 import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
 import static com.hotels.styx.api.HttpHeaderNames.HOST;
 import static com.hotels.styx.api.extension.Origin.newOriginBuilder;
+import static com.hotels.styx.client.HttpConfig.newHttpConfigBuilder;
 import static com.hotels.styx.client.HttpRequestOperationFactory.Builder.httpRequestOperationFactoryBuilder;
 import static com.hotels.styx.common.FreePorts.freePort;
 import static com.hotels.styx.support.server.UrlMatchingStrategies.urlStartingWith;
 import static io.netty.handler.codec.http.HttpMethod.GET;
 import static io.netty.handler.codec.http.HttpResponseStatus.OK;
 import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
+import static java.util.Collections.nCopies;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.StreamSupport.stream;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.Is.is;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
 
 @TestInstance(PER_CLASS)
@@ -64,6 +69,7 @@ public class NettyConnectionFactoryTest {
 
     private final NettyConnectionFactory connectionFactory = new NettyConnectionFactory.Builder()
             .httpRequestOperationFactory(httpRequestOperationFactoryBuilder().build())
+            .httpConfig(newHttpConfigBuilder().setMaxHeadersSize(100).build())
             .build();
 
     private Origin healthyOrigin;
@@ -113,6 +119,18 @@ public void createsWorkingHttpConnection() {
         assertThat(response.getStatus(), is(OK));
     }
 
+    @Test
+    public void responseFailsIfItExceedsMaxHeaderSize() {
+        server.stub(urlStartingWith("/"), aResponse().withStatus(200).withHeader("AHEADER", String.join("", nCopies(100, "A"))));
+
+        assertThrows(TooLongFrameException.class, () ->
+                sendRequestAndReceiveResponse(
+                        requestToOrigin(),
+                        ((NettyConnection) connectionFactory.createConnection(healthyOrigin, connectionSettings).block()).channel())
+        );
+
+    }
+
     private FullHttpRequest requestToOrigin() {
         DefaultFullHttpRequest request = new DefaultFullHttpRequest(HTTP_1_1, GET, "/");
         request.headers().set(HOST, "localhost:" + server.port());
@@ -135,6 +153,12 @@ private Flux<HttpObject> channelRequestResponse(Channel channel, FullHttpRequest
                 protected void channelRead0(ChannelHandlerContext ctx, HttpObject msg) throws Exception {
                     sink.next(msg);
 
+                    if (msg instanceof DefaultHttpResponse) {
+                        DefaultHttpResponse response = (DefaultHttpResponse) msg;
+                        if (response.decoderResult().isFailure()) {
+                            sink.error(response.decoderResult().cause());
+                        }
+                    }
                     if (msg instanceof LastHttpContent) {
                         sink.complete();
                     }

components/proxy/src/main/java/com/hotels/styx/infrastructure/configuration/json/mixins/BackendServiceMixin.java

@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2013-2018 Expedia Inc.
+  Copyright (C) 2013-2020 Expedia Inc.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -60,6 +60,9 @@ public interface BackendServiceMixin {
     @JsonProperty("responseTimeoutMillis")
     int responseTimeoutMillis();
 
+    @JsonProperty("maxHeaderSize")
+    int maxHeaderSize();
+
     @JsonProperty("tlsSettings")
     TlsSettings getTlsSettings();
 
@@ -77,6 +80,9 @@ interface Builder {
         @JsonProperty("responseTimeoutMillis")
         BackendService.Builder responseTimeoutMillis(int timeout);
 
+        @JsonProperty("maxHeaderSize")
+        BackendService.Builder maxHeaderSize(int maxHeaderSize);
+
         @JsonProperty("origins")
         BackendService.Builder origins(Set<Origin> origins);
 

components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java

@@ -52,6 +52,7 @@
 import java.util.concurrent.ConcurrentSkipListMap;
 
 import static com.google.common.collect.Iterables.concat;
+import static com.hotels.styx.client.HttpConfig.newHttpConfigBuilder;
 import static com.hotels.styx.client.HttpRequestOperationFactory.Builder.httpRequestOperationFactoryBuilder;
 import static java.util.Comparator.comparingInt;
 import static java.util.Comparator.naturalOrder;
@@ -192,6 +193,7 @@ private Connection.Factory connectionFactory(
                                 .build()
                 )
                 .tlsSettings(backendService.tlsSettings().orElse(null))
+                .httpConfig(newHttpConfigBuilder().setMaxHeadersSize(backendService.maxHeaderSize()).build())
                 .build();
 
         if (connectionExpiration > 0) {