From 9fffe9f1371a1f72cd4834dc2be966ade9aca9fe Mon Sep 17 00:00:00 2001 From: liuhy Date: Fri, 24 Jul 2026 02:04:39 -0700 Subject: [PATCH] fix: validate metrics query windows --- .../cluster/metrics/MetricsService.java | 75 +++++++++++++++++++ .../cluster/metrics/MetricsServiceTest.java | 73 ++++++++++++++++++ 2 files changed, 148 insertions(+) diff --git a/server/src/main/java/com/rocketmq/studio/cluster/metrics/MetricsService.java b/server/src/main/java/com/rocketmq/studio/cluster/metrics/MetricsService.java index 65bbe77f..cfc20c00 100644 --- a/server/src/main/java/com/rocketmq/studio/cluster/metrics/MetricsService.java +++ b/server/src/main/java/com/rocketmq/studio/cluster/metrics/MetricsService.java @@ -18,18 +18,93 @@ import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.springframework.http.HttpStatus; import org.springframework.stereotype.Service; +import org.springframework.util.StringUtils; + +import java.math.BigDecimal; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; @Slf4j @Service @RequiredArgsConstructor public class MetricsService { + private static final long MAX_RANGE_SECONDS = 31L * 24 * 60 * 60; + private static final long MAX_SAMPLE_POINTS = 11_000L; + private static final Pattern NUMBER_PATTERN = Pattern.compile("\\d+(?:\\.\\d+)?"); + private static final Pattern DURATION_PART_PATTERN = Pattern.compile("(\\d+(?:\\.\\d+)?)(ms|s|m|h|d|w|y)"); + private static final Map UNIT_TO_MILLIS = Map.of( + "ms", BigDecimal.ONE, + "s", BigDecimal.valueOf(1_000L), + "m", BigDecimal.valueOf(60_000L), + "h", BigDecimal.valueOf(3_600_000L), + "d", BigDecimal.valueOf(86_400_000L), + "w", BigDecimal.valueOf(604_800_000L), + "y", BigDecimal.valueOf(31_536_000_000L) + ); private final MetricsSource metricsSource; public MetricDataVO query(MetricQueryDTO query) { + validateQueryWindow(query); log.debug("Querying metrics: start={}, end={}, step={}", query.getStart(), query.getEnd(), query.getStep()); return metricsSource.query(query); } + + private void validateQueryWindow(MetricQueryDTO query) { + if (query == null) { + throw badRequest("Metric query is required"); + } + long rangeSeconds = query.getEnd() - query.getStart(); + if (rangeSeconds <= 0) { + throw badRequest("Metric query end must be later than start"); + } + if (rangeSeconds > MAX_RANGE_SECONDS) { + throw badRequest("Metric query range must not exceed 31 days"); + } + BigDecimal stepMillis = parseStepMillis(query.getStep()); + if (stepMillis.signum() <= 0) { + throw badRequest("Metric query step must be positive"); + } + BigDecimal samplePoints = BigDecimal.valueOf(rangeSeconds) + .multiply(BigDecimal.valueOf(1_000L)) + .divideToIntegralValue(stepMillis) + .add(BigDecimal.ONE); + if (samplePoints.compareTo(BigDecimal.valueOf(MAX_SAMPLE_POINTS)) > 0) { + throw badRequest("Metric query returns too many samples; increase step or reduce range"); + } + } + + private BigDecimal parseStepMillis(String step) { + if (!StringUtils.hasText(step)) { + throw badRequest("Metric query step is required"); + } + String value = step.strip(); + if (NUMBER_PATTERN.matcher(value).matches()) { + return new BigDecimal(value).multiply(BigDecimal.valueOf(1_000L)); + } + + Matcher matcher = DURATION_PART_PATTERN.matcher(value); + BigDecimal millis = BigDecimal.ZERO; + int position = 0; + while (matcher.find()) { + if (matcher.start() != position) { + throw badRequest("Metric query step is invalid"); + } + BigDecimal amount = new BigDecimal(matcher.group(1)); + millis = millis.add(amount.multiply(UNIT_TO_MILLIS.get(matcher.group(2)))); + position = matcher.end(); + } + if (position != value.length()) { + throw badRequest("Metric query step is invalid"); + } + return millis; + } + + private PrometheusException badRequest(String message) { + return new PrometheusException(HttpStatus.BAD_REQUEST.value(), message); + } } diff --git a/server/src/test/java/com/rocketmq/studio/cluster/metrics/MetricsServiceTest.java b/server/src/test/java/com/rocketmq/studio/cluster/metrics/MetricsServiceTest.java index a8f7118a..bf9de233 100644 --- a/server/src/test/java/com/rocketmq/studio/cluster/metrics/MetricsServiceTest.java +++ b/server/src/test/java/com/rocketmq/studio/cluster/metrics/MetricsServiceTest.java @@ -26,9 +26,11 @@ import java.util.List; import java.util.Map; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -102,16 +104,26 @@ void queryShouldPassQueryDirectlyToSource() { void queryShouldHandleVariousStepSizes() { MetricQueryDTO query15s = MetricQueryDTO.builder().metric("cpu").start(1L).end(2L).step("15s").build(); MetricQueryDTO query1h = MetricQueryDTO.builder().metric("cpu").start(1L).end(2L).step("1h").build(); + MetricQueryDTO queryCombined = MetricQueryDTO.builder().metric("cpu").start(1L).end(7_201L) + .step("1h30m").build(); + MetricQueryDTO queryNumeric = MetricQueryDTO.builder().metric("cpu").start(1L).end(2L) + .step("0.5").build(); MetricDataVO data = emptyMetricData(); when(metricsSource.query(any(MetricQueryDTO.class))).thenReturn(data); MetricDataVO result15s = metricsService.query(query15s); MetricDataVO result1h = metricsService.query(query1h); + MetricDataVO resultCombined = metricsService.query(queryCombined); + MetricDataVO resultNumeric = metricsService.query(queryNumeric); assertThat(result15s).isNotNull(); assertThat(result1h).isNotNull(); + assertThat(resultCombined).isNotNull(); + assertThat(resultNumeric).isNotNull(); verify(metricsSource).query(query15s); verify(metricsSource).query(query1h); + verify(metricsSource).query(queryCombined); + verify(metricsSource).query(queryNumeric); } @Test @@ -151,6 +163,58 @@ void queryShouldPreserveMetricName() { } } + @Test + void queryShouldRejectInvalidWindow() { + MetricQueryDTO query = MetricQueryDTO.builder() + .metric("rocketmq_messages_in_total") + .start(1700003600L) + .end(1700000000L) + .step("1m") + .build(); + + assertBadRequest(query, "Metric query end must be later than start"); + verifyNoInteractions(metricsSource); + } + + @Test + void queryShouldRejectOversizedWindow() { + MetricQueryDTO query = MetricQueryDTO.builder() + .metric("rocketmq_messages_in_total") + .start(1700000000L) + .end(1702678401L) + .step("1h") + .build(); + + assertBadRequest(query, "Metric query range must not exceed 31 days"); + verifyNoInteractions(metricsSource); + } + + @Test + void queryShouldRejectInvalidStep() { + MetricQueryDTO query = MetricQueryDTO.builder() + .metric("rocketmq_messages_in_total") + .start(1700000000L) + .end(1700003600L) + .step("five minutes") + .build(); + + assertBadRequest(query, "Metric query step is invalid"); + verifyNoInteractions(metricsSource); + } + + @Test + void queryShouldRejectTooManySamples() { + MetricQueryDTO query = MetricQueryDTO.builder() + .metric("rocketmq_messages_in_total") + .start(1700000000L) + .end(1700011001L) + .step("1s") + .build(); + + assertBadRequest(query, "Metric query returns too many samples; increase step or reduce range"); + verifyNoInteractions(metricsSource); + } + private MetricDataVO emptyMetricData() { return MetricDataVO.builder() .resultType("matrix") @@ -177,4 +241,13 @@ private MetricDataVO.MetricSampleVO sample(double timestamp, String value) { .value(value) .build(); } + + private void assertBadRequest(MetricQueryDTO query, String message) { + assertThatExceptionOfType(PrometheusException.class) + .isThrownBy(() -> metricsService.query(query)) + .satisfies(exception -> { + assertThat(exception.getStatusCode()).isEqualTo(400); + assertThat(exception.getMessage()).isEqualTo(message); + }); + } }