Improves handling of invalid Accept types by returning 400 Bad Reques…

…t instead of 500 Internal Error. New tests. See issue helidon-io#8672.

webserver/webserver/pom.xml

@@ -247,5 +247,15 @@
             <artifactId>junit-jupiter-params</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>io.helidon.media</groupId>
+            <artifactId>helidon-media-jsonp</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>io.helidon.metrics</groupId>
+            <artifactId>helidon-metrics</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2022 Oracle and/or its affiliates.
+ * Copyright (c) 2017, 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.
@@ -118,7 +118,6 @@ public List<MediaType> acceptedTypes() {
 
             if (acceptValues.size() == 1 && HUC_ACCEPT_DEFAULT.equals(acceptValues.get(0))) {
                 result = HUC_ACCEPT_DEFAULT_TYPES;
-
             } else {
                 result = LazyList.create(acceptValues.stream()
                         .flatMap(h -> Utils.tokenize(',', "\"", false, h).stream())
@@ -145,29 +144,33 @@ public Optional<MediaType> bestAccepted(MediaType... mediaTypes) {
         if (mediaTypes == null || mediaTypes.length == 0) {
             return Optional.empty();
         }
-        List<MediaType> accepts = acceptedTypes();
-        if (accepts == null || accepts.isEmpty()) {
-            return Optional.ofNullable(mediaTypes[0]);
-        }
+        try {
+            List<MediaType> accepts = acceptedTypes();
+            if (accepts == null || accepts.isEmpty()) {
+                return Optional.ofNullable(mediaTypes[0]);
+            }
 
-        double best = 0;
-        MediaType result = null;
-        for (MediaType mt : mediaTypes) {
-            if (mt != null) {
-                for (MediaType acc : accepts) {
-                    double q = acc.qualityFactor();
-                    if (q > best && acc.test(mt)) {
-                        if (q == 1) {
-                            return Optional.of(mt);
-                        } else {
-                            best = q;
-                            result = mt;
+            double best = 0;
+            MediaType result = null;
+            for (MediaType mt : mediaTypes) {
+                if (mt != null) {
+                    for (MediaType acc : accepts) {
+                        double q = acc.qualityFactor();
+                        if (q > best && acc.test(mt)) {
+                            if (q == 1) {
+                                return Optional.of(mt);
+                            } else {
+                                best = q;
+                                result = mt;
+                            }
                         }
                     }
                 }
             }
+            return Optional.ofNullable(result);
+        } catch (IllegalArgumentException e) {
+            throw new BadRequestException("Unable to parse Accept header", e);
         }
-        return Optional.ofNullable(result);
     }
 
     @Override

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2022 Oracle and/or its affiliates.
+ * Copyright (c) 2017, 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.
@@ -181,6 +181,10 @@ public <T> Single<ServerResponse> send(T content) {
                 sendPublisher.subscribe(bareResponse);
             }, content == null);
             return whenSent();
+        } catch (IllegalStateException e) {
+            eventListener.finish();
+            throw e.getCause() instanceof IllegalArgumentException
+                    ? new BadRequestException("Unable to process request", e) : e;
         } catch (RuntimeException | Error e) {
             eventListener.finish();
             throw e;

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

@@ -0,0 +1,75 @@
+/*
+ * Copyright (c) 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.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.helidon.webserver;
+
+import java.util.List;
+
+import io.helidon.common.http.Http;
+import io.helidon.webserver.utils.SocketHttpClient;
+import io.helidon.media.jsonp.JsonpSupport;
+import io.helidon.metrics.MetricsSupport;
+
+import jakarta.json.JsonObject;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+
+/**
+ * Tests bad types in Accept header
+ */
+public class BadRequestTest {
+
+    private static WebServer server;
+
+    @BeforeAll
+    static void createAndStartServer() {
+        server = WebServer.builder()
+                .addMediaSupport(JsonpSupport.create())
+                .addRouting(Routing.builder()
+                        .register(MetricsSupport.create())
+                        .post("/echo", Handler.create(JsonObject.class,
+                                (req, res, entity) -> res.status(Http.Status.OK_200).send(entity))))
+                .build();
+        server.start().await();
+    }
+
+    @AfterAll
+    static void stopServer() {
+        server.shutdown().await();
+    }
+
+    @Test
+    void testBadAcceptType() throws Exception {
+        try (SocketHttpClient c = new SocketHttpClient(server)) {
+            c.request(Http.Method.POST, "/echo", "{ }", List.of("Accept: application.json"));
+            String result = c.receive();
+            assertThat(result, containsString("400 Bad Request"));
+        }
+    }
+
+    @Test
+    void testBadAcceptTypeMetrics() throws Exception {
+        try (SocketHttpClient c = new SocketHttpClient(server)) {
+            c.request(Http.Method.GET, "/metrics", "", List.of("Accept: application.json"));
+            String result = c.receive();
+            assertThat(result, containsString("400 Bad Request"));
+        }
+    }
+}