Copilot commented on code in PR #13821:
URL: https://github.com/apache/cloudstack/pull/13821#discussion_r3735071718
##########
server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java:
##########
@@ -162,14 +163,29 @@ private DnsProvider getProviderByType(DnsProviderType
type) {
throw new CloudRuntimeException("No plugin found for DNS provider
type: " + type);
}
+ /**
+ * Rejects a DNS provider URL that resolves to an illegal address before
any provider client is given
+ * the chance to connect to it. See {@link UriUtils#validateUrl(String)}
for the exact rules enforced
+ * (including the requirement that the URL declares an {@code http}/{@code
https} scheme).
+ * Expects {@code url} to already be trimmed.
+ */
+ private void validateDnsServerUrl(String url) {
+ if (StringUtils.isBlank(url)) {
+ return;
+ }
+ UriUtils.validateUrl(url);
+ }
Review Comment:
`validateDnsServerUrl` currently returns early for blank/empty values, which
allows `null`/"" URLs to pass through to the duplicate check and persistence.
Also, the Javadoc claims only `http/https` are allowed, but
`UriUtils.validateUrl` accepts `file` URLs (e.g. `file://192.0.2.1/...`) which
is unlikely to be a valid DNS provider endpoint and can open up unintended
access paths. Consider rejecting blank URLs and explicitly disallowing `file:`
before delegating to `UriUtils.validateUrl`.
--
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]
