diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfigurationFactory.java b/log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfigurationFactory.java index fb6f1383be5..9b95b7eea94 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfigurationFactory.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfigurationFactory.java @@ -43,17 +43,25 @@ public class PropertiesConfigurationFactory extends ConfigurationFactory { */ protected static final String DEFAULT_PREFIX = "log4j"; + @Override + protected boolean isActive() { + return PropertiesUtil.getProperties() + .getBooleanProperty(ConfigurationFactory.LOG4J1_EXPERIMENTAL, Boolean.FALSE); + } + @Override protected String[] getSupportedTypes() { - if (!PropertiesUtil.getProperties() - .getBooleanProperty(ConfigurationFactory.LOG4J1_EXPERIMENTAL, Boolean.FALSE)) { - return null; + if (!isActive()) { + throw new IllegalStateException(getClass().getName() + " is inactive"); } return new String[] {FILE_EXTENSION}; } @Override public Configuration getConfiguration(final LoggerContext loggerContext, final ConfigurationSource source) { + if (!isActive()) { + throw new IllegalStateException(getClass().getName() + " is inactive"); + } final int interval = PropertiesUtil.getProperties().getIntegerProperty(Log4j1Configuration.MONITOR_INTERVAL, 0); return new PropertiesConfiguration(loggerContext, source, interval); } diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/config/package-info.java b/log4j-1.2-api/src/main/java/org/apache/log4j/config/package-info.java index 396cf9bbd32..8473f9ce3b7 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/config/package-info.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/config/package-info.java @@ -18,7 +18,7 @@ * Log4j 1.x compatibility layer. */ @Export -@Version("2.20.1") +@Version("2.27.0") @Open("org.apache.logging.log4j.core") package org.apache.log4j.config; diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfigurationFactory.java b/log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfigurationFactory.java index a5b10fce848..08b18fa692d 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfigurationFactory.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfigurationFactory.java @@ -47,17 +47,25 @@ public class XmlConfigurationFactory extends ConfigurationFactory { */ protected static final String DEFAULT_PREFIX = "log4j"; + @Override + protected boolean isActive() { + return PropertiesUtil.getProperties() + .getBooleanProperty(ConfigurationFactory.LOG4J1_EXPERIMENTAL, Boolean.FALSE); + } + @Override protected String[] getSupportedTypes() { - if (!PropertiesUtil.getProperties() - .getBooleanProperty(ConfigurationFactory.LOG4J1_EXPERIMENTAL, Boolean.FALSE)) { - return null; + if (!isActive()) { + throw new IllegalStateException(getClass().getName() + " is inactive"); } return new String[] {FILE_EXTENSION}; } @Override public Configuration getConfiguration(final LoggerContext loggerContext, final ConfigurationSource source) { + if (!isActive()) { + throw new IllegalStateException(getClass().getName() + " is inactive"); + } final int interval = PropertiesUtil.getProperties().getIntegerProperty(Log4j1Configuration.MONITOR_INTERVAL, 0); return new XmlConfiguration(loggerContext, source, interval); } diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/xml/package-info.java b/log4j-1.2-api/src/main/java/org/apache/log4j/xml/package-info.java index f420386fa0e..e4c7b423d40 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/xml/package-info.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/xml/package-info.java @@ -18,7 +18,7 @@ * Log4j 1.x compatibility layer. */ @Export -@Version("2.25.3") +@Version("2.27.0") package org.apache.log4j.xml; import org.osgi.annotation.bundle.Export; diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/LoggerJira3410Test.java b/log4j-1.2-api/src/test/java/org/apache/log4j/LoggerJira3410Test.java index be16d3d0953..97fb31eab95 100644 --- a/log4j-1.2-api/src/test/java/org/apache/log4j/LoggerJira3410Test.java +++ b/log4j-1.2-api/src/test/java/org/apache/log4j/LoggerJira3410Test.java @@ -27,12 +27,14 @@ import org.apache.logging.log4j.core.Appender; import org.apache.logging.log4j.core.LoggerContext; import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.test.junit.SetTestProperty; import org.apache.logging.log4j.util.SortedArrayStringMap; import org.junit.jupiter.api.Test; /** * Tests Jira3410. */ +@SetTestProperty(key = "log4j1.compatibility", value = "true") class LoggerJira3410Test { @Test diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/AsyncAppenderTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/AsyncAppenderTest.java index b74c3365ae7..4901321ad00 100644 --- a/log4j-1.2-api/src/test/java/org/apache/log4j/config/AsyncAppenderTest.java +++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/AsyncAppenderTest.java @@ -29,6 +29,7 @@ import org.apache.log4j.Logger; import org.apache.log4j.bridge.AppenderAdapter; import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.test.junit.SetTestProperty; import org.apache.logging.log4j.test.junit.UsingStatusListener; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -36,6 +37,7 @@ /** * Test configuration from XML. */ +@SetTestProperty(key = "log4j1.compatibility", value = "true") @UsingStatusListener class AsyncAppenderTest { diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesConfigurationFactoryActivationTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesConfigurationFactoryActivationTest.java new file mode 100644 index 00000000000..8a9a8131297 --- /dev/null +++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesConfigurationFactoryActivationTest.java @@ -0,0 +1,82 @@ +/* + * 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 static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.net.URI; +import java.net.URL; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.config.ConfigurationFactory; +import org.apache.logging.log4j.core.config.ConfigurationSource; +import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.WritesSystemProperty; + +@WritesSystemProperty +class PropertiesConfigurationFactoryActivationTest { + + @Test + void activationTracksTheCompatibilityProperty() throws Exception { + final String property = ConfigurationFactory.LOG4J1_EXPERIMENTAL; + final String previousValue = System.getProperty(property); + final PropertiesConfigurationFactory factory = new PropertiesConfigurationFactory(); + try { + System.setProperty(property, "false"); + assertFalse(factory.isActive()); + assertInactive(factory); + + System.setProperty(property, "true"); + assertTrue(factory.isActive()); + assertArrayEquals(new String[] {".properties"}, factory.getSupportedTypes()); + final URL resource = getClass().getResource("/config-1.2/log4j-console-SimpleLayout.properties"); + final ConfigurationSource source = ConfigurationSource.fromUri(resource.toURI()); + try (final LoggerContext context = new LoggerContext("test")) { + assertInstanceOf(PropertiesConfiguration.class, factory.getConfiguration(context, source)); + } + + System.setProperty(property, "false"); + assertFalse(factory.isActive()); + assertInactive(factory); + } finally { + if (previousValue == null) { + System.clearProperty(property); + } else { + System.setProperty(property, previousValue); + } + } + } + + private static void assertInactive(final PropertiesConfigurationFactory factory) { + assertThrows(IllegalStateException.class, factory::getSupportedTypes); + assertThrows( + IllegalStateException.class, () -> factory.getConfiguration(null, ConfigurationSource.NULL_SOURCE)); + assertThrows( + IllegalStateException.class, + () -> factory.getConfiguration(null, "test", URI.create("classpath:log4j.properties"))); + assertThrows( + IllegalStateException.class, + () -> factory.getConfiguration( + null, + "test", + URI.create("classpath:log4j.properties"), + PropertiesConfigurationFactoryActivationTest.class.getClassLoader())); + } +} 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 fb8ed815671..dbfe77abdc3 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 @@ -50,11 +50,13 @@ import org.apache.logging.log4j.core.filter.Filterable; import org.apache.logging.log4j.core.filter.LevelRangeFilter; import org.apache.logging.log4j.core.layout.PatternLayout; +import org.apache.logging.log4j.test.junit.SetTestProperty; import org.junit.jupiter.api.Test; /** * Test configuration from Properties. */ +@SetTestProperty(key = "log4j1.compatibility", value = "true") class PropertiesConfigurationTest extends AbstractLog4j1ConfigurationTest { private static final String TEST_KEY = "log4j.test.tmpdir"; diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesReconfigurationTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesReconfigurationTest.java index dbdca87084c..f8937c0f1dd 100644 --- a/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesReconfigurationTest.java +++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesReconfigurationTest.java @@ -43,11 +43,13 @@ import org.apache.logging.log4j.core.config.ConfigurationListener; import org.apache.logging.log4j.core.config.LoggerConfig; import org.apache.logging.log4j.core.config.Reconfigurable; +import org.apache.logging.log4j.test.junit.SetTestProperty; import org.junit.jupiter.api.Test; /** * Test reconfiguring with an XML configuration. */ +@SetTestProperty(key = "log4j1.compatibility", value = "true") class PropertiesReconfigurationTest { private class TestListener implements ConfigurationListener { diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/SocketAppenderConfigurationTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/SocketAppenderConfigurationTest.java index 2294c556d18..92f1ee2a93b 100644 --- a/log4j-1.2-api/src/test/java/org/apache/log4j/config/SocketAppenderConfigurationTest.java +++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/SocketAppenderConfigurationTest.java @@ -31,11 +31,13 @@ import org.apache.logging.log4j.core.filter.ThresholdFilter; import org.apache.logging.log4j.core.net.Protocol; import org.apache.logging.log4j.core.net.TcpSocketManager; +import org.apache.logging.log4j.test.junit.SetTestProperty; import org.junit.jupiter.api.Test; /** * Tests configuring a Syslog appender. */ +@SetTestProperty(key = "log4j1.compatibility", value = "true") class SocketAppenderConfigurationTest { private SocketAppender check(final Protocol expected, final Configuration configuration) { diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/SyslogAppenderConfigurationTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/SyslogAppenderConfigurationTest.java index 1313cee2c44..929f43fc26d 100644 --- a/log4j-1.2-api/src/test/java/org/apache/log4j/config/SyslogAppenderConfigurationTest.java +++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/SyslogAppenderConfigurationTest.java @@ -30,6 +30,7 @@ import org.apache.logging.log4j.core.filter.ThresholdFilter; import org.apache.logging.log4j.core.net.AbstractSocketManager; import org.apache.logging.log4j.core.net.Protocol; +import org.apache.logging.log4j.test.junit.SetTestProperty; import org.apache.logging.log4j.test.junit.UsingStatusListener; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -39,6 +40,7 @@ /** * Tests configuring a Syslog appender. */ +@SetTestProperty(key = "log4j1.compatibility", value = "true") @UsingStatusListener @WritesSystemProperty class SyslogAppenderConfigurationTest { 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 8cf58620597..dfe099c7ec6 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 @@ -43,11 +43,13 @@ import org.apache.logging.log4j.core.appender.rolling.TriggeringPolicy; import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.config.ConfigurationSource; +import org.apache.logging.log4j.test.junit.SetTestProperty; import org.junit.jupiter.api.Test; /** * Test configuration from XML. */ +@SetTestProperty(key = "log4j1.compatibility", value = "true") class XmlConfigurationTest extends AbstractLog4j1ConfigurationTest { private static final String SUFFIX = ".xml"; diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlReconfigurationTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlReconfigurationTest.java index ee9dbfcdb9a..a5797d3937a 100644 --- a/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlReconfigurationTest.java +++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlReconfigurationTest.java @@ -30,11 +30,13 @@ import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.config.ConfigurationListener; import org.apache.logging.log4j.core.config.Reconfigurable; +import org.apache.logging.log4j.test.junit.SetTestProperty; import org.junit.jupiter.api.Test; /** * Test reconfiguring with an XML configuration. */ +@SetTestProperty(key = "log4j1.compatibility", value = "true") class XmlReconfigurationTest { private static final String CONFIG = "target/test-classes/log4j1-file.xml"; diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/xml/XmlConfigurationFactoryActivationTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/xml/XmlConfigurationFactoryActivationTest.java new file mode 100644 index 00000000000..8cc81f626e4 --- /dev/null +++ b/log4j-1.2-api/src/test/java/org/apache/log4j/xml/XmlConfigurationFactoryActivationTest.java @@ -0,0 +1,82 @@ +/* + * 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.xml; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.net.URI; +import java.net.URL; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.config.ConfigurationFactory; +import org.apache.logging.log4j.core.config.ConfigurationSource; +import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.WritesSystemProperty; + +@WritesSystemProperty +class XmlConfigurationFactoryActivationTest { + + @Test + void activationTracksTheCompatibilityProperty() throws Exception { + final String property = ConfigurationFactory.LOG4J1_EXPERIMENTAL; + final String previousValue = System.getProperty(property); + final XmlConfigurationFactory factory = new XmlConfigurationFactory(); + try { + System.setProperty(property, "false"); + assertFalse(factory.isActive()); + assertInactive(factory); + + System.setProperty(property, "true"); + assertTrue(factory.isActive()); + assertArrayEquals(new String[] {".xml"}, factory.getSupportedTypes()); + final URL resource = getClass().getResource("/config-1.2/log4j-console-SimpleLayout.xml"); + final ConfigurationSource source = ConfigurationSource.fromUri(resource.toURI()); + try (final LoggerContext context = new LoggerContext("test")) { + assertInstanceOf(XmlConfiguration.class, factory.getConfiguration(context, source)); + } + + System.setProperty(property, "false"); + assertFalse(factory.isActive()); + assertInactive(factory); + } finally { + if (previousValue == null) { + System.clearProperty(property); + } else { + System.setProperty(property, previousValue); + } + } + } + + private static void assertInactive(final XmlConfigurationFactory factory) { + assertThrows(IllegalStateException.class, factory::getSupportedTypes); + assertThrows( + IllegalStateException.class, () -> factory.getConfiguration(null, ConfigurationSource.NULL_SOURCE)); + assertThrows( + IllegalStateException.class, + () -> factory.getConfiguration(null, "test", URI.create("classpath:log4j.xml"))); + assertThrows( + IllegalStateException.class, + () -> factory.getConfiguration( + null, + "test", + URI.create("classpath:log4j.xml"), + XmlConfigurationFactoryActivationTest.class.getClassLoader())); + } +} diff --git a/log4j-core-test/pom.xml b/log4j-core-test/pom.xml index 34ccfc0453b..133bb098bb2 100644 --- a/log4j-core-test/pom.xml +++ b/log4j-core-test/pom.xml @@ -436,6 +436,34 @@ org.apache.logging.log4j.core.test.categories.AsyncLoggers + + test-without-jackson-databind + + test + + + + com.fasterxml.jackson.core:jackson-databind + + + **/JsonConfigurationFactoryMissingDependenciesIT.java + + + + + test-without-jackson-dataformat-yaml + + test + + + + com.fasterxml.jackson.dataformat:jackson-dataformat-yaml + + + **/YamlConfigurationFactoryMissingDependenciesIT.java + + + diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryActivationTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryActivationTest.java new file mode 100644 index 00000000000..701ace7c31a --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryActivationTest.java @@ -0,0 +1,236 @@ +/* + * 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.config; + +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.reflect.Field; +import java.net.URI; +import java.net.URL; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.config.json.JsonConfigurationFactory; +import org.apache.logging.log4j.core.config.yaml.YamlConfigurationFactory; +import org.apache.logging.log4j.core.util.ReflectionUtil; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.WritesSystemProperty; + +@WritesSystemProperty +class ConfigurationFactoryActivationTest { + + private static final String[] CONFIGURATION_PROPERTIES = { + ConfigurationFactory.CONFIGURATION_FILE_PROPERTY, + ConfigurationFactory.LOG4J1_CONFIGURATION_FILE_PROPERTY, + ConfigurationFactory.LOG4J1_EXPERIMENTAL + }; + + private final InactiveConfigurationFactory inactiveFactory = new InactiveConfigurationFactory(); + private final Map previousProperties = new HashMap<>(); + + private Field factoriesField; + private List previousFactories; + private ConfigurationFactory previousConfigurationFactory; + + @BeforeEach + void setUp() throws Exception { + previousConfigurationFactory = ConfigurationFactory.getInstance(); + previousFactories = ConfigurationFactory.getFactories(); + factoriesField = ConfigurationFactory.class.getDeclaredField("factories"); + ReflectionUtil.setStaticFieldValue(factoriesField, Collections.singletonList(inactiveFactory)); + ConfigurationFactory.resetConfigurationFactory(); + for (final String property : CONFIGURATION_PROPERTIES) { + previousProperties.put(property, System.getProperty(property)); + System.clearProperty(property); + } + } + + @AfterEach + void tearDown() { + ReflectionUtil.setStaticFieldValue(factoriesField, previousFactories); + ConfigurationFactory.setConfigurationFactory(previousConfigurationFactory); + for (final String property : CONFIGURATION_PROPERTIES) { + final String previousValue = previousProperties.get(property); + if (previousValue == null) { + System.clearProperty(property); + } else { + System.setProperty(property, previousValue); + } + } + } + + @Test + void skipsInactiveFactoryForWildcardAndDefaultResourceSearch() { + try (final LoggerContext context = new LoggerContext("test")) { + assertInstanceOf( + DefaultConfiguration.class, + ConfigurationFactory.getInstance().getConfiguration(context, "test", (URI) null)); + } + assertFactoryWasChecked(); + } + + @Test + void skipsInactiveFactoryForExplicitUri() { + try (final LoggerContext context = new LoggerContext("test")) { + assertInstanceOf( + DefaultConfiguration.class, + ConfigurationFactory.getInstance() + .getConfiguration(context, "test", URI.create("file:///configuration.inactive"))); + } + assertFactoryWasChecked(); + } + + @Test + void skipsInactiveFactoryForConfiguredLocation() throws Exception { + final URL resource = getClass().getResource("/log4j-test1.xml"); + assertNotNull(resource); + System.setProperty( + ConfigurationFactory.CONFIGURATION_FILE_PROPERTY, + resource.toURI().toString()); + try (final LoggerContext context = new LoggerContext("test")) { + assertNull(ConfigurationFactory.getInstance().getConfiguration(context, "test", (URI) null)); + } + assertFactoryWasChecked(); + } + + @Test + void skipsInactiveFactoryBeforeVersionCheck() throws Exception { + final URL resource = getClass().getResource("/log4j-test1.xml"); + assertNotNull(resource); + System.setProperty( + ConfigurationFactory.LOG4J1_CONFIGURATION_FILE_PROPERTY, + resource.toURI().toString()); + try (final LoggerContext context = new LoggerContext("test")) { + assertNull(ConfigurationFactory.getInstance().getConfiguration(context, "test", (URI) null)); + } + assertFactoryWasChecked(); + } + + @Test + void skipsInactiveFactoryForConfigurationSource() throws Exception { + final URL resource = getClass().getResource("/log4j-test1.xml"); + assertNotNull(resource); + final ConfigurationSource source = ConfigurationSource.fromUri(resource.toURI()); + assertNotNull(source); + try (final LoggerContext context = new LoggerContext("test")) { + assertNull(ConfigurationFactory.getInstance().getConfiguration(context, source)); + } finally { + source.getInputStream().close(); + } + assertFactoryWasChecked(); + } + + @Test + void jsonFactoryUsesOverriddenActiveState() { + final InactiveJsonConfigurationFactory factory = new InactiveJsonConfigurationFactory(); + assertTrue(factory.dependenciesAvailable()); + assertThrows(IllegalStateException.class, factory::getSupportedTypes); + assertThrows( + IllegalStateException.class, () -> factory.getConfiguration(null, ConfigurationSource.NULL_SOURCE)); + } + + @Test + void yamlFactoryUsesOverriddenActiveState() { + final InactiveYamlConfigurationFactory factory = new InactiveYamlConfigurationFactory(); + assertTrue(factory.dependenciesAvailable()); + assertThrows(IllegalStateException.class, factory::getSupportedTypes); + assertThrows( + IllegalStateException.class, () -> factory.getConfiguration(null, ConfigurationSource.NULL_SOURCE)); + } + + private void assertFactoryWasChecked() { + assertTrue(inactiveFactory.activeChecks > 0); + } + + private static final class InactiveConfigurationFactory extends ConfigurationFactory { + + private int activeChecks; + + @Override + protected boolean isActive() { + activeChecks++; + return false; + } + + @Override + protected String getVersion() { + throw unexpectedCall("getVersion"); + } + + @Override + protected String getTestPrefix() { + throw unexpectedCall("getTestPrefix"); + } + + @Override + protected String getDefaultPrefix() { + throw unexpectedCall("getDefaultPrefix"); + } + + @Override + protected String[] getSupportedTypes() { + throw unexpectedCall("getSupportedTypes"); + } + + @Override + public Configuration getConfiguration( + final LoggerContext loggerContext, final String name, final URI configLocation) { + throw unexpectedCall("getConfiguration(LoggerContext, String, URI)"); + } + + @Override + public Configuration getConfiguration(final LoggerContext loggerContext, final ConfigurationSource source) { + throw unexpectedCall("getConfiguration(LoggerContext, ConfigurationSource)"); + } + + private static AssertionError unexpectedCall(final String method) { + return new AssertionError(method + " should not be called for an inactive factory"); + } + } + + private static final class InactiveJsonConfigurationFactory extends JsonConfigurationFactory { + + @Override + protected boolean isActive() { + return false; + } + + private boolean dependenciesAvailable() { + return super.isActive(); + } + } + + private static final class InactiveYamlConfigurationFactory extends YamlConfigurationFactory { + + @Override + protected boolean isActive() { + return false; + } + + private boolean dependenciesAvailable() { + return super.isActive(); + } + } +} diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/json/JsonConfigurationFactoryMissingDependenciesIT.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/json/JsonConfigurationFactoryMissingDependenciesIT.java new file mode 100644 index 00000000000..126fc13a1e7 --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/json/JsonConfigurationFactoryMissingDependenciesIT.java @@ -0,0 +1,74 @@ +/* + * 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.config.json; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.net.URI; +import java.net.URL; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.config.ConfigurationFactory; +import org.apache.logging.log4j.core.config.ConfigurationSource; +import org.apache.logging.log4j.core.config.xml.XmlConfiguration; +import org.junit.jupiter.api.Test; + +class JsonConfigurationFactoryMissingDependenciesIT { + + @Test + void inactiveFactoryRejectsDirectUse() { + final ClassLoader loader = getClass().getClassLoader(); + assertDoesNotThrow(() -> Class.forName("com.fasterxml.jackson.core.JsonParser", false, loader)); + assertThrows( + ClassNotFoundException.class, + () -> Class.forName("com.fasterxml.jackson.databind.ObjectMapper", false, loader)); + + final JsonConfigurationFactory factory = new JsonConfigurationFactory(); + assertFalse(factory.isActive()); + assertThrows(IllegalStateException.class, factory::getSupportedTypes); + assertThrows( + IllegalStateException.class, () -> factory.getConfiguration(null, ConfigurationSource.NULL_SOURCE)); + assertThrows( + IllegalStateException.class, + () -> factory.getConfiguration(null, "test", URI.create("classpath:log4j-test1.json"))); + assertThrows( + IllegalStateException.class, + () -> factory.getConfiguration( + null, + "test", + URI.create("classpath:log4j-test1.json"), + getClass().getClassLoader())); + } + + @Test + void aggregateFactorySkipsInactiveFactories() throws Exception { + final URL resource = getClass().getResource("/log4j-test1.xml"); + assertNotNull(resource); + try (final LoggerContext context = new LoggerContext("test")) { + final Configuration configuration = + ConfigurationFactory.getInstance().getConfiguration(context, "test", resource.toURI()); + assertInstanceOf(XmlConfiguration.class, configuration); + assertEquals( + resource.toURI(), configuration.getConfigurationSource().getURI()); + } + } +} diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/yaml/YamlConfigurationFactoryMissingDependenciesIT.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/yaml/YamlConfigurationFactoryMissingDependenciesIT.java new file mode 100644 index 00000000000..1b9783cb5eb --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/yaml/YamlConfigurationFactoryMissingDependenciesIT.java @@ -0,0 +1,79 @@ +/* + * 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.config.yaml; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.net.URI; +import java.net.URL; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.config.ConfigurationFactory; +import org.apache.logging.log4j.core.config.ConfigurationSource; +import org.apache.logging.log4j.core.config.json.JsonConfigurationFactory; +import org.apache.logging.log4j.core.config.xml.XmlConfiguration; +import org.junit.jupiter.api.Test; + +class YamlConfigurationFactoryMissingDependenciesIT { + + @Test + void inactiveFactoryRejectsDirectUseWithoutDisablingJson() { + final ClassLoader loader = getClass().getClassLoader(); + assertDoesNotThrow(() -> Class.forName("com.fasterxml.jackson.core.JsonParser", false, loader)); + assertDoesNotThrow(() -> Class.forName("com.fasterxml.jackson.databind.ObjectMapper", false, loader)); + assertThrows( + ClassNotFoundException.class, + () -> Class.forName("com.fasterxml.jackson.dataformat.yaml.YAMLFactory", false, loader)); + + assertArrayEquals(new String[] {".json", ".jsn"}, new JsonConfigurationFactory().getSupportedTypes()); + + final YamlConfigurationFactory factory = new YamlConfigurationFactory(); + assertFalse(factory.isActive()); + assertThrows(IllegalStateException.class, factory::getSupportedTypes); + assertThrows( + IllegalStateException.class, () -> factory.getConfiguration(null, ConfigurationSource.NULL_SOURCE)); + assertThrows( + IllegalStateException.class, + () -> factory.getConfiguration(null, "test", URI.create("classpath:log4j-test1.yaml"))); + assertThrows( + IllegalStateException.class, + () -> factory.getConfiguration( + null, + "test", + URI.create("classpath:log4j-test1.yaml"), + getClass().getClassLoader())); + } + + @Test + void aggregateFactorySkipsInactiveFactories() throws Exception { + final URL resource = getClass().getResource("/log4j-test1.xml"); + assertNotNull(resource); + try (final LoggerContext context = new LoggerContext("test")) { + final Configuration configuration = + ConfigurationFactory.getInstance().getConfiguration(context, "test", resource.toURI()); + assertInstanceOf(XmlConfiguration.class, configuration); + assertEquals( + resource.toURI(), configuration.getConfigurationSource().getURI()); + } + } +} diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java index bd0f6269bba..d5cd2babadd 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java @@ -293,7 +293,7 @@ protected boolean isActive() { public Configuration getConfiguration( final LoggerContext loggerContext, final String name, final URI configLocation) { if (!isActive()) { - return null; + throw new IllegalStateException(getClass().getName() + " is inactive"); } if (configLocation != null) { final ConfigurationSource source = ConfigurationSource.fromUri(configLocation); @@ -317,7 +317,7 @@ public Configuration getConfiguration( public Configuration getConfiguration( final LoggerContext loggerContext, final String name, final URI configLocation, final ClassLoader loader) { if (!isActive()) { - return null; + throw new IllegalStateException(getClass().getName() + " is inactive"); } if (loader == null) { return getConfiguration(loggerContext, name, configLocation); @@ -489,6 +489,9 @@ public Configuration getConfiguration( return getConfiguration(LOG4J1_VERSION, loggerContext, log4j1ConfigStr); } for (final ConfigurationFactory factory : getFactories()) { + if (!factory.isActive()) { + continue; + } final String[] types = factory.getSupportedTypes(); if (types != null) { for (final String type : types) { @@ -520,6 +523,9 @@ public Configuration getConfiguration( // configLocation != null final String configLocationStr = configLocation.toString(); for (final ConfigurationFactory factory : getFactories()) { + if (!factory.isActive()) { + continue; + } final String[] types = factory.getSupportedTypes(); if (types != null) { for (final String type : types) { @@ -577,6 +583,9 @@ private Configuration getConfiguration( } if (source != null) { for (final ConfigurationFactory factory : getFactories()) { + if (!factory.isActive()) { + continue; + } if (requiredVersion != null && !factory.getVersion().equals(requiredVersion)) { continue; } @@ -601,6 +610,9 @@ private Configuration getConfiguration( final boolean named = Strings.isNotEmpty(name); final ClassLoader loader = LoaderUtil.getThreadContextClassLoader(); for (final ConfigurationFactory factory : getFactories()) { + if (!factory.isActive()) { + continue; + } String configName; final String prefix = isTest ? factory.getTestPrefix() : factory.getDefaultPrefix(); final String[] types = factory.getSupportedTypes(); @@ -616,12 +628,6 @@ private Configuration getConfiguration( final ConfigurationSource source = ConfigurationSource.fromResource(configName, loader); if (source != null) { - if (!factory.isActive()) { - LOGGER.error( - "Found configuration file `{}` for the inactive `{}`. This `ConfigurationFactory` implementation might be inactive due to a missing dependency.", - configName, - factory.getClass().getName()); - } return factory.getConfiguration(loggerContext, source); } } @@ -639,6 +645,9 @@ public Configuration getConfiguration(final LoggerContext loggerContext, final C if (source != null) { final String config = source.getLocation(); for (final ConfigurationFactory factory : getFactories()) { + if (!factory.isActive()) { + continue; + } final String[] types = factory.getSupportedTypes(); if (types != null) { for (final String type : types) { diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/json/JsonConfigurationFactory.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/json/JsonConfigurationFactory.java index ddcc61b4a60..92e53da8642 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/json/JsonConfigurationFactory.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/json/JsonConfigurationFactory.java @@ -61,14 +61,17 @@ protected boolean isActive() { @Override public Configuration getConfiguration(final LoggerContext loggerContext, final ConfigurationSource source) { - if (!isActive) { - return null; + if (!isActive()) { + throw new IllegalStateException(getClass().getName() + " is inactive"); } return new JsonConfiguration(loggerContext, source); } @Override public String[] getSupportedTypes() { + if (!isActive()) { + throw new IllegalStateException(getClass().getName() + " is inactive"); + } return SUFFIXES; } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/yaml/YamlConfigurationFactory.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/yaml/YamlConfigurationFactory.java index d3cbaad03ef..25c5b9ac507 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/yaml/YamlConfigurationFactory.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/yaml/YamlConfigurationFactory.java @@ -62,14 +62,17 @@ protected boolean isActive() { @Override public Configuration getConfiguration(final LoggerContext loggerContext, final ConfigurationSource source) { - if (!isActive) { - return null; + if (!isActive()) { + throw new IllegalStateException(getClass().getName() + " is inactive"); } return new YamlConfiguration(loggerContext, source); } @Override public String[] getSupportedTypes() { + if (!isActive()) { + throw new IllegalStateException(getClass().getName() + " is inactive"); + } return SUFFIXES; } } diff --git a/src/changelog/.2.x.x/fix_configuration_factory_activation.xml b/src/changelog/.2.x.x/fix_configuration_factory_activation.xml new file mode 100644 index 00000000000..0e38a6505ee --- /dev/null +++ b/src/changelog/.2.x.x/fix_configuration_factory_activation.xml @@ -0,0 +1,10 @@ + + + + + Make `ConfigurationFactory` implementations consistently report and reject inactive state + +