logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LOG4J2-1731) SslSocketManager should respect connectTimeoutMillis
Date Sat, 10 Dec 2016 21:14:59 GMT

    [ 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-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Mime
View raw message