[ https://issues.apache.org/jira/browse/LOG4J2-1731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738498#comment-15738498 ]
ASF GitHub Bot commented on LOG4J2-1731: ---------------------------------------- Github user chrisribble commented on the issue: https://github.com/apache/logging-log4j2/pull/49 It looks like this test failed, but all tests - including this one - passed on my machine. TlsSyslogAppenderTest>SyslogAppenderTest.testUDPStructuredAppender:86->SyslogAppenderTestBase.sendAndCheckStructuredMessage:84->SyslogAppenderTestBase.checkTheNumberOfSentAndReceivedMessages:103 The number of received messages should be equal with the number of sent messages expected:<1> but was:<0> This is my first time contributing to log4j2, so I'm not really sure what the next step is. > 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 > Labels: patch > 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-unsubscr...@logging.apache.org For additional commands, e-mail: log4j-dev-h...@logging.apache.org