shashank created CAMEL-25615:
--------------------------------
Summary: camel-bonita - startCase uploads a file input to the
Bonita server and user of the bonita endpoint that was used first in the JVM
instead of its own, and never deletes the temporary file it creates for each
upload
Key: CAMEL-25615
URL: https://issues.apache.org/jira/browse/CAMEL-25615
Project: Camel
Issue Type: Bug
Reporter: shashank
Assignee: shashank
{{bonita:startCase}} sends three requests per exchange when the body contains a
{{FileInput}}: the process lookup, the upload of the file, and the case
creation. Two defects are in the upload step, both in {{BonitaAPIUtil}}: one in
how the upload helper is obtained ({{getInstance}}), one in {{uploadFile}}
(line numbers at main b5a64d3cc981).
h3. 1. The upload uses the server and the user of the endpoint that was used
first
{{BonitaStartProducer.process}} (line 44) builds a {{BonitaAPI}} with a client
for the configuration of its own endpoint, and the lookup and the case creation
use it. For the file inputs {{BonitaAPI.startCase}} (lines 77-78) calls
{code:java}
BonitaAPIUtil.getInstance(bonitaApiConfig).prepareInputs(processDefinition,
rawInputs);
{code}
and {{BonitaAPIUtil.getInstance}} (lines 47-61) is a lazily created static
singleton:
{code:java}
private static BonitaAPIUtil instance;
public static BonitaAPIUtil getInstance(BonitaAPIConfig bonitaAPIConfig) {
if (instance == null) {
instance = new BonitaAPIUtil();
...
client.register(new BonitaAuthFilter(bonitaAPIConfig));
instance.setWebTarget(client.target(bonitaAPIConfig.getBaseBonitaURI()));
}
return instance;
}
{code}
The configuration argument is only used by the first call in the JVM. Every
later upload goes to the host and port of that first configuration, after a
login with its username and password, whatever endpoint the exchange is on.
With one bonita endpoint, or with several that have the same hostname, port,
username and password, nothing is wrong. With two endpoints that differ in one
of these, for example
{code:java}
from("direct:a").to("bonita:startCase?hostname=bonita-a&port=8080&processName=orders&username=userA&password=...");
from("direct:b").to("bonita:startCase?hostname=bonita-b&port=8080&processName=orders&username=userB&password=...");
{code}
and a first exchange on {{direct:a}}, an exchange with a file input on
{{direct:b}} looks the process up on bonita-b, uploads the file to bonita-a as
userA, and then creates the case on bonita-b with the {{tempPath}} that
bonita-a returned, a path of a file that bonita-b never received. Which
endpoint "wins" depends on which one happens to process the first exchange
after the JVM started.
The singleton is also assigned before its web target is set, and without
synchronisation, so a second thread that enters {{getInstance}} during the
first call can get a helper whose web target is still null. This was read from
the code and shown by a small model, not reproduced in a test.
h3. 2. A temporary file per upload is never deleted
{{BonitaAPIUtil.uploadFile}} (lines 72-76) creates a file with
{{Files.createTempFile("tempFile", ".tmp")}}, writes the content of the file
input to it, and uses only its name, for the {{filename}} of the multipart
part; the content that is sent comes from the byte array. The file is never
deleted (no {{delete}}, no {{deleteOnExit}}). Every file input of every
exchange leaves a copy of its content in {{java.io.tmpdir}}. This needs no
second endpoint: it happens with the default configuration for every uploaded
file.
h3. Reproduction
New {{BonitaStartCaseFileUploadTest}} (4 tests): a CamelContext with two
{{bonita:startCase}} endpoints that point to two WireMock servers (the module
already uses WireMock) and have different users. Each stub server answers the
login with its own session cookie and API token and returns its own
{{tempPath}} for an upload, and records its requests. With the code of main,
each test run in a JVM of its own:
* {{fileOfSecondEndpointIsUploadedToItsOwnServer}} (A first, then B) and
{{fileOfFirstEndpointIsUploadedToItsOwnServerWhenTheOtherEndpointWasUsedFirst}}
(B first, then A): {{file uploads among the requests received by the Bonita
server on port ... ==> expected: <1> but was: <0>}}, for the server of the
endpoint that was used second;
* {{temporaryFileOfTheUploadIsDeleted}} and
{{temporaryFileOfTheUploadIsDeletedWhenTheUploadFails}}: {{the temporary file
of the upload was left behind: .../tempFile12892494639172391283.tmp ==>
expected: <false> but was: <true>}}. These two tests also assert that the
upload is one multipart part named {{tempFile<digits>.tmp}} with the content of
the file input, and the second one that the exchange fails with
{{InternalServerErrorException}} when the server answers the upload with 500;
those assertions pass on main.
When the four tests run in one JVM on main, only the first one sees the helper
being created; the others then fail with {{jakarta.ws.rs.ProcessingException:
java.net.ConnectException}}, because their uploads go to the stub server of the
first test, which has been stopped. That is the same defect seen from a later
exchange. In a run of the whole module on main the helper may already have been
created by {{BonitaAPIUtilPrepareInputsTest}}, so there the failure messages
depend on the order of the test classes as well as on the order of the tests;
all four tests fail in either case.
h3. Proposed fix
{{BonitaAPIBuilder.build}} creates the upload helper from the client of the
{{BonitaAPI}} it builds (new {{BonitaAPIUtil.forClient(Client,
BonitaAPIConfig)}}), so the three requests of an exchange use the same server
and the same user, and no static state is left.
{{BonitaAPIUtil.getInstance(BonitaAPIConfig)}} is kept, but no longer returns a
shared instance: every call returns a new helper for the given configuration;
the existing protected constructor of {{BonitaAPI}} is kept and uses it. The
response filter that turns the {{text/plain}} answer of the upload into JSON is
registered on the upload target only, so the other requests are read as before.
{{uploadFile}} no longer writes the content to the temporary file (nothing
reads it back) and deletes the file in a {{finally}} block with
{{File.delete()}}, which does not throw, so a failing clean-up cannot replace
the exception of the upload. The empty file is still created, because its
generated name is the file name of the part; the multipart request is the same
as on main (compared at the stub server: only the random boundary and the
random digits of the name differ).
No change for a route with a single bonita endpoint, other than the temporary
files not being left behind. No upgrade-guide entry.
camel-bonita tests with the fix: 22 run, 0 failures, 0 skipped (18 existing, 4
new).
How it was found: a review of static mutable state that is initialised from
endpoint configuration, and a small TLA+ model (two endpoints, interleaved
startCase calls, {{getInstance}} statement by statement). For the code of main
TLC reports an upload that goes to the other endpoint's server, a case created
on a server that does not have its file, a helper without web target, and a
temporary file left after the last call; with the fix all four properties hold.
The model is only a sketch of the code.
Not run: nothing was run against a Bonita server ({{BonitaProducerManualIT}} is
disabled and needs a local Bonita). What a Bonita server does with a case whose
file input names a {{tempPath}} it does not know, and whether an upload made by
one user can be used in a case started by another user of the same server, was
not checked; the test only shows which server and which user each request is
sent with. How often two differently configured bonita endpoints are used in
one JVM is not known; the temporary files are left for every file input.
Affected: main (b5a64d3cc981, tested). camel-4.23.x, camel-4.22.1, camel-4.18.4
and camel-4.14.9 have the same lines ({{BonitaAPIUtil}} 47, 51, 72;
{{BonitaAPI}} 77; {{BonitaStartProducer}} 44): read, not run.
Duplicate check (2026-10-13): JIRA text search "bonita" (9 issues: CAMEL-10350,
CAMEL-10858, CAMEL-11383, CAMEL-11845 and five that only mention the
component), none about the upload helper or the temporary file; GitHub pull
requests of apache/camel with "bonita" (9, among them #4455, which closed the
stream of the temporary file but did not delete the file), "BonitaAPIUtil",
"bonita upload", "bonita temp file": none.
_Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
