4.x: Fixed missing required query and header parameters in declarative (helidon-io#10668)

tomas-langer · web-flow · commit 32e9deb6a3cb · 2025-09-16T23:28:27.000+02:00
* Missing header or query parameter that is required in declarative method will now end with a bad request, rather than internal server error.
diff --git a/declarative/codegen/src/main/java/io/helidon/declarative/codegen/http/HttpTypes.java b/declarative/codegen/src/main/java/io/helidon/declarative/codegen/http/HttpTypes.java
@@ -86,6 +86,10 @@ public final class HttpTypes {
      * HTTP media type.
      */
     public static final TypeName HTTP_MEDIA_TYPE = TypeName.create("io.helidon.http.HttpMediaType");
+    /**
+     * Bad request exception.
+     */
+    public static final TypeName BAD_REQUEST_EXCEPTION = TypeName.create("io.helidon.http.BadRequestException");
 
     private HttpTypes() {
     }
diff --git a/declarative/codegen/src/main/java/io/helidon/declarative/codegen/http/webserver/AbstractParametersProvider.java b/declarative/codegen/src/main/java/io/helidon/declarative/codegen/http/webserver/AbstractParametersProvider.java
@@ -24,6 +24,7 @@
 import io.helidon.common.types.TypeNames;
 
 import static io.helidon.declarative.codegen.DeclarativeTypes.COMMON_MAPPERS;
+import static io.helidon.declarative.codegen.http.HttpTypes.BAD_REQUEST_EXCEPTION;
 
 abstract class AbstractParametersProvider {
     void codegenFromParameters(ContentBuilder<?> contentBuilder, TypeName parameterType, String paramName, boolean optional) {
@@ -55,12 +56,27 @@ void codegenFromParameters(ContentBuilder<?> contentBuilder, TypeName parameterT
             contentBuilder
                     .addContent(".first(\"")
                     .addContent(paramName)
-                    .addContent("\").");
-            getMethod(contentBuilder, parameterType);
+                    .addContentLine("\")")
+                    .increaseContentPadding()
+                    .increaseContentPadding()
+                    .addContent(".");
+            asMethod(contentBuilder, parameterType);
+            // add .orElseThrow() in case the parameter is missing
+            contentBuilder.addContentLine("")
+                    .addContent(".orElseThrow(() -> new ")
+                    .addContent(BAD_REQUEST_EXCEPTION)
+                    .addContent("(\"")
+                    .addContent(providerType())
+                    .addContent(" ")
+                    .addContent(paramName)
+                    .addContentLine(" is not present in the request.\"));")
+                    .decreaseContentPadding()
+                    .decreaseContentPadding();
         }
-
     }
 
+    abstract String providerType();
+
     void asMethod(ContentBuilder<?> content, TypeName type) {
         TypeName boxed = type.boxed();
 
diff --git a/declarative/codegen/src/main/java/io/helidon/declarative/codegen/http/webserver/ParamProviderHttpEntity.java b/declarative/codegen/src/main/java/io/helidon/declarative/codegen/http/webserver/ParamProviderHttpEntity.java
@@ -42,4 +42,9 @@ public boolean codegen(ParameterCodegenContext ctx) {
 
         return true;
     }
+
+    @Override
+    String providerType() {
+        return "Entity";
+    }
 }
diff --git a/declarative/codegen/src/main/java/io/helidon/declarative/codegen/http/webserver/ParamProviderHttpHeader.java b/declarative/codegen/src/main/java/io/helidon/declarative/codegen/http/webserver/ParamProviderHttpHeader.java
@@ -28,6 +28,7 @@
 import io.helidon.service.codegen.DefaultsParams;
 import io.helidon.service.codegen.FieldHandler;
 
+import static io.helidon.declarative.codegen.http.HttpTypes.BAD_REQUEST_EXCEPTION;
 import static io.helidon.declarative.codegen.http.HttpTypes.HTTP_HEADER_PARAM_ANNOTATION;
 
 class ParamProviderHttpHeader extends AbstractParametersProvider implements HttpParameterCodegenProvider {
@@ -62,17 +63,24 @@ public boolean codegen(ParameterCodegenContext ctx) {
         ContentBuilder<?> contentBuilder = ctx.contentBuilder();
         String serverRequestParamName = ctx.serverRequestParamName();
 
+        // we always want `req.headers().find(HEADER_CONSTANT).map(it -> it.get(SomeType.class)`
         contentBuilder.addContent(serverRequestParamName)
-                .addContent(".headers()");
+                .addContent(".headers()")
+                .addContent(".find(")
+                .addContent(headerConstantName)
+                .addContentLine(")")
+                .increaseContentPadding()
+                .increaseContentPadding()
+                .addContent(".map(it -> it.");
+        getMethod(contentBuilder, realType);
+        contentBuilder.addContent(")");
 
         // add generated code to obtain the header from request
         if (parameterType.isOptional() || defaultCode.isPresent()) {
-            contentBuilder.addContent(".find(")
-                    .addContent(headerConstantName)
-                    .addContentLine(")")
-                    .addContent(".map(it -> it.");
-            getMethod(contentBuilder, realType);
-            contentBuilder.addContentLine(")");
+            // optional is what we expect
+            contentBuilder.addContentLine(";")
+                    .decreaseContentPadding()
+                    .decreaseContentPadding();
 
             if (defaultCode.isPresent()) {
                 DefaultsCodegen.DefaultCode defaultInfo = defaultCode.get();
@@ -93,13 +101,24 @@ public boolean codegen(ParameterCodegenContext ctx) {
                 contentBuilder.addContentLine(";");
             }
         } else {
-            contentBuilder.addContent(".get(")
+            // add .orElseThrow() in case the header is missing
+            contentBuilder.addContentLine("")
+                    .addContent(".orElseThrow(() -> new ")
+                    .addContent(BAD_REQUEST_EXCEPTION)
+                    .addContent("(\"Header \" + ")
                     .addContent(headerConstantName)
-                    .addContent(").");
-            getMethod(contentBuilder, parameterType);
-            contentBuilder.addContentLine(";");
+                    .addContent(".defaultCase() + ")
+                    .addContentLiteral(" is not present in the request.")
+                    .addContentLine("));")
+                    .decreaseContentPadding()
+                    .decreaseContentPadding();
         }
 
         return true;
     }
+
+    @Override
+    String providerType() {
+        return "Header";
+    }
 }
diff --git a/declarative/codegen/src/main/java/io/helidon/declarative/codegen/http/webserver/ParamProviderHttpPathParam.java b/declarative/codegen/src/main/java/io/helidon/declarative/codegen/http/webserver/ParamProviderHttpPathParam.java
@@ -49,4 +49,9 @@ public boolean codegen(ParameterCodegenContext ctx) {
 
         return true;
     }
+
+    @Override
+    String providerType() {
+        return "Path parameter";
+    }
 }
diff --git a/declarative/codegen/src/main/java/io/helidon/declarative/codegen/http/webserver/ParamProviderHttpQuery.java b/declarative/codegen/src/main/java/io/helidon/declarative/codegen/http/webserver/ParamProviderHttpQuery.java
@@ -77,4 +77,8 @@ public boolean codegen(ParameterCodegenContext ctx) {
         return true;
     }
 
+    @Override
+    String providerType() {
+        return "Query parameter";
+    }
 }
diff --git a/declarative/tests/codegen/src/test/java/io/helidon/declarative/codegen/http/DeclarativeCodegenHttpTypesTest.java b/declarative/tests/codegen/src/test/java/io/helidon/declarative/codegen/http/DeclarativeCodegenHttpTypesTest.java
@@ -24,6 +24,7 @@
 import java.util.Set;
 
 import io.helidon.common.types.TypeName;
+import io.helidon.http.BadRequestException;
 import io.helidon.http.Header;
 import io.helidon.http.HeaderName;
 import io.helidon.http.HeaderNames;
@@ -83,6 +84,7 @@ void testTypes() {
         checkField(toCheck, checked, fields, "HTTP_ENTITY_ANNOTATION", Http.Entity.class);
         checkField(toCheck, checked, fields, "HTTP_HEADER_FUNCTION", Http.HeaderFunction.class);
         checkField(toCheck, checked, fields, "HTTP_MEDIA_TYPE", HttpMediaType.class);
+        checkField(toCheck, checked, fields, "BAD_REQUEST_EXCEPTION", BadRequestException.class);
 
         assertThat("If the collection is not empty, please add appropriate checkField line to this test",
                    toCheck,
diff --git a/declarative/tests/http/src/main/java/io/helidon/declarative/tests/http/GreetServiceEndpoint.java b/declarative/tests/http/src/main/java/io/helidon/declarative/tests/http/GreetServiceEndpoint.java
@@ -73,6 +73,18 @@ class GreetServiceEndpoint implements GreetService {
         this.greeting.set(greeting);
     }
 
+    @Http.GET
+    @Http.Path("/custom-header")
+    String customHeaderParam(@Http.HeaderParam("X-CUSTOM") String header) {
+        return header;
+    }
+
+    @Http.GET
+    @Http.Path("/query-param")
+    String queryParam(@Http.QueryParam("param") String queryParam) {
+        return queryParam;
+    }
+
     /**
      * Return a worldly greeting message.
      */
diff --git a/declarative/tests/http/src/main/resources/application.yaml b/declarative/tests/http/src/main/resources/application.yaml
@@ -20,6 +20,10 @@ app:
 server:
   port: 8080
   host: 0.0.0.0
+  error-handling:
+    # THIS MUST NOT BE USED IN PRODUCTION!
+    # exception message will be sent back over the network - this may give potential attackers information about your application
+    include-entity: true
 
 greet-service.client:
   uri: "http://localhost:${test.server.port}"
diff --git a/declarative/tests/http/src/test/java/io/helidon/declarative/tests/http/DeclarativeHttpTest.java b/declarative/tests/http/src/test/java/io/helidon/declarative/tests/http/DeclarativeHttpTest.java
@@ -20,6 +20,9 @@
 import java.util.Map;
 import java.util.Optional;
 
+import io.helidon.http.HeaderName;
+import io.helidon.http.HeaderNames;
+import io.helidon.http.HeaderValues;
 import io.helidon.http.HttpException;
 import io.helidon.http.Status;
 import io.helidon.service.registry.Lookup;
@@ -105,12 +108,54 @@ void testErrorHandler() {
                    hasItems(GreetServiceEndpoint.class.getName() + ".updateGreetingHandler(jakarta.json.JsonObject)"));
     }
 
+    @Test
+    void testCustomHeaderParamSuccess() {
+        HeaderName customHeader = HeaderNames.create("X-CUSTOM");
+
+        var response = client.get("/greet/custom-header")
+                .header(HeaderValues.create(customHeader, "Expected-value"))
+                .request(String.class);
+
+        assertThat(response.status(), is(Status.OK_200));
+        assertThat(response.entity(), is("Expected-value"));
+    }
+
+    @Test
+    void testCustomHeaderParamFailure() {
+        var response = client.get("/greet/custom-header")
+                .request(String.class);
+
+        assertThat(response.status(), is(Status.BAD_REQUEST_400));
+        // this requires configuration option server.error-handling.include-entity set to true
+        assertThat(response.entity(), is("Header X-CUSTOM is not present in the request."));
+    }
+
+    @Test
+    void testQueryParamSuccess() {
+        var response = client.get("/greet/query-param")
+                .queryParam("param", "Expected-value")
+                .request(String.class);
+
+        assertThat(response.status(), is(Status.OK_200));
+        assertThat(response.entity(), is("Expected-value"));
+    }
+
+    @Test
+    void testQueryParamFailure() {
+        var response = client.get("/greet/query-param")
+                .request(String.class);
+
+        assertThat(response.status(), is(Status.BAD_REQUEST_400));
+        // this requires configuration option server.error-handling.include-entity set to true
+        assertThat(response.entity(), is("Query parameter param is not present in the request."));
+    }
+
     @Test
     void testTypedClient() {
         GreetServiceClient typedClient = registry.get(Lookup.builder()
-                                                               .addContract(GreetServiceClient.class)
-                                                               .addQualifier(Qualifier.create(RestClient.Client.class))
-                                                               .build());
+                                                              .addContract(GreetServiceClient.class)
+                                                              .addQualifier(Qualifier.create(RestClient.Client.class))
+                                                              .build());
 
         String message = typedClient.getDefaultMessageHandlerPlain();
         assertThat(message, is("Hello World!"));

Original file line number	Diff line number	Diff line change
`@@ -42,4 +42,9 @@ public boolean codegen(ParameterCodegenContext ctx) {`
`42`	`42`
`43`	`43`	`return true;`
`44`	`44`	`}`
	`45`	`+`
	`46`	`+ @Override`
	`47`	`+ String providerType() {`
	`48`	`+ return "Entity";`
	`49`	`+ }`
`45`	`50`	`}`
Original file line number	Diff line number	Diff line change
`@@ -49,4 +49,9 @@ public boolean codegen(ParameterCodegenContext ctx) {`
`49`	`49`
`50`	`50`	`return true;`
`51`	`51`	`}`
	`52`	`+`
	`53`	`+ @Override`
	`54`	`+ String providerType() {`
	`55`	`+ return "Path parameter";`
	`56`	`+ }`
`52`	`57`	`}`
Original file line number	Diff line number	Diff line change
`@@ -77,4 +77,8 @@ public boolean codegen(ParameterCodegenContext ctx) {`
`77`	`77`	`return true;`
`78`	`78`	`}`
`79`	`79`
	`80`	`+ @Override`
	`81`	`+ String providerType() {`
	`82`	`+ return "Query parameter";`
	`83`	`+ }`
`80`	`84`	`}`