Federico Mariani created CAMEL-24120:
----------------------------------------
Summary: camel-smb: consumer downloads every file twice;
streamDownload=true leaks a remote file handle per file (SmbFile.getBody
bypasses the retrieved body)
Key: CAMEL-24120
URL: https://issues.apache.org/jira/browse/CAMEL-24120
Project: Camel
Issue Type: Bug
Components: camel-smb
Affects Versions: 4.21.0
Reporter: Federico Mariani
h3. Problem 1 - every consumed file is downloaded twice
{{SmbConsumer}} runs the standard retrieve pipeline:
{{GenericFileConsumer.processExchange}} -> {{SmbOperations.retrieveFile}} ->
{{retrieveFileToStreamInBody}} reads the *entire file* into a {{byte[]}} and
stores it on the {{GenericFile}} via {{target.setBody(body)}}
(SmbOperations.java:301-330). But the message body is resolved lazily through
{{GenericFileMessage.createBody()}} -> {{file.getBody()}}, and {{SmbFile}}
overrides {{getBody()}} (SmbFile.java:89-99) to bypass the stored body entirely:
{code:java}
@Override
public Object getBody() {
if (!download) {
return null;
}
if (streamDownload) {
return operations.getBodyAsInputStream(exchange,
this.getAbsoluteFilePath());
} else {
return operations.getBody(this.getAbsoluteFilePath()); // fresh full
download
}
}
{code}
The bytes fetched by {{retrieveFile}} are never used - the first body access
issues a *second full download*. Consequences with the default configuration:
* 2x network transfer and 2x server reads for every consumed file.
* {{localWorkDirectory}} is completely defeated:
{{retrieveFileToFileInLocalWorkDirectory}} spools the file to local disk (whose
whole point is keeping big files out of memory), then {{getBody()}}
re-downloads the whole file *into memory* anyway; the local file is never read.
* The bytes the route sees come from a later read than the one the {{changed}}
read lock validated.
h3. Problem 2 - streamDownload=true leaks one remote file handle per file
smbj 0.14.0's {{FileInputStream.close()}} only sets {{isClosed = true}} and
nulls its fields - verified in the bytecode, it does *not* close the underlying
{{File}} (the remote SMB handle), and neither does reading to EOF.
{{SmbOperations.getBodyAsInputStream}} (SmbOperations.java:626-641)
deliberately keeps the {{File}} open ({{// NOSONAR}}) but retains no reference
to it, and {{releaseRetrievedFileResources}} (SmbOperations.java:416-425) only
closes the *stream* from the {{CamelSmbFileInputStream}} header. Net effect:
every file consumed with {{streamDownload=true}} leaks one open SMB file handle
on the session until the consumer disconnects or stops ({{disconnect=false}} is
the default, so for a long-running consumer that is effectively forever).
Handles grow without bound; consumed-and-deleted files stay in delete-pending
state (removal deferred until the last - leaked - handle closes); other clients
can hit sharing violations on files Camel has already finished with.
Additionally, the stream created during the retrieve step is dead on arrival:
{{retrieveFileToStreamInBody}} opens the smbj {{File}} in try-with-resources,
stores {{getInputStream()}} as body and in the header, then closes the {{File}}
when the block exits - any read of that stream fails with STATUS_FILE_CLOSED.
This is currently masked only by Problem 1 (the body access re-opens the file
and overwrites the header).
h3. History
The lazy {{getBody()}} fetch dates from the CAMEL-21277 WrappedFile rework
(4.9.0), when the consumer did not yet run the retrieve pipeline. CAMEL-21352
then added the {{download}}/{{streamDownload}}/{{localWorkDirectory}} retrieve
machinery on top without removing the lazy override, producing both the
duplicate download and the second (leaked) open.
h3. Suggested fix
Both defects have the same root - body handling split across {{retrieveFile}}
and {{getBody()}} - and should be fixed together:
* {{getBody()}} should return the body captured by {{retrieveFile}} (via the
binding, like FTP's {{RemoteFile}}) and only fall back to a lazy fetch when
there is none (e.g. {{poll}}/{{pollEnrich}} via {{GenericFilePollingConsumer}}).
* The streamDownload stream must be wrapped in a {{FilterInputStream}} whose
{{close()}} also closes the smbj {{File}}, so
{{releaseRetrievedFileResources}}/route completion actually releases the remote
handle; and the retrieve step must not close the handle backing a stream it
hands out.
----
_This issue was researched and filed by Claude Code on behalf of [~fmariani]
(GitHub: Croway), as part of a deep code review of camel-smb. Verified against
the generic-file framework call chain and the smbj 0.14.0 bytecode; observing
the duplicate transfer / leaked handle requires server-side instrumentation, so
no JUnit reproducer is attached for this one._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
