diff --git a/docs/apidiffs/current_vs_latest/prometheus-metrics-exporter-common.txt b/docs/apidiffs/current_vs_latest/prometheus-metrics-exporter-common.txt index e83e9a6f5..daa5ef822 100644 --- a/docs/apidiffs/current_vs_latest/prometheus-metrics-exporter-common.txt +++ b/docs/apidiffs/current_vs_latest/prometheus-metrics-exporter-common.txt @@ -1,2 +1,4 @@ Comparing source compatibility of prometheus-metrics-exporter-common-1.8.1-SNAPSHOT.jar against prometheus-metrics-exporter-common-1.8.0.jar -No changes. +*** MODIFIED CLASS: PUBLIC io.prometheus.metrics.exporter.common.PrometheusScrapeHandler (not serializable) + === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 + diff --git a/prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/InvalidQueryParameterException.java b/prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/InvalidQueryParameterException.java new file mode 100644 index 000000000..0622952e7 --- /dev/null +++ b/prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/InvalidQueryParameterException.java @@ -0,0 +1,21 @@ +package io.prometheus.metrics.exporter.common; + +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; + +class InvalidQueryParameterException extends RuntimeException { + + InvalidQueryParameterException(String message) { + super(message); + } + + // decode with Charset is only available in Java 10+, but we want to support Java 8 + @SuppressWarnings("JdkObsolete") + static String urlDecode(String value) throws UnsupportedEncodingException { + try { + return URLDecoder.decode(value, "UTF-8"); + } catch (IllegalArgumentException e) { + throw new InvalidQueryParameterException("Invalid percent-encoding in query string"); + } + } +} diff --git a/prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/PrometheusHttpRequest.java b/prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/PrometheusHttpRequest.java index a0c692c23..31ce2cc73 100644 --- a/prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/PrometheusHttpRequest.java +++ b/prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/PrometheusHttpRequest.java @@ -3,7 +3,6 @@ import io.prometheus.metrics.annotations.StableApi; import io.prometheus.metrics.model.registry.PrometheusScrapeRequest; import java.io.UnsupportedEncodingException; -import java.net.URLDecoder; import java.util.ArrayList; import java.util.Enumeration; import javax.annotation.Nullable; @@ -49,15 +48,43 @@ default String getParameter(String name) { @SuppressWarnings("JdkObsolete") default String[] getParameterValues(String name) { try { + int maxQueryStringLength = 64 * 1024; + int maxQueryParameterCount = 1024; ArrayList result = new ArrayList<>(); String queryString = getQueryString(); if (queryString != null) { - String[] pairs = queryString.split("&"); - for (String pair : pairs) { + if (queryString.length() > maxQueryStringLength) { + throw new InvalidQueryParameterException( + "Query string too long: " + + queryString.length() + + " characters (max " + + maxQueryStringLength + + ")"); + } + int start = 0; + int parameterCount = 0; + for (int end = queryString.indexOf('&'); start <= queryString.length(); ) { + parameterCount++; + if (parameterCount > maxQueryParameterCount) { + throw new InvalidQueryParameterException( + "Too many query parameters: " + + parameterCount + + " (max " + + maxQueryParameterCount + + ")"); + } + String pair = + end == -1 ? queryString.substring(start) : queryString.substring(start, end); int idx = pair.indexOf("="); - if (idx != -1 && URLDecoder.decode(pair.substring(0, idx), "UTF-8").equals(name)) { - result.add(URLDecoder.decode(pair.substring(idx + 1), "UTF-8")); + if (idx != -1 + && InvalidQueryParameterException.urlDecode(pair.substring(0, idx)).equals(name)) { + result.add(InvalidQueryParameterException.urlDecode(pair.substring(idx + 1))); + } + if (end == -1) { + break; } + start = end + 1; + end = queryString.indexOf('&', start); } } if (result.isEmpty()) { diff --git a/prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/PrometheusScrapeHandler.java b/prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/PrometheusScrapeHandler.java index 20328382a..d97a90205 100644 --- a/prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/PrometheusScrapeHandler.java +++ b/prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/PrometheusScrapeHandler.java @@ -60,10 +60,19 @@ public PrometheusScrapeHandler(PrometheusProperties config, PrometheusRegistry r public void handleRequest(PrometheusHttpExchange exchange) throws IOException { try { PrometheusHttpRequest request = exchange.getRequest(); - MetricSnapshots snapshots = scrape(request); + String[] includedNames = null; + String debugParam = null; + try { + includedNames = request.getParameterValues("name[]"); + debugParam = request.getParameter("debug"); + } catch (InvalidQueryParameterException e) { + writeInvalidQueryParametersResponse(exchange); + return; + } + MetricSnapshots snapshots = scrape(request, includedNames); String acceptHeader = request.getHeader("Accept"); EscapingScheme escapingScheme = EscapingScheme.fromAcceptHeader(acceptHeader); - if (writeDebugResponse(snapshots, escapingScheme, exchange)) { + if (writeDebugResponse(snapshots, escapingScheme, debugParam, exchange)) { return; } ExpositionFormatWriter writer = expositionFormats.findWriter(acceptHeader); @@ -136,9 +145,9 @@ private Predicate makeNameFilter(@Nullable String[] includedNames) { return result; } - private MetricSnapshots scrape(PrometheusHttpRequest request) { + private MetricSnapshots scrape(PrometheusHttpRequest request, @Nullable String[] includedNames) { - Predicate filter = makeNameFilter(request.getParameterValues("name[]")); + Predicate filter = makeNameFilter(includedNames); if (filter != null) { return registry.scrape(filter, request); } else { @@ -147,9 +156,11 @@ private MetricSnapshots scrape(PrometheusHttpRequest request) { } private boolean writeDebugResponse( - MetricSnapshots snapshots, EscapingScheme escapingScheme, PrometheusHttpExchange exchange) + MetricSnapshots snapshots, + EscapingScheme escapingScheme, + @Nullable String debugParam, + PrometheusHttpExchange exchange) throws IOException { - String debugParam = exchange.getRequest().getParameter("debug"); PrometheusHttpResponse response = exchange.getResponse(); if (debugParam == null) { return false; @@ -184,6 +195,16 @@ private boolean writeDebugResponse( } } + private void writeInvalidQueryParametersResponse(PrometheusHttpExchange exchange) + throws IOException { + PrometheusHttpResponse response = exchange.getResponse(); + response.setHeader("Content-Type", "text/plain; charset=utf-8"); + byte[] message = "Invalid query parameters".getBytes(StandardCharsets.UTF_8); + try (OutputStream outputStream = response.sendHeadersAndGetBody(400, message.length)) { + outputStream.write(message); + } + } + private boolean shouldUseCompression(PrometheusHttpRequest request) { if (preferUncompressedResponse) { return false; diff --git a/prometheus-metrics-exporter-common/src/test/java/io/prometheus/metrics/exporter/common/PrometheusHttpRequestTest.java b/prometheus-metrics-exporter-common/src/test/java/io/prometheus/metrics/exporter/common/PrometheusHttpRequestTest.java index a2b630ecd..c2af705a4 100644 --- a/prometheus-metrics-exporter-common/src/test/java/io/prometheus/metrics/exporter/common/PrometheusHttpRequestTest.java +++ b/prometheus-metrics-exporter-common/src/test/java/io/prometheus/metrics/exporter/common/PrometheusHttpRequestTest.java @@ -1,6 +1,7 @@ package io.prometheus.metrics.exporter.common; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import java.util.Collections; import java.util.Enumeration; @@ -80,6 +81,22 @@ void testGetParameterValuesIgnoresParametersWithoutEquals() { assertThat(values).containsExactly("value1", "value2"); } + @Test + void testGetParameterValuesRejectsMalformedEncodedName() { + PrometheusHttpRequest request = new TestPrometheusHttpRequest("name%ZZ=value"); + + assertThatExceptionOfType(InvalidQueryParameterException.class) + .isThrownBy(() -> request.getParameterValues("name")); + } + + @Test + void testGetParameterValuesRejectsMalformedEncodedValue() { + PrometheusHttpRequest request = new TestPrometheusHttpRequest("name=value%ZZ"); + + assertThatExceptionOfType(InvalidQueryParameterException.class) + .isThrownBy(() -> request.getParameterValues("name")); + } + /** Test implementation of PrometheusHttpRequest for testing default methods. */ private static class TestPrometheusHttpRequest implements PrometheusHttpRequest { private final String queryString; diff --git a/prometheus-metrics-exporter-common/src/test/java/io/prometheus/metrics/exporter/common/PrometheusScrapeHandlerTest.java b/prometheus-metrics-exporter-common/src/test/java/io/prometheus/metrics/exporter/common/PrometheusScrapeHandlerTest.java index 07ea8ac52..4f190b61f 100644 --- a/prometheus-metrics-exporter-common/src/test/java/io/prometheus/metrics/exporter/common/PrometheusScrapeHandlerTest.java +++ b/prometheus-metrics-exporter-common/src/test/java/io/prometheus/metrics/exporter/common/PrometheusScrapeHandlerTest.java @@ -164,6 +164,36 @@ void testMultipleMetricNameFilters() throws IOException { assertThat(body).doesNotContain("metric_three"); } + @Test + void testRejectsTooManyQueryParameters() throws IOException { + StringBuilder queryString = new StringBuilder("name[]=test_counter"); + for (int i = 0; i < 1024; i++) { + queryString.append("&name[]=metric_").append(i); + } + + TestHttpExchange exchange = new TestHttpExchange("GET", queryString.toString()); + handler.handleRequest(exchange); + + assertThat(exchange.getResponseCode()).isEqualTo(400); + assertThat(exchange.getResponseHeaders().get("Content-Type")) + .isEqualTo("text/plain; charset=utf-8"); + assertThat(exchange.getResponseBody()).isEqualTo("Invalid query parameters"); + } + + @Test + void testRejectsTooLongQueryString() throws IOException { + StringBuilder queryString = new StringBuilder("name[]="); + for (int i = 0; i < 64 * 1024; i++) { + queryString.append("a"); + } + + TestHttpExchange exchange = new TestHttpExchange("GET", queryString.toString()); + handler.handleRequest(exchange); + + assertThat(exchange.getResponseCode()).isEqualTo(400); + assertThat(exchange.getResponseBody()).isEqualTo("Invalid query parameters"); + } + /** Test implementation of PrometheusHttpExchange for testing. */ private static class TestHttpExchange implements PrometheusHttpExchange { private final TestHttpRequest request; @@ -216,7 +246,7 @@ public Map getResponseHeaders() { } public String getResponseBody() { - return rawResponseBody.toString(StandardCharsets.UTF_8); + return new String(rawResponseBody.toByteArray(), StandardCharsets.UTF_8); } public boolean isGzipCompressed() { diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MetricNameFilter.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MetricNameFilter.java index 642f27150..ded171500 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MetricNameFilter.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MetricNameFilter.java @@ -6,6 +6,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.LinkedHashSet; import java.util.function.Predicate; import javax.annotation.Nullable; @@ -26,10 +27,10 @@ private MetricNameFilter( Collection nameIsNotEqualTo, Collection nameStartsWith, Collection nameDoesNotStartWith) { - this.nameIsEqualTo = unmodifiableCollection(new ArrayList<>(nameIsEqualTo)); - this.nameIsNotEqualTo = unmodifiableCollection(new ArrayList<>(nameIsNotEqualTo)); - this.nameStartsWith = unmodifiableCollection(new ArrayList<>(nameStartsWith)); - this.nameDoesNotStartWith = unmodifiableCollection(new ArrayList<>(nameDoesNotStartWith)); + this.nameIsEqualTo = unmodifiableCollection(new LinkedHashSet<>(nameIsEqualTo)); + this.nameIsNotEqualTo = unmodifiableCollection(new LinkedHashSet<>(nameIsNotEqualTo)); + this.nameStartsWith = unmodifiableCollection(new LinkedHashSet<>(nameStartsWith)); + this.nameDoesNotStartWith = unmodifiableCollection(new LinkedHashSet<>(nameDoesNotStartWith)); } @Override @@ -44,6 +45,9 @@ private boolean matchesNameEqualTo(String metricName) { if (nameIsEqualTo.isEmpty()) { return true; } + if (nameIsEqualTo.contains(metricName)) { + return true; + } for (String name : nameIsEqualTo) { // The following ignores suffixes like _total. // "request_count" and "request_count_total" both match a metric named "request_count". @@ -58,6 +62,9 @@ private boolean matchesNameNotEqualTo(String metricName) { if (nameIsNotEqualTo.isEmpty()) { return false; } + if (nameIsNotEqualTo.contains(metricName)) { + return true; + } for (String name : nameIsNotEqualTo) { // The following ignores suffixes like _total. // "request_count" and "request_count_total" both match a metric named "request_count".