You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2022/06/13 12:38:59 UTC
[sling-org-apache-sling-commons-log] branch master updated: SLING-11393 : Escape characters in log entries
This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-commons-log.git
The following commit(s) were added to refs/heads/master by this push:
new 6b1e837 SLING-11393 : Escape characters in log entries
6b1e837 is described below
commit 6b1e8372cc0ecfc4bec8b8749dc317533aeb2b2f
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Mon Jun 13 14:38:53 2022 +0200
SLING-11393 : Escape characters in log entries
---
.../commons/log/logback/internal/LogConfig.java | 4 +-
.../log/logback/internal/LogConfigManager.java | 10 +-
.../log/logback/internal/MaskingMessageUtil.java | 212 +++++++++++++++++++++
.../stacktrace/OSGiAwareExceptionHandling.java | 5 +-
.../log/logback/internal/TestLogConfig.java | 38 ++++
5 files changed, 257 insertions(+), 12 deletions(-)
diff --git a/src/main/java/org/apache/sling/commons/log/logback/internal/LogConfig.java b/src/main/java/org/apache/sling/commons/log/logback/internal/LogConfig.java
index ea13ce9..c18cbe5 100644
--- a/src/main/java/org/apache/sling/commons/log/logback/internal/LogConfig.java
+++ b/src/main/java/org/apache/sling/commons/log/logback/internal/LogConfig.java
@@ -145,10 +145,11 @@ public class LogConfig {
}
}
- PatternLayout pl = new PatternLayout();
+ final PatternLayout pl = new PatternLayout();
pl.setPattern(logBackPattern);
pl.setOutputPatternAsHeader(false);
pl.setContext(loggerContext);
+ MaskingMessageUtil.setMessageConverter(pl);
if (postProcessor != null) {
pl.setPostCompileProcessor(postProcessor);
@@ -171,5 +172,4 @@ public class LogConfig {
public interface LogWriterProvider {
LogWriter getLogWriter(String writerName);
}
-
}
diff --git a/src/main/java/org/apache/sling/commons/log/logback/internal/LogConfigManager.java b/src/main/java/org/apache/sling/commons/log/logback/internal/LogConfigManager.java
index 1f153f6..8bb560a 100644
--- a/src/main/java/org/apache/sling/commons/log/logback/internal/LogConfigManager.java
+++ b/src/main/java/org/apache/sling/commons/log/logback/internal/LogConfigManager.java
@@ -32,7 +32,6 @@ import java.util.concurrent.ConcurrentHashMap;
import ch.qos.logback.classic.ClassicConstants;
import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.LoggerContext;
-import ch.qos.logback.classic.encoder.PatternLayoutEncoder;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.Appender;
import ch.qos.logback.core.ConsoleAppender;
@@ -240,12 +239,7 @@ public class LogConfigManager implements LogbackResetListener, LogConfig.LogWrit
appender.setName(DEFAULT_CONSOLE_APPENDER_NAME);
appender.setContext(loggerContext);
- PatternLayoutEncoder encoder = new PatternLayoutEncoder();
- encoder.setPattern(LOG_PATTERN_DEFAULT);
- encoder.setContext(loggerContext);
- encoder.start();
-
- appender.setEncoder(encoder);
+ appender.setEncoder(MaskingMessageUtil.getDefaultEncoder(loggerContext));
appender.start();
return appender;
@@ -274,7 +268,7 @@ public class LogConfigManager implements LogbackResetListener, LogConfig.LogWrit
configuredAppenders = Collections.emptyMap();
}
- Map<Appender, LoggerSpecificEncoder> encoders = new HashMap<Appender, LoggerSpecificEncoder>();
+ final Map<Appender<ILoggingEvent>, LoggerSpecificEncoder> encoders = new HashMap<>();
Set<String> configPids = new HashSet<>();
for (LogConfig config : getLogConfigs()) {
diff --git a/src/main/java/org/apache/sling/commons/log/logback/internal/MaskingMessageUtil.java b/src/main/java/org/apache/sling/commons/log/logback/internal/MaskingMessageUtil.java
new file mode 100644
index 0000000..64af8e9
--- /dev/null
+++ b/src/main/java/org/apache/sling/commons/log/logback/internal/MaskingMessageUtil.java
@@ -0,0 +1,212 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.commons.log.logback.internal;
+
+import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.PatternLayout;
+import ch.qos.logback.classic.pattern.EnsureExceptionHandling;
+import ch.qos.logback.classic.pattern.ExtendedThrowableProxyConverter;
+import ch.qos.logback.classic.pattern.MessageConverter;
+import ch.qos.logback.classic.pattern.RootCauseFirstThrowableProxyConverter;
+import ch.qos.logback.classic.pattern.ThrowableProxyConverter;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.classic.spi.IThrowableProxy;
+import ch.qos.logback.classic.spi.StackTraceElementProxy;
+import ch.qos.logback.core.Context;
+import ch.qos.logback.core.encoder.Encoder;
+import ch.qos.logback.core.pattern.Converter;
+import ch.qos.logback.core.pattern.ConverterUtil;
+import ch.qos.logback.core.pattern.PatternLayoutEncoderBase;
+
+/**
+ * Converter util to mask certain characters in messages
+ */
+public class MaskingMessageUtil extends MessageConverter {
+
+ /**
+ * Set the message converter for the layout
+ * @param pl The layout
+ */
+ public static void setMessageConverter(final PatternLayout pl) {
+ // need to overwrite all converter for messages and exceptions
+ // see https://logback.qos.ch/manual/layouts.html
+ pl.getInstanceConverterMap().put("m", MaskingMessageConverter.class.getName());
+ pl.getInstanceConverterMap().put("msg", MaskingMessageConverter.class.getName());
+ pl.getInstanceConverterMap().put("message", MaskingMessageConverter.class.getName());
+
+ pl.getInstanceConverterMap().put("ex", MaskingThrowableProxyConverter.class.getName());
+ pl.getInstanceConverterMap().put("exception", MaskingThrowableProxyConverter.class.getName());
+ pl.getInstanceConverterMap().put("rEx", MaskingRootCauseFirstThrowableProxyConverter.class.getName());
+ pl.getInstanceConverterMap().put("rootException", MaskingRootCauseFirstThrowableProxyConverter.class.getName());
+ pl.getInstanceConverterMap().put("throwable", MaskingThrowableProxyConverter.class.getName());
+
+ pl.getInstanceConverterMap().put("xEx", MaskingExtendedThrowableProxyConverter.class.getName());
+ pl.getInstanceConverterMap().put("xException", MaskingExtendedThrowableProxyConverter.class.getName());
+ pl.getInstanceConverterMap().put("xThrowable", MaskingExtendedThrowableProxyConverter.class.getName());
+
+ // override post processor for ensuring exception handling
+ pl.setPostCompileProcessor(new MaskingEnsureExceptionHandling());
+ }
+
+ /**
+ * Create an encoder with the default pattern
+ * @param loggerContext Logging context
+ * @return The encoder
+ */
+ public static Encoder<ILoggingEvent> getDefaultEncoder(final Context loggerContext) {
+ final PatternLayoutEncoderBase<ILoggingEvent> encoder = new MaskingPatternLayoutEncoder();
+ encoder.setPattern(LogConfigManager.LOG_PATTERN_DEFAULT);
+ encoder.setContext(loggerContext);
+ encoder.start();
+ return encoder;
+ }
+
+ /**
+ * Replace any carriage returns and line feeds with an underscore
+ * @param msg The message
+ * @return converted string
+ */
+ static String mask(final String msg) {
+ if ( msg == null ) {
+ return null;
+ }
+ return msg.replace('\n', '_').replace('\r', '_');
+ }
+
+ public static final class MaskingMessageConverter extends MessageConverter {
+ @Override
+ public String convert(final ILoggingEvent event) {
+ return mask(super.convert(event));
+ }
+ }
+
+ public static final class MaskingThrowableProxyConverter extends ThrowableProxyConverter {
+ @Override
+ protected String throwableProxyToString(final IThrowableProxy tp) {
+ return super.throwableProxyToString(new MaskingThrowableProxy(tp));
+ }
+ }
+
+ public static final class MaskingRootCauseFirstThrowableProxyConverter extends RootCauseFirstThrowableProxyConverter {
+ @Override
+ protected String throwableProxyToString(final IThrowableProxy tp) {
+ return super.throwableProxyToString(new MaskingThrowableProxy(tp));
+ }
+ }
+
+ public static class MaskingExtendedThrowableProxyConverter extends ExtendedThrowableProxyConverter {
+ @Override
+ protected String throwableProxyToString(final IThrowableProxy tp) {
+ return super.throwableProxyToString(new MaskingThrowableProxy(tp));
+ }
+ }
+
+ public static final class MaskingThrowableProxy implements IThrowableProxy {
+ private final IThrowableProxy proxied;
+
+ public MaskingThrowableProxy(final IThrowableProxy proxied) {
+ this.proxied = proxied;
+ }
+
+ @Override
+ public String getMessage() {
+ return mask(proxied.getMessage());
+ }
+
+ private IThrowableProxy getProxy(final IThrowableProxy p) {
+ if ( p == null ) {
+ return null;
+ }
+ if ( p == proxied || p == this ) {
+ return this;
+ }
+ return new MaskingThrowableProxy(p);
+ }
+
+ @Override
+ public IThrowableProxy getCause() {
+ return getProxy(proxied.getCause());
+ }
+
+ @Override
+ public String getClassName() {
+ return proxied.getClassName();
+ }
+
+ @Override
+ public StackTraceElementProxy[] getStackTraceElementProxyArray() {
+ return proxied.getStackTraceElementProxyArray();
+ }
+
+ @Override
+ public int getCommonFrames() {
+ return proxied.getCommonFrames();
+ }
+
+ @Override
+ public IThrowableProxy[] getSuppressed() {
+ final IThrowableProxy[] result = proxied.getSuppressed();
+ if ( result == null || result.length == 0 ) {
+ return result;
+ }
+ final IThrowableProxy[] proxies = new IThrowableProxy[result.length];
+ for(int i=0;i<proxies.length;i++) {
+ proxies[i] = getProxy(result[i]);
+ }
+ return proxies;
+ }
+ }
+
+ static final class MaskingEnsureExceptionHandling extends EnsureExceptionHandling {
+
+ public void process(Context context, Converter<ILoggingEvent> head) {
+ if (head == null) {
+ // this should never happen
+ throw new IllegalArgumentException("cannot process empty chain");
+ }
+ if (!chainHandlesThrowable(head)) {
+ Converter<ILoggingEvent> tail = ConverterUtil.findTail(head);
+ Converter<ILoggingEvent> exConverter = null;
+ LoggerContext loggerContext = (LoggerContext) context;
+ if (loggerContext.isPackagingDataEnabled()) {
+ exConverter = new MaskingExtendedThrowableProxyConverter();
+ } else {
+ exConverter = new MaskingThrowableProxyConverter();
+ }
+ tail.setNext(exConverter);
+ }
+ }
+ }
+
+ static final class MaskingPatternLayoutEncoder extends PatternLayoutEncoderBase<ILoggingEvent> {
+
+ @Override
+ public void start() {
+ PatternLayout patternLayout = new PatternLayout();
+ patternLayout.setContext(context);
+ patternLayout.setPattern(getPattern());
+ patternLayout.setOutputPatternAsHeader(outputPatternAsHeader);
+ MaskingMessageUtil.setMessageConverter(patternLayout);
+ patternLayout.start();
+ this.layout = patternLayout;
+ super.start();
+ }
+ }
+}
+
diff --git a/src/main/java/org/apache/sling/commons/log/logback/internal/stacktrace/OSGiAwareExceptionHandling.java b/src/main/java/org/apache/sling/commons/log/logback/internal/stacktrace/OSGiAwareExceptionHandling.java
index 165d65a..00cb7d0 100644
--- a/src/main/java/org/apache/sling/commons/log/logback/internal/stacktrace/OSGiAwareExceptionHandling.java
+++ b/src/main/java/org/apache/sling/commons/log/logback/internal/stacktrace/OSGiAwareExceptionHandling.java
@@ -19,8 +19,9 @@
package org.apache.sling.commons.log.logback.internal.stacktrace;
+import org.apache.sling.commons.log.logback.internal.MaskingMessageUtil;
+
import ch.qos.logback.classic.pattern.EnsureExceptionHandling;
-import ch.qos.logback.classic.pattern.ExtendedThrowableProxyConverter;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.classic.spi.StackTraceElementProxy;
import ch.qos.logback.core.Context;
@@ -47,7 +48,7 @@ public class OSGiAwareExceptionHandling extends EnsureExceptionHandling {
}
}
- private class OSGiAwareConverter extends ExtendedThrowableProxyConverter {
+ private class OSGiAwareConverter extends MaskingMessageUtil.MaskingExtendedThrowableProxyConverter {
@Override
protected void extraData(StringBuilder builder, StackTraceElementProxy step) {
if (step != null) {
diff --git a/src/test/java/org/apache/sling/commons/log/logback/internal/TestLogConfig.java b/src/test/java/org/apache/sling/commons/log/logback/internal/TestLogConfig.java
index 408cc88..a6b85a1 100644
--- a/src/test/java/org/apache/sling/commons/log/logback/internal/TestLogConfig.java
+++ b/src/test/java/org/apache/sling/commons/log/logback/internal/TestLogConfig.java
@@ -20,14 +20,20 @@
package org.apache.sling.commons.log.logback.internal;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import java.io.IOException;
import java.util.Collections;
import org.junit.Test;
+import org.mockito.Mockito;
import org.slf4j.LoggerFactory;
import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.PatternLayout;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.classic.spi.ThrowableProxy;
public class TestLogConfig {
@@ -47,6 +53,38 @@ public class TestLogConfig {
assertEquals(convertedPattern, logConfig.createLayout().getPattern());
}
+ @Test
+ public void testMessageEscaping() {
+ final LogConfig logConfig = createConfig("%message %m %msg");
+ final ILoggingEvent event = Mockito.mock(ILoggingEvent.class);
+ Mockito.when(event.getFormattedMessage()).thenReturn("foo\r\nbar");
+ final String result = logConfig.createLayout().doLayout(event);
+ assertEquals("foo__bar foo__bar foo__bar", result);
+ }
+
+ @Test
+ public void testExceptionMessageEscaping() {
+ final String[] patterns = new String[] {"%ex", "%exception", "%rEx", "%rootException", "%throwable", "%xEx", "%xException", "%xThrowable", "%m"};
+ for(final String p : patterns) {
+ final LogConfig logConfig = createConfig(p);
+ final PatternLayout layout = logConfig.createLayout();
+
+ final ILoggingEvent event = Mockito.mock(ILoggingEvent.class);
+ Mockito.when(event.getFormattedMessage()).thenReturn("message");
+
+ // single exception
+ Mockito.when(event.getThrowableProxy()).thenReturn(new ThrowableProxy(new RuntimeException("foo\r\nbar")));
+ String result = layout.doLayout(event);
+ assertTrue("pattern " + p + " : " + result, result.contains("foo__bar"));
+
+ // nested exception
+ Mockito.when(event.getThrowableProxy()).thenReturn(new ThrowableProxy(new RuntimeException("foo\r\nbar", new IOException("a\r\nb"))));
+ result = layout.doLayout(event);
+ assertTrue("pattern " + p + " : " + result, result.contains("foo__bar"));
+ assertTrue("pattern " + p + " : " + result, result.contains("a__b"));
+ }
+ }
+
private LogConfig createConfig(String pattern) {
return new LogConfig(new DummyLogWriterProvider(), pattern, Collections.<String> emptySet(), Level.DEBUG,
"test", false, null, (LoggerContext) LoggerFactory.getILoggerFactory(), false);