fix: harden metric scrape edge cases

[zeitlinger]

Summary

This addresses the bug reports from #2282, #2283, #2284, #2285, #2286, and #2287 in one PR.

• Fix Buffer stripe indexing so large thread IDs cannot produce negative stripe indexes.
• Bound the Buffer spin wait during collection and replay buffered observations before failing the scrape on timeout.
• Return a generic HTTPServer error response instead of exposing stack traces to clients.
• Avoid blocking the HTTPServer dispatcher on executor saturation by using a bounded queue with the default rejection policy.
• Close rejected authenticated request bodies instead of draining them without a size limit.
• Bound default query parsing by query length and parameter count, and return HTTP 400 for invalid query parameters.
• De-duplicate exact metric-name filters for faster lookup.
• Redact invalid configuration values from PrometheusPropertiesException messages.

Fixes #2282
Fixes #2283
Fixes #2284
Fixes #2285
Fixes #2286
Fixes #2287

Validation

• mise run lint:fix
• ./mvnw -pl prometheus-metrics-core,prometheus-metrics-config,prometheus-metrics-exporter-common,prometheus-metrics-exporter-httpserver,prometheus-metrics-model test -Dcoverage.skip=true -Dcheckstyle.skip=true
• mise run build

[github-actions]

⚠️ API changes detected — maintainer review required

This PR modifies the published API diff for the following module(s):

• prometheus-metrics-core
• prometheus-metrics-exporter-common
• prometheus-metrics-exporter-httpserver

Please review the changes in docs/apidiffs/current_vs_latest/ carefully before approving.

[Copilot]

Pull request overview

This PR hardens the metrics scrape path across core buffering, query parsing/filtering, and the standalone HTTPServer exporter to address multiple reported scrape-triggered crashes/DoS vectors and information disclosure.

Changes:

• Make Buffer thread striping overflow-safe and bound spin-wait during collection (replaying buffered observations before failing).
• Stop exposing stack traces to HTTP clients; return a generic 500 error body while logging server-side.
• Bound query parsing (length + parameter count) and de-duplicate metric-name filters for more predictable scrape cost.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MetricNameFilter.java	De-duplicates filter inputs and adds fast-path contains() checks.
prometheus-metrics-exporter-httpserver/src/test/java/io/prometheus/metrics/exporter/httpserver/HTTPServerTest.java	Updates tests for new default executor + generic error responses.
prometheus-metrics-exporter-httpserver/src/test/java/io/prometheus/metrics/exporter/httpserver/HttpExchangeAdapterTest.java	Verifies generic 500 body and absence of stack traces.
prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/HTTPServer.java	Switches default executor to bounded queue and changes unauthorized-body handling.
prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/HttpExchangeAdapter.java	Replaces stack-trace responses with a generic error message + server-side logging.
prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/BlockingRejectedExecutionHandler.java	Removes the blocking rejection handler to avoid dispatcher thread stalls.
prometheus-metrics-exporter-common/src/test/java/io/prometheus/metrics/exporter/common/PrometheusScrapeHandlerTest.java	Adds tests for rejecting overly-long / overly-many query parameters.
prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/PrometheusScrapeHandler.java	Catches invalid query parameter parsing and returns HTTP 400.
prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/PrometheusHttpRequest.java	Implements bounded query parsing for getParameterValues().
prometheus-metrics-exporter-common/src/main/java/io/prometheus/metrics/exporter/common/InvalidQueryParameterException.java	Introduces an internal exception for invalid query parsing.
prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/BufferTest.java	Adds regression tests for stripe indexing and timeout/replay behavior.
prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Buffer.java	Uses floorMod for striping; adds spin-wait timeout + replay-on-timeout.
prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/UtilTest.java	Updates tests for redacted/escaped invalid-value error messages.
prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/OpenMetrics2PropertiesTest.java	Updates expected messages to quote/escape invalid values.
prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/ExporterPushgatewayPropertiesTest.java	Updates expected messages to quote/escape invalid values.
prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/ExporterPropertiesTest.java	Updates expected messages to quote/escape invalid values.
prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/Util.java	Centralizes invalid-value message formatting with escaping + truncation.
prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/ExporterPushgatewayProperties.java	Uses Util.invalidValueMessage(...) to avoid leaking raw config values.
integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/HttpServerIT.java	Validates generic HTTPServer error body in integration tests.
integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/ExporterIT.java	Factors error-body assertions behind an overridable hook.
docs/apidiffs/current_vs_latest/prometheus-metrics-exporter-httpserver.txt	Records API-diff output for exporter-httpserver changes.
docs/apidiffs/current_vs_latest/prometheus-metrics-exporter-common.txt	Records API-diff output for exporter-common changes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jaydeluca]

Review of the scrape-hardening changes (#2282–#2287). A few questions/suggestions inline — two worth a look (the new throw reaching the observe() path, and escape-vs-redact for config values); the rest are minor/optional.

[jaydeluca]

Throwing here is the intended behavior on the scrape path (Histogram.collect → buffer.run), but run() is also invoked from maybeResetOrScaleDown() / maybeScaleDown() (Histogram.java:439, :470), which sit on the application recording path: observe → doObserve → maybeResetOrScaleDown → buffer.run. Before this change observe() could never throw from here. Under sustained contention or a GC pause > 1 s where in-flight fast-path observers don't settle before the deadline, a native-histogram observe() will now throw an undocumented IllegalStateException into user code — a new failure mode close to the crash class #2282 fixes.

On the internal reset/scale-down path, should we log and defer (skip this cycle's scale-down) instead of throwing, and then only surface the timeout on the scrape/collect() path?

[zeitlinger]

Fixed in ec7f83d. Histogram.collect still calls run with timeout failure enabled, while the reset/scale-down paths call it with failOnTimeout=false. A timeout during maintenance now deactivates and drains the generation, then skips that maintenance cycle instead of throwing into observe(). I also added regression coverage for the timeout/replay behavior.

[jaydeluca]

Escaping control chars (log-injection / CWE-117 defense) and the 100-char cap are good, but the raw value stays in the message. The client-facing leak via HttpExchangeAdapter is closed by the #2283 fix; however the servlet adapters (servlet-jakarta / javax) re-throw to the container, so up to 100 chars of the offending config value can still land in a container error page.

[zeitlinger]

Fixed in ec7f83d. Invalid configuration messages now include only the property key and expected format; the offending raw value is no longer included. This is also applied to the PushGateway credential-related paths, so servlet/container error propagation cannot expose the configured value.

[jaydeluca]

observationBuffer still grows unbounded; the cap is now only implicit via the 1 s active window. Reasonable, but a hard MAX_BUFFER_SIZE with drop-on-overflow would make the bound explicit and independent of the timeout value.

[zeitlinger]

Addressed in ec7f83d. The observation generation now has an explicit DEFAULT_MAX_BUFFER_SIZE of 1,000,000, independent of the timeout. Appends wait for space while the generation is active and stop when it is deactivated; I chose bounded blocking rather than drop-on-overflow so recording does not silently lose observations.

[jaydeluca]

The spin-wait timeout covers this first loop, but the second wait loop below (while (bufferPos < expectedBufferSize) bufferFilled.await(), ~line 137) has no timeout: a thread stalled between the stripe increment in append() and doAppend() can still block run() there indefinitely. Narrow window, but the "hang forever" class isn't fully eliminated — consider a bounded await there too.

[zeitlinger]

Fixed in ec7f83d. The buffer handoff is now coordinated through generation/phase transitions, so the second unbounded bufferFilled.await() is gone. Appenders that straddle activation/deactivation are accounted for, and there is regression coverage for a stalled appender.

[jaydeluca]

The 1 s deadline is only reachable via this package-private constructor (tests). Should we expose a knob for this?

[zeitlinger]

The deadline constructor remains package-private and test-only; there is no public configuration knob. The production constructor continues to use the default one-second deadline.

[jaydeluca]

This reverses the prior "avoid logging in Java-agent mode" behavior — correct, since the client no longer gets details. But a persistent misconfiguration will emit a SEVERE line on every scrape. Maybe we should add rate-limiting/deduping the log to avoid flooding logs.

[zeitlinger]

Agreed. I left rate limiting/deduplication as a follow-up rather than expanding this change's scope. This PR removes exception details from the client response and logs the exception server-side, but a persistent misconfiguration can still produce one SEVERE entry per failing scrape. I am leaving this thread unresolved.

[zeitlinger]

Closing this combined PR because its long review history makes the individual fixes difficult to evaluate. I’ll replace it with focused PRs for the separate reports, and each PR will call out any discussion that is still ongoing.

[zeitlinger]

Replacement draft PRs:

• fix: prevent buffer stripe index overflow #2331 — Security Report: Deterministic ArrayIndexOutOfBoundsException via Math.abs(Integer.MIN_VALUE) in prometheus/client_java Buffer Thread Striping #2282: buffer stripe-index overflow
• fix: hide HTTP scrape error details #2332 — Security Report: Full Stack Trace Disclosure in HTTP Error Responses #2283: HTTP scrape error disclosure
• fix: bound HTTPServer request resources #2333 — # Security Report: Thread Pool Exhaustion via Slow Clients (Slowloris-style DoS) in prometheus/client_java HTTPServer #2284: HTTPServer resource bounds
• fix: bound scrape query parameters #2334 — # Security Report: Denial of Service via Unbounded name[] Query Parameter Flooding in prometheus/client_java #2285: scrape query parameter bounds
• fix: redact invalid configuration values #2335 — # Security Report: Sensitive Configuration Values Leaked via PrometheusPropertiesException to HTTP Clients #2286: configuration-value redaction
• fix: bound observation buffering during collection #2336 — # Security Report: Unbounded Observation Buffer Growth During Stalled Scrape in prometheus/client_java #2287: observation-buffer bounds (stacked on fix: prevent buffer stripe index overflow #2331)

Each PR has an Ongoing discussion section that records the remaining decisions from this PR's review history.