[jira] [Resolved] (LOG4J2-1731) SslSocketManager should respect connectTimeoutMillis

["Remko Popma (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/LOG4J2-1731?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Remko Popma resolved LOG4J2-1731.
---------------------------------
       Resolution: Fixed
    Fix Version/s: 2.8

Patch applied without issues. All core tests pass on my machine.

Thank you for the patch!
Please verify and close.

> SslSocketManager should respect connectTimeoutMillis
> ----------------------------------------------------
>
>                 Key: LOG4J2-1731
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-1731
>             Project: Log4j 2
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 2.7
>            Reporter: Chris Ribble
>            Assignee: Remko Popma
>              Labels: patch
>             Fix For: 2.8
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> Creating a SocketAppender with an SSLConfiguration causes log4j to use SslSocketManager to connect to the remote server.
> When SSL is not configured, TcpSocketManager correctly uses connectTimeoutMillis both during the initial socket setup and in the reconnection manager thread.
> But when SSL is configured SslSocketManager does not use connectTimeoutMillis when creating SSL sockets. The affects both the initial connection and the reconnection manager thread.
> In my testing, it takes nearly a minute for the connection to time out when the other side does not exists (i.e. use invalid host like 10.0.0.0).
> Example code to set up the SocketAppender:
> {code}
> SocketAppender appender = SyslogAppender.newBuilder()
> 				.withHost("10.0.0.0")
> 				.withPort(514)
> 				.withProtocol(Protocol.SSL)
> 				.withSslConfiguration(SslConfiguration.createSSLConfiguration("TLSv1.2", null, null))
> 				.withConnectTimeoutMillis(1000)
> 				.withReconnectDelayMillis(500)
> 				.withImmediateFail(true)
> 				.withImmediateFlush(true)
> 				.withAdvertise(false)
> 				.withIgnoreExceptions(true)
> 				.withLayout(layout)
> 				.withName("MY_APPENDER")
> 				.build();
> appender.start();
> {code}
> If you run this, you should see the call to build() block for ~60s until the underlying operating system timeout occurs.
> Interestingly, the reconnection code and the initial connection code ignore the timeout for different reasons.
> 1. The reconnect code uses a pattern that allows for specification of the timeout, but 0 is hard-coded in the constructor call for SslSocketManager.
> 2. The initial connection code doesn't use a pattern that allows for specification of the connection time.
> Suggested fixes:
> 1. Use the configuration data's connectTimeoutMillis when instantiating SslSocketManager instead of hard-coding timeout to 0
> 2. Use the same pattern that the reconnection code uses for socket connect where timeout is specified when connecting the socket.
> Suggested patch:
> {code}
> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SslSocketManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SslSocketManager.java
> index 57a4b43..8c9eddc 100644
> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SslSocketManager.java
> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SslSocketManager.java
> @@ -190,8 +190,8 @@ public class SslSocketManager extends TcpSocketManager {
>                  LOGGER.catching(Level.DEBUG, e);
>                  return null;
>              }
> -            return new SslSocketManager(name, os, socket, data.sslConfiguration, inetAddress, data.host, data.port, 0,
> -                    data.delayMillis, data.immediateFail, data.layout, data.bufferSize, data.socketOptions);
> +            return new SslSocketManager(name, os, socket, data.sslConfiguration, inetAddress, data.host, data.port,
> +                    data.connectTimeoutMillis, data.delayMillis, data.immediateFail, data.layout, data.bufferSize, data.socketOptions);
>          }
>          private InetAddress resolveAddress(final String hostName) throws TlsSocketManagerFactoryException {
> @@ -218,11 +218,12 @@ public class SslSocketManager extends TcpSocketManager {
>              SSLSocket socket;
>              socketFactory = createSslSocketFactory(data.sslConfiguration);
> -            socket = (SSLSocket) socketFactory.createSocket(data.host, data.port);
> +            socket = (SSLSocket) socketFactory.createSocket();
>              final SocketOptions socketOptions = data.socketOptions;
>              if (socketOptions != null) {
>                  socketOptions.apply(socket);
>              }
> +            socket.connect(new InetSocketAddress(data.host, data.port), data.connectTimeoutMillis);
>              return socket;
>          }
>      }
> {code}
> With this patch applied, both the initial connect and reconnect respect connectTimeoutMillis and my tests/applications don't hang on startup when the server is not available.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org