configured validation in RoutingHandler to fail fast if the content-l…

…ength header is set and greater than the configured max request size once we know the endpoint for the request

riposte-core/src/main/java/com/nike/riposte/server/channelpipeline/HttpChannelInitializer.java

@@ -476,7 +476,7 @@ public void initChannel(SocketChannel ch) {
 
         // INBOUND - Add RoutingHandler to figure out which endpoint should handle the request and set it on our request
         //           state for later execution
-        p.addLast(ROUTING_HANDLER_NAME, new RoutingHandler(endpoints));
+        p.addLast(ROUTING_HANDLER_NAME, new RoutingHandler(endpoints, maxRequestSizeInBytes));
 
         // INBOUND - Add SecurityValidationHandler to validate the RequestInfo object for the matching endpoint
         p.addLast(SECURITY_VALIDATION_HANDLER_NAME, new SecurityValidationHandler(requestSecurityValidator));

riposte-core/src/main/java/com/nike/riposte/server/handler/RequestInfoSetterHandler.java

@@ -4,7 +4,6 @@
 import com.nike.riposte.server.error.exception.InvalidHttpRequestException;
 import com.nike.riposte.server.handler.base.BaseInboundHandlerWithTracingAndMdcSupport;
 import com.nike.riposte.server.handler.base.PipelineContinuationBehavior;
-import com.nike.riposte.server.http.Endpoint;
 import com.nike.riposte.server.http.HttpProcessingState;
 import com.nike.riposte.server.http.RequestInfo;
 import com.nike.riposte.server.http.impl.RequestInfoImpl;
@@ -19,6 +18,9 @@
 import io.netty.handler.codec.http.HttpRequest;
 import io.netty.util.ReferenceCountUtil;
 
+import static com.nike.riposte.util.HttpUtils.getConfiguredMaxRequestSize;
+import static com.nike.riposte.util.HttpUtils.isMaxRequestSizeValidationDisabled;
+
 /**
  * Monitors the incoming messages - when it sees a {@link HttpRequest} then it creates a new {@link RequestInfo} from it
  * and sets it on the channel's current request state via {@link HttpProcessingState#setRequestInfo(RequestInfo)}. If
@@ -76,7 +78,7 @@ else if (msg instanceof HttpContent) {
                 }
 
                 int currentRequestLengthInBytes = requestInfo.addContentChunk(httpContentMsg);
-                int configuredMaxRequestSize = getConfiguredMaxRequestSize(state);
+                int configuredMaxRequestSize = getConfiguredMaxRequestSize(state.getEndpointForExecution(), globalConfiguredMaxRequestSizeInBytes);
 
                 if (!isMaxRequestSizeValidationDisabled(configuredMaxRequestSize)
                     && currentRequestLengthInBytes > configuredMaxRequestSize) {
@@ -104,21 +106,6 @@ private void throwExceptionIfNotSuccessfullyDecoded(HttpObject httpObject) {
         }
     }
 
-    private boolean isMaxRequestSizeValidationDisabled(int configuredMaxRequestSize) {
-        return configuredMaxRequestSize <= 0;
-    }
-
-    private int getConfiguredMaxRequestSize(HttpProcessingState state) {
-        Endpoint<?> endpoint = state.getEndpointForExecution();
-
-        //if the endpoint is null or the endpoint is not overriding, we should return the globally configured value
-        if (endpoint == null || endpoint.maxRequestSizeInBytesOverride() == null) {
-            return globalConfiguredMaxRequestSizeInBytes;
-        }
-
-        return endpoint.maxRequestSizeInBytesOverride();
-    }
-
     @Override
     protected boolean argsAreEligibleForLinkingAndUnlinkingDistributedTracingInfo(
         HandlerMethodToExecute methodToExecute, ChannelHandlerContext ctx, Object msgOrEvt, Throwable cause

riposte-core/src/main/java/com/nike/riposte/server/handler/RoutingHandler.java

@@ -17,8 +17,13 @@
 import java.util.Optional;
 
 import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.TooLongFrameException;
+import io.netty.handler.codec.http.HttpHeaders;
 import io.netty.handler.codec.http.HttpRequest;
 
+import static com.nike.riposte.util.HttpUtils.getConfiguredMaxRequestSize;
+import static com.nike.riposte.util.HttpUtils.isMaxRequestSizeValidationDisabled;
+
 /**
  * Handles the logic of determining which {@link Endpoint} matches the {@link RequestInfo} in the channel's current
  * state ({@link com.nike.riposte.server.http.HttpProcessingState#getRequestInfo()}). It also performs some error
@@ -32,12 +37,14 @@
 public class RoutingHandler extends BaseInboundHandlerWithTracingAndMdcSupport {
 
     private final Collection<Endpoint<?>> endpoints;
+    private final int globalConfiguredMaxRequestSizeInBytes;
 
-    public RoutingHandler(Collection<Endpoint<?>> endpoints) {
+    public RoutingHandler(Collection<Endpoint<?>> endpoints, int globalMaxRequestSizeInBytes) {
         if (endpoints == null || endpoints.isEmpty())
             throw new IllegalArgumentException("endpoints cannot be empty");
 
         this.endpoints = endpoints;
+        this.globalConfiguredMaxRequestSizeInBytes = globalMaxRequestSizeInBytes;
     }
 
     /**
@@ -104,6 +111,8 @@ public PipelineContinuationBehavior doChannelRead(ChannelHandlerContext ctx, Obj
             RequestInfo request = state.getRequestInfo();
             Pair<Endpoint<?>, String> endpointForExecution = findSingleEndpointForExecution(request);
 
+            throwExceptionIfContentLengthHeaderIsLargerThanConfiguredMaxRequestSize((HttpRequest) msg, endpointForExecution.getLeft());
+
             request.setPathParamsBasedOnPathTemplate(endpointForExecution.getRight());
 
             state.setEndpointForExecution(endpointForExecution.getLeft(), endpointForExecution.getRight());
@@ -112,6 +121,16 @@ public PipelineContinuationBehavior doChannelRead(ChannelHandlerContext ctx, Obj
         return PipelineContinuationBehavior.CONTINUE;
     }
 
+    private void throwExceptionIfContentLengthHeaderIsLargerThanConfiguredMaxRequestSize(HttpRequest msg, Endpoint<?> endpoint) {
+        int configuredMaxRequestSize = getConfiguredMaxRequestSize(endpoint, globalConfiguredMaxRequestSizeInBytes);
+
+        if (!isMaxRequestSizeValidationDisabled(configuredMaxRequestSize)
+                && HttpHeaders.isContentLengthSet(msg)
+                && HttpHeaders.getContentLength(msg) > configuredMaxRequestSize) {
+            throw new TooLongFrameException("Content-Length header value exceeded configured max request size of " + configuredMaxRequestSize);
+        }
+    }
+
     @Override
     protected boolean argsAreEligibleForLinkingAndUnlinkingDistributedTracingInfo(
         HandlerMethodToExecute methodToExecute, ChannelHandlerContext ctx, Object msgOrEvt, Throwable cause

riposte-core/src/test/java/com/nike/riposte/server/handler/RoutingHandlerTest.java

@@ -11,7 +11,15 @@
 import com.nike.riposte.server.http.RequestInfo;
 import com.nike.riposte.server.http.StandardEndpoint;
 import com.nike.riposte.util.Matcher;
-
+import io.netty.channel.Channel;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.TooLongFrameException;
+import io.netty.handler.codec.http.DefaultHttpHeaders;
+import io.netty.handler.codec.http.HttpHeaders;
+import io.netty.handler.codec.http.HttpObject;
+import io.netty.handler.codec.http.HttpRequest;
+import io.netty.util.Attribute;
+import org.assertj.core.api.Assertions;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.internal.util.reflection.Whitebox;
@@ -22,12 +30,11 @@
 import java.util.Collections;
 import java.util.Optional;
 
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelHandlerContext;
-import io.netty.handler.codec.http.HttpObject;
-import io.netty.handler.codec.http.HttpRequest;
-import io.netty.util.Attribute;
-
+import static io.netty.handler.codec.http.HttpHeaders.Names.CONTENT_LENGTH;
+import static io.netty.handler.codec.http.HttpHeaders.Names.CONTENT_TYPE;
+import static io.netty.handler.codec.http.HttpHeaders.Names.TRANSFER_ENCODING;
+import static io.netty.handler.codec.http.HttpHeaders.Values.CHUNKED;
+import static org.apache.http.entity.ContentType.APPLICATION_JSON;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.doReturn;
@@ -53,6 +60,9 @@ public class RoutingHandlerTest {
     private Matcher matcherMock;
     private Collection<Endpoint<?>> endpoints;
     private String defaultPath = "/*/*/{vipname}/**";
+    private HttpHeaders httpHeaders;
+    private int maxRequestSizeInBytes;
+    private HttpRequest msg;
 
     @Before
     public void beforeMethod() {
@@ -64,6 +74,9 @@ public void beforeMethod() {
         endpointMock = mock(StandardEndpoint.class);
         matcherMock = mock(Matcher.class);
         endpoints = new ArrayList<>(Collections.singleton(endpointMock));
+        httpHeaders = new DefaultHttpHeaders();
+        maxRequestSizeInBytes = 10;
+        msg = mock(HttpRequest.class);
 
         doReturn(channelMock).when(ctxMock).channel();
         doReturn(stateAttrMock).when(channelMock).attr(ChannelAttributes.HTTP_PROCESSING_STATE_ATTRIBUTE_KEY);
@@ -73,14 +86,15 @@ public void beforeMethod() {
         doReturn(Optional.of(defaultPath)).when(matcherMock).matchesPath(any(RequestInfo.class));
         doReturn(true).when(matcherMock).matchesMethod(any(RequestInfo.class));
         doReturn(requestInfoMock).when(stateMock).getRequestInfo();
+        doReturn(httpHeaders).when(msg).headers();
 
-        handlerSpy = spy(new RoutingHandler(endpoints));
+        handlerSpy = spy(new RoutingHandler(endpoints, maxRequestSizeInBytes));
     }
 
     @Test
     public void constructor_sets_fields_based_on_incoming_args() {
         // when
-        RoutingHandler theHandler = new RoutingHandler(endpoints);
+        RoutingHandler theHandler = new RoutingHandler(endpoints, maxRequestSizeInBytes);
 
         // then
         Collection<Endpoint<?>>
@@ -91,13 +105,13 @@ public void constructor_sets_fields_based_on_incoming_args() {
     @Test(expected = IllegalArgumentException.class)
     public void constructor_throws_IllegalArgumentException_if_arg_is_null() {
         // expect
-        new RoutingHandler(null);
+        new RoutingHandler(null, maxRequestSizeInBytes);
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void constructor_throws_IllegalArgumentException_if_arg_is_empty() {
         // expect
-        new RoutingHandler(Collections.emptyList());
+        new RoutingHandler(Collections.emptyList(), maxRequestSizeInBytes);
     }
 
     @Test
@@ -182,4 +196,99 @@ public void findSingleEndpointForExecution_throws_MultipleMatchingEndpointsExcep
         // when
         handlerSpy.findSingleEndpointForExecution(requestInfoMock);
     }
+
+    @Test
+    public void doChannelRead_HttpRequest_throws_exception_when_content_length_header_greater_than_configured_global_request_limit() {
+        // given
+        doReturn(null).when(endpointMock).maxRequestSizeInBytesOverride();
+
+        maxRequestSizeInBytes = 10;
+        httpHeaders.set(CONTENT_LENGTH, 100);
+        httpHeaders.set(CONTENT_TYPE, APPLICATION_JSON);
+        handlerSpy = spy(new RoutingHandler(endpoints, maxRequestSizeInBytes));
+
+        // when
+        Throwable thrownException = Assertions.catchThrowable(() -> handlerSpy.doChannelRead(ctxMock, msg));
+
+        // then
+        assertThat(thrownException).isExactlyInstanceOf(TooLongFrameException.class);
+        assertThat(thrownException.getMessage()).isEqualTo("Content-Length header value exceeded configured max request size of 10");
+    }
+
+    @Test
+    public void doChannelRead_HttpRequest_endpoint_overridden_max_request_size_throws_exception() {
+        // given
+        doReturn(100).when(endpointMock).maxRequestSizeInBytesOverride();
+
+        maxRequestSizeInBytes = 99;
+        httpHeaders.set(CONTENT_LENGTH, 101);
+        httpHeaders.set(CONTENT_TYPE, APPLICATION_JSON);
+
+        handlerSpy = spy(new RoutingHandler(endpoints, maxRequestSizeInBytes));
+
+        // when
+        Throwable thrownException = Assertions.catchThrowable(() -> handlerSpy.doChannelRead(ctxMock, msg));
+
+        // then
+        assertThat(thrownException).isExactlyInstanceOf(TooLongFrameException.class);
+        assertThat(thrownException.getMessage()).isEqualTo("Content-Length header value exceeded configured max request size of 100");
+    }
+
+    @Test
+    public void doChannelRead_HttpRequest_under_max_global_request_size_processed_successfully() {
+        // given
+        doReturn(null).when(endpointMock).maxRequestSizeInBytesOverride();
+
+        maxRequestSizeInBytes = 101;
+        httpHeaders.set(CONTENT_LENGTH, 100);
+        httpHeaders.set(CONTENT_TYPE, APPLICATION_JSON);
+        handlerSpy = spy(new RoutingHandler(endpoints, maxRequestSizeInBytes));
+
+        // when
+        PipelineContinuationBehavior result = handlerSpy.doChannelRead(ctxMock, msg);
+
+        // then
+        assertThat(result).isEqualTo(PipelineContinuationBehavior.CONTINUE);
+    }
+
+    @Test
+    public void doChannelRead_HttpRequest_transfer_encoding_chunked_processed() {
+        // given
+        doReturn(null).when(endpointMock).maxRequestSizeInBytesOverride();
+
+        maxRequestSizeInBytes = 101;
+        httpHeaders.set(TRANSFER_ENCODING, CHUNKED);
+        httpHeaders.set(CONTENT_TYPE, APPLICATION_JSON);
+        handlerSpy = spy(new RoutingHandler(endpoints, maxRequestSizeInBytes));
+
+        // when
+        PipelineContinuationBehavior result = handlerSpy.doChannelRead(ctxMock, msg);
+
+        // then
+        assertThat(result).isEqualTo(PipelineContinuationBehavior.CONTINUE);
+    }
+
+    @Test
+    public void doChannelRead_HttpRequest_request_size_validation_endpoint_overridden_processed_message() {
+        // given
+        doReturn(100).when(endpointMock).maxRequestSizeInBytesOverride();
+
+        maxRequestSizeInBytes = 99;
+        httpHeaders.set(CONTENT_LENGTH, 100);
+        httpHeaders.set(CONTENT_TYPE, APPLICATION_JSON);
+
+        handlerSpy = spy(new RoutingHandler(endpoints, maxRequestSizeInBytes));
+
+        // when
+        PipelineContinuationBehavior result = handlerSpy.doChannelRead(ctxMock, msg);
+
+        // then
+        assertThat(result).isEqualTo(PipelineContinuationBehavior.CONTINUE);
+    }
+
+    @Test
+    public void argsAreEligibleForLinkingAndUnlinkingDistributedTracingInfo_returns_false() {
+        assertThat(handlerSpy.argsAreEligibleForLinkingAndUnlinkingDistributedTracingInfo(null,null,null,null)).isFalse();
+    }
+
 }

riposte-spi/src/main/java/com/nike/riposte/util/HttpUtils.java

@@ -2,6 +2,7 @@
 
 import com.nike.riposte.server.error.exception.InvalidCharsetInContentTypeHeaderException;
 import com.nike.riposte.server.error.exception.PathParameterMatchingException;
+import com.nike.riposte.server.http.Endpoint;
 import com.nike.riposte.server.http.RequestInfo;
 
 import java.nio.charset.Charset;
@@ -213,4 +214,17 @@ public static String replaceUriPathVariables(RequestInfo<?> request, String down
 
         return downstreamDestinationUriPath;
     }
+
+    public static boolean isMaxRequestSizeValidationDisabled(int configuredMaxRequestSize) {
+        return configuredMaxRequestSize <= 0;
+    }
+
+    public static int getConfiguredMaxRequestSize(Endpoint<?> endpoint, int globalConfiguredMaxRequestSizeInBytes) {
+        //if the endpoint is null or the endpoint is not overriding, we should return the globally configured value
+        if (endpoint == null || endpoint.maxRequestSizeInBytesOverride() == null) {
+            return globalConfiguredMaxRequestSizeInBytes;
+        }
+
+        return endpoint.maxRequestSizeInBytesOverride();
+    }
 }