Some FTP servers and platforms requires stepwise. And it has been the default ever since Camel 1.x.
On Thu, May 30, 2013 at 10:23 AM, Bengt Rodehav <be...@rodehav.com> wrote: > Hello Aki - sorry for the late reply. > > Yes, I do realize that the reason for this code is to do a stepwise change > of directories when going back to the "home" directory. What I don't > understand is why this is necessary. It is only done in sftp - not in > ftp/ftps. > > I have never been in any situation where stepwise is needed at all. But, > since stepwise by default is true, I assume that these situations exist > (but I think the default should be not to use stepwise...). But, why would > you have to change directories stepwise when going back up only in sftp but > not in ftp/ftps? > > Also, I always disconnect after each poll (I've found it much more stable) > which means that going back up is totally unnecessary since I will > re-connect on next poll and therefore start in the "home"directory again. > > Of course, fixing the logic might be preferrable to removing it if there is > a need for this kind of logic... > > BTW, do you know why the stepwise option exists? What scenario requires us > to change to a directory in a stepwise fashion instead of changing to that > directory directly? > > /Bengt > > > 2013/5/27 Aki Yoshida <elak...@gmail.com> > >> Hi Bengt, >> the quoted code itself is needed for the normal case to ensure the stepwise >> operation moves the path upwards from the current directory to where it >> started without touching its upper directory which you potentially have no >> authorization to access. >> >> but the code does not handle the case when the path is at the root, as you >> describe. In that case, this code needs to be skipped as it is not needed >> (because you started out from the root and you have authorization down from >> the root to the current directory). >> >> I commented to the ticket. >> >> regards, aki >> >> >> 2013/5/6 Bengt Rodehav <be...@rodehav.com> >> >> > JIRA created: >> > >> > https://issues.apache.org/jira/browse/CAMEL-6335 >> > >> > /Bengt >> > >> > >> > 2013/5/6 Bengt Rodehav <be...@rodehav.com> >> > >> > > Hello Hadrian, >> > > >> > > I'm not really interested in the blame issue. I'm really glad that >> there >> > > are so many competent people developing Camel which I think is a great >> > > product. It might be interesting, though, to see how long the bug has >> > been >> > > there and what versions are affected. >> > > >> > > I've also been thinking why it hasn't been picked up by any tests. I >> > > realize that it must be very difficult to design tests for integration >> > > components like sftp. Ideally you would have a number of ftp/ftps/sftp >> > > servers to perform integration tests against. But I guess that is hard >> to >> > > maintain. I don't think unit tests (with mocking) will catch bugs like >> > the >> > > above. How are ftp/ftps/sftp integrations tested? >> > > >> > > I have noticed that some sftp servers silently ignore attempts to >> change >> > > directory to a non-existing directory - you won't get there but you >> won't >> > > get an error code either. But some will give you an error code (like >> the >> > > one we are integrating to right now). >> > > >> > > I will create a JIRA. As for patching I guess the most important part >> is >> > > to determine if it is necessary to change back to the home directory >> in a >> > > stepwise fashion or not. If not, then I think we can just remove that >> > code. >> > > If it is necessary, then we must instead fix it. >> > > >> > > /Bengt >> > > >> > > >> > > >> > > 2013/5/3 Hadrian Zbarcea <hzbar...@gmail.com> >> > > >> > >> Bengt, many thanks for reporting this. >> > >> >> > >> Either svn or git tells you pretty quickly how that code got there >> (git >> > >> blame). Do you mind raising a jira for this embarrassing bug? If you >> > want >> > >> to contribute a patch it'd be highly appreciated. That aside, I'll >> look >> > >> into it in the afternoon. I'll need to check if that piece of code was >> > >> meant to address another usecase or was an oversight and insufficient >> > >> testing. >> > >> >> > >> Cheers, >> > >> Hadrian >> > >> >> > >> >> > >> >> > >> On 05/03/2013 10:35 AM, Bengt Rodehav wrote: >> > >> >> > >>> Unfortunately I seem to have found another bug in Camel 2.11.0 >> > regarding >> > >>> sftp. I noticed it when I removed the "stepwise=false" argument from >> my >> > >>> routes thus enabling the default stepwise way of changing directory. >> > >>> >> > >>> The problematic section begins at line 404 in SftpOperations.java: >> > >>> >> > >>> *404:* if (getCurrentDirectory().**startsWith(path)) { >> > >>> *405:* // use relative path >> > >>> *406:* String p = getCurrentDirectory().** >> > >>> substring(path.length()); >> > >>> *407:* if (p.length() == 0) { >> > >>> *408:* return; >> > >>> *409:* } >> > >>> *410:* // the first character must be '/' and hence >> removed >> > >>> *411:* path = >> > >>> UP_DIR_PATTERN.matcher(p).**replaceAll("/..").substring(1)**; >> > >>> *412:* } >> > >>> >> > >>> >> > >>> >> > >>> The problem does not arise when stepping down into the directory to >> > poll >> > >>> but when going back to the directory where we started. Assume that >> the >> > >>> home >> > >>> directory in the sftp server is "/" and I want to poll the directory >> > "in" >> > >>> relative to the root. My endpoint would look something like this: >> > >>> >> > >>> sftp://user@server/in?**password=secret >> > >>> >> > >>> What happens is that the "i" in "in" is removed and Camel attempts to >> > >>> change to directory "n". Looking at the code above, the following >> > >>> happens: >> > >>> >> > >>> - The current directory is "/in" since we have previously succeeded >> in >> > >>> moving there and polling for files. We are now attempting to go back >> to >> > >>> where we started, therefore "path" is "/". >> > >>> - On line 406, the variable "p" will be set to "in", thus removing >> the >> > >>> leading "/". >> > >>> - On line 411 an incorrect assumption is made: It is assumed that the >> > >>> first >> > >>> character must be "/" but the leading "/" was removed on line 406. >> The >> > >>> result is therefore that "path" is set to "n" instead of "in". >> > >>> >> > >>> I haven't investigated when this error was introduced. I did however >> > >>> compare with the corresponding logic in FtpOperations.java. In that >> > file >> > >>> the section quoted above does not exist at all. It seems to me that >> the >> > >>> purpose with this code is to, stepwise, change directory up to ".." - >> > one >> > >>> directory at a time instead of going straight to "/". It seems very >> > >>> unnecessary to me but I assume there is a reason for it. Otherwise, >> the >> > >>> easiest change would be to just remove the code above and use the >> same >> > >>> logic as in FtpOperations.java. >> > >>> >> > >>> Currently my only workaround is to use "stepwise=false". >> > >>> >> > >>> /Bengt >> > >>> >> > >>> >> > > >> > >> -- Claus Ibsen ----------------- www.camelone.org: The open source integration conference. Red Hat, Inc. FuseSource is now part of Red Hat Email: cib...@redhat.com Web: http://fusesource.com Twitter: davsclaus Blog: http://davsclaus.com Author of Camel in Action: http://www.manning.com/ibsen
