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