You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2026/08/07 09:58:09 UTC
(cloudstack) 02/02: fixes
This is an automated email from the ASF dual-hosted git repository.
DaanHoogland pushed a commit to branch dnsProviderUrlValidate
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit c0b5e8963e9d55f590dfd97964aa37f422f7d1be
Author: Daan Hoogland <da...@apache.org>
AuthorDate: Fri Aug 7 11:38:57 2026 +0200
fixes
---
.../cloudstack/dns/DnsProviderManagerImpl.java | 33 ++++++-------
.../cloudstack/dns/DnsProviderManagerImplTest.java | 55 +++++++++++++++++++---
2 files changed, 64 insertions(+), 24 deletions(-)
diff --git a/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java b/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java
index 3718967ba5a..f8a0aa5dd84 100644
--- a/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java
+++ b/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java
@@ -164,32 +164,28 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa
}
/**
- * Rejects DNS provider URLs that resolve to an illegal address (per {@link UriUtils#validateUrl(String)},
- * currently any-local/link-local/loopback/multicast; RFC1918 site-local coverage follows once #271/#277
- * lands) before any provider client is given the chance to connect to it. A scheme is assumed to be
- * `http` when the caller omits one, matching how DNS provider clients (e.g. PowerDnsClient) already
- * tolerate bare host/IP values.
+ * 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;
}
- String urlToValidate = url.trim();
- if (!urlToValidate.startsWith("http://") && !urlToValidate.startsWith("https://")) {
- urlToValidate = "http://" + urlToValidate;
- }
- UriUtils.validateUrl(urlToValidate);
+ UriUtils.validateUrl(url);
}
@Override
@ActionEvent(eventType = EventTypes.EVENT_DNS_SERVER_ADD, eventDescription = "Adding a DNS Server")
public DnsServer addDnsServer(AddDnsServerCmd cmd) {
- validateDnsServerUrl(cmd.getUrl());
+ String url = StringUtils.trim(cmd.getUrl());
+ validateDnsServerUrl(url);
Account caller = CallContext.current().getCallingAccount();
- DnsServer existing = dnsServerDao.findByUrlAndAccount(cmd.getUrl(), caller.getId());
+ DnsServer existing = dnsServerDao.findByUrlAndAccount(url, caller.getId());
if (existing != null) {
throw new InvalidParameterValueException(
- "This Account already has a DNS server integration for URL: " + cmd.getUrl());
+ "This Account already has a DNS server integration for URL: " + url);
}
boolean isDnsPublic = cmd.isPublic();
@@ -205,7 +201,7 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa
}
DnsProviderType type = cmd.getProvider();
- DnsServerVO server = new DnsServerVO(cmd.getName(), cmd.getUrl(), cmd.getPort(), type,
+ DnsServerVO server = new DnsServerVO(cmd.getName(), url, cmd.getPort(), type,
cmd.getDnsUserName(), cmd.getDnsApiKey(), isDnsPublic, publicDomainSuffix, cmd.getNameServers(),
caller.getAccountId(), caller.getDomainId());
@@ -271,13 +267,14 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa
}
if (cmd.getUrl() != null) {
- if (!cmd.getUrl().equals(originalUrl)) {
- validateDnsServerUrl(cmd.getUrl());
- DnsServer duplicate = dnsServerDao.findByUrlAndAccount(cmd.getUrl(), dnsServer.getAccountId());
+ String url = StringUtils.trim(cmd.getUrl());
+ if (!url.equals(originalUrl)) {
+ validateDnsServerUrl(url);
+ DnsServer duplicate = dnsServerDao.findByUrlAndAccount(url, dnsServer.getAccountId());
if (duplicate != null && duplicate.getId() != dnsServer.getId()) {
throw new InvalidParameterValueException("Another DNS server with this URL already exists.");
}
- dnsServer.setUrl(cmd.getUrl());
+ dnsServer.setUrl(url);
validationRequired = true;
}
}
diff --git a/server/src/test/java/org/apache/cloudstack/dns/DnsProviderManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/dns/DnsProviderManagerImplTest.java
index ec239239abd..94efacad285 100644
--- a/server/src/test/java/org/apache/cloudstack/dns/DnsProviderManagerImplTest.java
+++ b/server/src/test/java/org/apache/cloudstack/dns/DnsProviderManagerImplTest.java
@@ -718,7 +718,7 @@ public class DnsProviderManagerImplTest {
org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd cmd = mock(
org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd.class);
when(accountMgr.isRootAdmin(callerMock.getId())).thenReturn(true);
- when(cmd.getUrl()).thenReturn("http://93.184.216.34:8081");
+ when(cmd.getUrl()).thenReturn("http://192.0.2.1:8081");
when(cmd.getProvider()).thenReturn(DnsProviderType.PowerDNS);
when(dnsServerDao.findByUrlAndAccount(anyString(), anyLong())).thenReturn(null);
when(dnsProviderMock.validateAndResolveServer(any())).thenReturn("resolved-id");
@@ -781,11 +781,28 @@ public class DnsProviderManagerImplTest {
public void testAddDnsServerAlreadyExists() {
org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd cmd = mock(
org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd.class);
- when(cmd.getUrl()).thenReturn("http://93.184.216.34:8081");
+ when(cmd.getUrl()).thenReturn("http://192.0.2.1:8081");
when(dnsServerDao.findByUrlAndAccount(anyString(), anyLong())).thenReturn(serverVO);
manager.addDnsServer(cmd);
}
+ @Test
+ public void testAddDnsServerTrimsUrlBeforeDuplicateCheckAndPersistence() throws Exception {
+ org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd cmd = mock(
+ org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd.class);
+ when(accountMgr.isRootAdmin(callerMock.getId())).thenReturn(true);
+ when(cmd.getUrl()).thenReturn(" http://192.0.2.1:8081 ");
+ when(cmd.getProvider()).thenReturn(DnsProviderType.PowerDNS);
+ when(dnsServerDao.findByUrlAndAccount(anyString(), anyLong())).thenReturn(null);
+ when(dnsProviderMock.validateAndResolveServer(any())).thenReturn("resolved-id");
+ when(dnsServerDao.persist(any())).thenReturn(serverVO);
+
+ manager.addDnsServer(cmd);
+
+ verify(dnsServerDao).findByUrlAndAccount(eq("http://192.0.2.1:8081"), anyLong());
+ verify(dnsServerDao).persist(Mockito.argThat(s -> "http://192.0.2.1:8081".equals(((DnsServerVO) s).getUrl())));
+ }
+
@Test(expected = IllegalArgumentException.class)
public void testAddDnsServerRejectsLoopbackUrl() {
org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd cmd = mock(
@@ -794,13 +811,21 @@ public class DnsProviderManagerImplTest {
manager.addDnsServer(cmd);
}
+ @Test(expected = IllegalArgumentException.class)
+ public void testAddDnsServerRejectsUrlWithoutScheme() {
+ org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd cmd = mock(
+ org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd.class);
+ when(cmd.getUrl()).thenReturn("192.0.2.1:8081");
+ manager.addDnsServer(cmd);
+ }
+
@Test
public void testAddDnsServerNormalUser() throws Exception {
org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd cmd = mock(
org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd.class);
when(accountMgr.isRootAdmin(callerMock.getId())).thenReturn(false);
when(accountMgr.isDomainAdmin(callerMock.getId())).thenReturn(false);
- when(cmd.getUrl()).thenReturn("http://93.184.216.34:8081");
+ when(cmd.getUrl()).thenReturn("http://192.0.2.1:8081");
when(cmd.getProvider()).thenReturn(DnsProviderType.PowerDNS);
when(cmd.getNameServers()).thenReturn(Collections.emptyList());
when(cmd.isPublic()).thenReturn(true);
@@ -819,7 +844,7 @@ public class DnsProviderManagerImplTest {
org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd cmd = mock(
org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd.class);
when(accountMgr.isRootAdmin(callerMock.getId())).thenReturn(true);
- when(cmd.getUrl()).thenReturn("http://93.184.216.34:8081");
+ when(cmd.getUrl()).thenReturn("http://192.0.2.1:8081");
when(cmd.getProvider()).thenReturn(DnsProviderType.PowerDNS);
when(cmd.getNameServers()).thenReturn(Collections.emptyList());
when(dnsServerDao.findByUrlAndAccount(anyString(), anyLong())).thenReturn(null);
@@ -832,7 +857,7 @@ public class DnsProviderManagerImplTest {
org.apache.cloudstack.api.command.user.dns.UpdateDnsServerCmd cmd = mock(
org.apache.cloudstack.api.command.user.dns.UpdateDnsServerCmd.class);
when(cmd.getId()).thenReturn(SERVER_ID);
- when(cmd.getUrl()).thenReturn("http://93.184.216.34:8081");
+ when(cmd.getUrl()).thenReturn("http://192.0.2.1:8081");
DnsServerVO existingServer = mock(DnsServerVO.class);
when(existingServer.getId()).thenReturn(SERVER_ID + 1); // Different ID implies duplicate
@@ -855,12 +880,30 @@ public class DnsProviderManagerImplTest {
manager.updateDnsServer(cmd);
}
+ @Test
+ public void testUpdateDnsServerTreatsWhitespaceOnlyUrlChangeAsUnchanged() throws Exception {
+ org.apache.cloudstack.api.command.user.dns.UpdateDnsServerCmd cmd = mock(
+ org.apache.cloudstack.api.command.user.dns.UpdateDnsServerCmd.class);
+ Integer unchangedPort = serverVO.getPort();
+ when(cmd.getId()).thenReturn(SERVER_ID);
+ when(cmd.getUrl()).thenReturn(" http://192.0.2.1:8081 ");
+ when(cmd.getPort()).thenReturn(unchangedPort);
+ when(dnsServerDao.findById(SERVER_ID)).thenReturn(serverVO);
+ Mockito.doReturn("http://192.0.2.1:8081").when(serverVO).getUrl();
+ when(dnsServerDao.update(anyLong(), any())).thenReturn(true);
+
+ DnsServer result = manager.updateDnsServer(cmd);
+ assertNotNull(result);
+ verify(dnsProviderMock, never()).validate(any());
+ verify(serverVO, never()).setUrl(anyString());
+ }
+
@Test
public void testUpdateDnsServerUrlValid() throws Exception {
org.apache.cloudstack.api.command.user.dns.UpdateDnsServerCmd cmd = mock(
org.apache.cloudstack.api.command.user.dns.UpdateDnsServerCmd.class);
when(cmd.getId()).thenReturn(SERVER_ID);
- when(cmd.getUrl()).thenReturn("http://93.184.216.34:8081");
+ when(cmd.getUrl()).thenReturn("http://192.0.2.1:8081");
when(dnsServerDao.findById(SERVER_ID)).thenReturn(serverVO);
Mockito.doReturn("http://original:8081").when(serverVO).getUrl();