Return a 400 response for a request with an invalid URL (#796)

* Return a 400 response for a request with an invalid URL. URL validation/creation moved to constructor of BareRequestImpl. New test added.

Signed-off-by: Santiago Pericas-Geertsen <santiago.pericasgeertsen@oracle.com>

* Fixed import order.

Signed-off-by: Santiago Pericas-Geertsen <santiago.pericasgeertsen@oracle.com>

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

@@ -44,6 +44,7 @@ class BareRequestImpl implements BareRequest {
     private final ChannelHandlerContext ctx;
     private final SSLEngine sslEngine;
     private final long requestId;
+    private final URI uri;
 
     BareRequestImpl(HttpRequest request,
                     Flow.Publisher<DataChunk> publisher,
@@ -57,6 +58,7 @@ class BareRequestImpl implements BareRequest {
         this.ctx = ctx;
         this.sslEngine = sslEngine;
         this.requestId = requestId;
+        this.uri = URI.create(nettyRequest.uri());
     }
 
     @Override
@@ -76,7 +78,7 @@ public Http.Version version() {
 
     @Override
     public URI uri() {
-        return URI.create(nettyRequest.uri());
+        return uri;
     }
 
     @Override

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

@@ -16,23 +16,27 @@
 
 package io.helidon.webserver;
 
+import java.nio.charset.StandardCharsets;
 import java.util.Queue;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.logging.Logger;
 
 import javax.net.ssl.SSLEngine;
 
 import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.SimpleChannelInboundHandler;
 import io.netty.handler.codec.http.DefaultFullHttpResponse;
 import io.netty.handler.codec.http.FullHttpResponse;
 import io.netty.handler.codec.http.HttpContent;
+import io.netty.handler.codec.http.HttpHeaderNames;
 import io.netty.handler.codec.http.HttpMethod;
 import io.netty.handler.codec.http.HttpRequest;
 import io.netty.handler.codec.http.HttpUtil;
 import io.netty.handler.codec.http.LastHttpContent;
 
+import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST;
 import static io.netty.handler.codec.http.HttpResponseStatus.CONTINUE;
 import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
 
@@ -97,11 +101,18 @@ protected void channelRead0(ChannelHandlerContext ctx, Object msg) {
             HttpRequestScopedPublisher publisherRef = requestContext.publisher();
             long requestId = REQUEST_ID_GENERATOR.incrementAndGet();
 
-            BareRequestImpl bareRequest =
-                new BareRequestImpl((HttpRequest) msg, requestContext.publisher(), webServer, ctx, sslEngine, requestId);
-            BareResponseImpl bareResponse =
-                new BareResponseImpl(ctx, request, publisherRef::isCompleted, Thread.currentThread(), requestId);
+            // If a problem with the request URI, return 400 response
+            BareRequestImpl bareRequest;
+            try {
+                bareRequest = new BareRequestImpl((HttpRequest) msg, requestContext.publisher(),
+                        webServer, ctx, sslEngine, requestId);
+            } catch (IllegalArgumentException e) {
+                send400BadRequest(ctx, e.getMessage());
+                return;
+            }
 
+            BareResponseImpl bareResponse =
+                    new BareResponseImpl(ctx, request, publisherRef::isCompleted, Thread.currentThread(), requestId);
             bareResponse.whenCompleted()
                         .thenRun(() -> {
                             RequestContext requestContext = this.requestContext;
@@ -177,6 +188,20 @@ private static void send100Continue(ChannelHandlerContext ctx) {
         ctx.write(response);
     }
 
+    /**
+     * Returns a 400 (Bad Request) response with a message as content.
+     *
+     * @param ctx Channel context.
+     * @param message The message.
+     */
+    private static void send400BadRequest(ChannelHandlerContext ctx, String message) {
+        byte[] entity = message.getBytes(StandardCharsets.UTF_8);
+        FullHttpResponse response = new DefaultFullHttpResponse(HTTP_1_1, BAD_REQUEST, Unpooled.wrappedBuffer(entity));
+        response.headers().add(HttpHeaderNames.CONTENT_TYPE, "text/plain");
+        response.headers().add(HttpHeaderNames.CONTENT_LENGTH, entity.length);
+        ctx.write(response);
+    }
+
     @Override
     public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
         if (requestContext != null) {

webserver/webserver/src/test/java/io/helidon/webserver/PlainTest.java

@@ -362,6 +362,20 @@ public void testConnectionCloseHeader() throws Exception {
         assertThat(headers, not(IsMapContaining.hasKey("connection")));
     }
 
+    @Test
+    public void testBadURL() throws Exception {
+        String s = SocketHttpClient.sendAndReceive("/?p=|",
+                Http.Method.GET,
+                null,
+                CollectionsHelper.listOf("Connection: close"),
+                webServer);
+        assertThat(s, containsString("400 Bad Request"));
+        Map<String, String> headers = cutHeaders(s);
+        assertThat(headers, IsMapContaining.hasKey("content-type"));
+        assertThat(headers, IsMapContaining.hasKey("content-length"));
+    }
+
+
     private Map<String, String> cutHeaders(String response) {
         assertThat(response, notNullValue());
         int index = response.indexOf("\n\n");