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
The following commit(s) were added to refs/heads/master by this push:
new 58e1e125 Simplify XML FSP (#731)
58e1e125 is described below
commit 58e1e125daaa0aebf8c5ffaa82af48821a1ccf2d
Author: Gary Gregory <[email protected]>
AuthorDate: Thu Dec 4 10:27:20 2025 -0500
Simplify XML FSP (#731)
* Simplify XML FSP
* Simplify XML FSP
---
src/changes/changes.xml | 2 +-
.../commons/text/lookup/StringLookupFactory.java | 5 ++---
.../apache/commons/text/lookup/XmlStringLookup.java | 19 ++++++-------------
.../commons/text/lookup/XmlStringLookupTest.java | 15 +++++----------
4 files changed, 14 insertions(+), 27 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index f8912d20..9a4b221e 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -53,7 +53,7 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="Gary Gregory">Fix Apache RAT
plugin console warnings.</action>
<action type="fix" dev="ggregory" due-to="Gary Gregory">Fix site XML to
use version 2.0.0 XML schema.</action>
<action type="fix" dev="ggregory" due-to="Michael Hausegger">Removed
unreachable threshold verification code in
src/main/java/org/apache/commons/text/similarity #730.</action>
- <action type="fix" dev="ggregory" due-to="김민재, Gary Gregory">Enable secure
processing for the XML parser in XmlStringLookup #729.</action>
+ <action type="fix" dev="ggregory" due-to="김민재, Gary Gregory, Piotr
Karwasz">Enable secure processing for the XML parser in XmlStringLookup in case
the underlying JAXP implementation doesn't #729.</action>
<!-- ADD -->
<action type="add" dev="ggregory" due-to="Piotr P. Karwasz, Gary
Gregory">Add experimental CycloneDX VEX file #683.</action>
<action type="add" dev="ggregory" due-to="LorgeN, Gary Gregory"
issue="TEXT-235">Add Damerau-Levenshtein distance #687.</action>
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 863baa7f..699b7dea 100644
--- a/src/main/java/org/apache/commons/text/lookup/StringLookupFactory.java
+++ b/src/main/java/org/apache/commons/text/lookup/StringLookupFactory.java
@@ -1718,15 +1718,14 @@ public final class StringLookupFactory {
* <li>{@code "com/domain/document.xml:/path/to/node"}</li>
* </ul>
* <p>
- * 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)}.
+ * Secure processing is enabled by default and can be overridden with this
constructor.
* </p>
* <p>
* Using a {@link StringLookup} from the {@link StringLookupFactory}
fenced by the current directory ({@code Paths.get("")}):
* </p>
*
* <pre>
- * StringLookupFactory.INSTANCE.xmlStringLookup(map,
Pathe.get("")).lookup("com/domain/document.xml:/path/to/node");
+ * StringLookupFactory.INSTANCE.xmlStringLookup(map,
Path.get("")).lookup("com/domain/document.xml:/path/to/node");
* </pre>
* <p>
* To use a {@link StringLookup} fenced by the current directory, use:
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 85747eeb..ee334419 100644
--- a/src/main/java/org/apache/commons/text/lookup/XmlStringLookup.java
+++ b/src/main/java/org/apache/commons/text/lookup/XmlStringLookup.java
@@ -30,7 +30,6 @@ 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;
/**
@@ -42,8 +41,7 @@ import org.w3c.dom.Document;
* <li>{@code "com/domain/document.xml:/path/to/node"}</li>
* </ul>
* <p>
- * 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)}.
+ * Secure processing is enabled by default and can be overridden with {@link
StringLookupFactory#xmlStringLookup(Map, Path...)}.
* </p>
*
* @since 1.5
@@ -72,14 +70,13 @@ final class XmlStringLookup extends
AbstractPathFencedLookup {
}
/**
- * Defines the singleton for this class with secure processing enabled.
+ * Defines the singleton for this class with secure processing enabled by
default.
+ * <p>
+ * Secure processing is enabled by default and can be overridden with
{@link StringLookupFactory#xmlStringLookup(Map, Path...)}.
+ * </p>
*/
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.
*/
@@ -113,8 +110,7 @@ final class XmlStringLookup extends
AbstractPathFencedLookup {
* <li>{@code "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
{@link StringLookupFactory#xmlStringLookup(Map, Path...)}.
* </p>
*
* @param key the key to be looked up, may be null.
@@ -130,7 +126,6 @@ final class XmlStringLookup extends
AbstractPathFencedLookup {
if (keyLen != KEY_PARTS_LEN) {
throw IllegalArgumentExceptions.format("Bad XML key format '%s';
the expected format is 'DocumentPath:XPath'.", key);
}
- final boolean secure = isSecure();
final String documentPath = keys[0];
final String xpath = StringUtils.substringAfterLast(key, SPLIT_CH);
final DocumentBuilderFactory dbFactory =
DocumentBuilderFactory.newInstance();
@@ -138,14 +133,12 @@ final class XmlStringLookup extends
AbstractPathFencedLookup {
for (final Entry<String, Boolean> p :
xmlFactoryFeatures.entrySet()) {
dbFactory.setFeature(p.getKey(), p.getValue());
}
- dbFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING,
secure);
try (InputStream inputStream =
Files.newInputStream(getPath(documentPath))) {
final Document doc =
dbFactory.newDocumentBuilder().parse(inputStream);
final XPathFactory xpFactory = XPathFactory.newInstance();
for (final Entry<String, Boolean> p :
xPathFactoryFeatures.entrySet()) {
xpFactory.setFeature(p.getKey(), p.getValue());
}
- xpFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING,
secure);
return xpFactory.newXPath().evaluate(xpath, doc);
}
} catch (final Exception e) {
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 f0cc50ba..7327bca7 100644
--- a/src/test/java/org/apache/commons/text/lookup/XmlStringLookupTest.java
+++ b/src/test/java/org/apache/commons/text/lookup/XmlStringLookupTest.java
@@ -69,7 +69,6 @@ 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());
@@ -79,16 +78,14 @@ class XmlStringLookupTest {
@Test
void testInterpolatorExternalDtdOff() {
final StringSubstitutor stringSubstitutor =
StringSubstitutor.createInterpolator();
- assertThrows(IllegalArgumentException.class, () ->
stringSubstitutor.replace("${xml:" + DOC_DIR
- + "document-external-dtd.xml:/document/content}"));
+ assertThrows(IllegalArgumentException.class, () ->
stringSubstitutor.replace("${xml:" + DOC_DIR +
"document-external-dtd.xml:/document/content}"));
}
@Test
- @SetSystemProperty(key = "XmlStringLookup.secure", value = "false")
+ @SetSystemProperty(key = "javax.xml.accessExternalDTD", value = "file")
void testInterpolatorExternalDtdOn() {
final StringSubstitutor stringSubstitutor =
StringSubstitutor.createInterpolator();
- assertEquals("This is an external entity.",
- stringSubstitutor.replace("${xml:" + DOC_DIR +
"document-external-dtd.xml:/document/content}").trim());
+ assertEquals("This is an external entity.",
stringSubstitutor.replace("${xml:" + DOC_DIR +
"document-external-dtd.xml:/document/content}").trim());
}
@Test
@@ -98,7 +95,7 @@ class XmlStringLookupTest {
}
@Test
- @SetSystemProperty(key = "XmlStringLookup.secure", value = "false")
+ @SetSystemProperty(key = "javax.xml.accessExternalDTD", value = "file")
void testInterpolatorExternalEntityOffOverride() {
final StringSubstitutor stringSubstitutor =
StringSubstitutor.createInterpolator();
assertEquals(DATA, stringSubstitutor.replace("${xml:" + DOC_DIR +
"document-entity-ref.xml:/document/content}").trim());
@@ -111,11 +108,9 @@ class XmlStringLookupTest {
}
@Test
- @SetSystemProperty(key = "XmlStringLookup.secure", value = "true")
void testInterpolatorExternalEntityOnOverride() {
final StringSubstitutor stringSubstitutor =
StringSubstitutor.createInterpolator();
- assertThrows(IllegalArgumentException.class,
- () -> stringSubstitutor.replace("${xml:" + DOC_DIR +
"document-entity-ref.xml:/document/content}"));
+ assertThrows(IllegalArgumentException.class, () ->
stringSubstitutor.replace("${xml:" + DOC_DIR +
"document-entity-ref.xml:/document/content}"));
}
@Test