Correctly handling invalid requests and custom configuration for HTTP… (

#2244)

* Correctly handling invalid requests and custom configuration for HTTP parsing.

Signed-off-by: Tomas Langer <tomas.langer@oracle.com>

docs/src/main/docs/webserver/02_configuration.adoc

@@ -1,6 +1,6 @@
 ///////////////////////////////////////////////////////////////////////////////
 
-    Copyright (c) 2018, 2019 Oracle and/or its affiliates. All rights reserved.
+    Copyright (c) 2018, 2020 Oracle and/or its affiliates.
 
     Licensed under the Apache License, Version 2.0 (the "License");
     you may not use this file except in compliance with the License.
@@ -69,3 +69,20 @@ WebServer webServer = WebServer.create(configuration, routing);
 
 See all configuration options 
  link:{javadoc-base-url-api}/ServerConfiguration.html[here].
+
+Available socket configuration options:
+[cols="^2s,<2,<2,<6"]
+|===
+|Configuration key |Default value ^|Java type  ^|Description
+|`port` |{nbsp} |int |Port to open server socket on, defaults to an available ephemeral port
+|`bind-address` |all local addresses |String |Address to listen on (may be an IPV6 address as well)
+|`backlog` |`1024` |int |Maximum length of the queue of incoming connections on the server socket.
+|`max-header-size` |`8192` |int |Maximal number of bytes of all header values combined. Returns `400` if headers are bigger
+|`max-initial-line-length` |`4096` |int |Maximal number of characters in the initial HTTP line. Returns `400` if line is longer
+|`timeout-millis` |no timeout| long |Server socket timeout.
+|`receive-buffer-size` |implementation default |int |Proposed value of the TCP receive window that is advertised to the remote peer on the server socket.
+|`max-chunk-size` | `8192` |int |Maximal size of a chunk to read from incoming requests
+|`validate-headers` |`true` |boolean |Whether to validate header names, if they contain illegal characters.
+|`initial-buffer-size` |`128` |int |Initial size of buffer used to parse HTTP line and headers
+|`ssl` |{nbsp} |Object |Configuration of SSL, please see our SSL example in repository
+|===

webserver/webserver/src/main/java/io/helidon/webserver/ForwardingHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2019 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2020 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@
 import io.netty.buffer.Unpooled;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.SimpleChannelInboundHandler;
+import io.netty.handler.codec.DecoderResult;
 import io.netty.handler.codec.http.DefaultFullHttpResponse;
 import io.netty.handler.codec.http.FullHttpResponse;
 import io.netty.handler.codec.http.HttpContent;
@@ -96,6 +97,12 @@ protected void channelRead0(ChannelHandlerContext ctx, Object msg) {
             ctx.channel().config().setAutoRead(false);
 
             HttpRequest request = (HttpRequest) msg;
+            try {
+                checkDecoderResult(request);
+            } catch (Throwable e) {
+                send400BadRequest(ctx, e.getMessage());
+                return;
+            }
             ReferenceHoldingQueue<DataChunk> queue = new ReferenceHoldingQueue<>();
             queues.add(queue);
             requestContext = new RequestContext(new HttpRequestScopedPublisher(ctx, queue), request);
@@ -214,4 +221,14 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
         }
         ctx.close();
     }
+
+    private static void checkDecoderResult(HttpRequest request) {
+        DecoderResult decoderResult = request.decoderResult();
+        if (decoderResult.isFailure()) {
+            LOGGER.info(String.format("Request %s to %s rejected: %s", request.method()
+                    .asciiName(), request.uri(), decoderResult.cause().getMessage()));
+            throw new BadRequestException(String.format("Request was rejected: %s", decoderResult.cause().getMessage()),
+                                          decoderResult.cause());
+        }
+    }
 }

webserver/webserver/src/main/java/io/helidon/webserver/HttpInitializer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2019 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2020 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -50,10 +50,15 @@ class HttpInitializer extends ChannelInitializer<SocketChannel> {
 
     private final SslContext sslContext;
     private final NettyWebServer webServer;
+    private final SocketConfiguration soConfig;
     private final Routing routing;
     private final Queue<ReferenceHoldingQueue<DataChunk>> queues = new ConcurrentLinkedQueue<>();
 
-    HttpInitializer(SslContext sslContext, Routing routing, NettyWebServer webServer) {
+    HttpInitializer(SocketConfiguration soConfig,
+                    SslContext sslContext,
+                    Routing routing,
+                    NettyWebServer webServer) {
+        this.soConfig = soConfig;
         this.routing = routing;
         this.sslContext = sslContext;
         this.webServer = webServer;
@@ -100,7 +105,12 @@ public void initChannel(SocketChannel ch) {
             p.addLast(cleartextHttp2ServerUpgradeHandler);
             p.addLast(new HelidonEventLogger());
         } else {
-            p.addLast(new HttpRequestDecoder());
+            // explicit configuration of http handling
+            p.addLast(new HttpRequestDecoder(soConfig.maxInitialLineLength(),
+                                             soConfig.maxHeaderSize(),
+                                             soConfig.maxChunkSize(),
+                                             soConfig.validateHeaders(),
+                                             soConfig.initialBufferSize()));
             // Uncomment the following line if you don't want to handle HttpChunks.
             //        p.addLast(new HttpObjectAggregator(1048576));
             p.addLast(new HttpResponseEncoder());

webserver/webserver/src/main/java/io/helidon/webserver/NettyWebServer.java

@@ -149,7 +149,10 @@ class NettyWebServer implements WebServer {
                 bootstrap.option(ChannelOption.SO_RCVBUF, soConfig.receiveBufferSize());
             }
 
-            HttpInitializer childHandler = new HttpInitializer(sslContext, namedRoutings.getOrDefault(name, routing), this);
+            HttpInitializer childHandler = new HttpInitializer(soConfig,
+                                                               sslContext,
+                                                               namedRoutings.getOrDefault(name, routing),
+                                                               this);
             initializers.add(childHandler);
             bootstrap.group(bossGroup, workerGroup)
                      .channel(NioServerSocketChannel.class)
@@ -192,7 +195,7 @@ public synchronized CompletionStage<WebServer> start() {
                     throw new IllegalStateException(
                             "no socket configuration found for name: " + name);
                 }
-                int port = socketConfig.port() <= 0 ? 0 : socketConfig.port();
+                int port = Math.max(socketConfig.port(), 0);
                 if (channelsUpFuture.isCompletedExceptionally()) {
                     // break because one of the previous channels already failed
                     break;

webserver/webserver/src/main/java/io/helidon/webserver/ServerBasicConfig.java

@@ -19,6 +19,7 @@
 import java.net.InetAddress;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
@@ -102,6 +103,31 @@ public int receiveBufferSize() {
         return socketConfig.receiveBufferSize();
     }
 
+    @Override
+    public int maxHeaderSize() {
+        return socketConfig.maxHeaderSize();
+    }
+
+    @Override
+    public int maxInitialLineLength() {
+        return socketConfig.maxInitialLineLength();
+    }
+
+    @Override
+    public int maxChunkSize() {
+        return socketConfig.maxChunkSize();
+    }
+
+    @Override
+    public boolean validateHeaders() {
+        return socketConfig.validateHeaders();
+    }
+
+    @Override
+    public int initialBufferSize() {
+        return socketConfig.initialBufferSize();
+    }
+
     @Override
     public Tracer tracer() {
         return tracer;
@@ -132,41 +158,34 @@ static class SocketConfig implements SocketConfiguration {
         private final SSLContext sslContext;
         private final Set<String> enabledSslProtocols;
         private final ClientAuthentication clientAuth;
+        private final int maxHeaderSize;
+        private final int maxInitialLineLength;
+        private final int maxChunkSize;
+        private final boolean validateHeaders;
+        private final int initialBufferSize;
 
         /**
          * Creates new instance.
          *
-         * @param port              a server port - ff port is {@code 0} or less then any available ephemeral port will be used
-         * @param bindAddress       an address to bind the server or {@code null} for all local addresses
-         * @param sslContext        the ssl context to associate with this socket configuration
-         * @param backlog           a maximum length of the queue of incoming connections
-         * @param timeoutMillis     a socket timeout in milliseconds or {@code 0} for infinite
-         * @param receiveBufferSize proposed TCP receive window size in bytes
-         * @param clientAuth        whether client authentication is required
+         * Used only from builder {@link io.helidon.webserver.SocketConfiguration.Builder}
+         *
+         * @param builder a builder instance
          */
-        SocketConfig(int port,
-                     InetAddress bindAddress,
-                     SSLContext sslContext,
-                     Set<String> sslProtocols,
-                     int backlog,
-                     int timeoutMillis,
-                     int receiveBufferSize,
-                     ClientAuthentication clientAuth) {
-            this.port = port <= 0 ? 0 : port;
-            this.bindAddress = bindAddress;
-            this.backlog = backlog <= 0 ? DEFAULT_BACKLOG_SIZE : backlog;
-            this.timeoutMillis = timeoutMillis <= 0 ? 0 : timeoutMillis;
-            this.receiveBufferSize = receiveBufferSize <= 0 ? 0 : receiveBufferSize;
-            this.sslContext = sslContext;
-            this.enabledSslProtocols = sslProtocols;
-            this.clientAuth = clientAuth;
-        }
+        SocketConfig(Builder builder) {
+            this.port = Math.max(0, builder.port());
+            this.bindAddress = builder.bindAddress();
+            this.backlog = builder.backlog() <= 0 ? DEFAULT_BACKLOG_SIZE : builder.backlog();
+            this.timeoutMillis = Math.max(builder.timeoutMillis(), 0);
+            this.receiveBufferSize = Math.max(builder.receiveBufferSize(), 0);
+            this.maxHeaderSize = builder.maxHeaderSize();
+            this.maxInitialLineLength = builder.maxInitialLineLength();
+            this.maxChunkSize = builder.maxChunkSize();
+            this.validateHeaders = builder.validateHeaders();
+            this.initialBufferSize = builder.initialBufferSize();
+            this.clientAuth = builder.clientAuth();
+            this.sslContext = builder.sslContext();
+            this.enabledSslProtocols = new HashSet<>(builder.enabledSslProtocols());
 
-        /**
-         * Creates default values instance.
-         */
-        SocketConfig() {
-            this(0, null, null, null, 0, 0, 0, ClientAuthentication.NONE);
         }
 
         @Override
@@ -208,5 +227,30 @@ public Set<String> enabledSslProtocols() {
         public ClientAuthentication clientAuth() {
             return clientAuth;
         }
+
+        @Override
+        public int maxHeaderSize() {
+            return maxHeaderSize;
+        }
+
+        @Override
+        public int maxInitialLineLength() {
+            return maxInitialLineLength;
+        }
+
+        @Override
+        public int maxChunkSize() {
+            return maxChunkSize;
+        }
+
+        @Override
+        public boolean validateHeaders() {
+            return validateHeaders;
+        }
+
+        @Override
+        public int initialBufferSize() {
+            return initialBufferSize;
+        }
     }
 }

webserver/webserver/src/main/java/io/helidon/webserver/ServerConfiguration.java

@@ -335,6 +335,28 @@ public Builder receiveBufferSize(int bytes) {
             return this;
         }
 
+        /**
+         * Configure maximal header size (all headers combined) in number of characters for default socket.
+         *
+         * @param size header size
+         * @return an updated builder
+         */
+        public Builder maxHeaderSize(int size) {
+            defaultSocketBuilder.maxHeaderSize(size);
+            return this;
+        }
+
+        /**
+         * Configure maximal length of the initial HTTP line for default socket.
+         *
+         * @param length line length
+         * @return an updated buidler
+         */
+        public Builder maxInitialLineLength(int length) {
+            defaultSocketBuilder.maxInitialLineLength(length);
+            return this;
+        }
+
         /**
          * Adds an additional named server socket configuration. As a result, the server will listen
          * on multiple ports.
@@ -535,6 +557,11 @@ private SocketConfiguration.Builder configureSocket(Config config, SocketConfigu
             config.get("timeout").asInt().ifPresent(soConfigBuilder::timeoutMillis);
             config.get("receive-buffer").asInt().ifPresent(soConfigBuilder::receiveBufferSize);
             config.get("ssl-protocols").asList(String.class).ifPresent(soConfigBuilder::enabledSSlProtocols);
+            config.get("max-header-size").asInt().ifPresent(soConfigBuilder::maxHeaderSize);
+            config.get("max-initial-line-length").asInt().ifPresent(soConfigBuilder::maxInitialLineLength);
+            config.get("max-chunk-size").asInt().ifPresent(soConfigBuilder::maxChunkSize);
+            config.get("validate-headers").asBoolean().ifPresent(soConfigBuilder::validateHeaders);
+            config.get("initial-buffer-size").asInt().ifPresent(soConfigBuilder::initialBufferSize);
 
             // ssl
             Config sslConfig = config.get("ssl");