I'm also wondering if we could perform better with SSL, if we cache the SSLContext and SSLSocketFactory. Currently, every data connection creates the SSLContext (even though the context parameters are the same) and gets the SocketFacotry from the context. Instead, we should create the SSLContext and SSLSocketFactory once per each SslConfiguration and reuse it. Obviously, this reduces the unnecessary object creation and eliminates the unnecessary method calls and context initialization. I've made some changes locally and things seem to be working fine. If you think we should apply this suggested change, I can post the patch for review. Basically, the changes we would need are:
Add the following new method to the SslConfiguration interface - SSLSocketFactory getSocketFactory() throws GeneralSecurityException; Implement the above method method in the DefaultSslConfiguration. When constructing the DefaultSslConfiguration, create the SSLContext and an SSLSocketFactory and save them as members. Then when we need a SSLSocket, we simply call SSLConfiguration.getSocketFactory() which returns the pre-created socket factory. I know we may not notice much time difference with this change, but every little things add up, especially under high loads. Let me know what you think. Regards, Sai Pullabhotla www.jMethods.com On Wed, Aug 5, 2009 at 10:05 AM, Sai Pullabhotla<sai.pullabho...@jmethods.com> wrote: > I'm not sure, I think it should go to the main trunk as well. > > Sai Pullabhotla > www.jMethods.com > > > > > On Wed, Aug 5, 2009 at 9:31 AM, Niklas Gustavsson<nik...@protocol7.com> wrote: >> On Wed, Aug 5, 2009 at 2:41 PM, Sai >> Pullabhotla<sai.pullabho...@jmethods.com> wrote: >>> I also have another question around the same code...Should we be >>> checking the remote address and make sure it matches with the IP >>> address of the remote host on the control connection. If we do not do >>> this check, it is possible for a hacker to connect to this port before >>> the original client and may gain access to the data? I know it is not >>> very easy to do this, but just in case. What do you think? >> >> I think this makes sense. We already do the logically same for active >> connections. Probably only should apply this to the 1.1.X (trunk) >> code, right? >> >> /niklas >> >
