allthingssecurity opened a new pull request, #28715: URL: https://github.com/apache/camel/pull/28715
# Description [CAMEL-26312](https://issues.apache.org/jira/browse/CAMEL-26312) `replace`, `substring`, `substringBefore`, `substringAfter`, `substringBetween` and `contains` split their arguments with `StringQuoteHelper.splitSafeQuote(values, ',', false)`, which keeps the white space around a value, also outside its quotes. A space after the comma became part of the next argument: ``` body: Hello World ${replace('o', '0')} -> Hell 0 W 0rld (replaced with " 0") ${replace('o', '0', ${header.foo})} -> " Hell 0 W 0rld" (the expression is " ${header.foo}") ${substringBetween(${body}, ' ', 'o')} -> null (looks for " " and " o") ${contains(${body}, 'hello')} -> false ${substring(1, -1)} -> TypeConversionException (" -1") ``` The examples of the replace function on the simple-advanced page ("Replacing double and single quotes", in Java, XML and YAML) write a space around the comma, and none of them works, since `replace` was added in 4.7.0 with these examples (CAMEL-20845): ``` body: say "hi" it's ${replace(" , ')} -> say "hi 'it's (expected: say 'hi' it's) ${replace(' , ")} -> say "hi" it's (expected: say "hi" it"s) ${replace(" , ∅)} -> say "hi ∅it's ``` The collection functions had the same bug, and CAMEL-24969 (4.23.0, part of #26806) fixed it for them with `CollectionFunctionFactory.splitArguments` ("a space after a comma no longer leaks into the argument: `${sort(${body}, true)}`, mapAdd, listAdd, ..."); the string functions were not in that list. Other functions, such as `escape`, `concat` and `pad`, already take an argument without the white space around it (`${escape(html, 'Tom & Jerry')}` on the functions page). Changes: - The `splitArguments` helper of CAMEL-24969 moves, unchanged, from `CollectionFunctionFactory` to `SimpleFunctionHelper`, and the six string functions use it too. It splits the arguments keeping their quotes (`splitSafeQuote(values, ',', false, true)`), then takes each one without the white space around it and without the quotes around it. The spaces inside quotes are kept (`' '`), and an argument of only white space is kept as it is, so `${replace( ,-,${header.msg})}`, which `StringFunctionFactoryTest.testReplaceWithExpression` pins, still replaces a space. That test is why the split cannot simply trim every argument (`splitSafeQuote` with trim=true would make that argument empty). - Only the quotes around a whole argument are removed, as in the collection functions: a quote inside an argument (`a'b'c`, `'it''s'`) is now kept; it was removed before. - Upgrade guide (4.24): `=== camel-core-languages - Simple: a space after a comma is not part of a function argument`. Not changed: `join` (`JoinFunctionFactory`) splits its arguments the same way as these functions did, so `${join(&, id=)}` gives ` id=A& id=B& id=C`; it is left for a follow-up JIRA, as its separator can be white space on purpose (`${join(; ,'')}` gives `A; B; C` today). Which routes see a difference: in apache/camel (main, code, tests, docs) and camel-kamelets 4.23.0 the six functions are written in 97 distinct ways; the arguments split differently for 4 of them, the documented examples above (in their Java/YAML and XML spellings), and the same for the other 93. Tests: - New `SimpleTest.testReplaceAllExpressionWithSpacesAroundTheComma` (the three documented examples, with a space after the comma, a nested expression), `StringFunctionFactoryTest.testArgumentsWithASpaceAfterTheComma` (each of the six functions, spaces inside quotes kept) and `StringFunctionFactoryTest.testQuoteInsideAnArgumentIsKept`. All three fail on main and pass with the change; the existing replace tests, including the bare space, and `CollectionFunctionFactoryTest` pass on both. - Run offline on main as of e4c4dbbeb5da (the date of the local snapshot jars of the other core modules; `StringFunctionFactory`, `SimpleFunctionHelper` and `CollectionFunctionFactory` are the same there as on the base of this branch) with this change: the language tests of camel-core and camel-core-languages (`org/apache/camel/language/**/*Test`, 884 run, 1 skipped) pass, except 8 `SimpleTest` / `SimpleSyntaxHintsTest` methods that came with those test files from later commits to other files and fail the same without this change. The whole camel-core suite, run with this change and the one of the camel-simple-predicate-in-braces-operators branch, gives the same picture (8259 tests, 54 skipped; those 8, and `ValidatorExternalResourceTest`, which needs the network). The formatter and import-sort plugins ran and changed nothing. Backport: main only (users can notice the change). The same split is in every release since 4.7.0 (`replace`, `substring`) and 4.18.0 (`substringBefore`, `substringAfter`, `substringBetween`, `contains`); up to 4.20.x it is in `SimpleFunctionExpression`, and the helper is from 4.23.0, so a backport would not be a cherry-pick. How it was found: a readable semantics of the Simple language written from its documentation (Rocq), extracted to an oracle and compared with the language on about 4.9 million generated programs. This class gave about 8 200 of the disagreements; the same programs agree with this change. The from and to texts that `StringFunctionFactory` gives the replace function are also checked on the real class files of main and of this change in a JVM model in Rocq (kernel-checked). # 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-26312) filed for the change (usually before you start working on it): [CAMEL-26312](https://issues.apache.org/jira/browse/CAMEL-26312). # 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 ran the tests above with the formatter and import-sort plugins; no option or catalog metadata changed. 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]
