This is an automated email from the ASF dual-hosted git repository. mattjuntunen pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-configuration.git
The following commit(s) were added to refs/heads/master by this push: new f025bc39 Make default interpolation prefix lookups configurable via system property. Remove dns, url, and script lookups from defaults. f025bc39 is described below commit f025bc399e8125ffc7701ac74f09b833c5b5e152 Author: Matt Juntunen <mattjuntu...@apache.org> AuthorDate: Wed Jun 8 23:08:26 2022 -0400 Make default interpolation prefix lookups configurable via system property. Remove dns, url, and script lookups from defaults. --- src/changes/changes.xml | 5 + .../configuration2/AbstractConfiguration.java | 4 + .../interpol/ConfigurationInterpolator.java | 225 +++++++++++++++++++-- .../configuration2/interpol/DefaultLookups.java | 13 +- .../interpol/TestConfigurationInterpolator.java | 160 ++++++++++++++- 5 files changed, 378 insertions(+), 29 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 51b3ee68..de979283 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -98,6 +98,11 @@ Add ImmutableConfiguration.getDuration() methods. </action> <!-- UPDATES --> + <action type="update" dev="mattjuntunen"> + Make default interpolation prefix lookups configurable via system property. Remove dns, url, and script + lookups from defaults. If these lookups are required for use in AbstractConfiguration subclasses, they must + be enabled via system property. See ConfigurationInterpolator.getDefaultPrefixLookups() for details. + </action> <action type="update" dev="ggregory" due-to="Dependabot, Gary Gregory"> Bump actions/cache from 2 to 3.0.4 #99, #151, #169. </action> diff --git a/src/main/java/org/apache/commons/configuration2/AbstractConfiguration.java b/src/main/java/org/apache/commons/configuration2/AbstractConfiguration.java index 3cde89c7..c1b0f0dd 100644 --- a/src/main/java/org/apache/commons/configuration2/AbstractConfiguration.java +++ b/src/main/java/org/apache/commons/configuration2/AbstractConfiguration.java @@ -211,9 +211,13 @@ public abstract class AbstractConfiguration extends BaseEventSource implements C /** * Returns the {@code ConfigurationInterpolator} object that manages the lookup objects for resolving variables. + * Unless a custom interpolator has been set or the instance has been modified, the returned interpolator will + * resolve values from this configuration instance and support the + * {@link ConfigurationInterpolator#getDefaultPrefixLookups() default prefix lookups}. * * @return the {@code ConfigurationInterpolator} associated with this configuration * @since 1.4 + * @see ConfigurationInterpolator#getDefaultPrefixLookups() */ @Override public ConfigurationInterpolator getInterpolator() { diff --git a/src/main/java/org/apache/commons/configuration2/interpol/ConfigurationInterpolator.java b/src/main/java/org/apache/commons/configuration2/interpol/ConfigurationInterpolator.java index 8c32ed04..c0b1a8ad 100644 --- a/src/main/java/org/apache/commons/configuration2/interpol/ConfigurationInterpolator.java +++ b/src/main/java/org/apache/commons/configuration2/interpol/ConfigurationInterpolator.java @@ -25,13 +25,13 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Properties; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.Function; import org.apache.commons.text.StringSubstitutor; -import org.apache.commons.text.lookup.DefaultStringLookup; /** * <p> @@ -89,6 +89,17 @@ import org.apache.commons.text.lookup.DefaultStringLookup; * @since 1.4 */ public class ConfigurationInterpolator { + + /** + * Name of the system property used to determine the lookups added by the + * {@link #getDefaultPrefixLookups()} method. Use of this property is only required + * in cases where the set of default lookups must be modified. + * + * @since 2.8 + */ + public static final String DEFAULT_PREFIX_LOOKUPS_PROPERTY = + "org.apache.commons.configuration2.interpol.ConfigurationInterpolator.defaultPrefixLookups"; + /** Constant for the prefix separator. */ private static final char PREFIX_SEPARATOR = ':'; @@ -104,23 +115,6 @@ public class ConfigurationInterpolator { /** The length of {@link #VAR_END}. */ private static final int VAR_END_LENGTH = VAR_END.length(); - /** A map containing the default prefix lookups. */ - private static final Map<String, Lookup> DEFAULT_PREFIX_LOOKUPS; - - static { - // TODO Perhaps a 3.0 version should only use Commons Text lookups. - // Add our own lookups. - final Map<String, Lookup> lookups = new HashMap<>(); - for (final DefaultLookups lookup : DefaultLookups.values()) { - lookups.put(lookup.getPrefix(), lookup.getLookup()); - } - // Add Apache Commons Text lookups but don't override existing keys. - for (final DefaultStringLookup lookup : DefaultStringLookup.values()) { - lookups.putIfAbsent(lookup.getKey(), new StringLookupAdapter(lookup.getStringLookup())); - } - DEFAULT_PREFIX_LOOKUPS = Collections.unmodifiableMap(lookups); - } - /** A map with the currently registered lookup objects. */ private final Map<String, Lookup> prefixLookups; @@ -189,14 +183,111 @@ public class ConfigurationInterpolator { /** * Returns a map containing the default prefix lookups. Every configuration object derived from - * {@code AbstractConfiguration} is by default initialized with a {@code ConfigurationInterpolator} containing these - * {@code Lookup} objects and their prefixes. The map cannot be modified + * {@code AbstractConfiguration} is by default initialized with a {@code ConfigurationInterpolator} containing + * these {@code Lookup} objects and their prefixes. The map cannot be modified. + * + * <p> + * All of the lookups present in the returned map are from {@link DefaultLookups}. However, not all of the + * available lookups are included by default. Specifically, lookups that can execute code (e.g., + * {@link DefaultLookups#SCRIPT SCRIPT}) and those that can result in contact with remote servers (e.g., + * {@link DefaultLookups#URL URL} and {@link DefaultLookups#DNS DNS}) are not included. If this behavior + * must be modified, users can define the {@value #DEFAULT_PREFIX_LOOKUPS_PROPERTY} system property + * with a comma-separated list of {@link DefaultLookups} enum names to be included in the set of defaults. + * For example, setting this system property to {@code "BASE64_ENCODER,ENVIRONMENT"} will only include the + * {@link DefaultLookups#BASE64_ENCODER BASE64_ENCODER} and + * {@link DefaultLookups#ENVIRONMENT ENVIRONMENT} lookups. Setting the property to the empty string will + * cause no defaults to be configured. + * </p> + * + * <p><strong>Default Lookups</strong></p> + * <table> + * <tr> + * <th>Prefix</th> + * <th>Lookup</th> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_BASE64_DECODER}</td> + * <td>{@link DefaultLookups#BASE64_DECODER BASE64_DECODER}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_BASE64_ENCODER}</td> + * <td>{@link DefaultLookups#BASE64_ENCODER BASE64_ENCODER}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_CONST}</td> + * <td>{@link DefaultLookups#CONST CONST}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_DATE}</td> + * <td>{@link DefaultLookups#DATE DATE}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_ENV}</td> + * <td>{@link DefaultLookups#ENVIRONMENT ENVIRONMENT}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_FILE}</td> + * <td>{@link DefaultLookups#FILE FILE}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_JAVA}</td> + * <td>{@link DefaultLookups#JAVA JAVA}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_LOCALHOST}</td> + * <td>{@link DefaultLookups#LOCAL_HOST LOCAL_HOST}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_PROPERTIES}</td> + * <td>{@link DefaultLookups#PROPERTIES PROPERTIES}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_RESOURCE_BUNDLE}</td> + * <td>{@link DefaultLookups#RESOURCE_BUNDLE RESOURCE_BUNDLE}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_SYS}</td> + * <td>{@link DefaultLookups#SYSTEM_PROPERTIES SYSTEM_PROPERTIES}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_URL_DECODER}</td> + * <td>{@link DefaultLookups#URL_DECODER URL_DECODER}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_URL_ENCODER}</td> + * <td>{@link DefaultLookups#URL_ENCODER URL_ENCODER}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_XML}</td> + * <td>{@link DefaultLookups#XML XML}</td> + * </tr> + * </table> + * + * <p><strong>Additional Lookups (not included by default)</strong></p> + * <table> + * <tr> + * <th>Prefix</th> + * <th>Lookup</th> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_DNS}</td> + * <td>{@link DefaultLookups#DNS DNS}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_URL}</td> + * <td>{@link DefaultLookups#URL URL}</td> + * </tr> + * <tr> + * <td>{@value org.apache.commons.text.lookup.StringLookupFactory#KEY_SCRIPT}</td> + * <td>{@link DefaultLookups#SCRIPT SCRIPT}</td> + * </tr> + * </table> * * @return a map with the default prefix {@code Lookup} objects and their prefixes * @since 2.0 */ public static Map<String, Lookup> getDefaultPrefixLookups() { - return DEFAULT_PREFIX_LOOKUPS; + return DefaultPrefixLookupsHolder.INSTANCE.getDefaultPrefixLookups(); } /** @@ -513,6 +604,98 @@ public class ConfigurationInterpolator { this.parentInterpolator = parentInterpolator; } + /** + * Internal class used to construct the default {@link Lookup} map used by + * {@link ConfigurationInterpolator#getDefaultPrefixLookups()}. + */ + static final class DefaultPrefixLookupsHolder { + + /** Singleton instance, initialized with the system properties. */ + static final DefaultPrefixLookupsHolder INSTANCE = new DefaultPrefixLookupsHolder(System.getProperties()); + + /** Default lookup map. */ + private final Map<String, Lookup> defaultLookups; + + /** + * Construct a new instance initialized with the given properties. + * @param props initialization properties + */ + DefaultPrefixLookupsHolder(final Properties props) { + final Map<String, Lookup> lookups = + props.containsKey(ConfigurationInterpolator.DEFAULT_PREFIX_LOOKUPS_PROPERTY) + ? parseLookups(props.getProperty(ConfigurationInterpolator.DEFAULT_PREFIX_LOOKUPS_PROPERTY)) + : createDefaultLookups(); + + defaultLookups = Collections.unmodifiableMap(lookups); + } + + /** + * Get the default prefix lookups map. + * @return default prefix lookups map + */ + Map<String, Lookup> getDefaultPrefixLookups() { + return defaultLookups; + } + + /** + * Create the lookup map used when the user has requested no customization. + * @return default lookup map + */ + private static Map<String, Lookup> createDefaultLookups() { + final Map<String, Lookup> lookupMap = new HashMap<>(); + + addLookup(DefaultLookups.BASE64_DECODER, lookupMap); + addLookup(DefaultLookups.BASE64_ENCODER, lookupMap); + addLookup(DefaultLookups.CONST, lookupMap); + addLookup(DefaultLookups.DATE, lookupMap); + addLookup(DefaultLookups.ENVIRONMENT, lookupMap); + addLookup(DefaultLookups.FILE, lookupMap); + addLookup(DefaultLookups.JAVA, lookupMap); + addLookup(DefaultLookups.LOCAL_HOST, lookupMap); + addLookup(DefaultLookups.PROPERTIES, lookupMap); + addLookup(DefaultLookups.RESOURCE_BUNDLE, lookupMap); + addLookup(DefaultLookups.SYSTEM_PROPERTIES, lookupMap); + addLookup(DefaultLookups.URL_DECODER, lookupMap); + addLookup(DefaultLookups.URL_ENCODER, lookupMap); + addLookup(DefaultLookups.XML, lookupMap); + + return lookupMap; + } + + /** + * Construct a lookup map by parsing the given string. The string is expected to contain + * comma or space-separated names of values from the {@link DefaultLookups} enum. + * @param str string to parse; not null + * @return lookup map parsed from the given string + * @throws IllegalArgumentException if the string does not contain a valid default lookup + * definition + */ + private static Map<String, Lookup> parseLookups(final String str) { + final Map<String, Lookup> lookupMap = new HashMap<>(); + + try { + for (final String lookupName : str.split("[\\s,]+")) { + if (!lookupName.isEmpty()) { + addLookup(DefaultLookups.valueOf(lookupName.toUpperCase()), lookupMap); + } + } + } catch (IllegalArgumentException exc) { + throw new IllegalArgumentException("Invalid default lookups definition: " + str, exc); + } + + return lookupMap; + } + + /** + * Add the prefix and lookup from {@code lookup} to {@code map}. + * @param lookup lookup to add + * @param map map to add to + */ + private static void addLookup(final DefaultLookups lookup, final Map<String, Lookup> map) { + map.put(lookup.getPrefix(), lookup.getLookup()); + } + } + /** Class encapsulating the default logic to convert resolved variable values into strings. * This class is thread-safe. */ diff --git a/src/main/java/org/apache/commons/configuration2/interpol/DefaultLookups.java b/src/main/java/org/apache/commons/configuration2/interpol/DefaultLookups.java index 00f73ff7..423e3df1 100644 --- a/src/main/java/org/apache/commons/configuration2/interpol/DefaultLookups.java +++ b/src/main/java/org/apache/commons/configuration2/interpol/DefaultLookups.java @@ -20,16 +20,17 @@ import org.apache.commons.text.lookup.StringLookupFactory; /** * <p> - * An enumeration class defining constants for the {@code Lookup} objects available for each {@code Configuration} - * object per default. + * An enumeration class defining constants for built-in {@code Lookup} objects available for + * {@code Configuration} instances. * </p> * <p> - * When a new configuration object derived from {@code AbstractConfiguration} is created it installs a - * {@link ConfigurationInterpolator} with a default set of {@link Lookup} objects. These lookups are defined by this - * enumeration class. + * When a new configuration object derived from {@code AbstractConfiguration} is created, it installs a + * {@link ConfigurationInterpolator} containing a default set of {@link Lookup} objects. These lookups are + * defined by this enumeration class, however not all lookups may be included in the defaults. See + * {@link ConfigurationInterpolator#getDefaultPrefixLookups()} for details. * </p> * <p> - * All the default {@code Lookup} classes are state-less, thus their instances can be shared between multiple + * All the {@code Lookup}s defined here are state-less, thus their instances can be shared between multiple * configuration objects. Therefore, it makes sense to keep shared instances in this enumeration class. * </p> * diff --git a/src/test/java/org/apache/commons/configuration2/interpol/TestConfigurationInterpolator.java b/src/test/java/org/apache/commons/configuration2/interpol/TestConfigurationInterpolator.java index 2a1bb903..c1b69f7b 100644 --- a/src/test/java/org/apache/commons/configuration2/interpol/TestConfigurationInterpolator.java +++ b/src/test/java/org/apache/commons/configuration2/interpol/TestConfigurationInterpolator.java @@ -23,17 +23,23 @@ import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.EnumSet; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Properties; +import java.util.Set; import java.util.function.Function; +import org.apache.commons.text.lookup.StringLookupFactory; import org.easymock.EasyMock; import org.junit.Before; import org.junit.Test; @@ -206,11 +212,23 @@ public class TestConfigurationInterpolator { */ @Test public void testGetDefaultPrefixLookups() { + final EnumSet<DefaultLookups> excluded = EnumSet.of( + DefaultLookups.DNS, + DefaultLookups.URL, + DefaultLookups.SCRIPT); + + final EnumSet<DefaultLookups> included = EnumSet.complementOf(excluded); + final Map<String, Lookup> lookups = ConfigurationInterpolator.getDefaultPrefixLookups(); - assertEquals("Wrong number of lookups", DefaultLookups.values().length, lookups.size()); - for (final DefaultLookups l : DefaultLookups.values()) { + + assertEquals("Wrong number of lookups", included.size(), lookups.size()); + for (final DefaultLookups l : included) { assertSame("Wrong entry for " + l, l.getLookup(), lookups.get(l.getPrefix())); } + + for (final DefaultLookups l : excluded) { + assertNull("Unexpected entry for " + l, lookups.get(l.getPrefix())); + } } /** @@ -684,4 +702,142 @@ public class TestConfigurationInterpolator { assertNull("Variable could be resolved", interpolator.resolve("UnknownPrefix:" + TEST_NAME)); assertNull("Variable with empty prefix could be resolved", interpolator.resolve(":" + TEST_NAME)); } + + @Test + public void testDefaultStringLookupsHolder_lookupsPropertyNotPresent() { + checkDefaultPrefixLookupsHolder(new Properties(), + "base64", + StringLookupFactory.KEY_BASE64_DECODER, + StringLookupFactory.KEY_BASE64_ENCODER, + StringLookupFactory.KEY_CONST, + StringLookupFactory.KEY_DATE, + StringLookupFactory.KEY_ENV, + StringLookupFactory.KEY_FILE, + StringLookupFactory.KEY_JAVA, + StringLookupFactory.KEY_LOCALHOST, + StringLookupFactory.KEY_PROPERTIES, + StringLookupFactory.KEY_RESOURCE_BUNDLE, + StringLookupFactory.KEY_SYS, + StringLookupFactory.KEY_URL_DECODER, + StringLookupFactory.KEY_URL_ENCODER, + StringLookupFactory.KEY_XML); + } + + @Test + public void testDefaultStringLookupsHolder_lookupsPropertyEmptyAndBlank() { + final Properties propsWithNull = new Properties(); + propsWithNull.setProperty(ConfigurationInterpolator.DEFAULT_PREFIX_LOOKUPS_PROPERTY, ""); + + checkDefaultPrefixLookupsHolder(propsWithNull); + + final Properties propsWithBlank = new Properties(); + propsWithBlank.setProperty(ConfigurationInterpolator.DEFAULT_PREFIX_LOOKUPS_PROPERTY, " "); + + checkDefaultPrefixLookupsHolder(propsWithBlank); + } + + @Test + public void testDefaultStringLookupsHolder_givenSingleLookup() { + final Properties props = new Properties(); + props.setProperty(ConfigurationInterpolator.DEFAULT_PREFIX_LOOKUPS_PROPERTY, "base64_encoder"); + + checkDefaultPrefixLookupsHolder(props, + "base64", + StringLookupFactory.KEY_BASE64_ENCODER); + } + + @Test + public void testDefaultStringLookupsHolder_givenSingleLookup_weirdString() { + final Properties props = new Properties(); + props.setProperty(ConfigurationInterpolator.DEFAULT_PREFIX_LOOKUPS_PROPERTY, " \n \t ,, DnS , , "); + + checkDefaultPrefixLookupsHolder(props, StringLookupFactory.KEY_DNS); + } + + @Test + public void testDefaultStringLookupsHolder_multipleLookups() { + final Properties props = new Properties(); + props.setProperty(ConfigurationInterpolator.DEFAULT_PREFIX_LOOKUPS_PROPERTY, "dns, url script "); + + checkDefaultPrefixLookupsHolder(props, + StringLookupFactory.KEY_DNS, + StringLookupFactory.KEY_URL, + StringLookupFactory.KEY_SCRIPT); + } + + @Test + public void testDefaultStringLookupsHolder_allLookups() { + final Properties props = new Properties(); + props.setProperty(ConfigurationInterpolator.DEFAULT_PREFIX_LOOKUPS_PROPERTY, + "BASE64_DECODER BASE64_ENCODER const, date, dns, environment " + + "file ,java, local_host properties, resource_bundle,script,system_properties " + + "url url_decoder , url_encoder, xml"); + + checkDefaultPrefixLookupsHolder(props, + "base64", + StringLookupFactory.KEY_BASE64_DECODER, + StringLookupFactory.KEY_BASE64_ENCODER, + StringLookupFactory.KEY_CONST, + StringLookupFactory.KEY_DATE, + StringLookupFactory.KEY_ENV, + StringLookupFactory.KEY_FILE, + StringLookupFactory.KEY_JAVA, + StringLookupFactory.KEY_LOCALHOST, + StringLookupFactory.KEY_PROPERTIES, + StringLookupFactory.KEY_RESOURCE_BUNDLE, + StringLookupFactory.KEY_SYS, + StringLookupFactory.KEY_URL_DECODER, + StringLookupFactory.KEY_URL_ENCODER, + StringLookupFactory.KEY_XML, + + StringLookupFactory.KEY_DNS, + StringLookupFactory.KEY_URL, + StringLookupFactory.KEY_SCRIPT); + } + + @Test + public void testDefaultStringLookupsHolder_invalidLookupsDefinition() { + final Properties props = new Properties(); + props.setProperty(ConfigurationInterpolator.DEFAULT_PREFIX_LOOKUPS_PROPERTY, "base64_encoder nope"); + + try { + new ConfigurationInterpolator.DefaultPrefixLookupsHolder(props); + + fail("Operation should have failed"); + } catch (Exception exc) { + assertEquals("Invalid default lookups definition: base64_encoder nope", exc.getMessage()); + } + } + + private static void checkDefaultPrefixLookupsHolder(final Properties props, final String... keys) { + final ConfigurationInterpolator.DefaultPrefixLookupsHolder holder = + new ConfigurationInterpolator.DefaultPrefixLookupsHolder(props); + + final Map<String, Lookup> lookupMap = holder.getDefaultPrefixLookups(); + + assertMappedLookups(lookupMap, keys); + } + + private static void assertMappedLookups(final Map<String, Lookup> lookupMap, final String... keys) { + final Set<String> remainingKeys = new HashSet<>(lookupMap.keySet()); + + for (final String key : keys) { + assertNotNull("Expected map to contain string lookup for key " + key, key); + + remainingKeys.remove(key); + } + + assertTrue("Unexpected keys in lookup map: " + remainingKeys, remainingKeys.isEmpty()); + } + + /** + * Main method used to verify the default lookups resolved during JVM execution. + * @param args + */ + public static void main(final String[] args) { + System.out.println("Default lookups"); + for (final String key : ConfigurationInterpolator.getDefaultPrefixLookups().keySet()) { + System.out.println("- " + key); + } + } }