This is an automated email from the ASF dual-hosted git repository. ppkarwasz pushed a commit to branch feature/reduce-shade-footprint in repository https://gitbox.apache.org/repos/asf/commons-xml.git
commit 1170a9189a3ff12c7e134aa5f13e4f4700f85833 Author: Piotr P. Karwasz <[email protected]> AuthorDate: Wed Jul 8 11:07:49 2026 +0200 Break the XmlFactories cycle for TrAX, XPath and schema The TrAX, schema and Saxon wrappers re-harden their sub-parsers by calling XmlFactories.harden(Source/XMLReader); because those helpers live on XmlFactories, whose class also news up every hardener, referencing them dragged the whole library into each of those shade closures. Move harden(Source)'s body to package-private SAXParserHardener. hardenSource (it needs only the SAX path) and repoint the internal callers: the four Source callers to hardenSource, SaxonProvider to SAXParserHardener.hardenReader. The public XmlFactories.harden(Source)/ harden(XMLReader) stay as thin delegates, so the API is unchanged, but no hardener transitively reaches XmlFactories anymore. Each heavy entry point now pulls only its own wrappers, its floor and the shared SAX path: - TransformerHardener: 32 -> 17 classes, 75204 -> 37366 bytes - XPathHardener: 32 -> 13 classes, 75204 -> 26471 bytes - HardeningSchemaFactory: 32 -> 13 classes, 75204 -> 32763 bytes Pin the three closures in ShadingFootprintTest and assert only the public XmlFactories entry still pulls all 32 classes. Assisted-By: Claude Opus 4.8 <[email protected]> --- .../apache/commons/xml/HardeningSchemaFactory.java | 2 +- .../apache/commons/xml/HardeningTransformer.java | 2 +- .../commons/xml/HardeningTransformerFactory.java | 8 +- .../org/apache/commons/xml/HardeningValidator.java | 2 +- .../org/apache/commons/xml/SAXParserHardener.java | 30 +++++++ .../java/org/apache/commons/xml/SaxonProvider.java | 2 +- .../java/org/apache/commons/xml/XmlFactories.java | 23 +----- .../apache/commons/xml/ShadingFootprintTest.java | 96 ++++++++++++++++------ 8 files changed, 110 insertions(+), 55 deletions(-) diff --git a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java index dcdbb54..a8f4df2 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java @@ -88,7 +88,7 @@ private static Source[] harden(final Source[] schemas) throws SAXException { final Source[] hardened = new Source[schemas.length]; try { for (int i = 0; i < schemas.length; i++) { - hardened[i] = XmlFactories.harden(schemas[i]); + hardened[i] = SAXParserHardener.hardenSource(schemas[i]); } } catch (final TransformerConfigurationException e) { throw new SAXException("Failed to harden schema source", e); diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformer.java b/src/main/java/org/apache/commons/xml/HardeningTransformer.java index de18485..d8a1b2e 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTransformer.java +++ b/src/main/java/org/apache/commons/xml/HardeningTransformer.java @@ -60,7 +60,7 @@ public URIResolver getURIResolver() { @Override public void transform(final Source xmlSource, final Result outputTarget) throws TransformerException { try { - delegate.transform(XmlFactories.harden(xmlSource), outputTarget); + delegate.transform(SAXParserHardener.hardenSource(xmlSource), outputTarget); } catch (final TransformerConfigurationException e) { throw new TransformerException(e); } diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java index b12a1a4..c6146dc 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java @@ -81,12 +81,12 @@ public URIResolver getURIResolver() { @Override public Source getAssociatedStylesheet(final Source source, final String media, final String title, final String charset) throws TransformerConfigurationException { - return delegate.getAssociatedStylesheet(XmlFactories.harden(source), media, title, charset); + return delegate.getAssociatedStylesheet(SAXParserHardener.hardenSource(source), media, title, charset); } @Override public Templates newTemplates(final Source source) throws TransformerConfigurationException { - final Templates templates = delegate.newTemplates(XmlFactories.harden(source)); + final Templates templates = delegate.newTemplates(SAXParserHardener.hardenSource(source)); return templates == null ? null : new HardeningTemplates(templates, getURIResolver()); } @@ -99,13 +99,13 @@ public Transformer newTransformer() throws TransformerConfigurationException { @Override public Transformer newTransformer(final Source source) throws TransformerConfigurationException { - final Transformer transformer = delegate.newTransformer(XmlFactories.harden(source)); + final Transformer transformer = delegate.newTransformer(SAXParserHardener.hardenSource(source)); return transformer == null ? null : new HardeningTransformer(transformer, getURIResolver()); } @Override public TransformerHandler newTransformerHandler(final Source source) throws TransformerConfigurationException { - return delegate.newTransformerHandler(XmlFactories.harden(source)); + return delegate.newTransformerHandler(SAXParserHardener.hardenSource(source)); } // <editor-fold defaultstate="collapsed" desc="Trivial delegation"> diff --git a/src/main/java/org/apache/commons/xml/HardeningValidator.java b/src/main/java/org/apache/commons/xml/HardeningValidator.java index b3790c0..47fd6e5 100644 --- a/src/main/java/org/apache/commons/xml/HardeningValidator.java +++ b/src/main/java/org/apache/commons/xml/HardeningValidator.java @@ -97,7 +97,7 @@ public void setResourceResolver(final LSResourceResolver resourceResolver) { @Override public void validate(final Source source, final Result result) throws SAXException, IOException { try { - delegate.validate(XmlFactories.harden(source), result); + delegate.validate(SAXParserHardener.hardenSource(source), result); } catch (final TransformerConfigurationException e) { throw new SAXException("Failed to harden source for validation", e); } diff --git a/src/main/java/org/apache/commons/xml/SAXParserHardener.java b/src/main/java/org/apache/commons/xml/SAXParserHardener.java index cbc4b5d..272449e 100644 --- a/src/main/java/org/apache/commons/xml/SAXParserHardener.java +++ b/src/main/java/org/apache/commons/xml/SAXParserHardener.java @@ -21,7 +21,12 @@ import java.util.Objects; import javax.xml.XMLConstants; +import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParserFactory; +import javax.xml.transform.Source; +import javax.xml.transform.TransformerConfigurationException; +import javax.xml.transform.sax.SAXSource; +import javax.xml.transform.stream.StreamSource; import org.xml.sax.EntityResolver; import org.xml.sax.InputSource; @@ -176,6 +181,31 @@ static XMLReader hardenReader(final XMLReader reader) { return new HardeningXMLReader(reader); } + /** + * Rewrites a {@link Source} so that any SAX parsing it triggers runs through a hardened {@link XMLReader}. + * + * <p>Only a {@link StreamSource} or a {@link SAXSource} without a reader is enriched with a hardened, namespace-aware reader; other source kinds are returned + * as-is. Used by the TrAX and schema wrappers to route every source they parse through the SAX hardening path.</p> + * + * @param source the source to harden; never {@code null}. + * @return a hardened source. + * @throws TransformerConfigurationException if a hardened reader cannot be obtained. + */ + static Source hardenSource(final Source source) throws TransformerConfigurationException { + if (source instanceof StreamSource || source instanceof SAXSource && ((SAXSource) source).getXMLReader() == null) { + try { + final SAXParserFactory factory = harden(SAXParserFactory.newInstance()); + factory.setNamespaceAware(true); + final XMLReader reader = factory.newSAXParser().getXMLReader(); + final InputSource inputSource = SAXSource.sourceToInputSource(source); + return inputSource == null ? source : new SAXSource(reader, inputSource); + } catch (final ParserConfigurationException | SAXException e) { + throw new TransformerConfigurationException("Failed to obtain a hardened XMLReader for source parsing", e); + } + } + return source; + } + private static void setFeature(final SAXParserFactory factory, final String feature, final boolean value) { try { factory.setFeature(feature, value); diff --git a/src/main/java/org/apache/commons/xml/SaxonProvider.java b/src/main/java/org/apache/commons/xml/SaxonProvider.java index e7ea690..a6c5002 100644 --- a/src/main/java/org/apache/commons/xml/SaxonProvider.java +++ b/src/main/java/org/apache/commons/xml/SaxonProvider.java @@ -71,7 +71,7 @@ private HardenedConfiguration() { @Override public XMLReader makeParser(final String className) throws TransformerFactoryConfigurationError { try { - return XmlFactories.harden(super.makeParser(className)); + return SAXParserHardener.hardenReader(super.makeParser(className)); } catch (final HardeningException e) { throw new TransformerFactoryConfigurationError(e); } diff --git a/src/main/java/org/apache/commons/xml/XmlFactories.java b/src/main/java/org/apache/commons/xml/XmlFactories.java index 8d8a62f..a1d1358 100644 --- a/src/main/java/org/apache/commons/xml/XmlFactories.java +++ b/src/main/java/org/apache/commons/xml/XmlFactories.java @@ -20,19 +20,14 @@ import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParserFactory; import javax.xml.stream.XMLInputFactory; import javax.xml.transform.Source; import javax.xml.transform.TransformerConfigurationException; import javax.xml.transform.TransformerFactory; -import javax.xml.transform.sax.SAXSource; -import javax.xml.transform.stream.StreamSource; import javax.xml.validation.SchemaFactory; import javax.xml.xpath.XPathFactory; -import org.xml.sax.InputSource; -import org.xml.sax.SAXException; import org.xml.sax.XMLReader; /** @@ -77,27 +72,15 @@ public final class XmlFactories { /** * Rewrites a {@link Source} so that any SAX parsing it triggers runs through an {@link XmlFactories}-hardened {@link XMLReader}. * - * <p>Only {@link StreamSource} and {@link SAXSource} without a reader are enriched with a hardened reader. Other kinds of sources are returned as-is.</p> - * - * <p>The reader is namespace-aware.</p> + * <p>Only a {@code StreamSource} or a {@code SAXSource} without a reader is enriched with a hardened, namespace-aware reader; other kinds of sources are + * returned as-is.</p> * * @param source The source to harden; never {@code null}. * @return A hardened source. * @throws TransformerConfigurationException if a hardened reader cannot be obtained. */ public static Source harden(final Source source) throws TransformerConfigurationException { - if (source instanceof StreamSource || source instanceof SAXSource && ((SAXSource) source).getXMLReader() == null) { - try { - final SAXParserFactory factory = newSAXParserFactory(); - factory.setNamespaceAware(true); - final XMLReader reader = factory.newSAXParser().getXMLReader(); - final InputSource inputSource = SAXSource.sourceToInputSource(source); - return inputSource == null ? source : new SAXSource(reader, inputSource); - } catch (final ParserConfigurationException | SAXException e) { - throw new TransformerConfigurationException("Failed to obtain a hardened XMLReader for source parsing", e); - } - } - return source; + return SAXParserHardener.hardenSource(source); } /** diff --git a/src/test/java/org/apache/commons/xml/ShadingFootprintTest.java b/src/test/java/org/apache/commons/xml/ShadingFootprintTest.java index 80dbbc9..2aa7c6a 100644 --- a/src/test/java/org/apache/commons/xml/ShadingFootprintTest.java +++ b/src/test/java/org/apache/commons/xml/ShadingFootprintTest.java @@ -39,37 +39,52 @@ * Guards the shade footprint: the set of classes a consumer pulls in when they shade a single hardener entry point. * * <p>Using {@code jdependency}, the same library {@code maven-shade-plugin}'s {@code minimizeJar} uses, this test computes each entry point's transitive class - * closure over the compiled {@code target/classes} and pins it to an expected set. It keeps the DOM, SAX and StAX hardeners from silently regaining a dependency - * on classes they should not need (for example the sibling resolver floors, or another hardener), and records that the TrAX, XPath and schema entry - * points still pull the whole library through the {@link XmlFactories} re-hardening cycle. Update the expected sets deliberately: a change here is a change to what - * a downstream shade includes.</p> + * closure over the compiled {@code target/classes} and pins it to an expected set. It keeps each hardener from silently regaining a dependency on classes it + * should not need (for example a sibling resolver floor or another hardener), so TrAX, XPath and schema build only on the shared SAX path while only the public + * {@link XmlFactories} entry pulls the whole library. Update the expected sets deliberately: a change here is a change to what a downstream shade includes.</p> */ class ShadingFootprintTest { private static final String PKG = "org.apache.commons.xml."; - /** Every hardener needs this shared exception (its {@code settingFailed}/{@code forbidden} message helpers). */ - private static final String CORE = "HardeningException"; + /** + * Every hardener needs this shared exception (its {@code settingFailed}/{@code forbidden} message helpers). + */ + private static final String HARDENING_EXCEPTION = "HardeningException"; - private static final Set<String> DOCUMENT_BUILDER_HARDENER = set( - "DocumentBuilderHardener", "HardeningDocumentBuilder", "HardeningDocumentBuilderFactory", CORE, - "FallbackDenyEntityResolver2"); + private static final Set<String> DOCUMENT_BUILDER_HARDENER = set("DocumentBuilderHardener", "HardeningDocumentBuilder", "HardeningDocumentBuilderFactory" + , HARDENING_EXCEPTION, "FallbackDenyEntityResolver2"); - private static final Set<String> SAX_PARSER_HARDENER = set( - "SAXParserHardener", "SAXParserHardener$DtdAwareDenyResolver", "SAXParserHardener$HardeningExpatXMLReader", - "HardeningSAXParser", "HardeningSAXParserFactory", "HardeningXMLReader", CORE, + private static final Set<String> SAX_PARSER_HARDENER = set("SAXParserHardener", "SAXParserHardener$DtdAwareDenyResolver", + "SAXParserHardener$HardeningExpatXMLReader", "HardeningSAXParser", "HardeningSAXParserFactory", "HardeningXMLReader", HARDENING_EXCEPTION, "FallbackDenyEntityResolver2"); - private static final Set<String> STAX_HARDENER = set( - "StaxHardener", "StaxHardener$DtdSubsetFloor", "HardeningXMLInputFactory", CORE, + private static final Set<String> STAX_HARDENER = set("StaxHardener", "StaxHardener$DtdSubsetFloor", "HardeningXMLInputFactory", HARDENING_EXCEPTION, "FallbackDenyXMLResolver", "FallbackIgnoreXMLResolver"); - /** The TrAX/XPath/schema entry points all pull the whole library through {@link XmlFactories}; this is its class count (Phase 4 territory to reduce). */ + /** + * TrAX, XPath and schema re-harden their sub-parsers through {@link SAXParserHardener#hardenSource}, so each builds on the full SAX closure below. + */ + private static final Set<String> TRANSFORMER_HARDENER = saxParsersHardenerPlus("TransformerHardener", "HardeningTransformerFactory", + "HardeningTransformer", "HardeningTemplates", "FallbackDenyURIResolver", "SaxonProvider", "SaxonProvider$1", "SaxonProvider$HardenedConfiguration" + , "SaxonProvider$SaxonProviderConfigurer"); + + private static final Set<String> XPATH_HARDENER = saxParsersHardenerPlus("XPathHardener", "SaxonProvider", "SaxonProvider$1", + "SaxonProvider$HardenedConfiguration", "SaxonProvider$SaxonProviderConfigurer"); + + private static final Set<String> SCHEMA_FACTORY = saxParsersHardenerPlus("HardeningSchemaFactory", "HardeningValidator", "HardeningValidatorHandler", + "HardeningSchema", "FallbackDenyLSResourceResolver"); + + /** + * Only the public {@link XmlFactories} entry, which news up every hardener, still pulls the whole library; this is its class count. + */ private static final int WHOLE_LIBRARY_SIZE = 32; - /** Entry points reported by the {@link #reportFootprint()} diagnostic, most-focused first, ending with the whole library. */ - private static final String[] REPORTED = { - "DocumentBuilderHardener", "SAXParserHardener", "StaxHardener", "TransformerHardener", "XPathHardener", "HardeningSchemaFactory", "XmlFactories"}; + /** + * Entry points reported by the {@link #reportFootprint()} diagnostic, most-focused first, ending with the whole library. + */ + private static final String[] REPORTED = {"DocumentBuilderHardener", "SAXParserHardener", "StaxHardener", "TransformerHardener", "XPathHardener", + "HardeningSchemaFactory", "XmlFactories"}; private static Clazzpath clazzpath; private static Path classesDir; @@ -81,7 +96,10 @@ static void indexCompiledClasses() throws Exception { clazzpath.addClazzpathUnit(classesDir); } - /** Prints each entry point's shade closure size (uncompressed {@code .class} bytes) and its share of the full library, to track the footprint over the refactor. */ + /** + * Prints each entry point's shade closure size (uncompressed {@code .class} bytes) and its share of the full library, to track the footprint over the + * refactor. + */ @AfterAll static void reportFootprint() { final long library = bytesOf(closureOf("XmlFactories")); @@ -110,15 +128,28 @@ void staxHardenerFootprint() { } @Test - void traxXPathAndSchemaPullTheWholeLibrary() { - final Set<String> whole = closureOf("XmlFactories"); - assertEquals(WHOLE_LIBRARY_SIZE, whole.size(), "XmlFactories closure size drifted: " + whole); - assertEquals(whole, closureOf("TransformerHardener"), "TransformerHardener no longer pulls exactly the whole library"); - assertEquals(whole, closureOf("XPathHardener"), "XPathHardener no longer pulls exactly the whole library"); - assertEquals(whole, closureOf("HardeningSchemaFactory"), "HardeningSchemaFactory no longer pulls exactly the whole library"); + void transformerHardenerFootprint() { + assertEquals(TRANSFORMER_HARDENER, closureOf("TransformerHardener")); } - /** Transitive class closure of {@code PKG + simpleName}, restricted to this library's own package and reported by simple name. */ + @Test + void xPathHardenerFootprint() { + assertEquals(XPATH_HARDENER, closureOf("XPathHardener")); + } + + @Test + void schemaFactoryFootprint() { + assertEquals(SCHEMA_FACTORY, closureOf("HardeningSchemaFactory")); + } + + @Test + void onlyXmlFactoriesPullsTheWholeLibrary() { + assertEquals(WHOLE_LIBRARY_SIZE, closureOf("XmlFactories").size(), "XmlFactories closure size drifted"); + } + + /** + * Transitive class closure of {@code PKG + simpleName}, restricted to this library's own package and reported by simple name. + */ private static Set<String> closureOf(final String simpleName) { final Clazz entry = clazzpath.getClazz(PKG + simpleName); if (entry == null) { @@ -134,7 +165,9 @@ private static Set<String> closureOf(final String simpleName) { return names; } - /** Sums the uncompressed {@code .class} file sizes of a closure's classes, as they would land in a shaded jar. */ + /** + * Sums the uncompressed {@code .class} file sizes of a closure's classes, as they would land in a shaded jar. + */ private static long bytesOf(final Set<String> simpleNames) { long total = 0; for (final String name : simpleNames) { @@ -154,4 +187,13 @@ private static String strip(final String qualifiedName) { private static Set<String> set(final String... names) { return new TreeSet<>(Arrays.asList(names)); } + + /** + * {@link #SAX_PARSER_HARDENER} plus the extra names; used where an entry point's closure is the SAX path plus its own classes. + */ + private static Set<String> saxParsersHardenerPlus(final String... more) { + final Set<String> union = new TreeSet<>(SAX_PARSER_HARDENER); + union.addAll(Arrays.asList(more)); + return union; + } }
