Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/3183#discussion_r237764346
--- Diff:
nifi-commons/nifi-record-path/src/main/java/org/apache/nifi/record/path/util/RecordPathUtils.java
---
@@ -39,4 +39,52 @@ public static String getFirstStringValue(final
RecordPathSegment segment, final
return stringValue;
}
+
+ /**
+ * This method handles backslash sequences after ANTLR parser converts
all backslash into double ones
+ * with exception for \t, \r and \n. See
+ * <a
href="file:../../../../../../../../../src/main/antlr3/org/apache/nifi/record/path/RecordPathParser.g">org/apache/nifi/record/path/RecordPathParser.g</a>
--- End diff --
@bdesert Thanks for the explanation. I read through your comment over and
over, and tested different input string. However, that differs from what what
I'm seeing.
I debugged how Lexer works, too, and found that:
- The `ESC` fragment handles an escaped special character in String
representation. I.e. String `\t` will be converted to actual tab character.
- The string values user input from NiFi UI are passed to
`RecordPath.compile` method as it is. E.g. the input string
`replaceRegex(/name, '\[', '')` is passed to as is, then the single back-slash
is converted to double back-slash by the ESC fragment line 155.
- Since the escaped characters such as `\n` are already unescaped by this
Lexer, the `RecordPathUtils.unescapeBackslash` method added by this PR will not
see any **escaped** characters other than the doubled back-slash `//`. And the
doubled back-slash is converted back to a single back-slash by
`RecordPathUtils.unescapeBackslash`.
- I believe the line 153-156 is aimed to preserve escaped characters as it
is, because such escape doesn't mean anything for the RecordPath/AttrExpLang
spec. And those should be unescaped later by underlying syntaxes such as RegEx.
- And current line 155 does it wrongly. It should append a single
back-slash..
- Other Lexers (AttributeExpressionLexer.g and HL7QueryLexer.g) have
the same issue.
- So, I think we should fix all Lexers instead of adding another conversion.
Here is the [Lexer
code](https://github.com/apache/nifi/blob/master/nifi-commons/nifi-record-path/src/main/antlr3/org/apache/nifi/record/path/RecordPathLexer.g#L143)
for reference:
```
143 fragment
144 ESC
145 : '\\'
146 (
147 '"' { setText("\""); }
148 | '\'' { setText("\'"); }
149 | 'r' { setText("\r"); }
150 | 'n' { setText("\n"); }
151 | 't' { setText("\t"); }
152 | '\\' { setText("\\\\"); }
153 | nextChar = ~('"' | '\'' | 'r' | 'n' | 't' | '\\')
154 {
155 StringBuilder lBuf = new StringBuilder();
lBuf.append("\\\\").appendCodePoint(nextChar); setText(lBuf.toString());
156 }
157 )
158 ;
```
Can share any input String value that a user would set into a processor
property text box (or passed to RecordPath.compile method), that will go the
proposed `RecordPathUtils.unescapeBackslash` method lines of code for `\n`,
`\r` or `\t`? Current test case does have regex test cases using those escaped
characters, but coverage shows those branches are not used:
---
