allthingssecurity commented on code in PR #26923:
URL: https://github.com/apache/camel/pull/26923#discussion_r4115130178
##########
core/camel-support/src/main/java/org/apache/camel/support/builder/TokenXMLExpressionIterator.java:
##########
@@ -374,6 +374,22 @@ private static Map<String, String>
toStringStringMap(String text) {
return namespaces;
}
+ private static int skipMarkupDeclaration(String xmlhead, int p) {
+ String end;
+ if (xmlhead.startsWith("<!--", p)) {
+ end = "-->";
+ } else if (xmlhead.startsWith("<![CDATA[", p)) {
+ end = "]]>";
+ } else {
+ // DOCTYPE which may have an internal subset with declarations of
its own
+ int bracket = xmlhead.indexOf('[', p);
+ int gt = xmlhead.indexOf('>', p);
+ end = bracket >= 0 && (gt < 0 || bracket < gt) ? "]>" : ">";
Review Comment:
Minor: XML allows whitespace between the `]` of the internal subset and the
closing `>` (`'[' intSubset ']' S? '>'`). With `<!DOCTYPE root [ ... ] >`, or
with the `>` on the next line, `indexOf("]>")` is -1, so the method returns
`xmlhead.length()`; `buildXMLTail` then records no tags and the wrapped parts
lose `</root>` (that case was already broken before, just differently).
Searching for `]`, skipping whitespace and then expecting `>` would cover it.
_Claude Code on behalf of allthingssecurity_
##########
core/camel-core-languages/src/main/java/org/apache/camel/language/tokenizer/TokenizeLanguage.java:
##########
@@ -71,32 +71,33 @@ public Expression createExpression(Expression source,
String expression, Object[
throw new IllegalArgumentException("The option includeTokens
requires endToken to be specified.");
}
- Expression answer = null;
+ Expression answer;
if (xml) {
answer = ExpressionBuilder.tokenizeXMLExpression(source, token,
inheritNamespaceTagName);
} else if (endToken != null) {
- answer = ExpressionBuilder.tokenizePairExpression(token, endToken,
includeTokens);
+ answer = ExpressionBuilder.tokenizePairExpression(source, token,
endToken, includeTokens);
+ } else if (regex) {
+ answer = ExpressionBuilder.regexTokenizeExpression(source, token);
+ } else {
+ answer = ExpressionBuilder.tokenizeExpression(source, token);
}
- if (answer == null) {
- // use the regular tokenizer
- if (regex) {
- answer = ExpressionBuilder.regexTokenizeExpression(source,
token);
- } else {
- answer = ExpressionBuilder.tokenizeExpression(source, token);
- }
- if (group == null && skipFirst) {
- // wrap in skip first (if group then it has its own skip first
logic)
- answer = ExpressionBuilder.skipFirstExpression(answer);
- }
+ if (group == null && skipFirst) {
+ // wrap in skip first (if group then it has its own skip first
logic)
+ answer = ExpressionBuilder.skipFirstExpression(answer);
}
// if group then wrap answer in group expression
if (group != null) {
if (xml) {
- answer = ExpressionBuilder.groupXmlIteratorExpression(answer,
group);
+ answer = ExpressionBuilder.groupXmlIteratorExpression(answer,
group, skipFirst);
} else {
- String delim = groupDelimiter != null ? groupDelimiter : token;
+ String delim = groupDelimiter;
+ if (delim == null) {
+ // the parts of a pair are joined without a delimiter (as
in xml mode), as the start token
+ // between them would make them look like an unfinished
pair
+ delim = endToken != null ? "" : token;
Review Comment:
`""` works for `includeTokens(true)`, as in the test and the upgrade guide.
But `includeTokens` defaults to `false`, and then the parts are the bare
values, so they now run together:
`tokenize().token("[").endToken("]").group(2)` on `[1][2][3]` gives `12`, `3`
(before: `1[2`, `3`), and the boundary is lost with no error. Could the `""`
default apply only when `includeTokens` is true? Or the upgrade guide could say
that with `includeTokens=false`, `groupDelimiter` should be set.
Reproducer (not run), e.g. in `TokenizeEdgeCasesTest`: split `[1][2][3]`
with `tokenize().token("[").endToken("]").group(2)` and check the bodies.
_Claude Code on behalf of allthingssecurity_
--
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]
