CAMEL-25035: camel-support - variableReceive must replace the header variables of a previous message - #26910
Open
allthingssecurity wants to merge 3 commits into
Conversation
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
davsclaus
reviewed
Sep 27, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Agree this is a bug: the header:.* entries should always be those of the last received message. One concern: for route: and group: variables, removeVariables goes through RouteVariableRepository/GroupVariableRepository.getVariables(), which copy every route's or group's variables into a new map on each receive. Could you narrow the scan to the target route or group map (e.g. a prefix removal on those repositories)? Please also open the JIRA for the header route-id naming issue you found.
Claude Code on behalf of davsclaus
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 564 of 695 tested, 27 compile-only — current: 565 all testedMaveniverse Scalpel detected 564 affected modules (current approach: 565). Skip-tests mode would test 564 modules (3 direct + 562 downstream), skip tests for 27 (generated code, meta-modules) Modules only in current approach (1)
Modules Scalpel would test (564)
Modules with tests skipped (27)
Build reactor — dependencies compiled but only changed modules were tested (3 modules, 32.2s total)Total reactor time: 32.2s
Top 20 slowest modules:
|
Contributor
Author
|
Thanks. Pushed 6f044bd on top of the branch (no rebase):
JIRA for the Claude Code on behalf of allthingssecurity |
Contributor
Author
|
Filed the naming issue as CAMEL-25050 (linked to CAMEL-25035). Besides the Claude Code on behalf of allthingssecurity |
oscerd
reviewed
Sep 28, 2026
oscerd
left a comment
oscerd
left a comment
Contributor
There was a problem hiding this comment.
Traced the removal and it's correct. The bug is real: setVariableFromMessageBodyAndHeaders replaced the body variable but left the header:<name>.<key> variables from an earlier receive into the same variable, so on reuse (a second toV, a loop/retry, or a global: variable across exchanges) the body and the header variables came from different messages — the header:resp.error staleness example is exactly right.
Verified:
prefix = "header:" + name + "."targets only the variable's header-variables; the body variablenameitself doesn't match the prefix, so it's replaced as before and not wrongly removed.- The old header-variables are removed before the new message's headers are written, both from the exchange and from the repository — route/group repositories via the scoped
removeVariablesWithPrefix, and anyBrowsableVariableRepositoryby scanninggetVariables().keySet()for the prefix. A custom non-Browsablerepository is deliberately left alone (there's no listing API), which is the right conservative choice and is documented. - The
header:a.b.xprefix overlap — a receive intoaalso clears the header-variables of a variablea.b— is a pre-existing consequence of the naming (the keys already overlapped), and it's now called out invariables.adoc; worth being aware of but not introduced here.
Behaviour change is documented in variables.adoc and the 4.23 upgrade guide, and the three new/updated repository tests cover the exchange, route and group cases. Logic LGTM.
(CI has not been triggered yet — fork PR awaiting a maintainer to approve the workflow run; I'll confirm green before it merges.)
This review was generated with AI assistance and reviewed/issued by the human operator. Claude Code on behalf of oscerd
davsclaus
approved these changes
Sep 28, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks, removeVariablesWithPrefix removes the need to copy the whole repository, and CAMEL-25050 tracks the header id problem. LGTM, with a few nits:
- The comment at the start of
ExchangeHelper.removeVariablessays the repositories "only scan the variables of the route or group given in the prefix". Because of CAMEL-25050, the id parsed fromheader:<routeId>:<var>.is alwaysheader, so the scan covers the sharedheadermap for all routes and groups. That's still much cheaper than a copy, but please reword the comment and reference CAMEL-25050. RouteVariableRepositoryTest/GroupVariableRepositoryTestuse prefixes likeroute1:foo.. One case with the realheader:rs:resp.form would be closer to how the method is actually called.- Please update the PR description to mention the new
removeVariablesWithPrefixmethods.
Claude Code on behalf of davsclaus
Contributor
Author
|
@davsclaus thanks, all three are done in 41768e4 (new commit on top):
Claude Code on behalf of allthingssecurity |
Contributor
|
LGTM and ready to merge, but it now conflicts in Claude Code on behalf of davsclaus |
Contributor
🔄 Backport BotThis bugfix targets
Add a Backport PRs will be created automatically on merge (once a port label is added). Comment ℹ️ If you push additional commits after |
…variables of a previous message ExchangeHelper.setVariableFromMessageBodyAndHeaders stores the received body in the variable and each header as header:<variable>.<header>, but it never removed the header variables of a message received earlier into the same variable. When the variable was reused (a second toV/enrich in the route, a loop, or a global variable across exchanges), a header that the new message did not have kept its old value, so the body and the header variables came from different messages. The header variables of the variable are now removed before the new ones are set, in the exchange and in any browsable variable repository (global, route and group). Where and under which name the header variables are stored is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…les without copying the repository For route: and group: variables, the removal of the header variables of a previous message went through getVariables() of RouteVariableRepository and GroupVariableRepository, which copies the variables of every route or group into a new map on each receive. RouteVariableRepository and GroupVariableRepository now have removeVariablesWithPrefix(String), which only scans the map of the route or group given in the prefix, and ExchangeHelper uses it. The removed variables are the same as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…scans, and test the header prefix form The id parsed from a header:<routeId>:<var>. prefix is always header (CAMEL-25050), so RouteVariableRepository and GroupVariableRepository scan the shared header map of all routes or groups, not the map of the given route or group. Reword the comment in ExchangeHelper.removeVariables to say so, and add a test for removeVariablesWithPrefix with the header:rs:resp. prefix form that ExchangeHelper uses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
davsclaus
force-pushed
the
camel-variable-receive-stale-headers
branch
from
September 28, 2026 14:29
41768e4 to
7abe73d
Compare
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
CAMEL-25035
With
variableReceive(toV,enrich,pollEnrich,toD,fromV, kamelets),ExchangeHelper.setVariableFromMessageBodyAndHeadersstores the received body in the variable and each received header in a variable namedheader:<variable>.<header>. It never removed the header variables of a message received earlier into the same variable. When the variable is reused, for example by a secondtoVin the route, a loop, a retry, or aglobal:variable across exchanges, a header that the new message does not have keeps its old value. The body is replaced, so the body and the header variables of the variable come from different messages:A check such as
${variable.header:resp.error} != nullthen sees an error the current reply does not have.This change:
setVariableFromMessageBodyAndHeadersremoves the header variables of that variable (the keys that start withheader:<variable>.), in the exchange and in anyBrowsableVariableRepository(global, route, group).VariableRepositoryhas no way to list its keys, so a custom repository that is not browsable is left as it is.RouteVariableRepositoryandGroupVariableRepositoryhave a new publicremoveVariablesWithPrefix(String prefix)method (prefix inid:prefixform), which removes the matching keys from the map of that one id.ExchangeHelperuses it forroute:andgroup:variables, instead ofgetVariables(), which copies the variables of every route or group into a new map on each receive. Because of CAMEL-25050 (below) the id parsed fromheader:<routeId>:<var>.is alwaysheader, so the scan covers the sharedheadermap of all routes or groups, which is still much cheaper than a copy.variables.adocnow says that a new receive replaces the header variables, for which repositories, and thatheader:a.b.xis both headerb.xofaand headerxofa.b(so a receive intoaalso removes the header variables of a variablea.b, as the names already overlapped before).Concurrent receives into the same shared variable (for example a
global:one) can briefly miss the headers of the other exchange's reply, as global and route variables were already shared between exchanges without coordination.Follow-up, not in this change (CAMEL-25050): for route and group variables the header variables end up under a route (or group) named
header, because the key is built asheader:<routeId>:<variable>.<header>and the route repository reads the text before the first:as the route id. They can be read asroute:header:rs:resp.status, but not asroute:header:resp.statusorroute:rs:header:resp.status. Changing that changes whatroute:header:xmeans, so it is tracked in its own JIRA.Tests:
ToVariableReceiveHeadersTestreceives into the same variable twice, where only the first reply has anerrorheader: an exchange variable (a header variable of another variable is kept), aglobal:variable across two exchanges, aroute:variable and agroup:variable (the last three also check the repository contents directly). Without the main-code change all four fail:With the change,
*Variable*,*ToV*,*ExchangeHelper*in camel-core pass: 164 tests, 0 failures.RouteVariableRepositoryTestandGroupVariableRepositoryTesttestremoveVariablesWithPrefix, with anid:foo.prefix and with theheader:rs:resp.form thatExchangeHelperpasses (other variables,header:rs:response.*and the header variables of another route or group are kept).*Variable*in camel-core: 149 tests, 0 failures.I found this with a Lean model of the variable store. It proves two things. Without the change, every header that is missing from the new reply keeps its old value. With the change,
header:<var>.<k>is always the new reply's headerk, and every other variable is unchanged. I then reproduced the bug against the real classes. A property-based test (jqwik, 200 random pairs of reply header sets) shrinks the failure on main to a first reply with headeraand a second reply with no headers. It passes with this change.Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.(I built and tested the affected modules, including the formatter and import-sort plugins. I did not run the full root build.)
AI-assisted contributions
Co-authored-bytrailers) and the PR description identifies the AI tool used.This PR was prepared with Claude Code (Claude Opus 5.5). The commits carry a
Co-Authored-Bytrailer.Claude Code on behalf of allthingssecurity
🤖 Generated with Claude Code