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