Copilot commented on code in PR #13245:
URL: https://github.com/apache/cloudstack/pull/13245#discussion_r4441967838
##########
plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterServerImpl.java:
##########
@@ -47,11 +51,21 @@ private final class ExporterHandler implements HttpHandler {
@Override
public void handle(final HttpExchange httpExchange) throws IOException
{
- final String remoteClientAddress =
httpExchange.getRemoteAddress().getAddress().toString().replace("/", "");
+ final String remoteClientAddress =
httpExchange.getRemoteAddress().getAddress().getHostAddress();
logger.debug("Prometheus exporter received client request from: "
+ remoteClientAddress);
String response = "Forbidden";
int responseCode = 403;
- if
(Arrays.asList(PrometheusExporterAllowedAddresses.value().split(",")).contains(remoteClientAddress))
{
+ final List<String> allowedAddresses =
Arrays.stream(PrometheusExporterAllowedAddresses.value().split(","))
+ .map(addr -> {
+ try {
+ return
InetAddress.getByName(addr.trim()).getHostAddress();
+ } catch (UnknownHostException e) {
+ logger.warn("Invalid IP address in
prometheus.exporter.allowed.ips: " + addr.trim());
+ return addr.trim();
+ }
+ })
+ .collect(Collectors.toList());
Review Comment:
`InetAddress.getByName("")` resolves to the loopback address on the JDK, so
an empty allowlist entry (e.g., an empty config value, a trailing comma, or
`",,"`) would unexpectedly normalize to `127.0.0.1`/`::1` and potentially widen
access. Filter out blank entries before attempting resolution (e.g., trim then
`filter(s -> !s.isEmpty())`) so empty tokens are ignored rather than treated as
localhost.
##########
plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterServerImpl.java:
##########
@@ -47,11 +51,21 @@ private final class ExporterHandler implements HttpHandler {
@Override
public void handle(final HttpExchange httpExchange) throws IOException
{
- final String remoteClientAddress =
httpExchange.getRemoteAddress().getAddress().toString().replace("/", "");
+ final String remoteClientAddress =
httpExchange.getRemoteAddress().getAddress().getHostAddress();
logger.debug("Prometheus exporter received client request from: "
+ remoteClientAddress);
String response = "Forbidden";
int responseCode = 403;
- if
(Arrays.asList(PrometheusExporterAllowedAddresses.value().split(",")).contains(remoteClientAddress))
{
+ final List<String> allowedAddresses =
Arrays.stream(PrometheusExporterAllowedAddresses.value().split(","))
+ .map(addr -> {
+ try {
+ return
InetAddress.getByName(addr.trim()).getHostAddress();
+ } catch (UnknownHostException e) {
+ logger.warn("Invalid IP address in
prometheus.exporter.allowed.ips: " + addr.trim());
+ return addr.trim();
+ }
+ })
+ .collect(Collectors.toList());
+ if (allowedAddresses.contains(remoteClientAddress)) {
Review Comment:
The allowlist is parsed and normalized on every HTTP request; additionally,
`InetAddress.getByName(...)` may trigger DNS resolution if a non-literal
hostname is present, adding latency and potentially blocking the handler
thread. Prefer normalizing once (e.g., cache a normalized `Set<String>` and
refresh only when the config changes) and avoid DNS resolution by strictly
parsing numeric IP literals (and rejecting/ignoring non-literals) if the config
is intended to be “IPs only.”
##########
plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterServerImpl.java:
##########
@@ -47,11 +51,21 @@ private final class ExporterHandler implements HttpHandler {
@Override
public void handle(final HttpExchange httpExchange) throws IOException
{
- final String remoteClientAddress =
httpExchange.getRemoteAddress().getAddress().toString().replace("/", "");
+ final String remoteClientAddress =
httpExchange.getRemoteAddress().getAddress().getHostAddress();
logger.debug("Prometheus exporter received client request from: "
+ remoteClientAddress);
String response = "Forbidden";
int responseCode = 403;
- if
(Arrays.asList(PrometheusExporterAllowedAddresses.value().split(",")).contains(remoteClientAddress))
{
+ final List<String> allowedAddresses =
Arrays.stream(PrometheusExporterAllowedAddresses.value().split(","))
+ .map(addr -> {
+ try {
+ return
InetAddress.getByName(addr.trim()).getHostAddress();
+ } catch (UnknownHostException e) {
+ logger.warn("Invalid IP address in
prometheus.exporter.allowed.ips: " + addr.trim());
+ return addr.trim();
Review Comment:
The warning drops the exception context, which makes troubleshooting harder
(e.g., distinguishing parse failures from resolution failures). Include the
exception (and consider quoting the value) so logs show the underlying cause,
e.g., log the trimmed token and pass `e` as the throwable argument.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
