Fix handling of invalid end of line in HTTP header parsing. Added tes…

…ts (helidon-io#8831)
barchetta · Jun 5, 2024 · b031fe7 · b031fe7
1 parent 90c2806
commit b031fe7
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 3 deletions.
diff --git a/...ing/http-junit5/src/main/java/io/helidon/common/testing/http/junit5/SocketHttpClient.java b/...ing/http-junit5/src/main/java/io/helidon/common/testing/http/junit5/SocketHttpClient.java
@@ -460,6 +460,25 @@ public void request(String method, String path, String protocol, String host, It
         }
     }
 
+    /**
+     * Send raw data to the server.
+     *
+     * @param content content to send over the socket
+     */
+    public void requestRaw(String content) {
+        if (socket == null) {
+            connect();
+        }
+
+        try {
+            PrintWriter pw = new PrintWriter(new OutputStreamWriter(socket.getOutputStream(), StandardCharsets.UTF_8));
+            pw.print(content);
+            pw.flush();
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+    }
+
     /**
      * Write raw proxy protocol header before a request.
      *

diff --git a/webserver/tests/webserver/src/test/java/io/helidon/webserver/tests/BadRequestTest.java b/webserver/tests/webserver/src/test/java/io/helidon/webserver/tests/BadRequestTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2024 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.
@@ -148,6 +148,39 @@ void testBadHeaderWhitespace() {
         assertThat(response, containsString(CUSTOM_ENTITY));
     }
 
+    @Test
+    void testCrWithoutLf() {
+        socketClient.requestRaw("GET / HTTP/1.1\r\nhost: localhost:8080\rcustom: value\r\n");
+
+        String response = socketClient.receive();
+        assertThat(response, containsString("400 " + CUSTOM_REASON_PHRASE));
+        assertThat(response, containsString(CUSTOM_ENTITY));
+    }
+
+    @Test
+    void testLfWithoutCr() {
+        socketClient.requestRaw("GET / HTTP/1.1\r\nhost: localhost:8080\ncustom: value\r\n");
+
+        String response = socketClient.receive();
+        assertThat(response, containsString("400 " + CUSTOM_REASON_PHRASE));
+        assertThat(response, containsString(CUSTOM_ENTITY));
+    }
+
+    @Test
+    void testKeepAliveAndMissingLf() {
+        socketClient.request(Method.GET, "/", null, List.of("Accept: text/plain", "Connection: keep-alive"));
+        String response = socketClient.receive();
+        assertThat(response, containsString("200 OK"));
+        assertThat(response, containsString("Hi"));
+
+        socketClient.requestRaw("GET / HTTP/1.1\r\nhost: localhost:8080\rcustom: value\r\n");
+
+        response = socketClient.receive();
+        assertThat(response, containsString("400 " + CUSTOM_REASON_PHRASE));
+        assertThat(response, containsString("Connection: close"));
+        assertThat(response, containsString(CUSTOM_ENTITY));
+    }
+
     private static DirectHandler.TransportResponse badRequestHandler(DirectHandler.TransportRequest request,
                                                                      DirectHandler.EventType eventType,
                                                                      Status httpStatus,

diff --git a/webserver/webserver/src/main/java/io/helidon/webserver/http1/Http1Headers.java b/webserver/webserver/src/main/java/io/helidon/webserver/http1/Http1Headers.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2024 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.
@@ -55,7 +55,7 @@ public Http1Headers(DataReader reader, int maxHeadersSize, boolean validateHeade
     public WritableHeaders<?> readHeaders(HttpPrologue prologue) {
         try {
             return Http1HeadersParser.readHeaders(reader, maxHeadersSize, validateHeaders);
-        } catch (IllegalStateException | IllegalArgumentException e) {
+        } catch (IllegalStateException | IllegalArgumentException | DataReader.IncorrectNewLineException e) {
             throw RequestException.builder()
                     .type(DirectHandler.EventType.BAD_REQUEST)
                     .request(DirectTransportRequest.create(prologue, WritableHeaders.create()))