Skip to content

Commit f6d9df5

Browse files
authored
fix: hide HTTP scrape error details (#2332)
## Summary - return a generic HTTP 500 body instead of exposing exception details - include a safe hint explaining how to enable better diagnostics - avoid adding server-side logging by default - let users explicitly opt into either detailed HTTP responses or server-side error reporting - verify the secure default does not expose the exception type or message This is the focused replacement for the #2283 portion of #2297. Fixes #2283 ## Implementation plan 1. Make `HttpErrorHandlingPolicy` builder-based and pass the built policy through the `HTTPServer` builder to the exchange adapter. Response verbosity and error reporting remain orthogonal choices. 2. Make the default policy return a generic HTTP 500 response with a short hint to configure server-side error reporting for diagnostic details. The default must not add a new log entry for the scrape exception. 3. Let callers configure either axis independently: - attach a caller-supplied error reporter while keeping the generic response, so applications and Java agents can route diagnostics to an appropriate sink; and - enable an explicitly unsafe debug response mode that includes exception details in the HTTP body. Avoid “legacy” naming; the API and docs must make the disclosure risk clear. 4. Preserve the existing logging behavior for failures where the error response itself cannot be sent or response headers were already committed, because no useful client response remains possible in those paths. 5. Add focused tests for the secure default, diagnostic hint, independent verbosity/reporter configuration, reporter invocation and isolation, reporter failure handling, and the explicitly unsafe debug-response mode. Add user documentation for the available policies and their security tradeoffs, plus the repository’s release-please changelog entry linking to that documentation. ## Alternatives considered - **Unconditional server-side logging:** rejected because it replaces the response disclosure with a new operational regression: one stack trace per failed scrape, with possible log-ingestion cost and application-logging side effects for unshaded integrations. - **Keep the detailed response as the default:** rejected because it does not remediate #2283 unless every user discovers and enables the secure mode. - **Generic response with no diagnostic guidance:** rejected because it leaves operators with a silent, unexplained HTTP 500 and no discoverable path to better diagnostics. - **Hard-code rate limiting or deduplication in the adapter:** deferred in favor of a reporter abstraction. Correct suppression requires bounded state, concurrency handling, distinct-failure classification, and suppressed-count reporting; callers or a later reusable reporter can implement that policy without coupling it to HTTP response handling. - **Ambient debug/verbosity configuration:** rejected in favor of an explicit builder option on `HttpErrorHandlingPolicy`, so re-enabling unsafe debug responses is deliberate and carries a clear security warning. - **Network controls or authentication documentation alone:** rejected as the primary fix. They remain useful defense in depth but should not substitute for a safe default response. ## Validation - `mise run lint:fix` - `mise run build` - `./mvnw test -pl prometheus-metrics-exporter-httpserver -Dcoverage.skip=true -Dcheckstyle.skip=true` ## Release note Release Please will use this override for the generated changelog and GitHub release notes after a squash merge: BEGIN_COMMIT_OVERRIDE fix(httpserver): make scrape error responses secure and configurable Scrape failures now return a generic HTTP 500 response by default. Applications can configure a server-side error reporter or explicitly enable an unsafe debug response containing exception details. See the [HTTPServer scrape error handling documentation](https://github.com/prometheus/client_java/blob/main/docs/content/exporters/httpserver.md#scrape-error-handling). END_COMMIT_OVERRIDE --------- Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
1 parent 63149ae commit f6d9df5

11 files changed

Lines changed: 545 additions & 52 deletions

File tree

docs/apidiffs/current_vs_latest/prometheus-metrics-exporter-httpserver.txt

Lines changed: 20 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/content/exporters/httpserver.md

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,54 @@ or [inetAddress()](</client_java/api/io/prometheus/metrics/exporter/httpserver/H
2626
The default handler can be changed
2727
with [defaultHandler()](</client_java/api/io/prometheus/metrics/exporter/httpserver/HTTPServer.Builder.html#defaultHandler(com.sun.net.httpserver.HttpHandler)>).
2828

29+
## Scrape error handling
30+
31+
By default, scrape failures return a generic HTTP 500 response. Exception details are not
32+
included in the response, and are not logged when the error response can be delivered, because
33+
the server may run inside an application or a Java agent with its own diagnostic pipeline. If the
34+
error response itself cannot be delivered, the transport failure is logged because no useful
35+
client response remains possible.
36+
37+
Configure a reporter to send exception details to an appropriate logging or telemetry sink:
38+
39+
```java
40+
HTTPServer server = HTTPServer.builder()
41+
.port(9400)
42+
.errorHandlingPolicy(
43+
HttpErrorHandlingPolicy.builder()
44+
.errorReporter(error -> logger.log(Level.SEVERE, "Prometheus scrape failed", error))
45+
.build())
46+
.buildAndStart();
47+
```
48+
49+
The reporter runs synchronously on the request thread and may be called concurrently. Reporter
50+
runtime exceptions do not prevent the generic HTTP 500 response from being sent. Rate limiting
51+
or deduplication can be implemented in the reporter when needed.
52+
53+
For applications using Java Util Logging, the synchronous reporter can be enabled explicitly:
54+
55+
```java
56+
HttpErrorHandlingPolicy.builder()
57+
.errorReporter(HttpErrorHandlingPolicy.julReporter())
58+
.build()
59+
```
60+
61+
This logs scrape exceptions at `SEVERE`. It is intentionally opt-in so applications and Java
62+
agents do not receive an implicit logging side effect.
63+
64+
For local debugging, an unsafe response containing the full exception stack trace can be enabled
65+
explicitly:
66+
67+
```java
68+
HttpErrorHandlingPolicy.builder()
69+
.unsafeDebugResponse(true)
70+
.build()
71+
```
72+
73+
This setting is independent of the error reporter, so both can be configured when needed. The
74+
unsafe debug response can disclose application internals and must not be enabled for an endpoint
75+
reachable by untrusted clients.
76+
2977
## Authentication and HTTPS
3078

3179
- [authenticator()](</client_java/api/io/prometheus/metrics/exporter/httpserver/HTTPServer.Builder.html#authenticator(com.sun.net.httpserver.Authenticator)>)

integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/ExporterIT.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,11 @@ void testErrorHandling() throws IOException {
157157
start("error");
158158
Response response = scrape("GET", "");
159159
assertThat(response.status).isEqualTo(500);
160-
assertThat(response.stringBody()).contains("Simulating an error.");
160+
assertErrorResponseBody(response.stringBody());
161+
}
162+
163+
protected void assertErrorResponseBody(String body) {
164+
assertThat(body).contains("Simulating an error.");
161165
}
162166

163167
@Test
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,21 @@
11
package io.prometheus.metrics.it.exporter.test;
22

3+
import static org.assertj.core.api.Assertions.assertThat;
4+
35
import java.io.IOException;
46
import java.net.URISyntaxException;
57

68
class HttpServerIT extends ExporterIT {
79
public HttpServerIT() throws IOException, URISyntaxException {
810
super("exporter-httpserver-sample");
911
}
12+
13+
@Override
14+
protected void assertErrorResponseBody(String body) {
15+
assertThat(body)
16+
.isEqualTo(
17+
"An internal error occurred while scraping metrics. "
18+
+ "Configure an HTTP error reporter for details.\n")
19+
.doesNotContain("Simulating an error.");
20+
}
1021
}

prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/HTTPServer.java

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ private HTTPServer(
6161
@Nullable String authenticatedSubjectAttributeName,
6262
@Nullable HttpHandler defaultHandler,
6363
@Nullable String metricsHandlerPath,
64-
@Nullable Boolean registerHealthHandler) {
64+
@Nullable Boolean registerHealthHandler,
65+
HttpErrorHandlingPolicy errorHandlingPolicy) {
6566
if (httpServer.getAddress() == null) {
6667
throw new IllegalArgumentException("HttpServer hasn't been bound to an address");
6768
}
@@ -85,7 +86,7 @@ private HTTPServer(
8586
}
8687
registerHandler(
8788
metricsPath,
88-
new MetricsHandler(config, registry),
89+
new MetricsHandler(config, registry, errorHandlingPolicy),
8990
authenticator,
9091
authenticatedSubjectAttributeName);
9192
if (registerHealthHandler == null || registerHealthHandler) {
@@ -211,6 +212,7 @@ public static class Builder {
211212
@Nullable private HttpHandler defaultHandler = null;
212213
@Nullable private String metricsHandlerPath = null;
213214
@Nullable private Boolean registerHealthHandler = null;
215+
private HttpErrorHandlingPolicy errorHandlingPolicy = HttpErrorHandlingPolicy.builder().build();
214216

215217
private Builder(PrometheusProperties config) {
216218
this.config = config;
@@ -295,6 +297,20 @@ public Builder registerHealthHandler(boolean registerHealthHandler) {
295297
return this;
296298
}
297299

300+
/**
301+
* Configure how exceptions raised while scraping metrics are reported to the client and
302+
* optionally to a caller-supplied diagnostic sink.
303+
*
304+
* <p>Default is {@code HttpErrorHandlingPolicy.builder().build()}.
305+
*/
306+
public Builder errorHandlingPolicy(HttpErrorHandlingPolicy errorHandlingPolicy) {
307+
if (errorHandlingPolicy == null) {
308+
throw new NullPointerException("errorHandlingPolicy");
309+
}
310+
this.errorHandlingPolicy = errorHandlingPolicy;
311+
return this;
312+
}
313+
298314
/** Build and start the HTTPServer. */
299315
public HTTPServer buildAndStart() throws IOException {
300316
if (registry == null) {
@@ -318,7 +334,8 @@ public HTTPServer buildAndStart() throws IOException {
318334
authenticatedSubjectAttributeName,
319335
defaultHandler,
320336
metricsHandlerPath,
321-
registerHealthHandler);
337+
registerHealthHandler,
338+
errorHandlingPolicy);
322339
}
323340

324341
private InetSocketAddress makeInetSocketAddress() {
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
package io.prometheus.metrics.exporter.httpserver;
2+
3+
import io.prometheus.metrics.annotations.StableApi;
4+
import java.io.PrintWriter;
5+
import java.io.StringWriter;
6+
import java.nio.charset.StandardCharsets;
7+
import java.util.function.Consumer;
8+
import java.util.logging.Level;
9+
import java.util.logging.Logger;
10+
import javax.annotation.Nullable;
11+
12+
/**
13+
* Controls how the {@link HTTPServer} handles exceptions raised while scraping metrics.
14+
*
15+
* <p>The default policy built by {@link #builder()} does not expose exception details and does not
16+
* report the exception. Configure the builder to route diagnostic details to an
17+
* application-appropriate sink.
18+
*/
19+
@StableApi
20+
public final class HttpErrorHandlingPolicy {
21+
22+
private static final Logger logger = Logger.getLogger(HttpErrorHandlingPolicy.class.getName());
23+
24+
private static final byte[] GENERIC_RESPONSE =
25+
("An internal error occurred while scraping metrics. "
26+
+ "Configure an HTTP error reporter for details.\n")
27+
.getBytes(StandardCharsets.UTF_8);
28+
29+
private final boolean unsafeDebugResponse;
30+
@Nullable private final Consumer<? super Exception> errorReporter;
31+
32+
private HttpErrorHandlingPolicy(
33+
boolean unsafeDebugResponse, @Nullable Consumer<? super Exception> errorReporter) {
34+
this.unsafeDebugResponse = unsafeDebugResponse;
35+
this.errorReporter = errorReporter;
36+
}
37+
38+
/**
39+
* Returns a builder for configuring scrape error handling.
40+
*
41+
* <p>The builder defaults to a generic HTTP 500 response with no error reporter. This avoids
42+
* exposing exception details to scrape clients or adding an implicit dependency on an
43+
* application's logging configuration.
44+
*/
45+
public static Builder builder() {
46+
return new Builder();
47+
}
48+
49+
byte[] getErrorResponse(Exception exception) {
50+
if (!unsafeDebugResponse) {
51+
return GENERIC_RESPONSE;
52+
}
53+
StringWriter stringWriter = new StringWriter();
54+
PrintWriter printWriter = new PrintWriter(stringWriter);
55+
printWriter.write("An Exception occurred while scraping metrics: ");
56+
exception.printStackTrace(printWriter);
57+
return stringWriter.toString().getBytes(StandardCharsets.UTF_8);
58+
}
59+
60+
void report(Exception error) {
61+
if (errorReporter != null) {
62+
errorReporter.accept(error);
63+
}
64+
}
65+
66+
boolean hasErrorReporter() {
67+
return errorReporter != null;
68+
}
69+
70+
/**
71+
* Returns a synchronous reporter that logs scrape exceptions at {@link Level#SEVERE} using JUL.
72+
*
73+
* <p>Reporting is opt-in; the default policy does not log scrape exceptions.
74+
*/
75+
public static Consumer<Throwable> julReporter() {
76+
return error -> logger.log(Level.SEVERE, "Prometheus scrape failed", error);
77+
}
78+
79+
/** Builder for {@link HttpErrorHandlingPolicy}. */
80+
public static final class Builder {
81+
82+
private boolean unsafeDebugResponse = false;
83+
@Nullable private Consumer<? super Exception> errorReporter;
84+
85+
private Builder() {}
86+
87+
/**
88+
* Pass scrape exceptions to {@code errorReporter}.
89+
*
90+
* <p>The reporter runs synchronously on the HTTP request thread. It should return promptly and
91+
* must be safe to call concurrently. Runtime exceptions thrown by the reporter are isolated
92+
* from HTTP response handling.
93+
*/
94+
public Builder errorReporter(Consumer<? super Exception> errorReporter) {
95+
if (errorReporter == null) {
96+
throw new NullPointerException("errorReporter");
97+
}
98+
this.errorReporter = errorReporter;
99+
return this;
100+
}
101+
102+
/**
103+
* Configure whether the HTTP 500 response includes the full exception stack trace.
104+
*
105+
* <p><strong>Security warning:</strong> Setting this to {@code true} exposes internal exception
106+
* information to scrape clients. Do not enable it for endpoints reachable by untrusted clients.
107+
*
108+
* <p>This setting is independent of {@link #errorReporter(Consumer)}.
109+
*/
110+
public Builder unsafeDebugResponse(boolean unsafeDebugResponse) {
111+
this.unsafeDebugResponse = unsafeDebugResponse;
112+
return this;
113+
}
114+
115+
/** Build the policy. */
116+
public HttpErrorHandlingPolicy build() {
117+
return new HttpErrorHandlingPolicy(unsafeDebugResponse, errorReporter);
118+
}
119+
}
120+
}

0 commit comments

Comments
 (0)