Croway commented on PR #29273: URL: https://github.com/apache/camel/pull/29273#issuecomment-6669784729
@davsclaus thanks for the review. I checked each point against the PR head and added three commits. 1. **Fallback of `fastExistsFile`:** confirmed. In JSch 2.28.8, `ChannelSftp.ls(path)` of a file starts with a STAT of the path, then lists the parent. I extended `SftpExistsFileIT` with a server that refuses LSTAT and STAT of the file but lists its directory (`listsDirectoryWhenServerAnswersNeitherLstatNorStatOfFile`). On the PR head, the `fastExistsCheck=true` case failed with `SSH_FX_PERMISSION_DENIED` from the STAT inside `ls`. Fixed in d7722dd63910 as you suggested: `fastExistsFile` is now `getFileAttributes(name) != null`, so with and without `fastExistsCheck` the fallback is the listing of the parent directory. A server that refuses STAT on every path, the directory included, still fails, because JSch also starts `ls(directory)` with a STAT of the directory. That is the same as before. 2. **Which statuses fall back:** confirmed. I added BAD_MESSAGE and NO_SUCH_PATH (10) to `listsDirectoryWhenServerDoesNotAnswerLstat`, and the PR head threw for both, with `fastExistsCheck` true and false (4 errors). Fixed in 7c4edadfda7e: any status sent by the server (`getCause() == null`) other than NO_SUCH_FILE falls back to the listing. An I/O or session failure still fails the check, and the normal path is unchanged. The component page, its catalog copy and the 4.24 upgrade guide now describe this fallback. 3. **Comment:** right, only the separator between the directory and the name is sent as `/`. I kept the path as it is built, so that it matches the listing it falls back to, which also splits at the last separator. I reworded the comment in f8955b30bfd8. 4. **`fastExistsCheck` description:** left for a follow-up. It is the javadoc of `RemoteFileEndpoint.setFastExistsCheck` in camel-ftp-common, shared by ftp, ftps, sftp and mina-sftp. mina-sftp still behaves as it says, because its `readLock=changed` uses the option. So I'd rather change it with the mina-sftp follow-up below, once both SFTP components behave the same. The sftp page already says that the option makes no difference for these checks with sftp. 5. **camel-mina-sftp:** confirmed with a throwaway IT against the embedded server. `existsFile` of a dangling link is false (one STAT). An existing file whose STAT the server refuses is also false, as any `IOException` means "missing" there. And `readLock=changed` in a 1000-file directory lists it on every check: 3 OPENDIR and 2,218 entries listed for one file. The `fastExistsCheck=true` read lock is already CAMEL-26648. Filed as CAMEL-26671 (`existsFile` and the read lock, including the `fastExistsCheck` description), linked to CAMEL-26648. Whole `camel-ftp` module (`mvn verify`): 143 unit tests, 473 + 1 ITs, 0 failures. _Claude Code on behalf of Federico Mariani_ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
