allthingssecurity commented on PR #28466:
URL: https://github.com/apache/camel/pull/28466#issuecomment-6409817136
@davsclaus thanks. All three points are in 582d71b4d4f2, a new commit on top
(no squash).
**Docs.** The mina-sftp page has a new `=== Using Local Work Directory`
section at the end of the examples. It follows the ftp page's section: the
download to `<name>.inprogress`, the rename when the download is done, and the
delete of the local file when the exchange is done (also when it failed). There
is the same Java/XML/YAML example with a file producer, and the first file
producer renames the local work file instead of copying it. An `IMPORTANT` note
says the body still names the moved file, so a step that reads it again (a
second file producer, `convertBodyTo`, or a `wireTap` or `aggregate` that keeps
the `File` body) fails. The fix it gives is `convertBodyTo(byte[].class)`
first, with a short example. The catalog copy is byte-identical.
**Partly downloaded `.inprogress` file.** I used your snippet with one
change: `temp.exists() && !FileUtil.deleteFile(temp)`. When `sftpClient.read`
fails, the `FileOutputStream` was never opened, so there is no temp file, and
`FileUtil.deleteFile` returns false for a missing file. That would log a false
warning.
**`localWorkFileIsMovedToTheTargetOfAFileProducer`.** The route now checks,
right after the file producer, that the local work file is gone, and the test
waits for `whenCompleted(1)`. The check throws an `IllegalStateException`, not
a JUnit assertion. I tried `assertFalse` first, but an `AssertionFailedError`
is an `Error`. It escaped the route to the poll ("Fatal error occurred during
running scheduled task") and did not fail the exchange normally. To show the
test now tells a move from a copy, I planted a copy-then-delete locally in
`FileOperations.handleFileAsFileSource` (not committed). That plant copies the
local work file and keeps the header, so the consumer deletes the file when the
exchange is done. With it, the new test fails ("The exchange should complete";
the route log has "The file producer should move the local work file, not copy
it"), and the old version of the test passes. Without the header fix, the new
test also fails, together with the other three, as before.
**Backport.** Main only. The disk-filling part is a real bug on the
maintenance branches too. But a route with two file producers, or a `wireTap`
of the `File` body, that works on a patch release today would start to fail,
and that should not happen in a patch release. I changed the backport line of
the description to match.
Tests: `MinaSftpConsumerLocalWorkDirectoryIT` passes 5 of 5. The
camel-mina-sftp tests and ITs all run through surefire on the in-JVM embedded
SFTP server, with no Docker: 308 run, 0 failures, 8 skipped. There are 2 errors
in `MinaSftpConfigurationValidationIT` (the OS-user and ssh-config username
fallbacks). They are the same two as before: the embedded server only accepts
`admin`. The formatter and import sort made no changes.
_Claude Code on behalf of allthingssecurity_
--
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]
