Federico Mariani created CAMEL-26648:
----------------------------------------

             Summary: camel-mina-sftp - readLock=changed with fastExistsCheck 
never consumes the file, as listing errors escape unwrapped
                 Key: CAMEL-26648
                 URL: https://issues.apache.org/jira/browse/CAMEL-26648
             Project: Camel
          Issue Type: Bug
          Components: camel-mina-sftp
    Affects Versions: 4.18.5, 4.23.0, 4.22.2
            Reporter: Federico Mariani


h3. Problem
In Apache MINA SSHD, {{SftpClient.readDir(path)}} is lazy. It sends OPENDIR 
only when the result is iterated, and a failed OPENDIR is thrown as 
{{java.io.UncheckedIOException}} around the {{SftpException}} (sshd 2.20.0, 
{{SftpIterableDirEntry.iterator()}}). {{MinaSftpOperations.listFiles(String)}} 
(components/camel-mina-sftp/src/main/java/org/apache/camel/component/file/remote/mina/MinaSftpOperations.java,
 iteration at line 1184 on main 7978db3d42dd) catches only {{IOException}} 
(line 1189). So the error leaves Camel as a raw {{UncheckedIOException}}, not 
as a {{GenericFileOperationFailedException}}. This has two symptoms:

# *readLock=changed&fastExistsCheck=true never acquires the lock.* 
{{MinaSftpChangedExclusiveReadLockStrategy}} 
(.../mina/strategy/MinaSftpChangedExclusiveReadLockStrategy.java line 90) calls 
{{operations.listFiles(file.getAbsoluteFilePath())}}, i.e. it lists the path of 
the file itself. The strategy was written for JSch, whose {{ChannelSftp.ls}} 
accepts a file path. MINA sends OPENDIR for the regular file, and the server 
refuses it (the MINA SSHD server answers {{SSH_FX_NOT_A_DIRECTORY}}). 
{{GenericFileConsumer}} (lines 547-577) records the exception as a begin 
failure, aborts the file and logs a WARN. Every poll repeats this, so the file 
is never consumed. {{readLockTimeout}} does not help, because the very first 
check throws.
# *ignoreFileNotFoundOrPermissionError=true does not ignore a missing directory 
with the whole listing.* This needs {{stepwise=false}} and a whole listing 
({{maxMessagesPerPoll}} unset, or {{preSort}}, {{doneFileName}} or 
{{eagerMaxMessagesPerPoll=false}}). Every poll then fails with 
{{UncheckedIOException}} ({{SSH_FX_NO_SUCH_FILE}}): {{AbstractSftpConsumer}} 
only consults {{ignoreCannotRetrieveFile}} for a 
{{GenericFileOperationFailedException}}. The lazy listing added by CAMEL-26635 
({{listFiles(String, Predicate)}}, line 1215) already catches {{IOException | 
UncheckedIOException}}. So with {{maxMessagesPerPoll}} set, the same missing 
directory is ignored.

The comments at lines 950 and 1127 ("readDir doesn't throw for non-existent 
directories in MINA SSHD") are misleading: it does throw, but only once the 
result is iterated.

{code:java}
// MinaSftpOperations.listFiles(String), main
for (DirEntry entry : sftpClient.readDir(path)) {  // OPENDIR is sent here; a 
failure is an UncheckedIOException
    result.add(new MinaSftpRemoteFile(entry));
}
...
} catch (IOException e) {                          // the UncheckedIOException 
is not caught
{code}

h3. How to reproduce
Both cases run against the embedded MINA SSHD SFTP server 
({{SftpEmbeddedService}} from camel-test-infra-ftp) on main, with a consumer 
{{exceptionHandler}} that records failures. These results were measured:
||Case||Endpoint options||Outcome||
|Read lock: one static 
file|{{delete=true&readLock=changed&readLockCheckInterval=100&readLockTimeout=5000&fastExistsCheck=true}}|10
 begin failures in about 3 s, {{UncheckedIOException: SFTP error 
(SSH_FX_NOT_A_DIRECTORY): Accessed location is not a directory}}; 0 messages; 
the file stays on the server|
|Read lock control|same, {{fastExistsCheck=false}}|file consumed and deleted, 0 
failures|
|Missing 
directory|{{autoCreate=false&stepwise=false&ignoreFileNotFoundOrPermissionError=true}}|every
 poll fails with {{UncheckedIOException: SFTP error (SSH_FX_NO_SUCH_FILE)}}|
|Missing directory control|same, plus {{maxMessagesPerPoll=10}} (lazy 
listing)|0 failures|

camel-mina-sftp has no test with {{fastExistsCheck}} or 
{{ignoreFileNotFoundOrPermissionError}} today.

h3. Proposed change
* *Fast exists check:* in the {{fastExistsCheck}} branch of 
{{MinaSftpChangedExclusiveReadLockStrategy}}, read the length and modification 
time with one {{stat}} of the file, as {{MinaSftpOperations.existsFile}} 
already does. Treat {{SSH_FX_NO_SUCH_FILE}} as "not found" (length 0, the same 
as no match today). This is also one request per check instead of a listing. 
Pick {{stat}} or {{lstat}} deliberately for symbolic links, and add a test with 
a linked file.
* *Listing errors:* in {{MinaSftpOperations.listFiles(String)}}, catch 
{{IOException | UncheckedIOException}} and wrap the exception in 
{{GenericFileOperationFailedException}}, as the lazy listing does. 
{{MinaSftpConsumer.ignoreCannotRetrieveFile}} then finds the {{SftpException}} 
in the cause chain, as it already does for the lazy listing (the control 
above). Fix the two comments as well.
* *Tests:* add an IT for {{readLock=changed&fastExistsCheck=true}}, and one for 
{{ignoreFileNotFoundOrPermissionError}} with {{stepwise=false}} and the whole 
listing.

h3. Impact / upgrade
No default or option changes, so no upgrade guide entry is needed. Only 
camel-mina-sftp is affected: camel-sftp (JSch 2.28.8) {{ls}} accepts a file 
path.

The same code is in camel-4.22.2 and camel-4.18.5 (sshd 2.20.0 on both). I 
checked this by reading the source at the tags, without running it.
* On 4.22.x the strategy is the same 
{{MinaSftpChangedExclusiveReadLockStrategy}}.
* On 4.18.x, camel-mina-sftp still uses camel-ftp's 
{{SftpChangedExclusiveReadLockStrategy}}, shared with JSch. There the backport 
is better done in {{MinaSftpOperations}}: catch the exception, and when OPENDIR 
fails because the path is a regular file, return that one entry from a {{stat}}.
* Neither branch has the lazy listing, so the missing-directory symptom happens 
with {{stepwise=false}} whatever {{maxMessagesPerPoll}} is.

h3. Related
* CAMEL-26635 (7978db3d42dd): the lazy mina-sftp listing that already catches 
{{UncheckedIOException}}.
* CAMEL-23164 (627174305ac5): copied the changed read lock strategy into 
camel-mina-sftp from camel-ftp, where it targets JSch {{ls}}.
* CAMEL-5512: {{fastExistsCheck}} for {{readLock=changed}}.
* CAMEL-12337: the same {{ignoreFileNotFoundOrPermissionError}} symptom on 
camel-ftp (fixed in 2018).

_Claude Code on behalf of Federico Mariani_



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

Reply via email to