oscerd commented on code in PR #27427:
URL: https://github.com/apache/camel/pull/27427#discussion_r4193588641
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/EndpointChecks.java:
##########
@@ -367,6 +367,17 @@ static void checkRegexOptions(List<String> errors, String
fullUri, int uriLineId
if (!name.equals("include") && !name.equals("exclude") ||
value.startsWith("{{")) {
continue;
}
+ String unquoted = withoutQuoteCharacters(value);
+ if (!unquoted.equals(value)) {
+ // \".*\\.json\" written as a plain YAML value: a backslash
before a quote is not YAML escaping, so the
+ // quotes and backslashes are part of the regex and no file
matches (CAMEL-25374)
+ errors.add(linePrefix(optionLineMap.getOrDefault(name,
uriLineIdx)) + fullUri.substring(0, colon) + ": "
+ + name + " has quote characters in its value (" +
value + "): in YAML a backslash before a"
+ + " quote is text, not escaping, and the quotes
become part of the regex. Write the line as "
Review Comment:
**The YAML-only hint recommends single quotes where they are part of the
regex (Java/XML DSL, inline YAML uri)**
`checkRegexOptions` is not YAML-only. Since CAMEL-25208,
`ModelChecks.endpoint()` calls `EndpointChecks.checkUri(..., uriForm = true)`
for Java and XML DSL routes (`camel validate`, `camel_validate_source`,
`camel_write_file`, the TUI), and `checkUri` passes `uriForm` to
`checkSimplePlaceholders`/`checkRequiredPathOptions` but not to
`checkRegexOptions`.
So `from("file:inbox?include='.*\\\\.json'")` (or `<from
uri="file:inbox?include='.*\\.json'"/>`) reaches this branch with the value
`'.*\\.json'` and is told to `Write the line as include: '.*\.json' (single
quotes, ...)`. In a uri the quotes are always part of the value, so following
the hint keeps `include='.*\.json'` and produces the same message again: the
loop this PR sets out to break, moved to Java/XML. An inline YAML uri (`uri:
file:inbox?include='.*\.json'`, no `parameters:` entry) gets the same YAML-line
advice.
Passing `uriForm` through would let the fix match the form, e.g.:
```java
// in checkUri: checkRegexOptions(errors, fullUri, lineIdx, optionLineMap,
uriForm);
String regex = unquoted.replace("\\\\", "\\");
String fix = !uriForm && optionLineMap.containsKey(name)
? "write the line as " + name + ": '" + regex + "' (in single
quotes)"
: "write " + name + "=" + regex + " in the uri, without quotes";
```
keeping the "in YAML a backslash before a quote is text" explanation for the
YAML form only. (The existing CAMEL-24854 hint, `write include='...'`, has the
same quote trap in uri form.) A `RouteAssistTest` case with a Java or XML uri
holding `include='.*\\.json'` would cover it.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/EndpointChecks.java:
##########
@@ -367,6 +367,17 @@ static void checkRegexOptions(List<String> errors, String
fullUri, int uriLineId
if (!name.equals("include") && !name.equals("exclude") ||
value.startsWith("{{")) {
continue;
}
+ String unquoted = withoutQuoteCharacters(value);
+ if (!unquoted.equals(value)) {
+ // \".*\\.json\" written as a plain YAML value: a backslash
before a quote is not YAML escaping, so the
+ // quotes and backslashes are part of the regex and no file
matches (CAMEL-25374)
+ errors.add(linePrefix(optionLineMap.getOrDefault(name,
uriLineIdx)) + fullUri.substring(0, colon) + ": "
+ + name + " has quote characters in its value (" +
value + "): in YAML a backslash before a"
+ + " quote is text, not escaping, and the quotes
become part of the regex. Write the line as "
+ + name + ": '" + unquoted.replace("\\\\", "\\") +
"' (single quotes, one backslash before"
+ + " the dot)");
+ continue;
Review Comment:
**The suggested regex is not validated, and "one backslash before the dot"
is fixed text**
Because of the `continue`, the unquoted value never goes through the
`Pattern.compile`/`toRegex` branch below, and the text in parentheses is fixed:
- A glob inside the quotes, `include: \"*.json\"`, gets `Write the line as
include: '*.json' (single quotes, one backslash before the dot)`. `*.json` does
not compile (`Dangling meta character '*'`), so the next round reports
`include=*.json is not a regular expression ... write include=.*\.json`: one
more round trip, which this check is meant to save.
- A value without `\.`, such as `\"^order_.*\"`, still gets "one backslash
before the dot", which invites turning `.*` into `\.*`.
Building the suggestion the way the branch below does would keep it valid:
```java
String regex = unquoted.replace("\\\\", "\\");
try {
Pattern.compile(regex);
} catch (PatternSyntaxException e) {
regex = toRegex(regex); // a glob in quotes: *.json -> .*\.json
}
```
and mention the backslash only when `regex.contains("\\.")`. A test with a
quoted glob would pin this down.
--
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]
