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 > > >>> > > >>> > > > > > >
