Two failure modes share this type:
+ *Three failure modes share this type:
*The message names the unsupported factory class or the specific feature, attribute or property that failed; the cause, when present, is the original @@ -71,6 +72,19 @@ static String forbidden(final String type, final String namespace, final String SecureException.THROW_ON_UNRESOLVED, type, namespace, publicId, systemId, baseURI); } + /** + * Builds the standard exception for a failed internal reader provisioning. + * + *
Every supported implementation provides a reader as a routine capability, so the wrapped {@code ParserConfigurationException} or + * {@code SAXException} signals a broken environment, not a per-parse condition — hence unchecked.
+ * + * @param cause the original checked exception from the JAXP implementation. + * @return the exception to throw. + */ + static SecureException readerFailed(final Throwable cause) { + return new SecureException("Failed to create a secure XMLReader", cause); + } + /** * Whether unresolved external references must be rejected instead of resolved to empty content. * diff --git a/src/main/java/org/apache/commons/xml/SecureSAXParserFactory.java b/src/main/java/org/apache/commons/xml/SecureSAXParserFactory.java index d12b188a..9c1275fb 100644 --- a/src/main/java/org/apache/commons/xml/SecureSAXParserFactory.java +++ b/src/main/java/org/apache/commons/xml/SecureSAXParserFactory.java @@ -27,7 +27,6 @@ import javax.xml.parsers.SAXParser; 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 javax.xml.validation.Schema; @@ -301,15 +300,16 @@ public static SAXParserFactory newNSInstance(final String factoryClassName, fina * * @param overrideDefaultParser whether {@value #OVERRIDE_DEFAULT_PARSER} on the originating factory asks to override the JDK's default parser. * @return a secure reader. - * @throws TransformerConfigurationException if a secure reader cannot be obtained. + * @throws IllegalStateException Thrown if the underlying implementation cannot provide a secure reader; providing one is a routine capability of every + * supported implementation, so a failure signals a broken environment, not a per-parse condition. * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service * configuration error} or if the implementation is not available or cannot be instantiated. */ - static XMLReader newSecureXMLReader(final boolean overrideDefaultParser) throws TransformerConfigurationException { + static XMLReader newXMLReader(final boolean overrideDefaultParser) { try { return newNSInstance(overrideDefaultParser).newSAXParser().getXMLReader(); - } catch (final ParserConfigurationException | SAXException e) { - throw new TransformerConfigurationException("Failed to obtain a secure XMLReader for source parsing", e); + } catch (ParserConfigurationException | SAXException e) { + throw SecureException.readerFailed(e); } } @@ -355,14 +355,14 @@ static SAXParserFactory secure(final SAXParserFactory factory) { * @param source the source to secure; never {@code null}. * @param overrideDefaultParser whether {@value #OVERRIDE_DEFAULT_PARSER} on the originating factory asks to override the JDK's default parser. * @return a secure source. - * @throws TransformerConfigurationException if a secure reader cannot be obtained. - * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service - * configuration error} or if the implementation is not available or cannot be instantiated. + * @throws IllegalStateException Thrown if the underlying implementation cannot provide a secure reader. + * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service + * configuration error} or if the implementation is not available or cannot be instantiated. */ - static Source secure(final Source source, final boolean overrideDefaultParser) throws TransformerConfigurationException { + static Source secure(final Source source, final boolean overrideDefaultParser) { if (source instanceof StreamSource || source instanceof SAXSource && ((SAXSource) source).getXMLReader() == null) { final InputSource inputSource = SAXSource.sourceToInputSource(source); - return inputSource == null ? source : new SAXSource(newSecureXMLReader(overrideDefaultParser), inputSource); + return inputSource == null ? source : new SAXSource(newXMLReader(overrideDefaultParser), inputSource); } return source; } diff --git a/src/main/java/org/apache/commons/xml/SecureSchemaFactory.java b/src/main/java/org/apache/commons/xml/SecureSchemaFactory.java index a16cd2e5..76b47bc0 100644 --- a/src/main/java/org/apache/commons/xml/SecureSchemaFactory.java +++ b/src/main/java/org/apache/commons/xml/SecureSchemaFactory.java @@ -24,7 +24,6 @@ import javax.xml.XMLConstants; import javax.xml.parsers.FactoryConfigurationError; import javax.xml.transform.Source; -import javax.xml.transform.TransformerConfigurationException; import javax.xml.validation.Schema; import javax.xml.validation.SchemaFactory; import javax.xml.validation.SchemaFactoryConfigurationError; @@ -165,19 +164,15 @@ private boolean overrideDefaultParser() { * * @param schemas the schema sources to secure; must not be {@code null}. * @return a new array of secure sources. - * @throws SAXException if any source cannot be secure. + * @throws IllegalStateException Thrown if the underlying implementation cannot provide a secure reader. * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service * configuration error} or if the implementation is not available or cannot be instantiated. */ - private Source[] secure(final Source[] schemas) throws SAXException { + private Source[] secure(final Source[] schemas) { final Source[] secure = new Source[schemas.length]; final boolean overrideDefaultParser = overrideDefaultParser(); - try { - for (int i = 0; i < schemas.length; i++) { - secure[i] = SecureSAXParserFactory.secure(schemas[i], overrideDefaultParser); - } - } catch (final TransformerConfigurationException e) { - throw new SAXException("Failed to secure schema source", e); + for (int i = 0; i < schemas.length; i++) { + secure[i] = SecureSAXParserFactory.secure(schemas[i], overrideDefaultParser); } return secure; } diff --git a/src/main/java/org/apache/commons/xml/SecureTransformer.java b/src/main/java/org/apache/commons/xml/SecureTransformer.java index bfcdeaa7..f9cf9f21 100644 --- a/src/main/java/org/apache/commons/xml/SecureTransformer.java +++ b/src/main/java/org/apache/commons/xml/SecureTransformer.java @@ -26,7 +26,6 @@ import javax.xml.transform.Result; import javax.xml.transform.Source; import javax.xml.transform.Transformer; -import javax.xml.transform.TransformerConfigurationException; import javax.xml.transform.TransformerException; import javax.xml.transform.URIResolver; @@ -139,9 +138,9 @@ public void setURIResolver(final URIResolver resolver) { /** * {@inheritDoc} * - * @throws TransformerConfigurationException Thrown if a secure reader cannot be obtained. - * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service configuration error} or - * if the implementation is not available or cannot be instantiated. + * @throws IllegalStateException Thrown if the underlying implementation cannot provide a secure reader. + * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service configuration error} or + * if the implementation is not available or cannot be instantiated. */ @Override public void transform(final Source xmlSource, final Result outputTarget) throws TransformerException { diff --git a/src/main/java/org/apache/commons/xml/SecureValidator.java b/src/main/java/org/apache/commons/xml/SecureValidator.java index 6497b2f2..f49645ac 100644 --- a/src/main/java/org/apache/commons/xml/SecureValidator.java +++ b/src/main/java/org/apache/commons/xml/SecureValidator.java @@ -23,7 +23,6 @@ import javax.xml.parsers.FactoryConfigurationError; import javax.xml.transform.Result; import javax.xml.transform.Source; -import javax.xml.transform.TransformerConfigurationException; import javax.xml.validation.Validator; import org.w3c.dom.ls.LSResourceResolver; @@ -120,10 +119,6 @@ public void setResourceResolver(final LSResourceResolver resourceResolver) { */ @Override public void validate(final Source source, final Result result) throws SAXException, IOException { - try { - delegate.validate(SecureSAXParserFactory.secure(source, overrideDefaultParser), result); - } catch (final TransformerConfigurationException e) { - throw new SAXException("Failed to secure source for validation", e); - } + delegate.validate(SecureSAXParserFactory.secure(source, overrideDefaultParser), result); } } diff --git a/src/main/java/org/apache/commons/xml/SecureXMLFilter.java b/src/main/java/org/apache/commons/xml/SecureXMLFilter.java index 56c294be..87e5e334 100644 --- a/src/main/java/org/apache/commons/xml/SecureXMLFilter.java +++ b/src/main/java/org/apache/commons/xml/SecureXMLFilter.java @@ -21,6 +21,8 @@ import java.util.Objects; import javax.xml.parsers.FactoryConfigurationError; +import javax.xml.transform.ErrorListener; +import javax.xml.transform.SourceLocator; import javax.xml.transform.Transformer; import javax.xml.transform.TransformerException; import javax.xml.transform.sax.SAXResult; @@ -29,7 +31,9 @@ import org.xml.sax.ContentHandler; import org.xml.sax.InputSource; import org.xml.sax.SAXException; +import org.xml.sax.SAXParseException; import org.xml.sax.XMLFilter; +import org.xml.sax.XMLReader; import org.xml.sax.ext.LexicalHandler; import org.xml.sax.helpers.XMLFilterImpl; @@ -40,9 +44,10 @@ * unsecured reader for the input (the stock JDK's does so as early as {@code setContentHandler}) and cast a supplied {@link javax.xml.transform.Templates} to * their own type, which a wrapped Templates is not. Here the input is parsed by the parent reader, a secure one installed on first {@code parse} when the * caller has not set a parent (a caller-set parent is trusted configuration, used as-is), and the transformation runs on a {@link SecureTransformer}, so - * runtime {@code document()} sits on the resolver floor. + * runtime {@code document()} sits on the resolver floor. The filter is also the transformer's {@link ErrorListener}, forwarding TrAX error reports to the + * caller-set {@link org.xml.sax.ErrorHandler} the way the parent reader's SAX reports are. */ -final class SecureXMLFilter extends XMLFilterImpl { +final class SecureXMLFilter extends XMLFilterImpl implements ErrorListener { private final SecureTemplates templates; @@ -56,6 +61,33 @@ final class SecureXMLFilter extends XMLFilterImpl { this.templates = Objects.requireNonNull(templates, "templates"); } + /** + * Forwards a recoverable transformation error to the caller-set {@link org.xml.sax.ErrorHandler}, mirroring the SAX contract: the transformation continues + * unless that handler throws. + */ + @Override + public void error(final TransformerException e) throws TransformerException { + try { + error(toSAXParseException(e)); + } catch (final SAXException se) { + throw new TransformerException(se); + } + } + + /** + * Forwards a fatal transformation error to the caller-set {@link org.xml.sax.ErrorHandler}, then fails the parse like a SAX parser does after + * {@code fatalError}: some implementations' lenient default listeners would otherwise only print and truncate the parse silently. + */ + @Override + public void fatalError(final TransformerException e) throws TransformerException { + try { + fatalError(toSAXParseException(e)); + } catch (final SAXException se) { + throw new TransformerException(se); + } + throw e; + } + /** * {@inheritDoc} * @@ -69,12 +101,14 @@ public void parse(final InputSource input) throws SAXException, IOException { throw new SAXException("No ContentHandler set on the XMLFilter to receive the transformation result"); } if (getParent() == null) { - try { - setParent(SecureSAXParserFactory.newSecureXMLReader(templates.overrideDefaultParser)); - } catch (final TransformerException e) { - throw new SAXException(e); - } + setParent(SecureSAXParserFactory.newXMLReader(templates.overrideDefaultParser)); } + final XMLReader parent = getParent(); + // Like XMLFilterImpl.setupParse, minus the ContentHandler: the transformer owns the parent's content events and delivers the transformed stream to + // the caller's handler through the SAXResult instead. + parent.setEntityResolver(this); + parent.setDTDHandler(this); + parent.setErrorHandler(this); final SAXResult result = new SAXResult(handler); if (handler instanceof LexicalHandler) { result.setLexicalHandler((LexicalHandler) handler); @@ -82,9 +116,48 @@ public void parse(final InputSource input) throws SAXException, IOException { try { // A new SecureTransformer per parse: the floor is installed on it, and transformers are not reusable across concurrent parses. final Transformer transformer = templates.newTransformer(); - transformer.transform(new SAXSource(getParent(), input), result); + // The filter is the listener, so TrAX error reports reach the caller-set ErrorHandler like the parent reader's SAX reports do. + transformer.setErrorListener(this); + transformer.transform(new SAXSource(parent, input), result); } catch (final TransformerException e) { + // The parent reader's parse errors and the handler's own exceptions arrive wrapped; rethrow the original rather than nesting the hierarchies. + final Throwable cause = e.getCause(); + if (cause instanceof SAXException) { + throw (SAXException) cause; + } + if (cause instanceof IOException) { + throw (IOException) cause; + } throw new SAXException(e); } } + + /** + * Bridges a TrAX report to the SAX callback shape. + * + * @param e the reported exception. + * @return The original {@link SAXParseException} where one is the cause, otherwise a synthetic one carrying the locator. + */ + private static SAXParseException toSAXParseException(final TransformerException e) { + final Throwable cause = e.getCause(); + if (cause instanceof SAXParseException) { + return (SAXParseException) cause; + } + // Embed the cause rather than the TrAX wrapper, so the originating exception stays directly reachable in the reported chain. + final Exception embedded = cause instanceof Exception ? (Exception) cause : e; + final SourceLocator locator = e.getLocator(); + return locator == null + ? new SAXParseException(e.getMessage(), null, null, -1, -1, embedded) + : new SAXParseException(e.getMessage(), locator.getPublicId(), locator.getSystemId(), locator.getLineNumber(), locator.getColumnNumber(), embedded); + } + + /** Forwards a transformation warning to the caller-set {@link org.xml.sax.ErrorHandler}; the transformation continues unless that handler throws. */ + @Override + public void warning(final TransformerException e) throws TransformerException { + try { + warning(toSAXParseException(e)); + } catch (final SAXException se) { + throw new TransformerException(se); + } + } } diff --git a/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java b/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java index cf3e5d39..95af0d80 100644 --- a/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java +++ b/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java @@ -83,9 +83,9 @@ void schemaFactoryReadsFeatureAtCreation() throws Exception { @Test void secureReaderFollowsFlag() throws Exception { assumeFalse(AttackTestSupport.IS_ANDROID); - final XMLReader pinned = ((SecureXMLReader) SecureSAXParserFactory.newSecureXMLReader(false)).getDelegate(); + final XMLReader pinned = ((SecureXMLReader) SecureSAXParserFactory.newXMLReader(false)).getDelegate(); assertTrue(pinned.getClass().getName().startsWith(JDK_INTERNAL_PREFIX), pinned.getClass().getName()); - final XMLReader pluggable = ((SecureXMLReader) SecureSAXParserFactory.newSecureXMLReader(true)).getDelegate(); + final XMLReader pluggable = ((SecureXMLReader) SecureSAXParserFactory.newXMLReader(true)).getDelegate(); final XMLReader lookedUp = ((SecureXMLReader) SecureSAXParserFactory.newNSInstance().newSAXParser().getXMLReader()).getDelegate(); assertEquals(lookedUp.getClass(), pluggable.getClass()); if (xercesOnClasspath()) { diff --git a/src/test/java/org/apache/commons/xml/XMLFilterTest.java b/src/test/java/org/apache/commons/xml/XMLFilterTest.java index fde4ea98..7230e57f 100644 --- a/src/test/java/org/apache/commons/xml/XMLFilterTest.java +++ b/src/test/java/org/apache/commons/xml/XMLFilterTest.java @@ -17,22 +17,35 @@ package org.apache.commons.xml; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.StringReader; +import java.util.ArrayList; +import java.util.List; import javax.xml.XMLConstants; import javax.xml.transform.Templates; +import javax.xml.transform.TransformerException; import javax.xml.transform.TransformerFactory; import javax.xml.transform.sax.SAXTransformerFactory; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.xml.sax.ContentHandler; +import org.xml.sax.DTDHandler; +import org.xml.sax.EntityResolver; +import org.xml.sax.ErrorHandler; import org.xml.sax.InputSource; +import org.xml.sax.SAXException; import org.xml.sax.XMLFilter; +import org.xml.sax.helpers.AttributesImpl; +import org.xml.sax.helpers.DefaultHandler; +import org.xml.sax.helpers.XMLFilterImpl; /** * {@link XMLFilter} products of the secure factory: the input document is parsed by a secure reader (never a self-provisioned permissive one), and the @@ -48,6 +61,21 @@ class XMLFilterTest { + "