ppkarwasz commented on code in PR #53:
URL: https://github.com/apache/commons-xml/pull/53#discussion_r3873413830
##########
src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java:
##########
@@ -19,21 +19,47 @@
import java.util.Objects;
+import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.FactoryConfigurationError;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.validation.Schema;
import org.xml.sax.EntityResolver;
/**
* {@link DocumentBuilderFactory} wrapper that keeps an ignore-all {@link
EntityResolver} floor on every {@link DocumentBuilder} produced.
- *
- * <p>Wraps each produced builder in a {@link HardeningDocumentBuilder};
required when the underlying factory carries no resolver of its own and does
not honor
+ * <p>
+ * Wraps each produced builder in a {@link HardeningDocumentBuilder}; required
when the underlying factory carries no resolver of its own and does not honor
* JAXP 1.5 {@code ACCESS_EXTERNAL_*} (e.g. the external Xerces distribution).
A caller-set resolver is routed through the floor rather than replacing it. Kept
- * as a standalone wrapper so any hardener can reuse the floor.</p>
+ * as a standalone wrapper so any hardener can reuse the floor.
+ * </p>
+ *
+ * @see org.apache.commons.xml
*/
-final class HardeningDocumentBuilderFactory extends DocumentBuilderFactory {
+public final class HardeningDocumentBuilderFactory extends
DocumentBuilderFactory {
Review Comment:
I have mixed feelings about using the wrapper classes as public entry points:
**Pros**:
- We save up on the number of classes.
**Cons**:
- Users can use `instanceof` on the implementation class of the wrapper. I
am not sure if this is good or bad. Since users could always compare
implementation by class name, this is not a blocker.
- This makes a Multi-Release JAR harder to produce without duplicating a lot
of code. Therefore we are left with calling the Java 9 and Java 13 methods by
reflection. Knowing what users do with our JARs (e.g. `log4j-api`), maybe it is
better, since there are a lot of shaders out there that don't know how to shade
an MRJ.
If we choose this model (i.e. wrapper class == public entry point and Java
9/13 methods called by reflection), I would recommend to merge this class with
`DocumentBuilderHardener` in a follow-up PR.
--
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]
