fix(cloudwatch): reject PutMetricData datums without a MetricName (#6072)

* fix(cloudwatch): reject PutMetricData datums without a MetricName MetricName is the one required member of MetricDatum. The JSON handler stored a nameless datum under an empty name, and the Query handler treated a member without MetricName as the end of the list, silently dropping it and every later datum while still storing the earlier ones. Both handlers now reject a missing or empty MetricName in top-level MetricData and in EntityMetricData[].MetricData with MissingParameter (400), the error PutMetricData declares for a missing required input, naming the offending member. A whitespace name is still accepted, since AWS constrains MetricName only to 1 to 255 characters. Every datum is validated before any is stored, so a rejected request leaves no state behind. On Query, a member index with no keys at all still ends that MetricData list. * fix(cloudwatch): return MissingRequiredParameterException on JSON and CBOR CloudWatch is awsQueryCompatible. MissingParameter is the awsQueryError code of the MissingRequiredParameterException shape, so it belongs in the Query <Code> and the x-amzn-query-error header, while a JSON 1.0 or CBOR body's __type names the shape. With the code in __type the AWS SDK v2 raised a generic CloudWatchException instead of the modelled one. A package-private CloudWatchMetricsException keeps the Query code as its error code and overrides jsonType() with the shape name, the same split SwfException makes. The shared query-code table in AwsException is left alone because SQS reports MissingParameter as its JSON type. (cherry picked from commit b8d87df05413ea8c4f9497fa05f388c19d4124ec)
2026-10-20 19:37:57 +05:30
parent e90978ae94
commit 24314f4432
8 changed files with 260 additions and 7 deletions
@@ -233,4 +233,20 @@ class CloudWatchTest {
        assertThat(stats.datapoints()).hasSize(1);
        assertThat(stats.datapoints().get(0).sum()).isEqualTo(6.5);
    }
+
+    @Test
+    @DisplayName("PutMetricData without MetricName throws MissingRequiredParameterException")
+    void testPutMetricDataWithoutMetricNameIsRejected() {
+        String namespace = "JavaTestMissingMetricName";
+
+        assertThatThrownBy(() -> cloudWatchClient.putMetricData(request -> request
+                .namespace(namespace)
+                .entityMetricData(entityMetricData -> entityMetricData
+                        .metricData(MetricDatum.builder().value(1.0).build()))))
+                .isInstanceOf(MissingRequiredParameterException.class)
+                .satisfies(e -> assertThat(((MissingRequiredParameterException) e).awsErrorDetails().errorCode())
+                        .isEqualTo("MissingParameter"));
+
+        assertThat(cloudWatchClient.listMetrics(request -> request.namespace(namespace)).metrics()).isEmpty();
+    }
 }
@@ -0,0 +1,34 @@
+package io.github.hectorvent.floci.services.cloudwatch.metrics;
+
+import io.github.hectorvent.floci.core.common.AwsException;
+
+/**
+ * A CloudWatch error whose Query code and JSON shape name differ.
+ *
+ * <p>CloudWatch is awsQueryCompatible: the Query {@code <Code>} and the
+ * {@code x-amzn-query-error} header carry the {@code awsQueryError} code
+ * ({@code MissingParameter}), while a JSON 1.0 or CBOR body's {@code __type} names the
+ * model shape ({@code MissingRequiredParameterException}). The AWS SDK v2 picks the typed
+ * exception from {@code __type}, so the code alone yields a generic {@code CloudWatchException}.
+ * Keeping {@link #getErrorCode()} as the Query code and overriding {@link #jsonType()}
+ * reproduces both surfaces.
+ */
+class CloudWatchMetricsException extends AwsException {
+
+    private final String jsonType;
+
+    private CloudWatchMetricsException(String errorCode, String jsonType, String message, int httpStatus) {
+        super(errorCode, message, httpStatus);
+        this.jsonType = jsonType;
+    }
+
+    static CloudWatchMetricsException missingParameter(String parameter) {
+        return new CloudWatchMetricsException("MissingParameter", "MissingRequiredParameterException",
+                "The parameter " + parameter + " is required.", 400);
+    }
+
+    @Override
+    public String jsonType() {
+        return jsonType;
+    }
+}
@@ -82,9 +82,12 @@ public class CloudWatchMetricsJsonHandler {

    private Response handlePutMetricData(JsonNode request, String region) {
        String namespace = request.path("Namespace").asText();
-        List<MetricDatum> datums = parseMetricDataJson(request.path("MetricData"));
+        List<MetricDatum> datums = parseMetricDataJson(request.path("MetricData"), "MetricData");
+        int entityIndex = 0;
        for (JsonNode entityMetricData : request.path("EntityMetricData")) {
-            datums.addAll(parseMetricDataJson(entityMetricData.path("MetricData")));
+            entityIndex++;
+            datums.addAll(parseMetricDataJson(entityMetricData.path("MetricData"),
+                    "EntityMetricData.member." + entityIndex + ".MetricData"));
        }
        LOG.debugv("JSON PutMetricData parsed {0} datums for namespace {1}", datums.size(), namespace);
        metricsService.putMetricData(namespace, datums, region);
@@ -608,12 +611,18 @@ public class CloudWatchMetricsJsonHandler {
        return configurations;
    }

-    private List<MetricDatum> parseMetricDataJson(JsonNode node) {
+    private List<MetricDatum> parseMetricDataJson(JsonNode node, String listPath) {
        List<MetricDatum> datums = new ArrayList<>();
        if (!node.isArray()) return datums;
+        int index = 0;
        for (JsonNode item : node) {
+            index++;
+            JsonNode metricName = item.path("MetricName");
+            if (!metricName.isTextual() || metricName.asText().isEmpty()) {
+                throw CloudWatchMetricsException.missingParameter(listPath + ".member." + index + ".MetricName");
+            }
            MetricDatum datum = new MetricDatum();
-            datum.setMetricName(item.path("MetricName").asText());
+            datum.setMetricName(metricName.asText());
            datum.setValue(item.path("Value").asDouble(0));
            datum.setUnit(item.path("Unit").asText(null));
            JsonNode ts = item.path("Timestamp");
@@ -616,10 +616,14 @@ public class CloudWatchMetricsQueryHandler {
    private List<MetricDatum> parseMetricData(MultivaluedMap<String, String> params, String listPrefix) {
        List<MetricDatum> datums = new ArrayList<>();
        for (int i = 1; ; i++) {
-            String metricName = params.getFirst(listPrefix + ".member." + i + ".MetricName");
-            if (metricName == null) {
+            String memberPrefix = listPrefix + ".member." + i + ".";
+            String metricName = params.getFirst(memberPrefix + "MetricName");
+            if (metricName == null || metricName.isEmpty()) {
+                if (metricName == null && params.keySet().stream().noneMatch(key -> key.startsWith(memberPrefix))) {
                    break;
                }
+                throw CloudWatchMetricsException.missingParameter(memberPrefix + "MetricName");
+            }
            MetricDatum datum = new MetricDatum();
            datum.setMetricName(metricName);
            datum.setUnit(params.getFirst(listPrefix + ".member." + i + ".Unit"));
@@ -6,11 +6,13 @@ import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.fasterxml.jackson.dataformat.cbor.CBORFactory;
 import io.quarkus.test.junit.QuarkusTest;
+import io.restassured.response.Response;
 import org.junit.jupiter.api.Test;

 import java.time.Instant;

 import static io.restassured.RestAssured.given;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;

@@ -82,6 +84,29 @@ class AwsJsonCborCloudWatchTimestampIntegrationTest {
                        + driftSeconds + "s): " + datapoint);
    }

+    // CloudWatch is awsQueryCompatible: the header keeps the Query code while the body names
+    // the model shape, which is what the AWS SDK v2 uses to pick the typed exception.
+    @Test
+    void putMetricDataWithoutMetricNameReturnsTheModelledExceptionShape() throws Exception {
+        ObjectNode putRequest = JSON.createObjectNode();
+        putRequest.put("Namespace", "CborRpcV2MissingName-" + System.nanoTime());
+        putRequest.putArray("MetricData").addObject().put("Value", 1.0);
+
+        Response response = given()
+                .contentType("application/cbor")
+                .accept("application/cbor")
+                .header("smithy-protocol", "rpc-v2-cbor")
+                .body(AwsJsonCborController.nodeToSmithyCbor(putRequest))
+                .when().post("/service/GraniteServiceVersion20100801/operation/PutMetricData");
+
+        assertEquals(400, response.statusCode());
+        assertEquals("MissingParameter;Sender", response.header("x-amzn-query-error"));
+        JsonNode body = CBOR.readTree(response.asByteArray());
+        assertEquals("MissingRequiredParameterException", body.path("__type").asText(), body.toString());
+        assertEquals("The parameter MetricData.member.1.MetricName is required.",
+                body.path("message").asText(), body.toString());
+    }
+
    private static JsonNode rpcV2Call(String action, ObjectNode body) throws Exception {
        byte[] responseCbor = given()
                .contentType("application/cbor")
@@ -14,6 +14,7 @@ import java.time.Instant;
 import static io.restassured.RestAssured.config;
 import static io.restassured.RestAssured.given;
 import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasItem;

@@ -153,4 +154,50 @@ class CloudWatchMetricsEntityMetricDataIntegrationTest {
                .body("GetMetricStatisticsResponse.GetMetricStatisticsResult.Datapoints.member.Sum",
                        equalTo("6.5"));
    }
+
+    @Test
+    void jsonNestedDatumWithoutMetricNameIsRejectedAndNothingIsStored() {
+        json("PutMetricData")
+                .body("""
+                        {"Namespace":"CWAgent/JsonInvalid",
+                         "MetricData":[{"MetricName":"TopLevel","Value":1}],
+                         "EntityMetricData":[{"MetricData":[{"Value":2}]}]}
+                        """)
+        .when().post("/")
+        .then()
+                .statusCode(400)
+                .header("x-amzn-query-error", equalTo("MissingParameter;Sender"))
+                .body("__type", equalTo("MissingRequiredParameterException"))
+                .body("message", equalTo(
+                        "The parameter EntityMetricData.member.1.MetricData.member.1.MetricName is required."));
+
+        json("ListMetrics")
+                .body("{\"Namespace\":\"CWAgent/JsonInvalid\"}")
+        .when().post("/")
+        .then()
+                .statusCode(200)
+                .body("Metrics", empty());
+    }
+
+    @Test
+    void queryMemberWithoutMetricNameIsRejectedAndNothingIsStored() {
+        query("PutMetricData")
+                .formParam("Namespace", "CWAgent/QueryInvalid")
+                .formParam("MetricData.member.1.Value", "1")
+                .formParam("MetricData.member.2.MetricName", "Valid")
+                .formParam("MetricData.member.2.Value", "2")
+        .when().post("/")
+        .then()
+                .statusCode(400)
+                .body("ErrorResponse.Error.Code", equalTo("MissingParameter"))
+                .body("ErrorResponse.Error.Message",
+                        equalTo("The parameter MetricData.member.1.MetricName is required."));
+
+        query("ListMetrics")
+                .formParam("Namespace", "CWAgent/QueryInvalid")
+        .when().post("/")
+        .then()
+                .statusCode(200)
+                .body("ListMetricsResponse.ListMetricsResult.Metrics.member.size()", equalTo(0));
+    }
 }
@@ -8,6 +8,7 @@ import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
+import io.github.hectorvent.floci.core.common.AwsException;
 import io.github.hectorvent.floci.core.common.RegionResolver;
 import io.github.hectorvent.floci.core.storage.InMemoryStorage;
 import io.github.hectorvent.floci.services.cloudwatch.dashboards.CloudWatchDashboardsService;
@@ -22,6 +23,7 @@ import java.util.List;

 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;

 /**
@@ -205,6 +207,61 @@ class CloudWatchMetricsJsonHandlerTest {
        assertEquals(List.of("FirstEntity", "SecondEntity", "TopLevel"), names.stream().sorted().toList());
    }

+    @Test
+    void putMetricData_entityDatumWithoutMetricName_rejectsTheWholeRequest() {
+        ObjectNode req = MAPPER.createObjectNode();
+        req.put("Namespace", "Invalid");
+        req.putArray("MetricData").addObject()
+                .put("MetricName", "TopLevel").put("Value", 1.0);
+        ArrayNode nested = req.putArray("EntityMetricData").addObject().putArray("MetricData");
+        nested.addObject().put("MetricName", "Valid").put("Value", 2.0);
+        nested.addObject().put("Value", 3.0);
+
+        AwsException e = assertThrows(AwsException.class,
+                () -> handler.handle("PutMetricData", req, REGION));
+        assertEquals("MissingParameter", e.getErrorCode());
+        assertEquals("MissingRequiredParameterException", e.jsonType());
+        assertEquals(400, e.getHttpStatus());
+        assertEquals("The parameter EntityMetricData.member.1.MetricData.member.2.MetricName is required.",
+                e.getMessage());
+        assertEquals(0, listMetricNames("Invalid").size());
+    }
+
+    @Test
+    void putMetricData_topLevelDatumWithEmptyMetricName_rejectsTheWholeRequest() {
+        ObjectNode req = MAPPER.createObjectNode();
+        req.put("Namespace", "InvalidTop");
+        ArrayNode data = req.putArray("MetricData");
+        data.addObject().put("MetricName", "Valid").put("Value", 1.0);
+        data.addObject().put("MetricName", "").put("Value", 2.0);
+
+        AwsException e = assertThrows(AwsException.class,
+                () -> handler.handle("PutMetricData", req, REGION));
+        assertEquals("MissingParameter", e.getErrorCode());
+        assertEquals("The parameter MetricData.member.2.MetricName is required.", e.getMessage());
+        assertEquals(0, listMetricNames("InvalidTop").size());
+    }
+
+    // AWS constrains MetricName to 1 to 255 characters with no pattern, so whitespace is a name.
+    @Test
+    void putMetricData_whitespaceMetricName_isStored() {
+        ObjectNode req = MAPPER.createObjectNode();
+        req.put("Namespace", "WhitespaceName");
+        req.putArray("MetricData").addObject().put("MetricName", " ").put("Value", 1.0);
+
+        assertEquals(200, handler.handle("PutMetricData", req, REGION).getStatus());
+        assertEquals(List.of(" "), listMetricNames("WhitespaceName"));
+    }
+
+    private List<String> listMetricNames(String namespace) {
+        ObjectNode listReq = MAPPER.createObjectNode();
+        listReq.put("Namespace", namespace);
+        ObjectNode listed = (ObjectNode) handler.handle("ListMetrics", listReq, REGION).getEntity();
+        List<String> names = new ArrayList<>();
+        listed.get("Metrics").forEach(m -> names.add(m.get("MetricName").asText()));
+        return names;
+    }
+
    @Test
    void setAlarmState_updatesFieldsCorrectly() {
        putMetric("NS", "M", "type", "current", 100.0, EPOCH_NOW);
@@ -1,5 +1,6 @@
 package io.github.hectorvent.floci.services.cloudwatch.metrics;

+import io.github.hectorvent.floci.core.common.AwsException;
 import io.github.hectorvent.floci.services.cloudwatch.dashboards.CloudWatchDashboardsService;
 import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm;
 import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricDatum;
@@ -14,6 +15,7 @@ import java.util.List;

 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.mockito.ArgumentMatchers.any;
@@ -21,6 +23,7 @@ import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;

@@ -181,4 +184,62 @@ class CloudWatchMetricsQueryHandlerTest {
        assertEquals(1.0, datums.get(0).getValue());
        assertEquals(2.0, datums.get(1).getValue());
    }
+
+    @Test
+    void putMetricDataRejectsAMemberWithoutMetricNameBeforeStoringAnything() {
+        MultivaluedMap<String, String> params = new MultivaluedHashMap<>();
+        params.putSingle("Namespace", "NS");
+        params.putSingle("MetricData.member.1.Value", "1");
+        params.putSingle("MetricData.member.2.MetricName", "Valid");
+        params.putSingle("MetricData.member.2.Value", "2");
+
+        AwsException e = assertThrows(AwsException.class,
+                () -> handler.handle("PutMetricData", params, REGION));
+        assertEquals("MissingParameter", e.getErrorCode());
+        assertEquals("MissingRequiredParameterException", e.jsonType());
+        assertEquals(400, e.getHttpStatus());
+        assertEquals("The parameter MetricData.member.1.MetricName is required.", e.getMessage());
+        verify(metricsService, never()).putMetricData(any(), any(), any());
+    }
+
+    @Test
+    void putMetricDataRejectsANestedMemberWithoutMetricNameBeforeStoringAnything() {
+        MultivaluedMap<String, String> params = new MultivaluedHashMap<>();
+        params.putSingle("Namespace", "NS");
+        params.putSingle("MetricData.member.1.MetricName", "TopLevel");
+        params.putSingle("MetricData.member.1.Value", "1");
+        params.putSingle("EntityMetricData.member.1.MetricData.member.1.Unit", "Percent");
+        params.putSingle("EntityMetricData.member.1.MetricData.member.2.MetricName", "Valid");
+
+        AwsException e = assertThrows(AwsException.class,
+                () -> handler.handle("PutMetricData", params, REGION));
+        assertEquals("The parameter EntityMetricData.member.1.MetricData.member.1.MetricName is required.",
+                e.getMessage());
+        verify(metricsService, never()).putMetricData(any(), any(), any());
+    }
+
+    @Test
+    void putMetricDataRejectsAnEmptyMetricNameButAcceptsWhitespace() {
+        MultivaluedMap<String, String> empty = new MultivaluedHashMap<>();
+        empty.putSingle("Namespace", "NS");
+        empty.putSingle("MetricData.member.1.MetricName", "");
+        empty.putSingle("MetricData.member.1.Value", "1");
+
+        AwsException e = assertThrows(AwsException.class,
+                () -> handler.handle("PutMetricData", empty, REGION));
+        assertEquals("The parameter MetricData.member.1.MetricName is required.", e.getMessage());
+        verify(metricsService, never()).putMetricData(any(), any(), any());
+
+        MultivaluedMap<String, String> whitespace = new MultivaluedHashMap<>();
+        whitespace.putSingle("Namespace", "NS");
+        whitespace.putSingle("MetricData.member.1.MetricName", " ");
+        whitespace.putSingle("MetricData.member.1.Value", "1");
+
+        handler.handle("PutMetricData", whitespace, REGION);
+
+        @SuppressWarnings("unchecked")
+        ArgumentCaptor<List<MetricDatum>> captor = ArgumentCaptor.forClass(List.class);
+        verify(metricsService).putMetricData(eq("NS"), captor.capture(), eq(REGION));
+        assertEquals(List.of(" "), captor.getValue().stream().map(MetricDatum::getMetricName).toList());
+    }
 }