JIRA created: https://issues.apache.org/activemq/browse/CAMEL-2829
<https://issues.apache.org/activemq/browse/CAMEL-2829>/Bengt 2010/6/18 Bengt Rodehav <[email protected]> > 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 >> > >
