Use lookupHost = false by default for TCP & UDP (#3825)

* Use `lookupHost = false` by default for TCP & UDP

The applications these days more and more are deployed and managed in the containers
where DNS is not configured by default.
Having `lookupHost = true` by default leads to a bad experience when some delays happen
for reverse host lookups.

* Use `lookupHost = false` by default for both TCP & UDP to have a reliable behavior
independently of the environment.
The `hostName` is used for `connectionId` and as a header in the message -
semantically it doesn't matter for the application logic what value is present over there.

* * Fix language in docs

Co-authored-by: Gary Russell <[email protected]>

Co-authored-by: Gary Russell <[email protected]>

Author: artembilan

spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractConnectionFactory.java

@@ -119,7 +119,7 @@ public abstract class AbstractConnectionFactory extends IntegrationObjectSupport
 
 	private TcpConnectionInterceptorFactoryChain interceptorFactoryChain;
 
-	private boolean lookupHost = true;
+	private boolean lookupHost;
 
 	private TcpSocketSupport tcpSocketSupport = new DefaultTcpSocketSupport();
 
@@ -456,7 +456,8 @@ public void setInterceptorFactoryChain(TcpConnectionInterceptorFactoryChain inte
 
 	/**
 	 * If true, DNS reverse lookup is done on the remote ip address.
-	 * Default true.
+	 * Default false: not all environments (e.g. Docker containers) perform reliable DNS
+	 * resolution.
 	 * @param lookupHost the lookupHost to set
 	 */
 	public void setLookupHost(boolean lookupHost) {

spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/DatagramPacketMessageMapper.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2021 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -21,6 +21,7 @@
 import java.net.DatagramPacket;
 import java.nio.Buffer;
 import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
 import java.util.Map;
 import java.util.UUID;
 import java.util.regex.Matcher;
@@ -71,26 +72,26 @@
 public class DatagramPacketMessageMapper implements InboundMessageMapper<DatagramPacket>,
 		OutboundMessageMapper<DatagramPacket>, BeanFactoryAware {
 
-	private volatile String charset = "UTF-8";
+	private static final Pattern UDP_HEADERS_PATTERN =
+			Pattern.compile(RegexUtils.escapeRegexSpecials(IpHeaders.ACK_ADDRESS) +
+					"=" + "([^;]*);" +
+					RegexUtils.escapeRegexSpecials(MessageHeaders.ID) +
+					"=" + "([^;]*);");
+
+	private String charset = StandardCharsets.UTF_8.name();
 
-	private boolean acknowledge = false;
+	private boolean acknowledge;
 
 	private String ackAddress;
 
-	private boolean lengthCheck = false;
+	private boolean lengthCheck;
 
-	private boolean lookupHost = true;
+	private boolean lookupHost;
 
 	private volatile MessageBuilderFactory messageBuilderFactory = new DefaultMessageBuilderFactory();
 
 	private volatile boolean messageBuilderFactorySet;
 
-	private static Pattern udpHeadersPattern =
-			Pattern.compile(RegexUtils.escapeRegexSpecials(IpHeaders.ACK_ADDRESS) +
-					"=" + "([^;]*);" +
-					RegexUtils.escapeRegexSpecials(MessageHeaders.ID) +
-					"=" + "([^;]*);");
-
 	private BeanFactory beanFactory;
 
 	public void setCharset(String charset) {
@@ -110,6 +111,9 @@ public void setLengthCheck(boolean lengthCheck) {
 	}
 
 	/**
+	 * If true, DNS reverse lookup is done on the remote ip address.
+	 * Default false: not all environments (e.g. Docker containers) perform reliable DNS
+	 * resolution.
 	 * @param lookupHost the lookupHost to set
 	 */
 	public void setLookupHost(boolean lookupHost) {
@@ -232,20 +236,17 @@ public Message<byte[]> toMessage(DatagramPacket packet, @Nullable Map<String, Ob
 			length -= 4; // NOSONAR magic number
 		}
 		String hostAddress = packet.getAddress().getHostAddress();
-		String hostName;
+		String hostName = hostAddress;
 		if (this.lookupHost) {
 			hostName = packet.getAddress().getHostName();
 		}
-		else {
-			hostName = hostAddress;
-		}
 		int port = packet.getPort();
 		// Peek at the message in case they didn't configure us for ack but the sending
 		// side expects it.
 		if (this.acknowledge || startsWith(buffer, IpHeaders.ACK_ADDRESS)) {
 			try {
 				String headersString = new String(packet.getData(), offset, length, this.charset);
-				Matcher matcher = udpHeadersPattern.matcher(headersString);
+				Matcher matcher = UDP_HEADERS_PATTERN.matcher(headersString);
 				if (matcher.find()) {
 					// Strip off the ack headers and put in Message headers
 					length = length - matcher.end();
@@ -292,8 +293,8 @@ private boolean startsWith(ByteBuffer buffer, String prefix) {
 		try {
 			byte[] comparing;
 			comparing = prefix.getBytes(this.charset);
-			for (int i = 0; i < comparing.length; i++) {
-				if (buffer.get() != comparing[i]) {
+			for (byte b : comparing) {
+				if (buffer.get() != b) {
 					return false;
 				}
 			}

spring-integration-ip/src/main/resources/org/springframework/integration/ip/config/spring-integration-ip.xsd

@@ -663,9 +663,9 @@
 			<xsd:attribute name="lookup-host" type="xsd:string">
 				<xsd:annotation>
 					<xsd:documentation>
-						Whether or not to do a DNS reverse-lookup on the remote ip address to
+						Whether to perform a DNS reverse-lookup on the remote ip address to
 						insert the host name into the
-						message headers (ip_connectionId, ip_hostName). Default "true".
+						message headers (ip_connectionId, ip_hostName). Default "false".
 					</xsd:documentation>
 				</xsd:annotation>
 			</xsd:attribute>
@@ -853,9 +853,9 @@
 				<xsd:attribute name="lookup-host" type="xsd:string">
 					<xsd:annotation>
 						<xsd:documentation>
-							Whether or not to do a DNS reverse-lookup on the remote ip address to
+							Whether to do a DNS reverse-lookup on the remote ip address to
 							insert the host name into the
-							message headers (ip_hostName). Default "true".
+							message headers (ip_hostName). Default "false".
 						</xsd:documentation>
 					</xsd:annotation>
 				</xsd:attribute>

spring-integration-ip/src/test/java/org/springframework/integration/ip/config/ParserUnitTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -21,7 +21,6 @@
 import static org.mockito.Mockito.mock;
 
 import java.net.DatagramSocket;
-import java.net.SocketException;
 import java.time.Duration;
 import java.util.Iterator;
 import java.util.Set;
@@ -249,10 +248,12 @@ public class ParserUnitTests {
 	@Autowired
 	MessageChannel udpAutoChannel;
 
-	@Autowired @Qualifier("tcpAutoChannel.adapter")
+	@Autowired
+	@Qualifier("tcpAutoChannel.adapter")
 	TcpReceivingChannelAdapter tcpAutoAdapter;
 
-	@Autowired @Qualifier("udpAutoChannel.adapter")
+	@Autowired
+	@Qualifier("udpAutoChannel.adapter")
 	UnicastReceivingChannelAdapter udpAutoAdapter;
 
 	@Autowired
@@ -316,7 +317,7 @@ public void testInUdpMulticast() {
 		assertThat(dfa.getPropertyValue("errorChannel")).isNull();
 		DatagramPacketMessageMapper mapper = (DatagramPacketMessageMapper) dfa.getPropertyValue("mapper");
 		DirectFieldAccessor mapperAccessor = new DirectFieldAccessor(mapper);
-		assertThat((Boolean) mapperAccessor.getPropertyValue("lookupHost")).isTrue();
+		assertThat((Boolean) mapperAccessor.getPropertyValue("lookupHost")).isFalse();
 	}
 
 	@Test
@@ -611,10 +612,10 @@ public void testtCPOrder() {
 						TestUtils.getPropertyValue(this.tcpChannel, "dispatcher"),
 						"handlers");
 		Iterator<MessageHandler> iterator = handlers.iterator();
-		assertThat(iterator.next()).isSameAs(this.tcpNewOut2);			//15
-		assertThat(iterator.next()).isSameAs(this.tcpOutboundGateway);	//24
-		assertThat(iterator.next()).isSameAs(this.tcpNewOut1);			//25
-		assertThat(iterator.next()).isSameAs(this.tcpOut);				//35
+		assertThat(iterator.next()).isSameAs(this.tcpNewOut2);            //15
+		assertThat(iterator.next()).isSameAs(this.tcpOutboundGateway);    //24
+		assertThat(iterator.next()).isSameAs(this.tcpNewOut1);            //25
+		assertThat(iterator.next()).isSameAs(this.tcpOut);                //35
 	}
 
 	@Test
@@ -682,6 +683,7 @@ public static class EventSubclass1 extends TcpConnectionEvent {
 		public EventSubclass1(TcpConnectionSupport connection, String connectionFactoryName) {
 			super(connection, connectionFactoryName);
 		}
+
 	}
 
 	@SuppressWarnings("serial")
@@ -690,6 +692,7 @@ public static class EventSubclass2 extends TcpConnectionEvent {
 		public EventSubclass2(TcpConnectionSupport connection, String connectionFactoryName) {
 			super(connection, connectionFactoryName);
 		}
+
 	}
 
 	@Configuration
@@ -708,7 +711,7 @@ AbstractClientConnectionFactory mockClientCf() {
 	public static class SocketCust implements SocketCustomizer {
 
 		@Override
-		public void configure(DatagramSocket socket) throws SocketException {
+		public void configure(DatagramSocket socket) {
 		}
 
 	}

spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/TcpNioConnectionTests.java

@@ -133,8 +133,9 @@ public void testWriteTimeout(TestInfo testInfo) throws Exception {
 			}
 		});
 		assertThat(latch.await(10000, TimeUnit.MILLISECONDS)).isTrue();
-		TcpNioClientConnectionFactory factory = new TcpNioClientConnectionFactory("localhost",
-				serverSocket.get().getLocalPort());
+		TcpNioClientConnectionFactory factory =
+				new TcpNioClientConnectionFactory("localhost", serverSocket.get().getLocalPort());
+		factory.setLookupHost(true);
 		AtomicReference<String> connectionId = new AtomicReference<>();
 		factory.setApplicationEventPublisher(event -> {
 			if (event instanceof TcpConnectionOpenEvent) {

spring-integration-ip/src/test/java/org/springframework/integration/ip/udp/DatagramPacketMessageMapperTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -23,7 +23,7 @@
 import java.net.InetSocketAddress;
 import java.nio.ByteBuffer;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 import org.springframework.integration.ip.IpHeaders;
 import org.springframework.integration.mapping.MessageMappingException;
@@ -33,31 +33,33 @@
 /**
  * @author Gary Russell
  * @author Dave Syer
+ * @author Artem Bilan
+ *
  * @since 2.0
  */
 public class DatagramPacketMessageMapperTests {
 
 	@Test
-	public void testFromToMessageNoAckNoLengthCheck() throws Exception {
+	public void testFromToMessageNoAckNoLengthCheck() {
 		test(false, false);
 	}
 
 	@Test
-	public void testFromToMessageAckNoLengthCheck() throws Exception {
+	public void testFromToMessageAckNoLengthCheck() {
 		test(true, false);
 	}
 
 	@Test
-	public void testFromToMessageNoAckLengthCheck() throws Exception {
+	public void testFromToMessageNoAckLengthCheck() {
 		test(false, true);
 	}
 
 	@Test
-	public void testFromToMessageAckLengthCheck() throws Exception {
+	public void testFromToMessageAckLengthCheck() {
 		test(true, true);
 	}
 
-	private void test(boolean ack, boolean lengthCheck) throws Exception {
+	private void test(boolean ack, boolean lengthCheck) {
 		Message<byte[]> message = MessageBuilder.withPayload("ABCD".getBytes()).build();
 		DatagramPacketMessageMapper mapper = new DatagramPacketMessageMapper();
 		mapper.setAckAddress("localhost:11111");
@@ -71,19 +73,19 @@ private void test(boolean ack, boolean lengthCheck) throws Exception {
 			assertThat(message.getHeaders().getId().toString())
 					.isEqualTo(messageOut.getHeaders().get(IpHeaders.ACK_ID).toString());
 		}
-		assertThat(((String) messageOut.getHeaders().get(IpHeaders.HOSTNAME)).contains("localhost")).isTrue();
-		mapper.setLookupHost(false);
+		assertThat(((String) messageOut.getHeaders().get(IpHeaders.HOSTNAME))).doesNotContain("localhost");
+		mapper.setLookupHost(true);
 		messageOut = mapper.toMessage(packet);
 		assertThat(new String(messageOut.getPayload())).isEqualTo(new String(message.getPayload()));
 		if (ack) {
 			assertThat(message.getHeaders().getId().toString())
 					.isEqualTo(messageOut.getHeaders().get(IpHeaders.ACK_ID).toString());
 		}
-		assertThat(((String) messageOut.getHeaders().get(IpHeaders.HOSTNAME)).contains("localhost")).isFalse();
+		assertThat(((String) messageOut.getHeaders().get(IpHeaders.HOSTNAME))).contains("localhost");
 	}
 
 	@Test
-	public void testTruncation() throws Exception {
+	public void testTruncation() {
 		String test = "ABCD";
 		Message<byte[]> message = MessageBuilder.withPayload(test.getBytes()).build();
 		DatagramPacketMessageMapper mapper = new DatagramPacketMessageMapper();
@@ -101,8 +103,7 @@ public void testTruncation() throws Exception {
 			fail("Truncated message exception expected");
 		}
 		catch (MessageMappingException e) {
-			assertThat(e.getMessage().contains("expected " + (bigLen + 4) + ", received " + (test.length() + 4)))
-					.isTrue();
+			assertThat(e.getMessage()).contains("expected " + (bigLen + 4) + ", received " + (test.length() + 4));
 		}
 	}
 

src/reference/asciidoc/ip.adoc

@@ -200,9 +200,8 @@ The following example shows how to configure a basic multicast inbound udp chann
 ----
 ====
 
-By default, reverse DNS lookups are done on inbound packets to convert IP addresses to host names for use in message headers.
-In environments where DNS is not configured, this can cause delays.
-You can override this default behavior by setting the `lookup-host` attribute to `false`.
+By default, reverse DNS lookups are not performed on inbound packets: in environments where DNS is not configured (e.g. Docker containers), this can cause connection delays.
+To convert IP addresses to host names for use in message headers, the default behavior can be overridden by setting the `lookup-host` attribute to `true`.
 
 Starting with version 5.3.3, you can add a `SocketCustomizer` bean to modify the `DatagramSocket` after it is created.
 It is called for the receiving socket and any sockets created for sending acks.
@@ -489,13 +488,12 @@ A server connection factory that uses `java.net.Socket` connections and uses Jav
 
 For full details of the attributes available on connection factories, see <<ip-annotation,the reference>> at the end of this section.
 
-By default, reverse DNS lookups are done on inbound packets to convert IP addresses to host names for use in message headers.
-In environments where DNS is not configured, this can cause connection delays.
-You can override this default behavior by setting the `lookup-host` attribute to `false`.
+By default, reverse DNS lookups are not performed on inbound packets: in environments where DNS is not configured (e.g. Docker containers), this can cause connection delays.
+To convert IP addresses to host names for use in message headers, the default behavior can be overridden by setting the `lookup-host` attribute to `true`.
 
 NOTE: You can also modify the attributes of sockets and socket factories.
 See <<ssl-tls>> for more information.
-As noted there, such modifications are possible whether or not SSL is being used.
+As noted there, such modifications are possible if SSL is being used, or not.
 
 Also see <<ip-annotation>> and <<ip-dsl>>.
 
@@ -1584,7 +1582,7 @@ Defaults to `ByteArrayCrLfSerializer`
 | Y
 | Y
 | `true`, `false`
-| Whether or not connection uses NIO.
+| Whether the connection uses NIO.
 Refer to the `java.nio` package for more information.
 See <<note-nio>>.
 Default: `false`.
@@ -1693,7 +1691,7 @@ For backward compatibility, it sets the backlog, but you should use `backlog` to
 | `true`, `false`
 | Specifies whether reverse lookups are done on IP addresses to convert to host names for use in message headers.
 If false, the IP address is used instead.
-Default: `true`.
+Default: `false`.
 
 | `interceptor-factory-chain`
 | Y
@@ -1778,7 +1776,7 @@ You can detect this by using the `check-length` attribute..
 
 | `check-length`
 | `true`, `false`
-| Whether or not a UDP adapter expects a data length field in the packet received.
+| Whether a UDP adapter expects a data length field in the packet received.
 Used to detect packet truncation.
 
 | `so-timeout`
@@ -1806,7 +1804,7 @@ See the setSendBufferSize() methods in `java.net.DatagramSocket` for more inform
 | `true`, `false`
 | Specifies whether reverse lookups are done on IP addresses to convert to host names for use in message headers.
 If `false`, the IP address is used instead.
-Default: `true`.
+Default: `false`.
 
 |===
 

src/reference/asciidoc/whats-new.adoc

@@ -69,4 +69,8 @@ See <<./kafka.adoc#kafka,Spring for Apache Kafka Support>> for more information.
 
 === JDBC Changes
 
-The `DefaultLockRepository` can now be supplied with a `PlatformTransactionManager` instead of relying on the primary bean from the application context.
+The `DefaultLockRepository` can now be supplied with a `PlatformTransactionManager` instead of relying on the primary bean from the application context.
+
+=== TCP/IP Changes
+
+The `lookupHost` property of the `AbstractConnectionFactory` and `DatagramPacketMessageMapper` is now set to `false` by default to avoid delays in the environments where DNS is not configured.