allthingssecurity opened a new pull request, #26908: URL: https://github.com/apache/camel/pull/26908
# Description [CAMEL-25033](https://issues.apache.org/jira/browse/CAMEL-25033) The method option can bind parameters from Simple expressions, for example `bean(MyService.class, "process(${body})")` or `to("bean:foo?method=check(${header.ETag})")`. `MethodInfo.ParameterExpression` evaluates every parameter with the Simple language and then post-processes the **result**. It removed the leading and ending quotes of any `String` result, and turned the result `"null"` into a `null` parameter. That handling is meant for the text of the method name. `bean-binding.adoc` says a quoted String such as `'World'` is passed "without quotes", and the parameter `null` passes `null`. Because it ran on the evaluated value, it also changed data from the message: ```java from("direct:etag").bean(MyBean.class, "echo(${header.v})"); // header v = "33a64df5" (an ETag, with the quotes) -> echo receives 33a64df5 // body "ACME, Inc.","42" with echo(${body}) -> echo receives ACME, Inc.","42 // header v = the text null -> echo receives a Java null ``` Before changing it I checked whether stripping evaluated values was intended: - History: the quote removal came with CAMEL-3961 (2011), which added parameter values to the method name. The commit's comment reads "we need to unquote String parameters, as the enclosing quotes is there to denote a parameter value", which is about the quotes of the literal syntax. The `"null".equals(evaluated)` check was already there, so a header with the text `null` was passed as `null` from the start. CAMEL-6687 (2013) then made an expression that evaluates to `null` pass `null` by returning the marker string `"null"`, which reuses the same check, so the collision with the text `null` from the message is older than CAMEL-6687. Until CAMEL-19098 (fixed in 3.18.6, 3.20.3 and 3.21/4.0), `splitSafeQuote` removed the literal's quotes while splitting. Since then it keeps them, and the removal after the evaluation is what unquotes literals. - Docs: `bean-binding.adoc` lists the quoted String and `null` as rules for parameter values written in the method option, and describes `${body}`/`${header.high}` as binding the body or header. It says nothing about changing the value of an expression. The bean component, language and EIP docs don't mention quotes either. - Tests: no existing test expects an evaluated value to lose its quotes or the text `null` to become `null`. `BeanParameterMatchPerformanceIssueTest` (`myMethod("'fast'")` -> `'fast'`), `BeanParameterValueTest`, `BeanParameterInvalidValueTest` (`echo(null, 2)`), `BeanOgnlBodyMethodReturnNullValueTest` (`${body.foo}` that is `null`) and `BeanMethodValueWithCommaTest` cover literals and a `null` result, and they still pass. This change, in `MethodInfo.evaluateParameterValue`: - The parameter `null` returns `null` before it is evaluated. An expression that evaluates to `null` still passes `null`, using a real `null` check instead of the `"null"` marker. - The quotes are removed after the evaluation only when the parameter text itself is quoted (`'World'`, `"World"`, `'${header.v}'`). Literals give exactly the same values as before. - Values from an expression are passed as-is. - `bean-binding.adoc` says so. The 4.23 upgrade guide has an entry, because a method can now receive a different value than before (quotes kept, or the text `null` instead of `null`). Tests: `BeanParameterValueFromExpressionTest` checks `echo(${header.v})` (Java DSL and `bean:` URI) with `"33a64df5"`, `'abc'` and ` 'x' `, `echo(${body})` with a CSV line and `''`, `two(${header.a}, ${header.b})` with `'x'` and `"y"`, and the text `null`. It also has controls that pass on main as well: a missing header still passes `null`, and `echo('World')`, `echo("World")`, `echo(null)`, `echo('null')` and `echo('${header.v}')` behave as before. Without the main-code change four of the five tests fail: ``` testQuotedHeaderValue expected: <["33a64df5"]> but was: <[33a64df5]> testQuotedBody expected: <["ACME, Inc.","42"]> but was: <[ACME, Inc.","42]> testQuotedHeaderValues expected: <['x'|"y"]> but was: <[x|y]> testNullText expected: <[null]> but was: <<null>> ``` With the change, `*Bean*,*MethodCall*,SimpleTest` passes in camel-bean (9), camel-core (700) and camel-console (9): 718 tests, 0 failures. I found this with a Lean model of the parameter binding. It proves that for every string `w`, the values `"w"` and `'w'` from `${...}` are bound as `w`, so `w` and `'w'` can't be told apart. It also proves that values the old code left unchanged, and literal parameters, were already bound correctly, so the defect is exactly quoted data and the text `null`. The model also shows that deciding on the parameter text, as this change does, meets the documented behavior. I then reproduced the bug against the real classes. Property-based tests (jqwik) fail on main for every quoted header value (shrunk to `''`) and for random values over quotes and the letters of `null`. With this change they pass: 300 quoted values, 2000 values over that alphabet, and 500 plain values. Not changed here: `splitSafeQuote` splits the method parameters at every comma outside quotes, including commas inside `${...}`, so `echo(${body.substring(0, 2)})` fails to parse. That is a separate problem in the same parser. The jqwik property for it still fails, and it is left for its own change. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally 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 - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
