This is an automated email from the ASF dual-hosted git repository. pkarwasz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 3ff6b86ab225365321df7d2a6daae4fd32ac8e62 Author: ppkarwasz <[email protected]> AuthorDate: Sat Feb 19 17:05:20 2022 +0100 Issues with multiple Log4j 1.x filters (#753) * Fix issues with multiple Log4j 1.x filters This PR fixes a couple of issues concerning filters in the Log4j 1.x bridge: - there is an endless loop in `FilterAdapter#filter` that hangs the program if more than one filter is configured, - filters are executed multiple times per log message. The second problem comes from chains of Log4j 1.x filters: a filter chain is split into a `CompositeFilter` of `FilterAdapter`s (one for each filter). Each `FilterAdapter` executes its own filter and all those that come later in the chain. In order to preserve the behavior of `FilterAdapter` the following restrictions have been applied to the generated filters: 1. no chains of Log4j 1.x filters are generated by the configuration; a list of Log4j 1.x filters is represented as a wrapped `CompositeFilter` of `FilterAdapter`s, 2. the factories don't generate any `FilterWrapper`s of `FilterAdapter`s nor `FilterAdapter`s of `FilterWrapper`s. * Import and whitespace fixes * More unused import fixes * Adds regression test for CompositeFilter Conflicts: log4j-1.2-api/src/main/java/org/apache/log4j/bridge/AppenderAdapter.java log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/AsyncAppenderBuilder.java log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/ConsoleAppenderBuilder.java log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SocketAppenderBuilder.java log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfiguration.java log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfiguration.java log4j-1.2-api/src/test/java/org/apache/log4j/config/AbstractLog4j1ConfigurationTest.java --- .../java/org/apache/log4j/AppenderSkeleton.java | 5 +- .../org/apache/log4j/PropertyConfigurator.java | 5 +- .../org/apache/log4j/bridge/AppenderAdapter.java | 16 +-- .../org/apache/log4j/bridge/AppenderWrapper.java | 6 +- .../org/apache/log4j/bridge/FilterAdapter.java | 43 +++++++- .../org/apache/log4j/builders/AbstractBuilder.java | 38 ++----- .../builders/appender/AsyncAppenderBuilder.java | 39 +++---- .../builders/appender/ConsoleAppenderBuilder.java | 17 +-- .../appender/DailyRollingFileAppenderBuilder.java | 2 +- .../builders/appender/FileAppenderBuilder.java | 2 +- .../builders/appender/RewriteAppenderBuilder.java | 2 +- .../appender/RollingFileAppenderBuilder.java | 2 +- .../builders/appender/SocketAppenderBuilder.java | 16 +-- .../builders/appender/SyslogAppenderBuilder.java | 2 +- .../log4j/config/PropertiesConfiguration.java | 26 +---- .../src/main/java/org/apache/log4j/spi/Filter.java | 12 -- .../org/apache/log4j/xml/XmlConfiguration.java | 48 ++++++-- .../log4j/builders/Log4j2ListAppenderBuilder.java | 98 +++++++++++++++++ .../config/AbstractLog4j1ConfigurationTest.java | 122 +++++++++++++++++++++ .../log4j/config/PropertiesConfigurationTest.java | 12 +- .../org/apache/log4j/config/StartsWithFilter.java | 38 +++++++ .../apache/log4j/config/XmlConfigurationTest.java | 6 + .../resources/log4j-multipleFilters.properties | 69 ++++++++++++ .../src/test/resources/log4j-multipleFilters.xml | 92 ++++++++++++++++ .../log4j/core/filter/CompositeFilterTest.java | 49 +++++++++ .../logging/log4j/core/filter/CompositeFilter.java | 4 +- 26 files changed, 611 insertions(+), 160 deletions(-) diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/AppenderSkeleton.java b/log4j-1.2-api/src/main/java/org/apache/log4j/AppenderSkeleton.java index 1353dae..3660a19 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/AppenderSkeleton.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/AppenderSkeleton.java @@ -108,12 +108,9 @@ public abstract class AppenderSkeleton implements Appender, OptionHandler { return ((threshold == null) || priority.isGreaterOrEqual(threshold)); } - /** - * This method is never going to be called in Log4j 2 so there isn't much point in having any code in it. - * @param event The LoggingEvent. - */ @Override public void doAppend(final LoggingEvent event) { + // Threshold checks and filtering is performed by the AppenderWrapper. append(event); } diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/PropertyConfigurator.java b/log4j-1.2-api/src/main/java/org/apache/log4j/PropertyConfigurator.java index d522d57..483096e 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/PropertyConfigurator.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/PropertyConfigurator.java @@ -30,6 +30,7 @@ import java.util.Properties; import java.util.StringTokenizer; import java.util.Vector; +import org.apache.log4j.bridge.FilterAdapter; import org.apache.log4j.config.PropertiesConfiguration; import org.apache.log4j.config.PropertySetter; import org.apache.log4j.helpers.FileWatchdog; @@ -520,6 +521,7 @@ public class PropertyConfigurator implements Configurator { // sort filters by IDs, insantiate filters, set filter options, // add filters to the appender final Enumeration g = new SortedKeyEnumeration(filters); + Filter head = null; while (g.hasMoreElements()) { final String key = (String) g.nextElement(); final String clazz = properties.getProperty(key); @@ -536,12 +538,13 @@ public class PropertyConfigurator implements Configurator { } propSetter.activate(); LogLog.debug("Adding filter of type [" + filter.getClass() + "] to appender named [" + appender.getName() + "]."); - appender.addFilter(filter); + head = FilterAdapter.addFilter(head, filter); } } else { LogLog.warn("Missing class definition for filter: [" + key + "]"); } } + appender.addFilter(head); } /** diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/AppenderAdapter.java b/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/AppenderAdapter.java index 1ae0369..3334535 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/AppenderAdapter.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/AppenderAdapter.java @@ -26,7 +26,6 @@ import org.apache.logging.log4j.core.Layout; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.appender.AbstractAppender; import org.apache.logging.log4j.core.config.Property; -import org.apache.logging.log4j.core.filter.CompositeFilter; /** * Binds a Log4j 1.x Appender to Log4j 2. @@ -42,20 +41,7 @@ public class AppenderAdapter { */ public AppenderAdapter(Appender appender) { this.appender = appender; - org.apache.logging.log4j.core.Filter appenderFilter = null; - if (appender.getFilter() != null) { - if (appender.getFilter().getNext() != null) { - org.apache.log4j.spi.Filter filter = appender.getFilter(); - List<org.apache.logging.log4j.core.Filter> filters = new ArrayList<>(); - while (filter != null) { - filters.add(new FilterAdapter(filter)); - filter = filter.getNext(); - } - appenderFilter = CompositeFilter.createFilters(filters.toArray(new Filter[0])); - } else { - appenderFilter = new FilterAdapter(appender.getFilter()); - } - } + final org.apache.logging.log4j.core.Filter appenderFilter = FilterAdapter.convertFilter(appender.getFilter()); this.adapter = new Adapter(appender.getName(), appenderFilter, null, true, null); } diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/AppenderWrapper.java b/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/AppenderWrapper.java index d26c878..3d28fd7 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/AppenderWrapper.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/AppenderWrapper.java @@ -55,11 +55,7 @@ public class AppenderWrapper implements Appender { @Override public void addFilter(Filter newFilter) { if (appender instanceof AbstractFilterable) { - if (newFilter instanceof FilterWrapper) { - ((AbstractFilterable) appender).addFilter(((FilterWrapper) newFilter).getFilter()); - } else { - ((AbstractFilterable) appender).addFilter(new FilterAdapter(newFilter)); - } + ((AbstractFilterable) appender).addFilter(FilterAdapter.convertFilter(newFilter)); } else { LOGGER.warn("Unable to add filter to appender {}, it does not support filters", appender.getName()); } diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/FilterAdapter.java b/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/FilterAdapter.java index 4851125..5ce599e 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/FilterAdapter.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/FilterAdapter.java @@ -20,6 +20,7 @@ import org.apache.log4j.spi.Filter; import org.apache.log4j.spi.LoggingEvent; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.filter.AbstractFilter; +import org.apache.logging.log4j.core.filter.CompositeFilter; /** * Binds a Log4j 1.x Filter with Log4j 2. @@ -28,6 +29,44 @@ public class FilterAdapter extends AbstractFilter { private final Filter filter; + /** + * Converts a Log4j 1.x filter into a Log4j 2.x filter. + * + * @param filter + * a Log4j 1.x filter + * @return a Log4j 2.x filter + */ + public static org.apache.logging.log4j.core.Filter convertFilter(Filter filter) { + if (filter instanceof FilterWrapper && filter.getNext() == null) { + return ((FilterWrapper) filter).getFilter(); + } else if (filter != null) { + return new FilterAdapter(filter); + } + return null; + } + + /** + * Appends one filter to another using Log4j 2.x concatenation utilities. + * @param first + * @param second + * @return + */ + public static Filter addFilter(Filter first, Filter second) { + if (first == null) { + return second; + } + if (second == null) { + return first; + } + final CompositeFilter composite; + if (first instanceof FilterWrapper && ((FilterWrapper) first).getFilter() instanceof CompositeFilter) { + composite = (CompositeFilter) ((FilterWrapper) first).getFilter(); + } else { + composite = CompositeFilter.createFilters(new org.apache.logging.log4j.core.Filter[] {convertFilter(first)}); + } + return new FilterWrapper(composite.addFilter(convertFilter(second))); + } + public FilterAdapter(Filter filter) { this.filter = filter; } @@ -46,14 +85,14 @@ public class FilterAdapter extends AbstractFilter { LoggingEvent loggingEvent = new LogEventAdapter(event); Filter next = filter; while (next != null) { - switch (filter.decide(loggingEvent)) { + switch (next.decide(loggingEvent)) { case Filter.ACCEPT: return Result.ACCEPT; case Filter.DENY: return Result.DENY; default: } - next = filter.getNext(); + next = next.getNext(); } return Result.NEUTRAL; } diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java index 04b439c..b73fff4 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java @@ -19,14 +19,9 @@ package org.apache.log4j.builders; import static org.apache.log4j.xml.XmlConfiguration.NAME_ATTR; import static org.apache.log4j.xml.XmlConfiguration.VALUE_ATTR; -import java.util.ArrayList; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Properties; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; import org.apache.log4j.bridge.FilterAdapter; import org.apache.log4j.bridge.FilterWrapper; @@ -34,7 +29,6 @@ import org.apache.log4j.helpers.OptionConverter; import org.apache.log4j.spi.Filter; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.core.filter.CompositeFilter; import org.apache.logging.log4j.core.filter.ThresholdFilter; import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.Strings; @@ -74,33 +68,17 @@ public abstract class AbstractBuilder implements Builder { props.entrySet().forEach(e -> this.properties.put(toBeanKey(e.getKey().toString()), e.getValue())); } - protected org.apache.logging.log4j.core.Filter buildFilters(final String level, final Filter filter) { - if (level != null && filter != null) { - final List<org.apache.logging.log4j.core.Filter> filterList = new ArrayList<>(); + protected static org.apache.logging.log4j.core.Filter buildFilters(final String level, final Filter filter) { + Filter head = null; + if (level != null) { final org.apache.logging.log4j.core.Filter thresholdFilter = ThresholdFilter.createFilter(OptionConverter.convertLevel(level, Level.TRACE), org.apache.logging.log4j.core.Filter.Result.NEUTRAL, org.apache.logging.log4j.core.Filter.Result.DENY); - filterList.add(thresholdFilter); - Filter f = filter; - while (f != null) { - if (filter instanceof FilterWrapper) { - filterList.add(((FilterWrapper) f).getFilter()); - } else { - filterList.add(new FilterAdapter(f)); - } - f = f.getNext(); - } - return CompositeFilter.createFilters(filterList.toArray(org.apache.logging.log4j.core.Filter.EMPTY_ARRAY)); - } else if (level != null) { - return ThresholdFilter.createFilter(OptionConverter.convertLevel(level, Level.TRACE), org.apache.logging.log4j.core.Filter.Result.NEUTRAL, - org.apache.logging.log4j.core.Filter.Result.DENY); - } else if (filter != null) { - if (filter instanceof FilterWrapper) { - return ((FilterWrapper) filter).getFilter(); - } else { - return new FilterAdapter(filter); - } + head = new FilterWrapper(thresholdFilter); + } + if (filter != null) { + head = FilterAdapter.addFilter(head, filter); } - return null; + return FilterAdapter.convertFilter(head); } private String capitalize(final String value) { diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/AsyncAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/AsyncAppenderBuilder.java index 4b8b17e..ca1aa80 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/AsyncAppenderBuilder.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/AsyncAppenderBuilder.java @@ -19,45 +19,34 @@ package org.apache.log4j.builders.appender; import static org.apache.log4j.builders.BuilderManager.CATEGORY; import static org.apache.log4j.config.Log4j1Configuration.APPENDER_REF_TAG; import static org.apache.log4j.config.Log4j1Configuration.THRESHOLD_PARAM; +import static org.apache.log4j.xml.XmlConfiguration.FILTER_TAG; import static org.apache.log4j.xml.XmlConfiguration.PARAM_TAG; import static org.apache.log4j.xml.XmlConfiguration.forEachElement; import java.util.ArrayList; import java.util.List; import java.util.Properties; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; import org.apache.log4j.Appender; import org.apache.log4j.bridge.AppenderWrapper; +import org.apache.log4j.bridge.FilterAdapter; import org.apache.log4j.builders.AbstractBuilder; import org.apache.log4j.builders.BooleanHolder; import org.apache.log4j.builders.Holder; import org.apache.log4j.config.Log4j1Configuration; import org.apache.log4j.config.PropertiesConfiguration; import org.apache.log4j.helpers.OptionConverter; +import org.apache.log4j.spi.Filter; import org.apache.log4j.xml.XmlConfiguration; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.appender.AsyncAppender; +import org.apache.logging.log4j.core.appender.AsyncAppender.Builder; import org.apache.logging.log4j.core.config.AppenderRef; import org.apache.logging.log4j.plugins.Plugin; import org.apache.logging.log4j.status.StatusLogger; +import org.apache.logging.log4j.util.Strings; import org.w3c.dom.Element; -import java.util.ArrayList; -import java.util.List; -import java.util.Properties; - -import static org.apache.log4j.builders.BuilderManager.CATEGORY; -import static org.apache.log4j.xml.XmlConfiguration.NAME_ATTR; -import static org.apache.log4j.xml.XmlConfiguration.PARAM_TAG; -import static org.apache.log4j.xml.XmlConfiguration.VALUE_ATTR; -import static org.apache.log4j.xml.XmlConfiguration.forEachElement; -import static org.apache.log4j.config.Log4j1Configuration.APPENDER_REF_TAG; -import static org.apache.log4j.config.Log4j1Configuration.THRESHOLD_PARAM; - - /** * Build an Async Appender */ @@ -83,6 +72,7 @@ public class AsyncAppenderBuilder extends AbstractBuilder implements AppenderBui Holder<Boolean> includeLocation = new BooleanHolder(); Holder<String> level = new Holder<>("trace"); Holder<Integer> bufferSize = new Holder<>(1024); + Holder<Filter> filter = new Holder<>(); forEachElement(appenderElement.getChildNodes(), (currentElement) -> { switch (currentElement.getTagName()) { case APPENDER_REF_TAG: @@ -91,6 +81,9 @@ public class AsyncAppenderBuilder extends AbstractBuilder implements AppenderBui appenderRefs.get().add(appender.getName()); } break; + case FILTER_TAG: + config.addFilter(filter, currentElement); + break; case PARAM_TAG: { switch (getNameAttributeKey(currentElement)) { case BUFFER_SIZE_PARAM: @@ -110,14 +103,15 @@ public class AsyncAppenderBuilder extends AbstractBuilder implements AppenderBui } } }); - return createAppender(name, level.get(), appenderRefs.get().toArray(new String[0]), blocking.get(), - bufferSize.get(), includeLocation.get(), config); + return createAppender(name, level.get(), appenderRefs.get().toArray(Strings.EMPTY_ARRAY), blocking.get(), + bufferSize.get(), includeLocation.get(), filter.get(), config); } @Override public Appender parseAppender(final String name, final String appenderPrefix, final String layoutPrefix, final String filterPrefix, final Properties props, final PropertiesConfiguration configuration) { final String appenderRef = getProperty(APPENDER_REF_TAG); + final Filter filter = configuration.parseAppenderFilters(props, filterPrefix, name); final boolean blocking = getBooleanProperty(BLOCKING_PARAM); final boolean includeLocation = getBooleanProperty(INCLUDE_LOCATION_PARAM); final String level = getProperty(THRESHOLD_PARAM); @@ -131,13 +125,13 @@ public class AsyncAppenderBuilder extends AbstractBuilder implements AppenderBui LOGGER.warn("Cannot locate Appender {}", appenderRef); return null; } - return createAppender(name, level, new String[] {appenderRef}, blocking, bufferSize, includeLocation, + return createAppender(name, level, new String[]{appenderRef}, blocking, bufferSize, includeLocation, filter, configuration); } private <T extends Log4j1Configuration> Appender createAppender(final String name, final String level, final String[] appenderRefs, final boolean blocking, final int bufferSize, final boolean includeLocation, - final T configuration) { + final Filter filter, final T configuration) { final org.apache.logging.log4j.Level logLevel = OptionConverter.convertLevel(level, org.apache.logging.log4j.Level.TRACE); final AppenderRef[] refs = new AppenderRef[appenderRefs.length]; @@ -145,8 +139,9 @@ public class AsyncAppenderBuilder extends AbstractBuilder implements AppenderBui for (final String appenderRef : appenderRefs) { refs[index++] = AppenderRef.createAppenderRef(appenderRef, logLevel, null); } - return new AppenderWrapper(AsyncAppender.newBuilder() - .setName(name) + Builder builder = AsyncAppender.newBuilder(); + builder.setFilter(FilterAdapter.convertFilter(filter)); + return new AppenderWrapper(builder.setName(name) .setAppenderRefs(refs) .setBlocking(blocking) .setBufferSize(bufferSize) diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/ConsoleAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/ConsoleAppenderBuilder.java index aa8056e..1214452 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/ConsoleAppenderBuilder.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/ConsoleAppenderBuilder.java @@ -71,7 +71,7 @@ public class ConsoleAppenderBuilder extends AbstractBuilder implements AppenderB String name = getNameAttribute(appenderElement); Holder<String> target = new Holder<>(SYSTEM_OUT); Holder<Layout> layout = new Holder<>(); - Holder<List<Filter>> filters = new Holder<>(new ArrayList<>()); + Holder<Filter> filter = new Holder<>(); Holder<String> level = new Holder<>(); Holder<Boolean> follow = new BooleanHolder(); Holder<Boolean> immediateFlush = new BooleanHolder(true); @@ -81,7 +81,7 @@ public class ConsoleAppenderBuilder extends AbstractBuilder implements AppenderB layout.set(config.parseLayout(currentElement)); break; case FILTER_TAG: - filters.get().add(config.parseFilters(currentElement)); + config.addFilter(filter, currentElement); break; case PARAM_TAG: { switch (getNameAttributeKey(currentElement)) { @@ -116,18 +116,7 @@ public class ConsoleAppenderBuilder extends AbstractBuilder implements AppenderB } } }); - Filter head = null; - Filter current = null; - for (final Filter f : filters.get()) { - if (head == null) { - head = f; - current = f; - } else { - current.next = f; - current = f; - } - } - return createAppender(name, layout.get(), head, level.get(), target.get(), immediateFlush.get(), follow.get(), config); + return createAppender(name, layout.get(), filter.get(), level.get(), target.get(), immediateFlush.get(), follow.get(), config); } @Override diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/DailyRollingFileAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/DailyRollingFileAppenderBuilder.java index 576ccab..78ade8c 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/DailyRollingFileAppenderBuilder.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/DailyRollingFileAppenderBuilder.java @@ -82,7 +82,7 @@ public class DailyRollingFileAppenderBuilder extends AbstractBuilder implements layout.set(config.parseLayout(currentElement)); break; case FILTER_TAG: - filter.set(config.parseFilters(currentElement)); + config.addFilter(filter, currentElement); break; case PARAM_TAG: switch (getNameAttributeKey(currentElement)) { diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/FileAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/FileAppenderBuilder.java index 96d2f90..8ead958 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/FileAppenderBuilder.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/FileAppenderBuilder.java @@ -77,7 +77,7 @@ public class FileAppenderBuilder extends AbstractBuilder implements AppenderBuil layout.set(config.parseLayout(currentElement)); break; case FILTER_TAG: - filter.set(config.parseFilters(currentElement)); + config.addFilter(filter, currentElement); break; case PARAM_TAG: switch (getNameAttributeKey(currentElement)) { diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RewriteAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RewriteAppenderBuilder.java index cac02ee..4398995 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RewriteAppenderBuilder.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RewriteAppenderBuilder.java @@ -87,7 +87,7 @@ public class RewriteAppenderBuilder extends AbstractBuilder implements AppenderB } break; case FILTER_TAG: - filter.set(config.parseFilters(currentElement)); + config.addFilter(filter, currentElement); break; case PARAM_TAG: if (getNameAttributeKey(currentElement).equalsIgnoreCase(THRESHOLD_PARAM)) { diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RollingFileAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RollingFileAppenderBuilder.java index 3a74245..3ab8086 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RollingFileAppenderBuilder.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RollingFileAppenderBuilder.java @@ -84,7 +84,7 @@ public class RollingFileAppenderBuilder extends AbstractBuilder implements Appen layout.set(config.parseLayout(currentElement)); break; case FILTER_TAG: - filter.set(config.parseFilters(currentElement)); + config.addFilter(filter, currentElement); break; case PARAM_TAG: switch (getNameAttributeKey(currentElement)) { diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SocketAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SocketAppenderBuilder.java index 9034f05..a2997e4 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SocketAppenderBuilder.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SocketAppenderBuilder.java @@ -100,7 +100,7 @@ public class SocketAppenderBuilder extends AbstractBuilder implements AppenderBu final Holder<Integer> port = new Holder<>(DEFAULT_PORT); final Holder<Integer> reconnectDelay = new Holder<>(DEFAULT_RECONNECTION_DELAY); final Holder<Layout> layout = new Holder<>(); - final Holder<List<Filter>> filters = new Holder<>(new ArrayList<>()); + final Holder<Filter> filter = new Holder<>(); final Holder<String> level = new Holder<>(); final Holder<Boolean> immediateFlush = new BooleanHolder(true); forEachElement(appenderElement.getChildNodes(), currentElement -> { @@ -109,7 +109,7 @@ public class SocketAppenderBuilder extends AbstractBuilder implements AppenderBu layout.set(config.parseLayout(currentElement)); break; case FILTER_TAG: - filters.get().add(config.parseFilters(currentElement)); + config.addFilter(filter, currentElement); break; case PARAM_TAG: switch (getNameAttributeKey(currentElement)) { @@ -132,17 +132,7 @@ public class SocketAppenderBuilder extends AbstractBuilder implements AppenderBu break; } }); - Filter head = null; - Filter current = null; - for (final Filter f : filters.get()) { - if (head == null) { - head = f; - } else { - current.next = f; - } - current = f; - } - return createAppender(name, host.get(), port.get(), layout.get(), head, level.get(), immediateFlush.get(), reconnectDelay.get(), config); + return createAppender(name, host.get(), port.get(), layout.get(), filter.get(), level.get(), immediateFlush.get(), reconnectDelay.get(), config); } @Override diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SyslogAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SyslogAppenderBuilder.java index fe6ee59..ffbdedf 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SyslogAppenderBuilder.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SyslogAppenderBuilder.java @@ -85,7 +85,7 @@ public class SyslogAppenderBuilder extends AbstractBuilder implements AppenderBu layout.set(config.parseLayout(currentElement)); break; case FILTER_TAG: - filter.set(config.parseFilters(currentElement)); + config.addFilter(filter, currentElement); break; case PARAM_TAG: { switch (getNameAttributeKey(currentElement)) { diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfiguration.java b/log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfiguration.java index 405bd4b..32a523d 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfiguration.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfiguration.java @@ -35,7 +35,7 @@ import org.apache.log4j.LogManager; import org.apache.log4j.PatternLayout; import org.apache.log4j.bridge.AppenderAdapter; import org.apache.log4j.bridge.AppenderWrapper; -import org.apache.log4j.builders.BuilderManager; +import org.apache.log4j.bridge.FilterAdapter; import org.apache.log4j.helpers.OptionConverter; import org.apache.log4j.spi.ErrorHandler; import org.apache.log4j.spi.Filter; @@ -44,22 +44,8 @@ import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.config.ConfigurationSource; import org.apache.logging.log4j.core.config.LoggerConfig; import org.apache.logging.log4j.core.config.status.StatusConfiguration; -import org.apache.logging.log4j.core.filter.AbstractFilterable; import org.apache.logging.log4j.util.LoaderUtil; -import java.io.IOException; -import java.io.InputStream; -import java.lang.reflect.InvocationTargetException; -import java.util.ArrayList; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Properties; -import java.util.SortedMap; -import java.util.StringTokenizer; -import java.util.TreeMap; - /** * Constructs a configuration based on Log4j 1 properties. */ @@ -570,7 +556,6 @@ public class PropertiesConfiguration extends Log4j1Configuration { } Filter head = null; - Filter next = null; for (final Map.Entry<String, List<NameValue>> entry : filters.entrySet()) { final String clazz = props.getProperty(entry.getKey()); Filter filter = null; @@ -581,14 +566,7 @@ public class PropertiesConfiguration extends Log4j1Configuration { filter = buildFilter(clazz, appenderName, entry.getValue()); } } - if (filter != null) { - if (head == null) { - head = filter; - } else { - next.setNext(filter); - } - next = filter; - } + head = FilterAdapter.addFilter(head, filter); } return head; } diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/spi/Filter.java b/log4j-1.2-api/src/main/java/org/apache/log4j/spi/Filter.java index 17a15d6..91917ab 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/spi/Filter.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/spi/Filter.java @@ -16,8 +16,6 @@ */ package org.apache.log4j.spi; -import org.apache.log4j.bridge.FilterAdapter; - /** * @since 0.9.0 */ @@ -32,16 +30,6 @@ public abstract class Filter { } isCorePresent = temp; } - - // TODO Unused? - private final FilterAdapter adapter; - - /** - * Constructs a new instance. - */ - public Filter() { - this.adapter = isCorePresent ? new FilterAdapter(this) : null; - } /** * The log event must be dropped immediately without consulting diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfiguration.java b/log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfiguration.java index 65b6a2c..1b05d57 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfiguration.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfiguration.java @@ -21,6 +21,8 @@ import org.apache.log4j.Layout; import org.apache.log4j.Level; import org.apache.log4j.bridge.AppenderAdapter; import org.apache.log4j.bridge.AppenderWrapper; +import org.apache.log4j.bridge.FilterAdapter; +import org.apache.log4j.builders.Holder; import org.apache.log4j.config.Log4j1Configuration; import org.apache.log4j.config.PropertySetter; import org.apache.log4j.helpers.OptionConverter; @@ -375,7 +377,8 @@ public class XmlConfiguration extends Log4j1Configuration { PropertySetter propSetter = new PropertySetter(appender); appender.setName(subst(appenderElement.getAttribute(NAME_ATTR))); - forEachElement(appenderElement.getChildNodes(), (currentElement) -> { + final Holder<Filter> filterChain = new Holder<>(); + forEachElement(appenderElement.getChildNodes(), currentElement -> { // Parse appender parameters switch (currentElement.getTagName()) { case PARAM_TAG: @@ -385,12 +388,7 @@ public class XmlConfiguration extends Log4j1Configuration { appender.setLayout(parseLayout(currentElement)); break; case FILTER_TAG: - Filter filter = parseFilters(currentElement); - if (filter != null) { - LOGGER.debug("Adding filter of type [{}] to appender named [{}]", - filter.getClass(), appender.getName()); - appender.addFilter(filter); - } + addFilter(filterChain, currentElement); break; case ERROR_HANDLER_TAG: parseErrorHandler(currentElement, appender); @@ -417,6 +415,10 @@ public class XmlConfiguration extends Log4j1Configuration { } } }); + final Filter head = filterChain.get(); + if (head != null) { + appender.addFilter(head); + } propSetter.activate(); return appender; } catch (ConsumerException ex) { @@ -502,12 +504,30 @@ public class XmlConfiguration extends Log4j1Configuration { * @param filterElement The Filter Element. * @return The Filter. */ + public void addFilter(final Holder<Filter> ref, final Element filterElement) { + final Filter filter = parseFilters(filterElement); + final Filter newFilter = FilterAdapter.addFilter(ref.get(), filter); + ref.set(newFilter); + } + + /** + * Used internally to parse a filter element. + */ public Filter parseFilters(Element filterElement) { String className = subst(filterElement.getAttribute(CLASS_ATTR)); LOGGER.debug("Class name: [" + className + ']'); Filter filter = manager.parseFilter(className, filterElement, this); if (filter == null) { + filter = buildFilter(className, filterElement); + } + return filter; + } + + private Filter buildFilter(final String className, final Element filterElement) { + try { + Filter filter = LoaderUtil.newInstanceOf(className); PropertySetter propSetter = new PropertySetter(filter); + forEachElement(filterElement.getChildNodes(), (currentElement) -> { String tagName = currentElement.getTagName(); if (tagName.equals(PARAM_TAG)) { @@ -517,8 +537,20 @@ public class XmlConfiguration extends Log4j1Configuration { } }); propSetter.activate(); + return filter; + } catch (ConsumerException ex) { + Throwable t = ex.getCause(); + if (t instanceof InterruptedException || t instanceof InterruptedIOException) { + Thread.currentThread().interrupt(); + } + LOGGER.error("Could not create an Filter. Reported error follows.", t); + } catch (Exception oops) { + if (oops instanceof InterruptedException || oops instanceof InterruptedIOException) { + Thread.currentThread().interrupt(); + } + LOGGER.error("Could not create an Filter. Reported error follows.", oops); } - return filter; + return null; } /** diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/builders/Log4j2ListAppenderBuilder.java b/log4j-1.2-api/src/test/java/org/apache/log4j/builders/Log4j2ListAppenderBuilder.java new file mode 100644 index 0000000..ccef99b --- /dev/null +++ b/log4j-1.2-api/src/test/java/org/apache/log4j/builders/Log4j2ListAppenderBuilder.java @@ -0,0 +1,98 @@ +/* + * 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.log4j.builders; + +import static org.apache.log4j.builders.BuilderManager.CATEGORY; +import static org.apache.log4j.xml.XmlConfiguration.FILTER_TAG; +import static org.apache.log4j.xml.XmlConfiguration.LAYOUT_TAG; +import static org.apache.log4j.xml.XmlConfiguration.NAME_ATTR; +import static org.apache.log4j.xml.XmlConfiguration.forEachElement; + +import java.util.ArrayList; +import java.util.List; +import java.util.Properties; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import org.apache.log4j.Appender; +import org.apache.log4j.Layout; +import org.apache.log4j.bridge.AppenderWrapper; +import org.apache.log4j.bridge.LayoutAdapter; +import org.apache.log4j.bridge.LayoutWrapper; +import org.apache.log4j.builders.appender.AppenderBuilder; +import org.apache.log4j.config.PropertiesConfiguration; +import org.apache.log4j.spi.Filter; +import org.apache.log4j.xml.XmlConfiguration; +import org.apache.logging.log4j.core.test.appender.ListAppender; +import org.apache.logging.log4j.plugins.Plugin; +import org.w3c.dom.Element; + +/** + * Builder for the native Log4j 2.x list appender to be used in the tests. + */ +@Plugin(name = "org.apache.logging.log4j.test.appender.ListAppender", category = CATEGORY) +public class Log4j2ListAppenderBuilder extends AbstractBuilder implements AppenderBuilder { + + public Log4j2ListAppenderBuilder() { + } + + public Log4j2ListAppenderBuilder(final String prefix, final Properties props) { + super(prefix, props); + } + + @Override + public Appender parseAppender(Element element, XmlConfiguration configuration) { + final String name = getNameAttribute(element); + final Holder<Layout> layout = new Holder<>(); + final Holder<Filter> filter = new Holder<>(); + forEachElement(element.getChildNodes(), currentElement -> { + switch (currentElement.getTagName()) { + case LAYOUT_TAG : + layout.set(configuration.parseLayout(currentElement)); + break; + case FILTER_TAG : + configuration.addFilter(filter, currentElement); + break; + default : + } + }); + return createAppender(name, layout.get(), filter.get()); + } + + @Override + public Appender parseAppender(String name, String appenderPrefix, String layoutPrefix, String filterPrefix, + Properties props, PropertiesConfiguration configuration) { + final Layout layout = configuration.parseLayout(layoutPrefix, name, props); + final Filter filter = configuration.parseAppenderFilters(props, filterPrefix, name); + return createAppender(name, layout, filter); + } + + private Appender createAppender(String name, Layout layout, Filter filter) { + final org.apache.logging.log4j.core.Layout<?> log4j2Layout; + if (layout instanceof LayoutWrapper) { + log4j2Layout = ((LayoutWrapper) layout).getLayout(); + } else { + log4j2Layout = layout != null ? new LayoutAdapter(layout) : null; + } + return new AppenderWrapper( + ListAppender.newBuilder() + .setName(name) + .setLayout(log4j2Layout) + .setFilter(AbstractBuilder.buildFilters(null, filter)) + .build()); + } +} diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/AbstractLog4j1ConfigurationTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/AbstractLog4j1ConfigurationTest.java index 21c9b7e..e4ac132 100644 --- a/log4j-1.2-api/src/test/java/org/apache/log4j/config/AbstractLog4j1ConfigurationTest.java +++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/AbstractLog4j1ConfigurationTest.java @@ -17,7 +17,9 @@ package org.apache.log4j.config; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.io.File; @@ -33,9 +35,16 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.concurrent.TimeUnit; +import org.apache.log4j.ListAppender; +import org.apache.log4j.LogManager; +import org.apache.log4j.Logger; +import org.apache.log4j.bridge.FilterAdapter; +import org.apache.log4j.bridge.FilterWrapper; +import org.apache.log4j.bridge.AppenderAdapter.Adapter; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.core.Appender; import org.apache.logging.log4j.core.Layout; +import org.apache.logging.log4j.core.LoggerContext; import org.apache.logging.log4j.core.appender.ConsoleAppender; import org.apache.logging.log4j.core.appender.ConsoleAppender.Target; import org.apache.logging.log4j.core.appender.FileAppender; @@ -49,15 +58,28 @@ import org.apache.logging.log4j.core.appender.rolling.SizeBasedTriggeringPolicy; import org.apache.logging.log4j.core.appender.rolling.TimeBasedTriggeringPolicy; import org.apache.logging.log4j.core.appender.rolling.TriggeringPolicy; import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.config.Configurator; import org.apache.logging.log4j.core.config.LoggerConfig; +import org.apache.logging.log4j.core.filter.CompositeFilter; +import org.apache.logging.log4j.core.filter.Filterable; import org.apache.logging.log4j.core.layout.HtmlLayout; import org.apache.logging.log4j.core.layout.PatternLayout; import org.apache.logging.log4j.core.util.CloseShieldOutputStream; +import org.junit.Rule; +import org.junit.rules.TemporaryFolder; public abstract class AbstractLog4j1ConfigurationTest { + @Rule + public TemporaryFolder tempFolder = TemporaryFolder.builder().assureDeletion().build(); + abstract Configuration getConfiguration(String configResourcePrefix) throws URISyntaxException, IOException; + private LoggerContext configure(String configResourcePrefix) throws URISyntaxException, IOException { + Configurator.reconfigure(getConfiguration(configResourcePrefix)); + return (LoggerContext) org.apache.logging.log4j.LogManager.getContext(false); + } + public void testConsoleCapitalization() throws Exception { final Configuration config = getConfiguration("config-1.2/log4j-capitalization"); final Appender capitalized = config.getAppender("ConsoleCapitalized"); @@ -378,4 +400,104 @@ public abstract class AbstractLog4j1ConfigurationTest { config.start(); config.stop(); } + + /** + * Checks a hierarchy of filters. + * + * @param filter + * @return the number of filters + */ + private int checkFilters(final org.apache.logging.log4j.core.Filter filter) { + int count = 0; + if (filter instanceof CompositeFilter) { + for (final org.apache.logging.log4j.core.Filter part : ((CompositeFilter) filter).getFiltersArray()) { + count += checkFilters(part); + } + } else if (filter instanceof FilterAdapter) { + // Don't create adapters from wrappers + assertFalse("found FilterAdapter of a FilterWrapper", ((FilterAdapter) filter).getFilter() instanceof FilterWrapper); + count += checkFilters(((FilterAdapter) filter).getFilter()); + } else { + count++; + } + return count; + } + + /** + * Checks a hierarchy of filters. + * + * @param filter + * @return the number of filters + */ + private int checkFilters(final org.apache.log4j.spi.Filter filter) { + int count = 0; + if (filter instanceof FilterWrapper) { + // Don't create wrappers from adapters + assertFalse("found FilterWrapper of a FilterAdapter", ((FilterWrapper) filter).getFilter() instanceof FilterAdapter); + count += checkFilters(((FilterWrapper) filter).getFilter()); + } else { + count++; + } + // We prefer a: + // CompositeFilter of native Log4j 2.x filters + // over a: + // FilterAdapter of a chain of FilterWrappers. + assertNull("found chain of Log4j 1.x filters", filter.getNext()); + return count; + } + + public void testMultipleFilters() throws Exception { + final File folder = tempFolder.newFolder(); + System.setProperty("test.tmpDir", folder.getCanonicalPath()); + try (LoggerContext loggerContext = configure("log4j-multipleFilters")) { + final Configuration configuration = loggerContext.getConfiguration(); + assertNotNull(configuration); + // Check only number of filters. + final Filterable console = configuration.getAppender("CONSOLE"); + assertNotNull(console); + assertEquals(4, checkFilters(console.getFilter())); + final Filterable file = configuration.getAppender("FILE"); + assertNotNull(file); + assertEquals(4, checkFilters(file.getFilter())); + final Filterable rfa = configuration.getAppender("RFA"); + assertNotNull(rfa); + assertEquals(4, checkFilters(rfa.getFilter())); + final Filterable drfa = configuration.getAppender("DRFA"); + assertNotNull(drfa); + assertEquals(4, checkFilters(drfa.getFilter())); + // List appenders + final Appender appender = configuration.getAppender("LIST"); + assertNotNull(appender); + assertEquals(3, checkFilters(((Filterable)appender).getFilter())); + final ListAppender legacyAppender = (ListAppender) ((Adapter) appender).getAppender(); + final org.apache.logging.log4j.core.test.appender.ListAppender nativeAppender = configuration.getAppender("LIST2"); + assertEquals(3, checkFilters(((Filterable)nativeAppender).getFilter())); + final Logger logger = LogManager.getLogger(PropertiesConfigurationTest.class); + int expected = 0; + // message blocked by Threshold + logger.trace("NEUTRAL message"); + assertEquals(expected, legacyAppender.getEvents().size()); + assertEquals(expected, nativeAppender.getEvents().size()); + // message blocked by DenyAll filter + logger.warn("NEUTRAL message"); + assertEquals(expected, legacyAppender.getEvents().size()); + assertEquals(expected, nativeAppender.getEvents().size()); + // message accepted by level filter + logger.info("NEUTRAL message"); + expected++; + assertEquals(expected, legacyAppender.getEvents().size()); + assertEquals(expected, nativeAppender.getEvents().size()); + // message accepted by "StartsWith" filter + logger.warn("ACCEPT message"); + expected++; + assertEquals(expected, legacyAppender.getEvents().size()); + assertEquals(expected, nativeAppender.getEvents().size()); + // message blocked by "StartsWith" filter + logger.info("DENY message"); + assertEquals(expected, legacyAppender.getEvents().size()); + assertEquals(expected, nativeAppender.getEvents().size()); + } finally { + System.clearProperty("test.tmpDir"); + } + } } diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesConfigurationTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesConfigurationTest.java index 85d2859..21534f5 100644 --- a/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesConfigurationTest.java +++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesConfigurationTest.java @@ -21,7 +21,6 @@ import org.apache.log4j.LogManager; import org.apache.log4j.Logger; import org.apache.log4j.bridge.AppenderAdapter; import org.apache.log4j.bridge.FilterAdapter; -import org.apache.log4j.bridge.FilterWrapper; import org.apache.log4j.spi.LoggingEvent; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.core.Appender; @@ -114,9 +113,9 @@ public class PropertiesConfigurationTest extends AbstractLog4j1ConfigurationTest final Filterable filterable = (Filterable) appender; final CompositeFilter filter = (CompositeFilter) filterable.getFilter(); final org.apache.logging.log4j.core.Filter[] filters = filter.getFiltersArray(); - final LevelRangeFilter customFilterReal = (LevelRangeFilter) ((FilterWrapper) ((FilterAdapter) filters[0]).getFilter()).getFilter(); + final LevelRangeFilter customFilterReal = (LevelRangeFilter) filters[0]; assertEquals(Level.ALL, customFilterReal.getMinLevel()); - final LevelRangeFilter defaultFilter = (LevelRangeFilter) ((FilterWrapper) ((FilterAdapter) filters[1]).getFilter()).getFilter(); + final LevelRangeFilter defaultFilter = (LevelRangeFilter) filters[1]; assertEquals(Level.TRACE, defaultFilter.getMinLevel()); } } @@ -180,6 +179,7 @@ public class PropertiesConfigurationTest extends AbstractLog4j1ConfigurationTest } } + @Override @Test public void testConsoleEnhancedPatternLayout() throws Exception { @@ -263,4 +263,10 @@ public class PropertiesConfigurationTest extends AbstractLog4j1ConfigurationTest public void testDefaultValues() throws Exception { super.testDefaultValues(); } + + @Override + @Test + public void testMultipleFilters() throws Exception { + super.testMultipleFilters(); + } } diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/StartsWithFilter.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/StartsWithFilter.java new file mode 100644 index 0000000..db16d84 --- /dev/null +++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/StartsWithFilter.java @@ -0,0 +1,38 @@ +/* + * 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.log4j.config; + +import org.apache.log4j.spi.Filter; +import org.apache.log4j.spi.LoggingEvent; + +/** + * A Filter used in tests. + */ +public class StartsWithFilter extends Filter { + + @Override + public int decide(LoggingEvent event) { + String message = String.valueOf(event.getMessage()); + if (message.startsWith("DENY")) { + return DENY; + } else if (message.startsWith("ACCEPT")) { + return ACCEPT; + } + return NEUTRAL; + } + +} diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlConfigurationTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlConfigurationTest.java index 6cacbf8..98ac3ba 100644 --- a/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlConfigurationTest.java +++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlConfigurationTest.java @@ -186,4 +186,10 @@ public class XmlConfigurationTest extends AbstractLog4j1ConfigurationTest { super.testDefaultValues(); } + @Override + @Test + public void testMultipleFilters() throws Exception { + super.testMultipleFilters(); + } + } diff --git a/log4j-1.2-api/src/test/resources/log4j-multipleFilters.properties b/log4j-1.2-api/src/test/resources/log4j-multipleFilters.properties new file mode 100644 index 0000000..177d60c --- /dev/null +++ b/log4j-1.2-api/src/test/resources/log4j-multipleFilters.properties @@ -0,0 +1,69 @@ +# +# 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. +# + +log4j.appender.LIST=org.apache.log4j.ListAppender +log4j.appender.LIST.Threshold=DEBUG +log4j.appender.LIST.filter.1=org.apache.log4j.config.StartsWithFilter +log4j.appender.LIST.filter.2=org.apache.log4j.varia.LevelMatchFilter +log4j.appender.LIST.filter.2.LevelToMatch=INFO +log4j.appender.LIST.filter.2.AcceptOnMatch=true +log4j.appender.LIST.filter.3=org.apache.log4j.varia.DenyAllFilter + +log4j.appender.LIST2=org.apache.logging.log4j.test.appender.ListAppender +log4j.appender.LIST2.Threshold=DEBUG +log4j.appender.LIST2.filter.1=org.apache.log4j.config.StartsWithFilter +log4j.appender.LIST2.filter.2=org.apache.log4j.varia.LevelMatchFilter +log4j.appender.LIST2.filter.2.LevelToMatch=INFO +log4j.appender.LIST2.filter.2.AcceptOnMatch=true +log4j.appender.LIST2.filter.3=org.apache.log4j.varia.DenyAllFilter + +log4j.appender.CONSOLE=org.apache.log4j.ConsoleAppender +log4j.appender.CONSOLE.Threshold=DEBUG +log4j.appender.CONSOLE.filter.1=org.apache.log4j.config.StartsWithFilter +log4j.appender.CONSOLE.filter.2=org.apache.log4j.varia.LevelMatchFilter +log4j.appender.CONSOLE.filter.2.LevelToMatch=INFO +log4j.appender.CONSOLE.filter.2.AcceptOnMatch=true +log4j.appender.CONSOLE.filter.3=org.apache.log4j.varia.DenyAllFilter + +log4j.appender.FILE=org.apache.log4j.FileAppender +log4j.appender.FILE.Threshold=DEBUG +log4j.appender.FILE.File=${test.tmpDir}/file-appender.log +log4j.appender.FILE.filter.1=org.apache.log4j.config.StartsWithFilter +log4j.appender.FILE.filter.2=org.apache.log4j.varia.LevelMatchFilter +log4j.appender.FILE.filter.2.LevelToMatch=INFO +log4j.appender.FILE.filter.2.AcceptOnMatch=true +log4j.appender.FILE.filter.3=org.apache.log4j.varia.DenyAllFilter + +log4j.appender.RFA=org.apache.log4j.RollingFileAppender +log4j.appender.RFA.Threshold=DEBUG +log4j.appender.RFA.File=${test.tmpDir}/rolling-file-appender.log +log4j.appender.RFA.filter.1=org.apache.log4j.config.StartsWithFilter +log4j.appender.RFA.filter.2=org.apache.log4j.varia.LevelMatchFilter +log4j.appender.RFA.filter.2.LevelToMatch=INFO +log4j.appender.RFA.filter.2.AcceptOnMatch=true +log4j.appender.RFA.filter.3=org.apache.log4j.varia.DenyAllFilter + +log4j.appender.DRFA=org.apache.log4j.DailyRollingFileAppender +log4j.appender.DRFA.Threshold=DEBUG +log4j.appender.DRFA.File=${test.tmpDir}/daily-rolling-file-appender.log +log4j.appender.DRFA.filter.1=org.apache.log4j.config.StartsWithFilter +log4j.appender.DRFA.filter.2=org.apache.log4j.varia.LevelMatchFilter +log4j.appender.DRFA.filter.2.LevelToMatch=INFO +log4j.appender.DRFA.filter.2.AcceptOnMatch=true +log4j.appender.DRFA.filter.3=org.apache.log4j.varia.DenyAllFilter + +log4j.rootLogger=TRACE, LIST, LIST2, CONSOLE, FILE, RFA, DRFA \ No newline at end of file diff --git a/log4j-1.2-api/src/test/resources/log4j-multipleFilters.xml b/log4j-1.2-api/src/test/resources/log4j-multipleFilters.xml new file mode 100644 index 0000000..ea256e7 --- /dev/null +++ b/log4j-1.2-api/src/test/resources/log4j-multipleFilters.xml @@ -0,0 +1,92 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + 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. +--> +<!DOCTYPE log4j:configuration SYSTEM "log4j.dtd"> +<log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/";> + <appender name="LIST" class="org.apache.log4j.ListAppender"> + <param name="Threshold" value="DEBUG" /> + <filter class="org.apache.log4j.config.StartsWithFilter" /> + <filter class="org.apache.log4j.varia.LevelMatchFilter"> + <param name="LevelToMatch" value="INFO" /> + <param name="AcceptOnMatch" value="true" /> + </filter> + <filter class="org.apache.log4j.varia.DenyAllFilter" /> + </appender> + + <appender name="LIST2" class="org.apache.logging.log4j.test.appender.ListAppender"> + <param name="Threshold" value="DEBUG" /> + <filter class="org.apache.log4j.config.StartsWithFilter" /> + <filter class="org.apache.log4j.varia.LevelMatchFilter"> + <param name="LevelToMatch" value="INFO" /> + <param name="AcceptOnMatch" value="true" /> + </filter> + <filter class="org.apache.log4j.varia.DenyAllFilter" /> + </appender> + + <appender name="CONSOLE" class="org.apache.log4j.ConsoleAppender"> + <param name="Threshold" value="DEBUG" /> + <filter class="org.apache.log4j.config.StartsWithFilter" /> + <filter class="org.apache.log4j.varia.LevelMatchFilter"> + <param name="LevelToMatch" value="INFO" /> + <param name="AcceptOnMatch" value="true" /> + </filter> + <filter class="org.apache.log4j.varia.DenyAllFilter" /> + </appender> + + <appender name="FILE" class="org.apache.log4j.FileAppender"> + <param name="Threshold" value="DEBUG" /> + <param name="File" value="${test.tmpDir}/file-appender.log" /> + <filter class="org.apache.log4j.config.StartsWithFilter" /> + <filter class="org.apache.log4j.varia.LevelMatchFilter"> + <param name="LevelToMatch" value="INFO" /> + <param name="AcceptOnMatch" value="true" /> + </filter> + <filter class="org.apache.log4j.varia.DenyAllFilter" /> + </appender> + + <appender name="RFA" class="org.apache.log4j.RollingFileAppender"> + <param name="Threshold" value="DEBUG" /> + <param name="File" value="${test.tmpDir}/rolling-file-appender.log" /> + <filter class="org.apache.log4j.config.StartsWithFilter" /> + <filter class="org.apache.log4j.varia.LevelMatchFilter"> + <param name="LevelToMatch" value="INFO" /> + <param name="AcceptOnMatch" value="true" /> + </filter> + <filter class="org.apache.log4j.varia.DenyAllFilter" /> + </appender> + + <appender name="DRFA" class="org.apache.log4j.DailyRollingFileAppender"> + <param name="Threshold" value="DEBUG" /> + <param name="File" value="${test.tmpDir}/daily-rolling-file-appender.log" /> + <filter class="org.apache.log4j.config.StartsWithFilter" /> + <filter class="org.apache.log4j.varia.LevelMatchFilter"> + <param name="LevelToMatch" value="INFO" /> + <param name="AcceptOnMatch" value="true" /> + </filter> + <filter class="org.apache.log4j.varia.DenyAllFilter" /> + </appender> + + <root> + <priority value="TRACE" /> + <appender-ref ref="LIST" /> + <appender-ref ref="LIST2" /> + <appender-ref ref="CONSOLE" /> + <appender-ref ref="FILE" /> + <appender-ref ref="RFA" /> + <appender-ref ref="DRFA" /> + </root> +</log4j:configuration> diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/CompositeFilterTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/CompositeFilterTest.java new file mode 100644 index 0000000..0a5698b --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/CompositeFilterTest.java @@ -0,0 +1,49 @@ +/* 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.logging.log4j.core.filter; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +import org.apache.logging.log4j.core.Filter; +import org.apache.logging.log4j.core.Filter.Result; +import org.junit.jupiter.api.Test; + +public class CompositeFilterTest { + + @Test + public void testConcatenation() { + final Filter a = DenyAllFilter.newBuilder().setOnMatch(Result.ACCEPT).build(); + final Filter b = DenyAllFilter.newBuilder().setOnMatch(Result.NEUTRAL).build(); + final Filter c = DenyAllFilter.newBuilder().setOnMatch(Result.DENY).build(); + // The three values need to be distinguishable + assertNotEquals(a, b); + assertNotEquals(a, c); + assertNotEquals(b, c); + final Filter[] expected = new Filter[] {a, b, c}; + final CompositeFilter singleA = CompositeFilter.createFilters(new Filter[] {a}); + final CompositeFilter singleB = CompositeFilter.createFilters(new Filter[] {b}); + final CompositeFilter singleC = CompositeFilter.createFilters(new Filter[] {c}); + // Concatenating one at a time + final CompositeFilter concat1 = singleA.addFilter(b).addFilter(c); + assertArrayEquals(expected, concat1.getFiltersArray()); + // In reverse order + final CompositeFilter concat2 = singleA.addFilter(singleB.addFilter(singleC)); + assertArrayEquals(expected, concat2.getFiltersArray()); + } +} diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java index 4ac1a3f..5b870b4 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java @@ -63,9 +63,9 @@ public final class CompositeFilter extends AbstractLifeCycle implements Iterable if (filter instanceof CompositeFilter) { final int size = this.filters.length + ((CompositeFilter) filter).size(); final Filter[] copy = Arrays.copyOf(this.filters, size); - final int index = this.filters.length; + int index = this.filters.length; for (final Filter currentFilter : ((CompositeFilter) filter).filters) { - copy[index] = currentFilter; + copy[index++] = currentFilter; } return new CompositeFilter(copy); }
