Fail with 400 for bad application/x-www-form-urlencoded requests

a-dubaj · Mar 14, 2024 · b0e0e63 · b0e0e63
1 parent 6c76d7d
commit b0e0e63
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 1 deletion.
diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FormPostRequestHandler.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FormPostRequestHandler.java
@@ -26,7 +26,10 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
+import static com.yahoo.jdisc.Response.Status.BAD_REQUEST;
 import static com.yahoo.jdisc.Response.Status.UNSUPPORTED_MEDIA_TYPE;
 import static com.yahoo.jdisc.http.server.jetty.CompletionHandlerUtils.NOOP_COMPLETION_HANDLER;
 
@@ -40,6 +43,8 @@
  */
 class FormPostRequestHandler extends AbstractRequestHandler implements ContentChannel, DelegatedRequestHandler {
 
+    private static final Logger log = Logger.getLogger(FormPostRequestHandler.class.getName());
+
     private final ByteArrayOutputStream accumulatedRequestContent = new ByteArrayOutputStream();
     private final RequestHandler delegateHandler;
     private final String contentCharsetName;
@@ -86,7 +91,14 @@ public void close(CompletionHandler completionHandler) {
             byte[] requestContentBytes = accumulatedRequestContent.toByteArray();
             String content = new String(requestContentBytes, contentCharset);
             completionHandler.completed();
-            Map<String, List<String>> parameterMap = parseFormParameters(content);
+            Map<String, List<String>> parameterMap;
+            try {
+                parameterMap = parseFormParameters(content);
+            } catch (IllegalArgumentException e) {
+                // Log for now until this is solved properly
+                log.log(Level.INFO, "Failed to parse form parameters: %s".formatted(e.getMessage()));
+                throw new RequestException(BAD_REQUEST, "Failed to parse form parameters", e);
+            }
             mergeParameters(parameterMap, request.parameters());
             ContentChannel contentChannel = delegateHandler.handleRequest(request, responseHandler);
             if (contentChannel != null) {

diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
@@ -287,6 +287,19 @@ void requireThatFormPostRemovesContentWhenConfiguredTo() throws Exception {
         assertTrue(driver.close());
     }
 
+    @Test
+    void requireThatFormPostWithInvalidDataFailsWith400() throws Exception {
+        final JettyTestDriver driver = newDriverWithFormPostContentRemoved(new ParameterPrinterRequestHandler(), true);
+        final ResponseValidator response =
+                driver.client().newPost("/status.html")
+                        .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED)
+                        .setContent("%!Foo=bar")
+                        .execute();
+        response.expectStatusCode(is(BAD_REQUEST))
+                .expectContent(containsString("Failed to parse form parameters"));
+        assertTrue(driver.close());
+    }
+
     @Test
     void requireThatFormPostWithCharsetSpecifiedWorks() throws Exception {
         final JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler());