On Mon, Jun 21, 2010 at 8:52 AM, Bengt Rodehav <[email protected]> wrote: > I have now created a JIRA in commons-net for this issue: > > https://issues.apache.org/jira/browse/NET-327 >
Good > <https://issues.apache.org/jira/browse/NET-327>However, I still think we > must fix this in camel-ftp by some kind of work-around (maybe similar to the > one I proposed in https://issues.apache.org/activemq/browse/CAMEL-2829). > Have you had a chance to look at it Claus? > No sorry I am to busy with other issues but we got the ticket so we will look into it before 2.4 is cut. > /Bengt > > 2010/6/18 Bengt Rodehav <[email protected]> > >> 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 >>>> >>> >>> >> > -- 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
