Federico Mariani created CAMEL-26671:
----------------------------------------
Summary: camel-mina-sftp - existsFile counts dangling links and
server errors as missing; readLock=changed lists the directory
Key: CAMEL-26671
URL: https://issues.apache.org/jira/browse/CAMEL-26671
Project: Camel
Issue Type: Improvement
Components: camel-mina-sftp
Affects Versions: 4.18.5, 4.23.0, 4.22.2
Reporter: Federico Mariani
h3. Problem
With CAMEL-26656, the JSch-based {{sftp}} component checks whether a file
exists, and reads its size and modification time for {{readLock=changed}}, with
one {{lstat}} of the path. It lists the directory only when the server answers
that {{lstat}} with another status than "no such file". {{mina-sftp}} still
gives other answers, and still lists the directory:
* *A dangling symbolic link counts as missing.*
{{MinaSftpOperations.existsFile}}
(components/camel-mina-sftp/src/main/java/org/apache/camel/component/file/remote/mina/MinaSftpOperations.java
lines 1514-1530 on main, commit 2d97a2141484) sends {{stat}}, which follows
the link. A directory listing shows the link, and {{sftp}} reports it as
existing.
* *Any error counts as missing.* The same method returns {{false}} for every
{{IOException}}: {{PERMISSION_DENIED}} or any other status of the server, and
an I/O failure of the session. So for {{fileExist=Fail}} or {{Ignore}},
{{tempFileName}}, {{tempPrefix}} or a producer {{doneFileName}}, an error means
"no such file", and the producer goes on to write.
* *readLock=changed lists the whole directory on every check.*
{{MinaSftpChangedExclusiveReadLockStrategy}}
(.../mina/strategy/MinaSftpChangedExclusiveReadLockStrategy.java lines 76-121)
lists {{file.getParent()}} and compares the names. With
{{fastExistsCheck=true}} it lists the path of the file instead, which never
acquires the lock (CAMEL-26648).
* The description of {{fastExistsCheck}} (the javadoc of
{{RemoteFileEndpoint.setFastExistsCheck}} in camel-ftp-common, shared by ftp,
ftps, sftp and mina-sftp) still says that it influences {{readLock=changed}}.
That no longer holds for {{sftp}}, and should change once both SFTP components
behave the same.
h3. How to reproduce
A throwaway IT against the embedded MINA SSHD SFTP server
({{SftpEmbeddedService}} of camel-test-infra-ftp, sshd 2.20.0), on main. An
{{SftpEventListener}} counts the requests and the listed entries. Measured:
||Case||Outcome||
|{{existsFile}} of a regular file (control)|true, 1 STAT|
|{{existsFile}} of a dangling link {{link.txt -> missing.txt}}|false, 1 STAT|
|{{existsFile}} of an existing file, the server answers STAT with
PERMISSION_DENIED|false, no exception|
|{{readLock=changed&readLockCheckInterval=10&maxMessagesPerPoll=1}}, 1000
files, one file consumed|3 OPENDIR (the poll and 2 checks), 2,218 entries
listed|
|same with {{fastExistsCheck=true}} and {{readLockTimeout=2000}}, 3 files|not
consumed within 5 s (CAMEL-26648)|
For comparison, with {{sftp}} after CAMEL-26656 the same read lock costs 2
LSTAT and only the OPENDIR of the poll, and the dangling link exists
({{SftpExistsFileIT}} in camel-ftp).
h3. Proposed change
* {{existsFile}}: one {{lstat}} of the path, where {{SSH_FX_NO_SUCH_FILE}}
means false, as {{SftpOperations}} does since CAMEL-26656. On any other status
of the server (an {{org.apache.sshd.sftp.common.SftpException}}), list the
parent directory and compare the names. Throw an I/O failure as
{{GenericFileOperationFailedException}} instead of answering false.
* {{readLock=changed}}: read the size and modification time with the same
{{lstat}} on every check, whatever {{fastExistsCheck}} is, with the same
fallback. This replaces the {{fastExistsCheck}} branch that CAMEL-26648 fixes,
so both are best done together. CAMEL-26648 proposes {{stat}} there; {{lstat}}
keeps comparing what the listing reports for a link, as {{sftp}} does.
* Then reword the {{fastExistsCheck}} description, and regenerate the json, the
catalog and the endpoint DSL of ftp, ftps, sftp and mina-sftp.
* Tests: the cases above, plus the fallback when the server refuses {{lstat}}.
h3. Impact / upgrade
* No option or default changes. Two answers change, which needs an entry in the
4.24 upgrade guide: a dangling link now exists, and an error (other than "no
such file", when the listing does not answer either) fails the check instead of
meaning "missing".
* Each read lock check becomes one request instead of a listing of the
directory.
* The same {{existsFile}} is on camel-4.22.x and camel-4.18.x, and the read
lock lists the directory on both (checked by reading the source). On 4.18.x,
mina-sftp uses camel-ftp's {{SftpChangedExclusiveReadLockStrategy}}, shared
with JSch.
h3. Related
* CAMEL-26656: the same change for {{sftp}} (apache/camel PR #29273).
* CAMEL-26648: mina-sftp {{readLock=changed&fastExistsCheck=true}} never
consumes the file.
* CAMEL-22379 (9898d2b08a24): added {{existsFile}} with {{stat}}, together with
the component.
* CAMEL-26635 (7978db3d42dd): the lazy mina-sftp listing.
_Claude Code on behalf of Federico Mariani_
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
