This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-text.git
commit 0297671c98d184724a6a576186008cd28a7c9a45 Author: Gary Gregory <[email protected]> AuthorDate: Wed Dec 3 15:59:53 2025 -0500 Move the XmlStringLookup secure toggle to a system property --- pom.xml | 5 +++ .../commons/text/lookup/StringLookupFactory.java | 24 +++++------- .../commons/text/lookup/XmlStringLookup.java | 44 ++++++++-------------- .../text/lookup/StringLookupFactoryTest.java | 2 + .../commons/text/lookup/XmlStringLookupTest.java | 11 ++++-- 5 files changed, 39 insertions(+), 47 deletions(-) diff --git a/pom.xml b/pom.xml index 0c180d36..bcab4f57 100644 --- a/pom.xml +++ b/pom.xml @@ -92,6 +92,11 @@ <artifactId>junit-jupiter</artifactId> <scope>test</scope> </dependency> + <dependency> + <groupId>org.junit-pioneer</groupId> + <artifactId>junit-pioneer</artifactId> + <scope>test</scope> + </dependency> <dependency> <!-- Java 21 support, revisit for Mockito 5 --> <groupId>net.bytebuddy</groupId> diff --git a/src/main/java/org/apache/commons/text/lookup/StringLookupFactory.java b/src/main/java/org/apache/commons/text/lookup/StringLookupFactory.java index 3135f5fa..863baa7f 100644 --- a/src/main/java/org/apache/commons/text/lookup/StringLookupFactory.java +++ b/src/main/java/org/apache/commons/text/lookup/StringLookupFactory.java @@ -1618,19 +1618,17 @@ public final class StringLookupFactory { * if a lookup causes causes a path to resolve outside of these fences. Otherwise, the result is unfenced to preserved behavior from previous versions. * </p> * <p> - * We looks up values in an XML document in the format {@code "[secure=(true|false):]DocumentPath:XPath"}. + * We looks up values in an XML document in the format {@code "DocumentPath:XPath"}. * </p> * <p> * For example: * </p> * <ul> * <li>{@code "com/domain/document.xml:/path/to/node"}</li> - * <li>{@code "secure=false:com/domain/document.xml:/path/to/node"}</li> - * <li>{@code "secure=true:com/domain/document.xml:/path/to/node"}</li> * </ul> * <p> - * Secure processing is enabled by default. The secure boolean String parsing follows the syntax defined by {@link Boolean#parseBoolean(String)}. The secure - * value in the key overrides instance settings given in the constructor. + * Secure processing is enabled by default and can be overridden with the system property {@code "XmlStringLookup.secure"} set to {@code false}. The secure + * boolean String parsing follows the syntax defined by {@link Boolean#parseBoolean(String)}. * </p> * <p> * Using a {@link StringLookup} from the {@link StringLookupFactory}: @@ -1664,19 +1662,17 @@ public final class StringLookupFactory { * if a lookup causes causes a path to resolve outside of these fences. Otherwise, the result is unfenced to preserved behavior from previous versions. * </p> * <p> - * We looks up values in an XML document in the format {@code "[secure=(true|false):]DocumentPath:XPath"}. + * We looks up values in an XML document in the format {@code "]DocumentPath:XPath"}. * </p> * <p> * For example: * </p> * <ul> * <li>{@code "com/domain/document.xml:/path/to/node"}</li> - * <li>{@code "secure=false:com/domain/document.xml:/path/to/node"}</li> - * <li>{@code "secure=true:com/domain/document.xml:/path/to/node"}</li> * </ul> * <p> - * Secure processing is enabled by default. The secure boolean String parsing follows the syntax defined by {@link Boolean#parseBoolean(String)}. The secure - * value in the key overrides instance settings given in the constructor. + * Secure processing is enabled by default and can be overridden with the system property {@code "XmlStringLookup.secure"} set to {@code false}. The secure + * boolean String parsing follows the syntax defined by {@link Boolean#parseBoolean(String)}. * </p> * <p> * Using a {@link StringLookup} from the {@link StringLookupFactory}: @@ -1713,19 +1709,17 @@ public final class StringLookupFactory { * if a lookup causes causes a path to resolve outside of these fences. Otherwise, the result is unfenced to preserved behavior from previous versions. * </p> * <p> - * We looks up values in an XML document in the format {@code "[secure=(true|false):]DocumentPath:XPath"}. + * We looks up values in an XML document in the format {@code "DocumentPath:XPath"}. * </p> * <p> * For example: * </p> * <ul> * <li>{@code "com/domain/document.xml:/path/to/node"}</li> - * <li>{@code "secure=false:com/domain/document.xml:/path/to/node"}</li> - * <li>{@code "secure=true:com/domain/document.xml:/path/to/node"}</li> * </ul> * <p> - * Secure processing is enabled by default. The secure boolean String parsing follows the syntax defined by {@link Boolean#parseBoolean(String)}. The secure - * value in the key overrides instance settings given in the constructor. + * Secure processing is enabled by default and can be overridden with the system property {@code "XmlStringLookup.secure"} set to {@code false}. The secure + * boolean String parsing follows the syntax defined by {@link Boolean#parseBoolean(String)}. * </p> * <p> * Using a {@link StringLookup} from the {@link StringLookupFactory} fenced by the current directory ({@code Paths.get("")}): diff --git a/src/main/java/org/apache/commons/text/lookup/XmlStringLookup.java b/src/main/java/org/apache/commons/text/lookup/XmlStringLookup.java index aa873a00..26739c02 100644 --- a/src/main/java/org/apache/commons/text/lookup/XmlStringLookup.java +++ b/src/main/java/org/apache/commons/text/lookup/XmlStringLookup.java @@ -30,21 +30,20 @@ import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.xpath.XPathFactory; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.SystemProperties; import org.w3c.dom.Document; /** - * Looks up values in an XML document in the format {@code "[secure=(true|false):]DocumentPath:XPath"}. + * Looks up values in an XML document in the format {@code "DocumentPath:XPath"}. * <p> * For example: * </p> * <ul> * <li>{@code "com/domain/document.xml:/path/to/node"}</li> - * <li>{@code "secure=false:com/domain/document.xml:/path/to/node"}</li> - * <li>{@code "secure=true:com/domain/document.xml:/path/to/node"}</li> * </ul> * <p> - * Secure processing is enabled by default. The secure boolean String parsing follows the syntax defined by {@link Boolean#parseBoolean(String)}. The secure - * value in the key overrides instance settings given in the constructor. + * Secure processing is enabled by default and can be overridden with the system property {@code "XmlStringLookup.secure"} set to {@code false}. The secure + * boolean String parsing follows the syntax defined by {@link Boolean#parseBoolean(String)}. * </p> * * @since 1.5 @@ -52,14 +51,9 @@ import org.w3c.dom.Document; final class XmlStringLookup extends AbstractPathFencedLookup { /** - * Minimum number of key parts. + * The number of key parts. */ - private static final int KEY_PARTS_MIN = 2; - - /** - * Minimum number of key parts. - */ - private static final int KEY_PARTS_MAX = 3; + private static final int KEY_PARTS_LEN = 2; /** * Defines default XPath factory features. @@ -76,11 +70,16 @@ final class XmlStringLookup extends AbstractPathFencedLookup { DEFAULT_XML_FEATURES = new HashMap<>(1); DEFAULT_XML_FEATURES.put(XMLConstants.FEATURE_SECURE_PROCESSING, Boolean.TRUE); } + /** * Defines the singleton for this class with secure processing enabled. */ static final XmlStringLookup INSTANCE = new XmlStringLookup(DEFAULT_XML_FEATURES, DEFAULT_XPATH_FEATURES, (Path[]) null); + private static boolean isSecure() { + return SystemProperties.getBoolean(XmlStringLookup.class, "secure", () -> true); + } + /** * Defines XPath factory features. */ @@ -106,14 +105,12 @@ final class XmlStringLookup extends AbstractPathFencedLookup { } /** - * Looks up a value for the key in the format {@code "[secure=(true|false):]DocumentPath:XPath"}. + * Looks up a value for the key in the format {@code "DocumentPath:XPath"}. * <p> * For example: * </p> * <ul> * <li>{@code "com/domain/document.xml:/path/to/node"}</li> - * <li>{@code "secure=false:com/domain/document.xml:/path/to/node"}</li> - * <li>{@code "secure=true:com/domain/document.xml:/path/to/node"}</li> * </ul> * <p> * Secure processing is enabled by default. The secure boolean String parsing follows the syntax defined by {@link Boolean#parseBoolean(String)}. The secure @@ -130,12 +127,11 @@ final class XmlStringLookup extends AbstractPathFencedLookup { } final String[] keys = key.split(SPLIT_STR); final int keyLen = keys.length; - if (keyLen != KEY_PARTS_MIN && keyLen != KEY_PARTS_MAX) { - throw IllegalArgumentExceptions.format("Bad XML key format '%s'; the expected format is [secure=(true|false):]DocumentPath:XPath.", key); + if (keyLen != KEY_PARTS_LEN) { + throw IllegalArgumentExceptions.format("Bad XML key format '%s'; the expected format is 'DocumentPath:XPath'.", key); } - final boolean isKeySecure = keyLen == KEY_PARTS_MAX; - final Boolean secure = isKeySecure ? parseSecureKey(keys, key) : null; - final String documentPath = isKeySecure ? keys[1] : keys[0]; + final Boolean secure = isSecure(); + final String documentPath = keys[0]; final String xpath = StringUtils.substringAfterLast(key, SPLIT_CH); final DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance(); try { @@ -160,12 +156,4 @@ final class XmlStringLookup extends AbstractPathFencedLookup { throw new IllegalArgumentException(e); } } - - private Boolean parseSecureKey(final String[] args, final String key) { - final String[] secParts = args[0].split("="); - if (secParts.length != 2 && !Objects.equals(secParts[0], "secure")) { - throw IllegalArgumentExceptions.format("Bad XML key format '%s'; the expected format is [secure=(true|false):]DocumentPath:XPath.", key); - } - return Boolean.valueOf(secParts[1]); - } } diff --git a/src/test/java/org/apache/commons/text/lookup/StringLookupFactoryTest.java b/src/test/java/org/apache/commons/text/lookup/StringLookupFactoryTest.java index c037aa59..bdc6cac1 100644 --- a/src/test/java/org/apache/commons/text/lookup/StringLookupFactoryTest.java +++ b/src/test/java/org/apache/commons/text/lookup/StringLookupFactoryTest.java @@ -32,6 +32,7 @@ import java.util.Set; import javax.xml.XMLConstants; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.SetSystemProperty; /** * Tests {@link StringLookupFactory}. @@ -272,6 +273,7 @@ class StringLookupFactoryTest { } @Test + @SetSystemProperty(key = "XmlStringLookup.secure", value = "false") void testXmlStringLookupExternalEntityOn() { final String key = XmlStringLookupTest.DOC_DIR + "document-entity-ref.xml:/document/content"; assertEquals(XmlStringLookupTest.DATA, StringLookupFactory.INSTANCE.xmlStringLookup(XmlStringLookupTest.EMPTY_MAP).apply(key).trim()); diff --git a/src/test/java/org/apache/commons/text/lookup/XmlStringLookupTest.java b/src/test/java/org/apache/commons/text/lookup/XmlStringLookupTest.java index a37cafbe..ead417dd 100644 --- a/src/test/java/org/apache/commons/text/lookup/XmlStringLookupTest.java +++ b/src/test/java/org/apache/commons/text/lookup/XmlStringLookupTest.java @@ -35,6 +35,7 @@ import javax.xml.XMLConstants; import org.apache.commons.lang3.StringUtils; import org.apache.commons.text.StringSubstitutor; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.SetSystemProperty; /** * Tests {@link XmlStringLookup}. @@ -68,6 +69,7 @@ class XmlStringLookupTest { } @Test + @SetSystemProperty(key = "XmlStringLookup.secure", value = "false") void testExternalEntityOn() { final String key = DOC_DIR + "document-entity-ref.xml:/document/content"; assertEquals(DATA, new XmlStringLookup(EMPTY_MAP, EMPTY_MAP).apply(key).trim()); @@ -81,9 +83,10 @@ class XmlStringLookupTest { } @Test + @SetSystemProperty(key = "XmlStringLookup.secure", value = "false") void testInterpolatorExternalEntityOffOverride() { final StringSubstitutor stringSubstitutor = StringSubstitutor.createInterpolator(); - assertEquals(DATA, stringSubstitutor.replace("${xml:secure=false:" + DOC_DIR + "document-entity-ref.xml:/document/content}").trim()); + assertEquals(DATA, stringSubstitutor.replace("${xml:" + DOC_DIR + "document-entity-ref.xml:/document/content}").trim()); } @Test @@ -93,18 +96,18 @@ class XmlStringLookupTest { } @Test + @SetSystemProperty(key = "XmlStringLookup.secure", value = "true") void testInterpolatorExternalEntityOnOverride() { final StringSubstitutor stringSubstitutor = StringSubstitutor.createInterpolator(); assertThrows(IllegalArgumentException.class, - () -> stringSubstitutor.replace("${xml:secure=true:" + DOC_DIR + "document-entity-ref.xml:/document/content}")); + () -> stringSubstitutor.replace("${xml:" + DOC_DIR + "document-entity-ref.xml:/document/content}")); } @Test void testInterpolatorSecureOnBla() { final StringSubstitutor stringSubstitutor = StringSubstitutor.createInterpolator(); assertThrows(IllegalArgumentException.class, () -> stringSubstitutor.replace("${xml:" + DOC_DIR + "bla.xml:/document/content}")); - assertThrows(IllegalArgumentException.class, () -> stringSubstitutor.replace("${xml:secure=true:" + DOC_DIR + "bla.xml:/document/content}")); - // Using secure=false allows the BLA to occur. + // Using XmlStringLookup.secure=false allows the BLA to occur. } @Test
