OK 2010/6/21 Claus Ibsen <[email protected]>
> 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 >
