[
https://issues.apache.org/jira/browse/CAMEL-25615?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen resolved CAMEL-25615.
---------------------------------
Resolution: Fixed
> 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
> Priority: Major
> Fix For: 4.24.0
>
>
> {{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)
