Polish DNS SRV lookup method with port

Issue gh-9030

remoting/src/main/java/org/springframework/security/remoting/dns/JndiDnsResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2016 the original author or authors.
+ * Copyright 2009-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -36,6 +36,7 @@ import javax.naming.directory.InitialDirContext;
  * <b>com.sun.jndi.dns.DnsContextFactory</b>
  *
  * @author Mike Wiesner
+ * @author Kathryn Newbould
  * @since 3.0
  * @see DnsResolver
  * @see InitialContextFactory
@@ -44,6 +45,14 @@ public class JndiDnsResolver implements DnsResolver {
 
 	private InitialContextFactory ctxFactory = new DefaultInitialContextFactory();
 
+	private static final int SERVICE_RECORD_PRIORITY_INDEX = 0;
+
+	private static final int SERVICE_RECORD_WEIGHT_INDEX = 1;
+
+	private static final int SERVICE_RECORD_PORT_INDEX = 2;
+
+	private static final int SERVICE_RECORD_TARGET_INDEX = 3;
+
 	/**
 	 * Allows to inject an own JNDI context factory.
 	 * @param ctxFactory factory to use, when a DirContext is needed
@@ -72,10 +81,16 @@ public class JndiDnsResolver implements DnsResolver {
 	}
 
 	/**
-	 * @author Kathryn Newbould
+	 * Resolves the host name for the specified service and then the IP Address and port
-	 * @since 5.4.1
+	 * for this host in one call.
-	 * @return String of ip address and port, format [ip_address]:[port] of service if found
+	 * @param serviceType The service type you are searching for, e.g. ldap, kerberos, ...
-	 * @throws DnsLookupException if not found
+	 * @param domain The domain, in which you are searching for the service
+	 * @return IP address and port of the service, formatted [ip_address]:[port]
+	 * @throws DnsEntryNotFoundException No record found
+	 * @throws DnsLookupException Unknown DNS error
+	 * @since 5.6
+	 * @see #resolveServiceEntry(String, String)
+	 * @see #resolveServiceIpAddress(String, String)
 	 */
 	public String resolveServiceIpAddressAndPort(String serviceType, String domain) {
 		DirContext ctx = this.ctxFactory.getCtx();
@@ -101,7 +116,7 @@ public class JndiDnsResolver implements DnsResolver {
 	// This method is needed, so that we can use only one DirContext for
 	// resolveServiceIpAddress().
 	private ConnectionInfo resolveServiceEntry(String serviceType, String domain, DirContext ctx) {
-		String result = null;
+		String target = null;
 		String port = null;
 		try {
 			String query = new StringBuilder("_").append(serviceType).append("._tcp.").append(domain).toString();
@@ -117,20 +132,19 @@ public class JndiDnsResolver implements DnsResolver {
 					throw new DnsLookupException(
 							"Wrong service record for query " + query + ": [" + Arrays.toString(record) + "]");
 				}
-				int priority = Integer.parseInt(record[0]);
+				int priority = Integer.parseInt(record[SERVICE_RECORD_PRIORITY_INDEX]);
-				int weight = Integer.parseInt(record[1]);
+				int weight = Integer.parseInt(record[SERVICE_RECORD_WEIGHT_INDEX]);
 				// we have a new highest Priority, so forget also the highest weight
-				int SERVICE_RECORD_PORT_INDEX = 2;
 				if (priority < highestPriority || highestPriority == -1) {
 					highestPriority = priority;
 					highestWeight = weight;
-					result = record[3].trim();
+					target = record[SERVICE_RECORD_TARGET_INDEX].trim();
 					port = record[SERVICE_RECORD_PORT_INDEX].trim();
 				}
 				// same priority, but higher weight
 				if (priority == highestPriority && weight > highestWeight) {
 					highestWeight = weight;
-					result = record[3].trim();
+					target = record[SERVICE_RECORD_TARGET_INDEX].trim();
 					port = record[SERVICE_RECORD_PORT_INDEX].trim();
 				}
 			}
@@ -139,10 +153,10 @@ public class JndiDnsResolver implements DnsResolver {
 			throw new DnsLookupException("DNS lookup failed for service " + serviceType + " at " + domain, ex);
 		}
 		// remove the "." at the end
-		if (result.endsWith(".")) {
+		if (target.endsWith(".")) {
-			result = result.substring(0, result.length() - 1);
+			target = target.substring(0, target.length() - 1);
 		}
-		return new ConnectionInfo(result, port);
+		return new ConnectionInfo(target, port);
 	}
 
 	private Attribute lookup(String query, DirContext ictx, String recordType) {
@@ -176,21 +190,24 @@ public class JndiDnsResolver implements DnsResolver {
 	}
 
 	private static class ConnectionInfo {
+
 		private final String hostName;
+
 		private final String port;
 
-		public ConnectionInfo(String hostName, String port) {
+		ConnectionInfo(String hostName, String port) {
 			this.hostName = hostName;
 			this.port = port;
 		}
 
-		public String getHostName() {
+		String getHostName() {
-			return hostName;
+			return this.hostName;
 		}
 
-		public String getPort() {
+		String getPort() {
-			return port;
+			return this.port;
 		}
+
 	}
 
 }

remoting/src/test/java/org/springframework/security/remoting/dns/JndiDnsResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2016 the original author or authors.
+ * Copyright 2009-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -96,13 +96,13 @@ public class JndiDnsResolverTests {
 	}
 
 	@Test
-	public void resolveServiceIpAddressWithPort() throws Exception {
+	public void resolveServiceIpAddressAndPortWhenExistsThenReturnsIpAddressAndPort() throws Exception {
 		BasicAttributes srvRecords = createSrvRecords();
 		BasicAttributes aRecords = new BasicAttributes("A", "63.246.7.80");
 		given(this.context.getAttributes("_ldap._tcp.springsource.com", new String[] { "SRV" })).willReturn(srvRecords);
 		given(this.context.getAttributes("kdc.springsource.com", new String[] { "A" })).willReturn(aRecords);
-		String ipAddress = this.dnsResolver.resolveServiceIpAddressAndPort("ldap", "springsource.com");
+		String ipAddressAndPort = this.dnsResolver.resolveServiceIpAddressAndPort("ldap", "springsource.com");
-		assertThat(ipAddress).isEqualTo("63.246.7.80:389");
+		assertThat(ipAddressAndPort).isEqualTo("63.246.7.80:389");
 	}
 
 	@Test