ppkarwasz commented on code in PR #729:
URL: https://github.com/apache/commons-text/pull/729#discussion_r2585074457
##########
src/main/java/org/apache/commons/text/lookup/XmlStringLookup.java:
##########
@@ -26,62 +26,101 @@
import java.util.Objects;
import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.xpath.XPathFactory;
import org.apache.commons.lang3.StringUtils;
-import org.xml.sax.InputSource;
+import org.w3c.dom.Document;
/**
- * Looks up keys from an XML document.
+ * Looks up values in an XML document in the format {@code
"[secure=(true|false):]DocumentPath:XPath"}.
* <p>
- * Looks up the value for a given key in the format "Document:XPath".
+ * 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>
- * For example: "com/domain/document.xml:/path/to/node".
+ * 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.
* </p>
*
* @since 1.5
*/
final class XmlStringLookup extends AbstractPathFencedLookup {
+ /**
+ * Minimum 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;
+
/**
* Defines default XPath factory features.
*/
- static final Map<String, Boolean> DEFAULT_FEATURES;
+ static final Map<String, Boolean> DEFAULT_XPATH_FEATURES;
+ /**
+ * Defines default XML factory features.
+ */
+ static final Map<String, Boolean> DEFAULT_XML_FEATURES;
static {
- DEFAULT_FEATURES = new HashMap<>(1);
- DEFAULT_FEATURES.put(XMLConstants.FEATURE_SECURE_PROCESSING,
Boolean.TRUE);
+ DEFAULT_XPATH_FEATURES = new HashMap<>(1);
+ DEFAULT_XPATH_FEATURES.put(XMLConstants.FEATURE_SECURE_PROCESSING,
Boolean.TRUE);
+ DEFAULT_XML_FEATURES = new HashMap<>(1);
+ DEFAULT_XML_FEATURES.put(XMLConstants.FEATURE_SECURE_PROCESSING,
Boolean.TRUE);
Review Comment:
I mostly thought of it as a failsafe if a user doesn't read the memo and
uses it for untrusted data.
After a closer look, however, even in those cases:
- Disabling `DOCTYPE` entirely is not necessary, since modern parsers
disable external DTDs/entitities and limit entity expansion in “secure” mode.
Leaving `DOCTYPE` allows users to use some entitities in their configuration
files.
- `isXIncludeAware()` default to `false` anyway. If somebody wants to enable
it, there is a Xerces feature for it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
