Croway opened a new pull request, #29285: URL: https://github.com/apache/camel/pull/29285
JIRA: [CAMEL-26066](https://issues.apache.org/jira/browse/CAMEL-26066) ## What was wrong The scp producer sends the name of each directory and of the file on a protocol line that ends with a line feed: `D0775 0 <dir>`, then `C0<chmod> <size> <name>`, in `ScpOperations.writeFile`. The name comes from the `CamelFileName` (or `CamelOverruleFileName`) header or from the `fileName` option, and nothing checked it. On main, against the embedded Apache MINA SSHD server of the tests: - **Line feed in the file name** (`a\nb.txt`): the exchange **succeeds**. The server holds a file named `a` with the content `b.txt\nHello` instead of `Hello World`, because the rest of the name was read as file data. - **Line feed in a directory** (`d1\nd2/file.txt`): the exchange fails, but a stray directory `d1` is left on the server. - **NUL character**: the server refuses the name, after scp was started. OpenSSH servers before 10.6 read the line as a C string, so they cut the name short at the NUL and write a file with another name. A carriage return, a tab or any other character is written with the name as given, because the scp sinks of SSHD and OpenSSH read a line up to the line feed. These names keep working. ## What changed - `ScpOperations.storeFile` refuses a name with a line feed or a NUL character, with a `GenericFileOperationFailedException`. - The check runs before `session.openChannel("exec")`, so scp is not started on the remote host. - It covers the whole name: the directory segments as well as the file name. - The message gives the character code and its index (for example `U+000A at index 52`), not the name. - `storeFileDirectly` writes the name into the same protocol lines, so it does the same check. It is used for the checksum file of `checksumFileAlgorithm`. - Any other character is sent exactly as before. The check only reads the name, and `writeFile` and the exec command are unchanged, so a name without a line feed or a NUL gives the same protocol lines and the same files. ## Tests `ScpFileNameControlCharTest` (embedded Apache MINA SSHD) is a new class, because the route of `ScpSimpleProduceTest` consumes and deletes the files the producer writes. The new class records the scp commands the server starts, and lists the server directory. It covers: - **Refused names:** a line feed or a NUL, in the file name and in a directory, a trailing line feed, a nested `mysub/a\nb/file.txt`, and the `fileName` option. Each case asserts: - a `GenericFileOperationFailedException` whose message does not hold the name; - that no scp command was started; - that no file or directory was created. - **`storeFileDirectly`** with a line feed. - **Names that keep working:** these are still written, with the right content: - a carriage return (`a\rb.txt`, a trailing `hello.txt\r`, `my\rsub/a\rb.txt`); - a tab, also in a directory; - `\u001B` and `\u0085`; - a nested `mysub/mysubsub/hello.txt`. The test of these characters is disabled on Windows, where a file name cannot hold them. Results: - On main, 8 of 16 tests fail. The 8 that pass are the names that keep working. - With the fix, all 16 pass. - `mvn verify` of camel-jsch passes: 32 unit tests (3 already `@Disabled`) and the 7 embedded-server ITs. ## Docs and upgrade guide - `scp-component.adoc` and its catalog copy: a note that a name with a line feed or a NUL character is refused before anything is sent, and that any other character is sent as is. - Upgrade guide: - an entry in `camel-4x-upgrade-guide-4_24.adoc`; - the same entry in the 4.22.3 and 4.18.6 sections of the 4.22 and 4.18 guides, for the backports. ## Backport Backport PRs are planned for `camel-4.22.x` and `camel-4.18.x`. - The Java and test changes cherry-pick cleanly to both branches, and the camel-jsch build passes there: 30 unit tests and 7 ITs. - Only the doc placement differs. The "Usage" section of `scp-component.adoc` does not exist on those branches, so the note goes into "URI format" there. - Because of that, the backport PRs need a normal review rather than the straight-cherry-pick fast path. Unrelated to this change: with `checksumFileAlgorithm`, the scp producer writes the file but fails to write the checksum file. That will get its own JIRA. _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]
