ramanathan1504 commented on code in PR #4373:
URL: https://github.com/apache/logging-log4j2/pull/4373#discussion_r4329213393
##########
log4j-jpa/src/main/java/org/apache/logging/log4j/core/appender/db/jpa/converter/ThrowableAttributeConverter.java:
##########
@@ -243,4 +270,32 @@ private Throwable getThrowable(final Class<Throwable>
throwableClass) {
return null;
}
}
+
+ private static boolean requiresEscaping(final String message) {
+ return message.contains("\\")
+ || message.contains("\n")
Review Comment:
A backslash alone does not need the prefix. `unescape` only runs after
`!LOG4J2!:`, so `C:\new` stored as is reads back as is. The prefix check below
still covers a message that starts with `!LOG4J2!:`.
```suggestion
return message.contains("\n")
```
These two cases fail without the prefix check and without the `\r` escape:
```java
@Test
void testEscapedMessageEdgeCasesRoundTrip() {
for (final String message : new String[] {"!LOG4J2!:C:\\new", "ends
with\r"}) {
final Throwable reversed =
this.converter.convertToEntityAttribute(
this.converter.convertToDatabaseColumn(new
IllegalStateException(message)));
assertNotNull(reversed);
assertEquals(message, reversed.getMessage());
}
}
```
##########
log4j-jpa/src/main/java/org/apache/logging/log4j/core/appender/db/jpa/converter/ThrowableAttributeConverter.java:
##########
@@ -134,13 +159,15 @@ private Throwable getThrowable(
final Throwable cause,
final StackTraceElement[] stackTrace) {
try {
- @SuppressWarnings("unchecked")
- final Class<Throwable> throwableClass = (Class<Throwable>)
LoaderUtil.loadClass(throwableClassName);
+ final Class<?> loadedClass = Class.forName(throwableClassName,
false, LoaderUtil.getClassLoader());
Review Comment:
`LoaderUtil.loadClass` used the thread context class loader, or the
`log4j-api` loader when `log4j2.ignoreTCCL` is set.
`LoaderUtil.getClassLoader()` returns the context loader only when it is a
child of the `log4j-api` loader. When it is not, an application exception class
is not found and the converter returns `null`. Can this keep the loader from
`loadClass` and change only the `initialize` flag?
--
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]
