Claus, I'm not sure what you mean by:
"I think we should add an option on the endpoint to allow people to turn this on/off. Some would like to reuse existing connections to avoid the connect -> upload -> disconnect cycle when they upload many files etc." IMO we would not disconnect more often than today. We would just change the connect() method in FtpsOperations so that it creates a new instance of FTPSClient prior to a connection attempt. I assume that since the connect() method is called in the first place, we are not currently connected and there is no way to reuse an existing connection. Am I right? I have made some modifications in camel-ftp along these lines. I will create a JIRA ticket for this bug and attach my changes as diff files to the issue. I just tested my changes and they do solve the problem I have encountered but I haven't tested whether I have introduced any new problems or not. Anyway, this is what I did: *FtpsEndpoint* I added the following members to keep track of the configuration of the FTPSClient: private boolean isNeedClientAuth = false; private KeyManager keyManager; private TrustManager trustManager; I modified the createFtpClient() method so that it doesn't actually create a FTPSClient but only calculates and stores the configuration in the above members. I also renamed it to createFtpsConfiguration() to reflect that. The createRemoteFileOperations() method was also changed accordingly. It no longer interacts directly with an FTPSClient (since it is not created yet). *FtpsOperations* I added the private createFtpsClient() method that takes care of creating an FTPSClient from configuration stored in the FtpsEndpoint. The createFtpsClient() is called in the connect() method just before the call to the "real" connect() method in the superclass (FtpOperations). I changed the constructor so that it no longer takes a client as an argument. I send null as client to the super class constructor. This is of course a major change in behavior that you need to take a look at it. Neither FtpsOperations nor its super class FtpOperations can no longer assume that a client exists until the first connection attempt has been made. *FtpOperations* I had to make the "client" member non-final since I now set it on every call to the connect() method in FtpsOperations. BTW, I agree with you that the SFTPClient in commons-net is flawed. It should provide a way to re-initialize itself so that a new connect() could be made. The FTPClient seems to work that way and it also provides a disconnect() method. If they do not intend to support this, then they should at least state, as part of the documentation, that the SFTPClient cannot be reused across multiple connections. I've posted about this on their mailing list but hasn't got any reply yet. Meanwhile the above can be regarded as a (complicated...) work-around. /Bengt 2010/6/18 Claus Ibsen <[email protected]> > On Thu, Jun 17, 2010 at 11:56 PM, Bengt Rodehav <[email protected]> wrote: > > Claus, > > > > Having looked a bit more at the commons-net code it's becoming clear to > me > > that you probably can't just reconnect an FTPSClient once it has entered > > secure communication mode. I can't find a way to re-initialise an > > FTPSClient. I tried the disconnect() method (in the super class > FTPClient) > > but it didn't get the FTPSClient out of secure communication mode. > > > > To be safe I think we should always start with a newly instantiated > > FTPSClient when we try to connect. Today the FTPSClient is created by the > > FtpsEndpoint. I don't think it has to. It could just get all the righ > > configuration parameters and pass it to FtpsOperations which would then > > instantiate a new FTPSClient at every connect attempt. > > > > If we do it this way for ftps I guess it would make sense with the same > > setup for plain ftp. > > > > What do you think? > > > > I think we should add an option on the endpoint to allow people to > turn this on/off. > Some would like to reuse existing connections to avoid the connect -> > upload -> disconnect cycle when they upload many files etc. > > > > > And frankly I can't see the problem why a secure connection cannot > self heal and be able to re-establish itself. After all FTPSClient got > all the connection information. And hence I would consider it a > missing feature / glitch in the FTP library. > > Imagine if using secure connections for JMS brokers which couldn't > self heal just because a network outage. That would cause a lot of > pain for clients having to re-start. > > > > /Bengt > > > > 2010/6/17 Bengt Rodehav <[email protected]> > > > >> Claus, > >> > >> Yeah, maybe this is a commons-net issue. As always I find it surprising > >> that no one else has encountered this problem. > >> > >> As I started writing an email on the commons user list, I took another > look > >> at the code in commons-net. It turns out that the FTPSSocketFactory > class > >> actually does override the createSocket() methods in SocketFactory. A > >> similar work-around that I was talking about but on the factory class > >> instead of in the SocketClient class. FTPSSocketFactory delegates the > >> createSocket() calls to its SSLContect's socket factory. > >> > >> So why then are we initially able to connect but not after we have > called > >> execProt() and thus changed the connection factory? > >> > >> Moving back to the FTPSClient class now. When the initial connection is > up, > >> the _connectAction() method is called. It in turn calls sslNegotiation() > >> which in turn sets up the secure socket. In the sslNegotiation() method, > an > >> SSLSocketFactory is not instantiated directly. It is given by the > >> SSLContext's getSocketFactory() method. This connection factory > obviously > >> works otherwise the secure connection wouldn't work even initially. > >> > >> Compare this with what's being done in the execProt() method. Here the > >> FTPSSocketFactory is instantiated directly with the SSLContext passed as > a > >> constructor argument. Obviously this factory does NOT work... > >> > >> Another question that comes to mind is why the socket factory created in > >> the sslNegotiation() method is NOT set as the SocketClient's socket > factory > >> but the one created in the execProt() method is? It turns out that > >> execProt() calls a generic sendCommand() method that sends the command > and > >> if the reply is OK, then sets the socket factory to null! One wonders > why > >> (and there is also a comment in the code: "Check this - is this > necessary at > >> all?"). Anyway, this is probably the reason why execProt() needs to > "reset" > >> the connection factory. > >> > >> Still sounds like a commons-net problem though. I'll try to post this at > >> their user mailing list. > >> > >> Just one thought. Do we need to reuse the FTPSClient instance? Can't we > >> just create a fresh new instance everytime we need to connect? > >> > >> /Bengt > >> > >> > >> > >> 2010/6/17 Claus Ibsen <[email protected]> > >> > >> Hi Bengt > >>> > >>> I think the issue should be created as a ticket for Apache Commons > >>> Net. Then at least the people there can take a look, and maybe they > >>> got some ideas. And can fix it in a future release. > >>> > >>> Bengt fell free to experiment yourself with the subclassing, as it > >>> sounds like a good workaround. > >>> > >>> I assume the FTPSClient doesn't offer other methods to re connect > >>> which can recover this error? > >>> > >>> > >>> On Wed, Jun 16, 2010 at 2:44 PM, Bengt Rodehav <[email protected]> > wrote: > >>> > I have mentioned this problem in a previous conversation but after > >>> > investigating the subject further I decided to start a new thread for > >>> this. > >>> > > >>> > If some kind of problem is encountered while using ftps (I tested > this > >>> using > >>> > Filezilla server by copying a file that already existed which > Filezilla > >>> does > >>> > not allow), then at the next attempt to connect to the ftps server, I > >>> get > >>> > the following exception in my logfile: > >>> > > >>> > 13:03:27,606 | ERROR | %7Bfile%3Aext%7D | GenericFileOnCompletion > >>> | > >>> > rg.apache.camel.processor.Logger 248 | Caused by: > >>> > [org.apache.camel.component.file.GenericFileOperationFailedException > - > >>> File > >>> > operation failed: null Unconnected sockets not implemented. Code: > 221] > >>> > org.apache.camel.component.file.GenericFileOperationFailedException: > >>> File > >>> > operation failed: null Unconnected sockets not implemented. Code: 221 > >>> > at > >>> > > >>> > org.apache.camel.component.file.remote.FtpOperations.connect(FtpOperations.java:108)[76:org.apache.camel.camel-ftp:2.4.0.SNAPSHOT] > >>> > at > >>> > > >>> > org.apache.camel.component.file.remote.FtpsOperations.connect(FtpsOperations.java:40)[76:org.apache.camel.camel-ftp:2.4.0.SNAPSHOT] > >>> > at > >>> > > >>> > org.apache.camel.component.file.remote.RemoteFileProducer.connectIfNecessary(RemoteFileProducer.java:170)[76:org.apache.camel.camel-ftp:2.4.0.SNAPSHOT] > >>> > at > >>> > > >>> > org.apache.camel.component.file.remote.RemoteFileProducer.preWriteCheck(RemoteFileProducer.java:123)[76:org.apache.camel.camel-ftp:2.4.0.SNAPSHOT] > >>> > at > >>> > > >>> > org.apache.camel.component.file.GenericFileProducer.processExchange(GenericFileProducer.java:75)[65:org.apache.camel.camel-core:2.4.0.SNAPSHOT] > >>> > at > >>> > > >>> > org.apache.camel.component.file.remote.RemoteFileProducer.process(RemoteFileProducer.java:49)[76:org.apache.camel.camel-ftp:2.4.0.SNAPSHOT] > >>> > at > >>> > > >>> > org.apache.camel.processor.SendProcessor$1.doInProducer(SendProcessor.java:91)[65:org.apache.camel.camel-core:2.4.0.SNAPSHOT] > >>> > > >>> > I added some logging to see the actual stack trace which is: > >>> > > >>> > java.net.SocketException: Unconnected sockets not implemented > >>> > at > javax.net.SocketFactory.createSocket(SocketFactory.java:104) > >>> > at > >>> > org.apache.commons.net.SocketClient.connect(SocketClient.java:175) > >>> > at > >>> > > >>> > org.apache.camel.component.file.remote.FtpOperations.connect(FtpOperations.java:91) > >>> > at > >>> > > >>> > org.apache.camel.component.file.remote.FtpsOperations.connect(FtpsOperations.java:40) > >>> > at > >>> > > >>> > org.apache.camel.component.file.remote.RemoteFileProducer.connectIfNecessary(RemoteFileProducer.java:170) > >>> > at > >>> > > >>> > org.apache.camel.component.file.remote.RemoteFileProducer.preWriteCheck(RemoteFileProducer.java:99) > >>> > at > >>> > > >>> > org.apache.camel.component.file.GenericFileProducer.processExchange(GenericFileProducer.java:75) > >>> > at > >>> > > >>> > org.apache.camel.component.file.remote.RemoteFileProducer.process(RemoteFileProducer.java:49) > >>> > > >>> > commons-net SocketClient class tries to call the socket factory's > >>> > createSocket() method (with no parameters): > >>> > > >>> > _socket_= _socketFactory_.createSocket(); > >>> > _socket_.connect(new InetSocketAddress(hostname, port), > >>> > connectTimeout); > >>> > > >>> > Then intention is to create an "unconnected socket" (hence the error > >>> > message) and then subsequently connect it. In my case, the connection > >>> > factory being used is an FTPSSocketFactory > >>> > (package org.apache.commons.net.ftp) that inherits from > >>> > SocketFactory(package javax.net). The SocketFactory class implements > >>> the > >>> > createSocket() method but throws the exception ("Unconnected sockets > not > >>> > implemented"). > >>> > > >>> > The reason why this happens is that after having executed the > >>> > FTPSClient.execPROT() method (package org.apache.commons.net.ftp), > with > >>> any > >>> > other parameter than "C" (in my case I used "P"), the socket factory > is > >>> set > >>> > to an FTPSSocketFactory as follows: > >>> > > >>> > setSocketFactory(new FTPSSocketFactory(context)); > >>> > > >>> > After this, no connection attempt will succeed. This problem has been > >>> > introduced recently when Claus (on my initiative) added support for > >>> secure > >>> > data channel in ftps. I'm thus to blame... > >>> > > >>> > I'm not sure how this should be fixed. > >>> > > >>> > It's a bit strange that all the connect methods > >>> > in org.apache.commons.net.SocketClient (which is the ultimate base > class > >>> of > >>> > FTPSClient) always try to create an unconnected socket first and then > >>> > connects it. If commons-net uses this pattern, then it should make > sure > >>> that > >>> > all its connection factor's support unconnected sockets but the > >>> > FTPSSocketFactory dosn't. This sounds like a bug in commons-net. > >>> > > >>> > Knowing that commons-net doesn't release very frequently I think we > need > >>> a > >>> > workaround for this. I guess it would be possible to subclass the > >>> FTPClient > >>> > class and override all the connect methods to make sure that no > attempt > >>> is > >>> > made to create unconnected sockets. We would then change the > >>> > createFtpClient() method in the FtpsEndpoint class to instantiate our > >>> own > >>> > subclass instead of an FTPSClient. Doesn't sound like a nice clean > >>> solution > >>> > though. > >>> > > >>> > Any ideas? > >>> > > >>> > /Bengt > >>> > > >>> > >>> > >>> > >>> -- > >>> Claus Ibsen > >>> Apache Camel Committer > >>> > >>> Author of Camel in Action: http://www.manning.com/ibsen/ > >>> Open Source Integration: http://fusesource.com > >>> Blog: http://davsclaus.blogspot.com/ > >>> Twitter: http://twitter.com/davsclaus > >>> > >> > >> > > > > > > -- > Claus Ibsen > Apache Camel Committer > > Author of Camel in Action: http://www.manning.com/ibsen/ > Open Source Integration: http://fusesource.com > Blog: http://davsclaus.blogspot.com/ > Twitter: http://twitter.com/davsclaus >
