allthingssecurity opened a new pull request, #29126: URL: https://github.com/apache/camel/pull/29126
# Description [CAMEL-26465](https://issues.apache.org/jira/browse/CAMEL-26465) This change follows CAMEL-26420 (apache/camel#29044, merged), which rewrote the same lines of `JoinFunctionFactory`; it is one commit on main. `SimpleFunctionHelper.splitArguments` (used by `replace`, `substring`, `substringBefore`, `substringAfter`, `substringBetween`, `contains` and the collection functions) and the split of `join` used `StringQuoteHelper.splitSafeQuote(values, ',', false, true)`, which drops an empty argument and does not look at nesting: - an empty argument is dropped, so the next argument takes its place: `${replace(b,,${header.s})}` replaced `b` with the text `${header.s}` in the body, `${join(,id=)}` joined with the separator `id=`, `${join(&,,${body})}` used the text `${body}` as the prefix; - a comma inside a nested function splits the argument: `${replace(b,${header.s.substring(1,3)})}` replaced `b` with `3)}`, `${join(&,id=,${header.ids.subList(0,2)})}` and `${sort(${header.list.subList(0,2)},true)}` were refused, and `distinct` / `list` with such a value failed to parse; - `${join(,,)}` failed with `Index 0 out of bounds`, and `${join( )}` joined with the default separator (the arguments were checked with `isNotEmpty`, which trims). Where this differs from the JIRA text: the JIRA asks for the "Valid syntax" message of `join` for a malformed argument list, with `${join(,,)}` as the example. With the rule below `${join(,,)}` is not malformed (three arguments left out), so it is `${join()}`; four arguments still get the "Valid syntax" message. I chose the rule because it gives every function the same meaning for an empty argument, and an expression argument left out keeps meaning the body, as before. ## The fix - New `SimpleFunctionHelper.splitArgumentsAsWritten(String)`: splits at the commas outside quotes and outside parentheses and braces (as `indexOfClosingParenthesis` already skips nested functions and quotes), and keeps every argument as written, also an empty one. It follows the quote and nesting rules of `StringQuoteHelper.splitSafeQuote(values, ',', false, true, true)`; that method is not reused because it drops empty values, and it is used by many other callers (options, URIs, the value lists of `list` and `map`) that rely on that. `splitArguments` gives an argument left out (nothing between the commas) as `null`, so `''` stays the empty text. `splitArguments` is that plus the existing `trimArgument` per argument; `join` uses it for the arguments as written (so its separator is still kept as written). This covers the functions that use `splitArguments` (the string functions above and the collection functions that take arguments) and `join`. The other functions with arguments (s uch as `concat`, `pad`, `convertTo`, `iif`, `hash`, `assert`, `throwException` and the math functions) split their own way and are not changed here (a follow-up JIRA, CAMEL-YYYYY, covers them). - What an empty argument means: **it is the same as leaving the argument out where the function has a default for it, and it is refused where the function has none**: - defaults: an expression argument left out is the message body (new `SimpleFunctionHelper.expressionOrBody`), so `${replace(b,X,)}` still works on the body; the end of `substring` is `0`; the separator of `join` and of `split` is a comma (`${join(,,)}` is `${join()}`); the prefix of `join`, and the type of `setHeader` / `setVariable`, is none; - the empty text: the `to` of `replace` (`${replace(b,,${header.s})}` removes `b` from the header; `∅` still works); - refused with the "Valid syntax" message: the `from` of `replace`, the start of `substring`, the text of `contains`, `substringBefore`, `substringAfter` and `substringBetween`, the name and the value of `setHeader` / `setVariable`, the key and the value of `mapAdd` / `mapRemove`, the value of `listAdd` / `listRemove`, and the expressions of `forEach` / `filter`. Before, such an empty argument was dropped and the remaining ones moved, which changed their meaning silently: `${contains(xyz,)}` was `${contains(xyz)}`, `${setHeader(,x)}` was refused only by the count of arguments. - `join`: the arguments are split when there is any text (not `isNotEmpty`, which trims), so `${join( )}` joins with a space, as written (an unquoted separator is kept as written since CAMEL-26420); `${join('')}` joins with no separator. - `distinct`, `reverse`, `shuffle`, `list` and `map` split their values with `splitSafeQuote(values, ',', true, false, true)` (nested), so a nested function with a comma is one value; their empty values are dropped as before. ## Behaviour changes Upgrade guide 4.24, new section after the CAMEL-26312 one: "camel-core - Simple: an empty function argument is kept, and a comma in a nested function does not split". It names the changed inputs users are likely to have: - `${replace(b,,exp)}`, `${join(,prefix)}`, `${join(sep,,exp)}`: the argument left out now stays in its place (a quoted `''` is the empty text, as before); - `${join( )}`: joins with a space, as written (it joined with a comma); the same for a newline or tab separator (not named in the guide). `${split( )}` keeps its own handling and still splits at commas; - a trailing comma where the function reads its first argument as the text, such as `${contains(xyz,)}` or `${listAdd(x,)}`, and `${substring(,2)}`: refused (they were read as if the comma were not there); `${sort(true,)}` sorts the text `true`; - a comma after an unquoted `(` or `{` that is not closed before it no longer separates arguments: quote such an argument (`${replace('(',x)}`); - a comma after the last argument a function takes, such as `${replace(a,b,${body},)}`: refused (it was ignored); - a lone quote at the end of an argument is kept: `${replace(b,x')}` replaces with `x'` (it replaced with `x`). There is no option for the old splitting; the way back for each input is to write it without the empty argument, as the guide says. The Simple docs (`simple-functions.adoc`, and its copy in the catalog) say which functions this applies to and what an empty argument means. I evaluated every `${fn(...)}` with a comma, quote, `(` or `{` in its arguments in the repository (206 forms from docs, catalog, tests and other modules) on main, on the CAMEL-26420 base and on this branch: all give the same result, apart from the inputs this change is about. ## Tests camel-core, in the existing test classes (JUnit assertions, as they are): - `StringFunctionFactoryTest`: arguments left out of `replace`, `substring`, `substringBefore`, `substringAfter`, `substringBetween`, `contains`; a `from`, start or text left out refused, also a trailing comma after the last argument and an unquoted lone `(`; a quoted `''` is the empty text, and a lone quote at the end of an argument is kept; a nested function with a comma as an argument of each (also `replace`, whose `from` and `to` are text); a quoted `(`; - `JoinFunctionFactoryTest`: `${join(,id=)}`, `${join(&,,exp)}`, `${join(&,id=,)}`, `${join(,,)}`, `${join('')}`, `${join( )}`, a nested function with a comma, a quoted `(` in the prefix; four arguments refused (a control, refused before too); - `CollectionFunctionFactoryTest`: `sort`, `distinct`, `list`, `reverse`, `shuffle`, `map` and `setHeader` with a nested function with a comma; a type of `setHeader` and a separator of `split` left out, a quoted `''` value and separator; the arguments left out that are refused, also a trailing comma of `listAdd`, `mapAdd` and `filter`; an expression of `sort` and `listAdd` left out (a control: the body, as before), and `${sort(true,)}` sorting the text `true`; - `SimpleTest`: an argument left out, a nested function with a comma and a quoted `''`, through the whole Simple language. Against the CAMEL-26420 commit (its `SimpleFunctionHelper`, `JoinFunctionFactory`, `CollectionFunctionFactory` and `StringFunctionFactory` compiled over the branch), 10 of the 15 new tests fail; the 5 that pass there are controls (a quoted `''` and a quoted `(` as text, four arguments of `join` refused, a quoted `(` in the prefix of `join`, an expression left out is the body). - The three function test classes 20 times in one JVM: 1600 tests, 0 failures. - camel-core `org.apache.camel.language.**` and `builder.**`, plus `BeanParameterValueWithCommaTest` and `ExpressionBuilderEdgeCasesTest`: 1367 tests, 9 failures, all in `SimpleSyntaxHintsTest` (6) and the `testOGNLKeyOn*` tests of `SimpleTest` (3), which fail the same way on the CAMEL-26420 commit (they check hint texts that come from camel-bean, which my local build takes from an older jar). - Formatter and import-sort: no changes. # 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-26465) filed for the change (usually before you start working on it): [CAMEL-26465](https://issues.apache.org/jira/browse/CAMEL-26465). # 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 the core modules and ran the tests above, including the formatter and import-sort plugins. The catalog copy of the Simple docs is updated to match. 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]
