From 24314f44327d464e9f8db2505d8e6c0487b0b5d9 Mon Sep 17 00:00:00 2001 From: Mohith Date: Tue, 20 Oct 2026 19:37:57 +0530 Subject: [PATCH] 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 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) --- .../java/com/floci/test/CloudWatchTest.java | 16 +++++ .../metrics/CloudWatchMetricsException.java | 34 +++++++++++ .../metrics/CloudWatchMetricsJsonHandler.java | 17 ++++-- .../CloudWatchMetricsQueryHandler.java | 10 ++- ...borCloudWatchTimestampIntegrationTest.java | 25 ++++++++ ...etricsEntityMetricDataIntegrationTest.java | 47 ++++++++++++++ .../CloudWatchMetricsJsonHandlerTest.java | 57 +++++++++++++++++ .../CloudWatchMetricsQueryHandlerTest.java | 61 +++++++++++++++++++ 8 files changed, 260 insertions(+), 7 deletions(-) create mode 100644 src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsException.java diff --git a/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchTest.java b/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchTest.java index e48b58a..66dc0e9 100644 --- a/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchTest.java +++ b/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchTest.java @@ -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(); + } } diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsException.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsException.java new file mode 100644 index 0000000..1ea39ed --- /dev/null +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsException.java @@ -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. + * + *

CloudWatch is awsQueryCompatible: the Query {@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; + } +} diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java index acba84d..2aef32e 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java @@ -82,9 +82,12 @@ public class CloudWatchMetricsJsonHandler { private Response handlePutMetricData(JsonNode request, String region) { String namespace = request.path("Namespace").asText(); - List datums = parseMetricDataJson(request.path("MetricData")); + List 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 parseMetricDataJson(JsonNode node) { + private List parseMetricDataJson(JsonNode node, String listPath) { List 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"); diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java index 4865b89..d3a0542 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java @@ -616,9 +616,13 @@ public class CloudWatchMetricsQueryHandler { private List parseMetricData(MultivaluedMap params, String listPrefix) { List datums = new ArrayList<>(); for (int i = 1; ; i++) { - String metricName = params.getFirst(listPrefix + ".member." + i + ".MetricName"); - if (metricName == null) { - break; + 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); diff --git a/src/test/java/io/github/hectorvent/floci/core/common/AwsJsonCborCloudWatchTimestampIntegrationTest.java b/src/test/java/io/github/hectorvent/floci/core/common/AwsJsonCborCloudWatchTimestampIntegrationTest.java index 3cbf6c9..fbdc848 100644 --- a/src/test/java/io/github/hectorvent/floci/core/common/AwsJsonCborCloudWatchTimestampIntegrationTest.java +++ b/src/test/java/io/github/hectorvent/floci/core/common/AwsJsonCborCloudWatchTimestampIntegrationTest.java @@ -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") diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsEntityMetricDataIntegrationTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsEntityMetricDataIntegrationTest.java index 890bceb..27d81e0 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsEntityMetricDataIntegrationTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsEntityMetricDataIntegrationTest.java @@ -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)); + } } diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java index 9b57294..2d3a2a2 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java @@ -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 listMetricNames(String namespace) { + ObjectNode listReq = MAPPER.createObjectNode(); + listReq.put("Namespace", namespace); + ObjectNode listed = (ObjectNode) handler.handle("ListMetrics", listReq, REGION).getEntity(); + List 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); diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java index 94e7e3f..881de60 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java @@ -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 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 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 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 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> captor = ArgumentCaptor.forClass(List.class); + verify(metricsService).putMetricData(eq("NS"), captor.capture(), eq(REGION)); + assertEquals(List.of(" "), captor.getValue().stream().map(MetricDatum::getMetricName).toList()); + } }