ppkarwasz commented on code in PR #729:
URL: https://github.com/apache/commons-text/pull/729#discussion_r2584789202
##########
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 would follow the [OWASP
Cheatsheet](https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#jaxp-documentbuilderfactory-saxparserfactory-and-dom4j)
here:
- Enable the `http://apache.org/xml/features/disallow-doctype-decl` feature,
- Call `setXIncludeAware(false)`.
If somebody does not want these features, they can always pass a custom set
of features to enable.
It is also almost 2026: those features are supported by virtually all
maintained parsers. If somebody uses an old XML parser or a non-standard Xerces
configuration that lacks those features, they can always update to a more
recent parser/configuration.
--
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]
