Skip to content

2791 Fix handling of onMatch and onMismatch attributes in the properties configuration format #3505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: 2.x
Choose a base branch
from
Open

2791 Fix handling of onMatch and onMismatch attributes in the properties configuration format #3505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
322cc1d
Fix potential NPEs in DefaultComponentBuilder in addAttribute methods…
JWT007 Feb 27, 2025
3aa1ac2
Added JVerify annotations per review (#2791)
JWT007 Feb 27, 2025
406b097
Added DefaultComponentBuilderTest (#2791)
JWT007 Feb 27, 2025
5bc9762
Replaced DefaultComponentBuilder#put with DefaultComponentBuilder#put…
JWT007 Feb 27, 2025
e18ebf0
Reworked the builder.api and builder.api.impl packages (#2791)
JWT007 Mar 1, 2025
fcc5d3e
Update references to deprecations with new component builder APIs (#2…
JWT007 Mar 1, 2025
8415abf
Changed ConfigurationBuilder#setLoggerContext(...) to return builder …
JWT007 Mar 1, 2025
d7526eb
Adapt changelog entry for additional changes (#2791)
JWT007 Mar 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions log4j-1.2-api/src/main/java/org/apache/log4j/config/Log4j1ConfigurationConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import javax.xml.transform.stream.StreamSource;
import org.apache.logging.log4j.core.config.ConfigurationException;
import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilder;
import org.apache.logging.log4j.core.config.builder.impl.BuiltConfiguration;
import org.apache.logging.log4j.core.config.builder.impl.DefaultConfigurationBuilder;
import org.apache.logging.log4j.core.tools.BasicCommandLineArguments;
import org.apache.logging.log4j.core.tools.picocli.CommandLine;
Expand Down Expand Up @@ -160,8 +159,7 @@ private Log4j1ConfigurationConverter(final CommandLineArguments cla) {
}

protected void convert(final InputStream input, final OutputStream output) throws IOException {
final ConfigurationBuilder<BuiltConfiguration> builder =
new Log4j1ConfigurationParser().buildConfigurationBuilder(input);
final ConfigurationBuilder<?> builder = new Log4j1ConfigurationParser().buildConfigurationBuilder(input);
builder.writeXmlConfiguration(output);
}

Expand Down
3 changes: 1 addition & 2 deletions log4j-1.2-api/src/main/java/org/apache/log4j/config/Log4j1ConfigurationFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.apache.logging.log4j.core.config.ConfigurationFactory;
import org.apache.logging.log4j.core.config.ConfigurationSource;
import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilder;
import org.apache.logging.log4j.core.config.builder.impl.BuiltConfiguration;

/**
* Experimental ConfigurationFactory for Log4j 1.2 properties configuration files.
Expand All @@ -40,7 +39,7 @@ public class Log4j1ConfigurationFactory extends ConfigurationFactory {

@Override
public Configuration getConfiguration(final LoggerContext loggerContext, final ConfigurationSource source) {
final ConfigurationBuilder<BuiltConfiguration> builder;
final ConfigurationBuilder<?> builder;
try (final InputStream configStream = source.getInputStream()) {
builder = new Log4j1ConfigurationParser().buildConfigurationBuilder(configStream);
} catch (final IOException e) {
Expand Down
39 changes: 21 additions & 18 deletions log4j-1.2-api/src/main/java/org/apache/log4j/config/Log4j1ConfigurationParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilderFactory;
import org.apache.logging.log4j.core.config.builder.api.LayoutComponentBuilder;
import org.apache.logging.log4j.core.config.builder.api.LoggerComponentBuilder;
import org.apache.logging.log4j.core.config.builder.api.PropertyComponentBuilder;
import org.apache.logging.log4j.core.config.builder.api.RootLoggerComponentBuilder;
import org.apache.logging.log4j.core.config.builder.impl.BuiltConfiguration;
import org.apache.logging.log4j.status.StatusLogger;
Expand Down Expand Up @@ -108,7 +109,7 @@ public ConfigurationBuilder<BuiltConfiguration> buildConfigurationBuilder(final
if (threshold != null) {
final Level level = OptionConverter.convertLevel(threshold.trim(), Level.ALL);
builder.add(builder.newFilter("ThresholdFilter", Result.NEUTRAL, Result.DENY)
.addAttribute("level", level));
.setAttribute("level", level));
}
// Root
buildRootLogger(getLog4jValue(ROOTCATEGORY));
Expand All @@ -134,7 +135,9 @@ private void buildProperties() {
for (final Map.Entry<Object, Object> entry : new TreeMap<>(properties).entrySet()) {
final String key = entry.getKey().toString();
if (!key.startsWith("log4j.") && !key.equals(ROOTCATEGORY) && !key.equals(ROOTLOGGER)) {
builder.addProperty(key, Objects.toString(entry.getValue(), Strings.EMPTY));
PropertyComponentBuilder propertyComponentBuilder =
builder.newProperty(key, Objects.toString(entry.getValue(), Strings.EMPTY));
builder.add(propertyComponentBuilder);
}
}
}
Expand Down Expand Up @@ -204,7 +207,7 @@ private void buildConsoleAppender(final String appenderName) {
target = null;
}
if (target != null) {
appenderBuilder.addAttribute("target", target);
appenderBuilder.setAttribute("target", target);
}
}
buildAttribute(appenderName, appenderBuilder, "Follow", "follow");
Expand Down Expand Up @@ -236,13 +239,13 @@ private void buildDailyRollingFileAppender(final String appenderName) {
buildFileAppender(appenderName, appenderBuilder);
final String fileName = getLog4jAppenderValue(appenderName, "File");
final String datePattern = getLog4jAppenderValue(appenderName, "DatePattern", ".yyyy-MM-dd");
appenderBuilder.addAttribute("filePattern", fileName + "%d{" + datePattern + "}");
appenderBuilder.setAttribute("filePattern", fileName + "%d{" + datePattern + "}");
final ComponentBuilder<?> triggeringPolicy = builder.newComponent("Policies")
.addComponent(builder.newComponent("TimeBasedTriggeringPolicy").addAttribute("modulate", true));
.addComponent(builder.newComponent("TimeBasedTriggeringPolicy").setAttribute("modulate", true));
appenderBuilder.addComponent(triggeringPolicy);
appenderBuilder.addComponent(builder.newComponent("DefaultRolloverStrategy")
.addAttribute("max", Integer.MAX_VALUE)
.addAttribute("fileIndex", "min"));
.setAttribute("max", Integer.MAX_VALUE)
.setAttribute("fileIndex", "min"));
builder.add(appenderBuilder);
}

Expand All @@ -251,16 +254,16 @@ private void buildRollingFileAppender(final String appenderName) {
builder.newAppender(appenderName, RollingFileAppender.PLUGIN_NAME);
buildFileAppender(appenderName, appenderBuilder);
final String fileName = getLog4jAppenderValue(appenderName, "File");
appenderBuilder.addAttribute("filePattern", fileName + ".%i");
appenderBuilder.setAttribute("filePattern", fileName + ".%i");
final String maxFileSizeString = getLog4jAppenderValue(appenderName, "MaxFileSize", "10485760");
final String maxBackupIndexString = getLog4jAppenderValue(appenderName, "MaxBackupIndex", "1");
final ComponentBuilder<?> triggeringPolicy = builder.newComponent("Policies")
.addComponent(
builder.newComponent("SizeBasedTriggeringPolicy").addAttribute("size", maxFileSizeString));
builder.newComponent("SizeBasedTriggeringPolicy").setAttribute("size", maxFileSizeString));
appenderBuilder.addComponent(triggeringPolicy);
appenderBuilder.addComponent(builder.newComponent("DefaultRolloverStrategy")
.addAttribute("max", maxBackupIndexString)
.addAttribute("fileIndex", "min"));
.setAttribute("max", maxBackupIndexString)
.setAttribute("fileIndex", "min"));
builder.add(appenderBuilder);
}

Expand All @@ -271,7 +274,7 @@ private void buildAttribute(
final String targetAttributeName) {
final String attributeValue = getLog4jAppenderValue(componentName, sourceAttributeName);
if (attributeValue != null) {
componentBuilder.addAttribute(targetAttributeName, attributeValue);
componentBuilder.setAttribute(targetAttributeName, attributeValue);
}
}

Expand All @@ -282,7 +285,7 @@ private void buildMandatoryAttribute(
final String targetAttributeName) {
final String attributeValue = getLog4jAppenderValue(componentName, sourceAttributeName);
if (attributeValue != null) {
componentBuilder.addAttribute(targetAttributeName, attributeValue);
componentBuilder.setAttribute(targetAttributeName, attributeValue);
} else {
reportWarning("Missing " + sourceAttributeName + " for " + componentName);
}
Expand Down Expand Up @@ -358,19 +361,19 @@ private void buildAppenderLayout(final String name, final AppenderComponentBuild
}
case "org.apache.log4j.HTMLLayout": {
final LayoutComponentBuilder htmlLayout = builder.newLayout("HtmlLayout");
htmlLayout.addAttribute("title", getLog4jAppenderValue(name, "layout.Title", "Log4J Log Messages"));
htmlLayout.addAttribute(
htmlLayout.setAttribute("title", getLog4jAppenderValue(name, "layout.Title", "Log4J Log Messages"));
htmlLayout.setAttribute(
"locationInfo",
Boolean.parseBoolean(getLog4jAppenderValue(name, "layout.LocationInfo", FALSE)));
appenderBuilder.add(htmlLayout);
break;
}
case "org.apache.log4j.xml.XMLLayout": {
final LayoutComponentBuilder xmlLayout = builder.newLayout("Log4j1XmlLayout");
xmlLayout.addAttribute(
xmlLayout.setAttribute(
"locationInfo",
Boolean.parseBoolean(getLog4jAppenderValue(name, "layout.LocationInfo", FALSE)));
xmlLayout.addAttribute(
xmlLayout.setAttribute(
"properties",
Boolean.parseBoolean(getLog4jAppenderValue(name, "layout.Properties", FALSE)));
appenderBuilder.add(xmlLayout);
Expand All @@ -385,7 +388,7 @@ private void buildAppenderLayout(final String name, final AppenderComponentBuild
private LayoutComponentBuilder newPatternLayout(final String pattern) {
final LayoutComponentBuilder layoutBuilder = builder.newLayout("PatternLayout");
if (pattern != null) {
layoutBuilder.addAttribute("pattern", pattern);
layoutBuilder.setAttribute("pattern", pattern);
}
return layoutBuilder;
}
Expand Down
2 changes: 1 addition & 1 deletion ...-core-fuzz-test/src/main/java/org/apache/logging/log4j/core/fuzz/PatternLayoutFuzzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public static void fuzzerTestOneInput(final FuzzedDataProvider dataProvider) {
createLoggerContext(loggerContextName, EncodingAppender.PLUGIN_NAME, configBuilder -> configBuilder
.newLayout("PatternLayout")
// Enforce using a single message-based converter, i.e., `MessagePatternConverter`
.addAttribute("pattern", "%m"))) {
.setAttribute("pattern", "%m"))) {
final ExtendedLogger logger = loggerContext.getLogger(PatternLayoutFuzzer.class);
final LoggerFacade loggerFacade = new Log4jLoggerFacade(logger);
fuzzLogger(loggerFacade, dataProvider);
Expand Down
6 changes: 2 additions & 4 deletions ...j-core-test/src/test/java/org/apache/logging/log4j/core/EventParameterMemoryLeakTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilderFactory;
import org.apache.logging.log4j.core.config.builder.api.LayoutComponentBuilder;
import org.apache.logging.log4j.core.config.builder.api.RootLoggerComponentBuilder;
import org.apache.logging.log4j.core.config.builder.impl.BuiltConfiguration;
import org.apache.logging.log4j.core.test.appender.ListAppender;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -88,10 +87,9 @@ private static LoggerContext createLoggerContext(

@SuppressWarnings("SameParameterValue")
private static Configuration createConfiguration(final String appenderName) {
final ConfigurationBuilder<BuiltConfiguration> configBuilder =
ConfigurationBuilderFactory.newConfigurationBuilder();
final ConfigurationBuilder<?> configBuilder = ConfigurationBuilderFactory.newConfigurationBuilder();
final LayoutComponentBuilder layoutComponentBuilder =
configBuilder.newLayout("PatternLayout").addAttribute("pattern", "%m");
configBuilder.newLayout("PatternLayout").setAttribute("pattern", "%m");
final AppenderComponentBuilder appenderComponentBuilder =
configBuilder.newAppender(appenderName, "List").add(layoutComponentBuilder);
final RootLoggerComponentBuilder loggerComponentBuilder =
Expand Down
9 changes: 3 additions & 6 deletions ...re-test/src/test/java/org/apache/logging/log4j/core/appender/ReconfigureAppenderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@
import org.apache.logging.log4j.core.config.Configurator;
import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilder;
import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilderFactory;
import org.apache.logging.log4j.core.config.builder.impl.BuiltConfiguration;
import org.apache.logging.log4j.core.layout.PatternLayout;
import org.apache.logging.log4j.core.util.Builder;
import org.junit.jupiter.api.Test;

class ReconfigureAppenderTest {
Expand Down Expand Up @@ -127,8 +125,7 @@ private void removeAppender() {
}

private void createAndAddAppender() {
final ConfigurationBuilder<BuiltConfiguration> config_builder =
ConfigurationBuilderFactory.newConfigurationBuilder();
final ConfigurationBuilder<?> config_builder = ConfigurationBuilderFactory.newConfigurationBuilder();

// All loggers must have a root logger. The default root logger logs ERRORs to the console.
// Override this with a root logger that does not log anywhere as we leave it up the
Expand All @@ -141,10 +138,10 @@ private void createAndAddAppender() {
// Retrieve the logger.
final Logger logger = (Logger) LogManager.getLogger(this.getClass());

final Builder pattern_builder =
final PatternLayout.Builder pattern_builder =
PatternLayout.newBuilder().withPattern("[%d{dd-MM-yy HH:mm:ss}] %p %m %throwable %n");

final PatternLayout pattern_layout = (PatternLayout) pattern_builder.build();
final PatternLayout pattern_layout = pattern_builder.build();

appender = RollingFileAppender.newBuilder()
.withLayout(pattern_layout)
Expand Down
46 changes: 19 additions & 27 deletions ...est/src/test/java/org/apache/logging/log4j/core/appender/SocketAppenderReconnectTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import org.apache.logging.log4j.core.config.builder.api.ComponentBuilder;
import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilder;
import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilderFactory;
import org.apache.logging.log4j.core.config.builder.impl.BuiltConfiguration;
import org.apache.logging.log4j.core.net.TcpSocketManager;
import org.apache.logging.log4j.core.net.TcpSocketManager.HostResolver;
import org.apache.logging.log4j.core.net.ssl.SslKeyStoreConstants;
Expand Down Expand Up @@ -219,24 +218,21 @@ void key_store_changes_should_be_detected_at_reconfigure(
final int server1Port = server1.getServerSocket().getLocalPort();

// Create the configuration transformer to add the `<Ssl>`, `<KeyStore>`, and `<TrustStore>` elements
final BiFunction<
ConfigurationBuilder<BuiltConfiguration>,
AppenderComponentBuilder,
AppenderComponentBuilder>
final BiFunction<ConfigurationBuilder<?>, AppenderComponentBuilder, AppenderComponentBuilder>
appenderComponentBuilderTransformer = (configBuilder, appenderComponentBuilder) -> {
final ComponentBuilder<?> keyStoreComponentBuilder = configBuilder
.newComponent("KeyStore")
.addAttribute("type", keyStoreType)
.addAttribute("location", keyStoreFilePath.toString())
.addAttribute("passwordFile", keyStorePasswordFilePath);
.setAttribute("type", keyStoreType)
.setAttribute("location", keyStoreFilePath.toString())
.setAttribute("passwordFile", keyStorePasswordFilePath);
final ComponentBuilder<?> trustStoreComponentBuilder = configBuilder
.newComponent("TrustStore")
.addAttribute("type", trustStoreType)
.addAttribute("location", trustStoreFilePath.toString())
.addAttribute("passwordFile", trustStorePasswordFilePath);
.setAttribute("type", trustStoreType)
.setAttribute("location", trustStoreFilePath.toString())
.setAttribute("passwordFile", trustStorePasswordFilePath);
return appenderComponentBuilder.addComponent(configBuilder
.newComponent("Ssl")
.addAttribute("protocol", "TLS")
.setAttribute("protocol", "TLS")
.addComponent(keyStoreComponentBuilder)
.addComponent(trustStoreComponentBuilder));
};
Expand Down Expand Up @@ -301,27 +297,23 @@ private static Configuration createConfiguration(
final String host,
final int port,
@Nullable
final BiFunction<
ConfigurationBuilder<BuiltConfiguration>,
AppenderComponentBuilder,
AppenderComponentBuilder>
final BiFunction<ConfigurationBuilder<?>, AppenderComponentBuilder, AppenderComponentBuilder>
appenderComponentBuilderTransformer) {

// Create the configuration builder
final ConfigurationBuilder<BuiltConfiguration> configBuilder =
ConfigurationBuilderFactory.newConfigurationBuilder()
.setStatusLevel(Level.ERROR)
.setConfigurationName(SocketAppenderReconnectTest.class.getSimpleName());
final ConfigurationBuilder<?> configBuilder = ConfigurationBuilderFactory.newConfigurationBuilder()
.setStatusLevel(Level.ERROR)
.setConfigurationName(SocketAppenderReconnectTest.class.getSimpleName());

// Create the appender configuration
final AppenderComponentBuilder appenderComponentBuilder = configBuilder
.newAppender(APPENDER_NAME, "Socket")
.addAttribute("host", host)
.addAttribute("port", port)
.addAttribute("ignoreExceptions", false)
.addAttribute("reconnectionDelayMillis", 10)
.addAttribute("immediateFlush", true)
.add(configBuilder.newLayout("PatternLayout").addAttribute("pattern", "%m%n"));
.setAttribute("host", host)
.setAttribute("port", port)
.setAttribute("ignoreExceptions", false)
.setAttribute("reconnectionDelayMillis", 10)
.setAttribute("immediateFlush", true)
.add(configBuilder.newLayout("PatternLayout").setAttribute("pattern", "%m%n"));
final AppenderComponentBuilder transformedAppenderComponentBuilder = appenderComponentBuilderTransformer != null
? appenderComponentBuilderTransformer.apply(configBuilder, appenderComponentBuilder)
: appenderComponentBuilder;
Expand Down Expand Up @@ -406,7 +398,7 @@ private static void verifyLoggingFailure(
// Verify the failure
for (int i = 0; i < retryCount; i++) {
try {
logger.info("should fail #" + i);
logger.info("should fail #{}", i);
fail("should have failed #" + i);
} catch (final AppenderLoggingException ignored) {
assertThat(errorHandler.getBuffer()).hasSize(2 * (i + 1));
Expand Down
Loading