#2283 Escape special characters in subject #2284
Conversation
For example, in case of new line in subject the report schema is broken and all the report starts look ugly. We need to escape characters before the line length computing.
| case '\n' -> builder.append("\\n"); | ||
| case '\f' -> builder.append("\\f"); | ||
| case '\b' -> builder.append("\\b"); | ||
| default -> builder.append(symbol); |
There was a problem hiding this comment.
@Au6ojlut Maybe we should also escape all invisible characters below 32 (space)?
There was a problem hiding this comment.
Agree, I'll fix it
| while (!stack.isEmpty() && stack.peekFirst().getEndTime() <= event.getStartTime()) | ||
| var eventWithEscapedSubject = new LogEventWithNestingLevel.SimpleLogEvent( | ||
| event.getElement(), | ||
| escape(event.getSubject()), |
There was a problem hiding this comment.
At a first glance, the good moment to escape characters is not the construction of SimpleLogEvent, but when we render SimpleLogEvent. In other words, when we generate the resulting report. Then we could escape all fields, not only subject.
There was a problem hiding this comment.
But we need to know the max size of a cell of the report table. If the calculation was made before the escape was applied then the final max size may be incorrect.
There was a problem hiding this comment.
Since element and subject are just strings, I believe, that they are the only place where special characters can appear. So, it's enough to escape them, not all the report line/row.
But, I also didn't find a case when the element could contain some special characters (only some synthetic example), that's why I left it as is, but it can be easily fixed.
Also escape '\u00A0' aka "no-break space"
|
Kudos, SonarCloud Quality Gate passed!
|
For example, in case of new line in subject the report schema is broken and all the report starts look ugly.
We need to escape characters before the line length computing.
Proposed changes
Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request.
If it fixes a bug or resolves a feature request, be sure to link to that issue.
Checklist
gradlew check chrome_headless firefox_headlesscommand