This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/commons-xml.git
commit cd2a7a4087fe448372acfc92d2e6094cd9c0e417 Author: Gary Gregory <[email protected]> AuthorDate: Sun Aug 30 07:53:33 2026 -0400 Add missing unit tests. - Total coverage is now 75% (70% branches). - Sort members --- .../apache/commons/xml/MethodHandleFactory.java | 2 +- .../commons/xml/SecureTransformerFactory.java | 64 +-- .../org/apache/commons/xml/SecureXMLFilter.java | 38 +- .../commons/xml/MethodHandleFactoryTest.java | 74 +++ .../commons/xml/SecureXMLInputFactoryTest.java | 622 +++++++++++++++++++++ .../java/org/apache/commons/xml/XMLFilterTest.java | 16 +- 6 files changed, 756 insertions(+), 60 deletions(-) diff --git a/src/main/java/org/apache/commons/xml/MethodHandleFactory.java b/src/main/java/org/apache/commons/xml/MethodHandleFactory.java index 671819d..1468d0c 100644 --- a/src/main/java/org/apache/commons/xml/MethodHandleFactory.java +++ b/src/main/java/org/apache/commons/xml/MethodHandleFactory.java @@ -54,7 +54,7 @@ static <T, E extends Throwable> T invokeExact(final ThrowableCallable<T> methodH try { return methodHandle.call(); } catch (final Throwable e) { - if (e.getClass().isInstance(rethrow)) { + if (rethrow.isInstance(e)) { throw rethrow.cast(e); } // Unreachable: the looked-up method declares no other exceptions. diff --git a/src/main/java/org/apache/commons/xml/SecureTransformerFactory.java b/src/main/java/org/apache/commons/xml/SecureTransformerFactory.java index f6db179..4e5b98c 100644 --- a/src/main/java/org/apache/commons/xml/SecureTransformerFactory.java +++ b/src/main/java/org/apache/commons/xml/SecureTransformerFactory.java @@ -214,38 +214,6 @@ public URIResolver getURIResolver() { return floor.getDelegate(); } - private TransformerHandler secure(final TransformerHandler handler) { - return handler == null ? null : new SecureTransformerHandler(handler, getURIResolver(), emptySource, overrideDefaultParser()); - } - - /** - * Parses a reader-less source into a DOM through a secure, namespace-aware {@link javax.xml.parsers.DocumentBuilder} and returns a {@link DOMSource} - * carrying its system id, so the consumer walks the tree instead of provisioning its own reader. Any other source is left to - * {@link SecureSAXParserFactory#secure(Source, boolean)}. - * - * @param source The source to scan for an associated stylesheet. - * @return A {@link DOMSource} for a reader-less source, otherwise the result of {@link SecureSAXParserFactory#secure(Source, boolean)}. - * @throws TransformerConfigurationException if the source cannot be parsed. - * @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 SecureException Thrown if a (non-Android) factory cannot support the secure processing feature {@link XMLConstants#FEATURE_SECURE_PROCESSING}. - */ - private Source secureSourceToDom(final Source source) throws TransformerConfigurationException { - if (source instanceof StreamSource || source instanceof SAXSource && ((SAXSource) source).getXMLReader() == null) { - final InputSource inputSource = SAXSource.sourceToInputSource(source); - if (inputSource != null) { - try { - final DocumentBuilderFactory factory = SecureDocumentBuilderFactory.newNSInstance(overrideDefaultParser()); - final Document document = factory.newDocumentBuilder().parse(inputSource); - return new DOMSource(document, inputSource.getSystemId()); - } catch (final ParserConfigurationException | SAXException | IOException e) { - throw new TransformerConfigurationException("Failed to parse the source for associated-stylesheet lookup", e); - } - } - } - return SecureSAXParserFactory.secure(source, overrideDefaultParser()); - } - /** * {@inheritDoc} * @@ -335,6 +303,38 @@ private boolean overrideDefaultParser() { return !supportsOverrideDefaultParser || delegate.getFeature(SecureSAXParserFactory.OVERRIDE_DEFAULT_PARSER); } + private TransformerHandler secure(final TransformerHandler handler) { + return handler == null ? null : new SecureTransformerHandler(handler, getURIResolver(), emptySource, overrideDefaultParser()); + } + + /** + * Parses a reader-less source into a DOM through a secure, namespace-aware {@link javax.xml.parsers.DocumentBuilder} and returns a {@link DOMSource} + * carrying its system id, so the consumer walks the tree instead of provisioning its own reader. Any other source is left to + * {@link SecureSAXParserFactory#secure(Source, boolean)}. + * + * @param source The source to scan for an associated stylesheet. + * @return A {@link DOMSource} for a reader-less source, otherwise the result of {@link SecureSAXParserFactory#secure(Source, boolean)}. + * @throws TransformerConfigurationException if the source cannot be parsed. + * @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 SecureException Thrown if a (non-Android) factory cannot support the secure processing feature {@link XMLConstants#FEATURE_SECURE_PROCESSING}. + */ + private Source secureSourceToDom(final Source source) throws TransformerConfigurationException { + if (source instanceof StreamSource || source instanceof SAXSource && ((SAXSource) source).getXMLReader() == null) { + final InputSource inputSource = SAXSource.sourceToInputSource(source); + if (inputSource != null) { + try { + final DocumentBuilderFactory factory = SecureDocumentBuilderFactory.newNSInstance(overrideDefaultParser()); + final Document document = factory.newDocumentBuilder().parse(inputSource); + return new DOMSource(document, inputSource.getSystemId()); + } catch (final ParserConfigurationException | SAXException | IOException e) { + throw new TransformerConfigurationException("Failed to parse the source for associated-stylesheet lookup", e); + } + } + } + return SecureSAXParserFactory.secure(source, overrideDefaultParser()); + } + @Override public void setAttribute(final String name, final Object value) { delegate.setAttribute(name, value); diff --git a/src/main/java/org/apache/commons/xml/SecureXMLFilter.java b/src/main/java/org/apache/commons/xml/SecureXMLFilter.java index 87e5e33..42a45f9 100644 --- a/src/main/java/org/apache/commons/xml/SecureXMLFilter.java +++ b/src/main/java/org/apache/commons/xml/SecureXMLFilter.java @@ -49,6 +49,25 @@ */ final class SecureXMLFilter extends XMLFilterImpl implements ErrorListener { + /** + * 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); + } + private final SecureTemplates templates; /** @@ -132,25 +151,6 @@ public void parse(final InputSource input) throws SAXException, IOException { } } - /** - * 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 { diff --git a/src/test/java/org/apache/commons/xml/MethodHandleFactoryTest.java b/src/test/java/org/apache/commons/xml/MethodHandleFactoryTest.java new file mode 100644 index 0000000..bb934e4 --- /dev/null +++ b/src/test/java/org/apache/commons/xml/MethodHandleFactoryTest.java @@ -0,0 +1,74 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.commons.xml; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodType; + +import javax.xml.parsers.FactoryConfigurationError; +import javax.xml.stream.XMLInputFactory; + +import org.junit.jupiter.api.Test; + +/** + * Tests for {@link MethodHandleFactory}, the reflective lookup helper behind {@link SecureXMLInputFactory#newDefaultFactory()}. + * <p> + * The helper is exercised directly, including the miss and rethrow paths that no standard JVM factory lookup reaches, so the platform-specific fallback logic + * of the secure factory has full branch coverage. + * </p> + */ +class MethodHandleFactoryTest { + + @Test + void findStaticReturnsHandleForExistingMethod() { + final MethodHandle handle = MethodHandleFactory.findStatic(XMLInputFactory.class, "newDefaultFactory", MethodType.methodType(XMLInputFactory.class)); + assertNotNull(handle, "an existing static method must resolve to a handle"); + } + + @Test + void findStaticReturnsNullForMissingMethod() { + assertNull(MethodHandleFactory.findStatic(XMLInputFactory.class, "noSuchMethod", MethodType.methodType(void.class)), + "a missing method must resolve to null"); + } + + @Test + void invokeExactRethrowsDeclaredException() { + assertThrows(FactoryConfigurationError.class, () -> MethodHandleFactory.invokeExact(() -> { + throw new FactoryConfigurationError("boom"); + }, FactoryConfigurationError.class), "an exception of the declared type must be rethrown"); + } + + @Test + void invokeExactReturnsValue() { + assertEquals("value", MethodHandleFactory.invokeExact(() -> "value", IllegalStateException.class), "the callable's value must be returned"); + } + + @Test + void invokeExactWrapsUnexpectedException() { + final IllegalStateException wrapped = assertThrows(IllegalStateException.class, () -> MethodHandleFactory.invokeExact(() -> { + throw new IllegalArgumentException("boom"); + }, FactoryConfigurationError.class), "an undeclared exception must be wrapped"); + assertInstanceOf(IllegalArgumentException.class, wrapped.getCause(), "the unexpected exception must be the wrapper's cause"); + } +} diff --git a/src/test/java/org/apache/commons/xml/SecureXMLInputFactoryTest.java b/src/test/java/org/apache/commons/xml/SecureXMLInputFactoryTest.java new file mode 100644 index 0000000..82988dc --- /dev/null +++ b/src/test/java/org/apache/commons/xml/SecureXMLInputFactoryTest.java @@ -0,0 +1,622 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +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.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.io.Reader; +import java.io.StringReader; +import java.lang.reflect.Constructor; +import java.lang.reflect.Proxy; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; + +import javax.xml.stream.EventFilter; +import javax.xml.stream.StreamFilter; +import javax.xml.stream.XMLEventReader; +import javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLReporter; +import javax.xml.stream.XMLResolver; +import javax.xml.stream.XMLStreamConstants; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamReader; +import javax.xml.stream.events.XMLEvent; +import javax.xml.stream.util.XMLEventAllocator; +import javax.xml.transform.Source; +import javax.xml.transform.stream.StreamSource; + +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link SecureXMLInputFactory} and the {@link XMLInputFactory} wrapper it installs. + * <p> + * The wrapper's delegation and resolver-routing logic is exercised against a recording stand-in factory, so every branch is deterministic on every platform. + * The public factory methods are additionally exercised end-to-end against the platform's real implementation, including the Woodstox-specific resolver hooks + * where that implementation is present. + * </p> + */ +@Tag("stax") +class SecureXMLInputFactoryTest { + + /** + * A recording stand-in {@link XMLInputFactory} for the delegation tests. + * <p> + * Every call is recorded in {@link #calls} together with the runtime class and identity hash code of each argument, so the wrapper can be asserted to + * forward the caller's exact arguments. The resolver hook the fake reports through {@code getProperty} and {@code getXMLResolver} is whatever the wrapper + * or a test last installed, so the tests can steer the wrapper into each routing branch deterministically. + * </p> + */ + private static final class RecordingXMLInputFactory extends XMLInputFactory { + + /** The {@link XMLEventReader} every event-flavor creation method returns. */ + static final XMLEventReader EVENT_SENTINEL = proxy(XMLEventReader.class); + + /** The {@link XMLStreamReader} every stream-flavor creation method returns. */ + static final XMLStreamReader STREAM_SENTINEL = proxy(XMLStreamReader.class); + + /** A stand-in {@link XMLEventAllocator} for the round-trip tests. */ + static final XMLEventAllocator ALLOCATOR_SENTINEL = proxy(XMLEventAllocator.class); + + /** A stand-in {@link XMLReporter} for the round-trip tests. */ + static final XMLReporter REPORTER_SENTINEL = proxy(XMLReporter.class); + + /** A stand-in {@link EventFilter} for the delegation tests. */ + static final EventFilter EVENT_FILTER_SENTINEL = proxy(EventFilter.class); + + /** A stand-in {@link StreamFilter} for the delegation tests. */ + static final StreamFilter STREAM_FILTER_SENTINEL = proxy(StreamFilter.class); + + /** + * Formats a recorded call so a test can assert the wrapper forwarded the exact arguments. + */ + static String call(final String method, final Object... args) { + final StringBuilder entry = new StringBuilder(method).append('('); + for (int i = 0; i < args.length; i++) { + if (i > 0) { + entry.append(", "); + } + final Object arg = args[i]; + entry.append(arg == null ? "null" : arg.getClass().getName()).append('@').append(System.identityHashCode(arg)); + } + return entry.append(')').toString(); + } + + /** + * Builds an unbacked instance of the given interface whose boolean and int methods answer their neutral values and whose other methods answer + * {@code null}; the sentinel readers returned from the fake's creation methods. + */ + private static <T> T proxy(final Class<T> type) { + return (T) Proxy.newProxyInstance(type.getClassLoader(), new Class<?>[] { type }, (p, method, args) -> { + if (method.getReturnType() == boolean.class) { + return Boolean.FALSE; + } + if (method.getReturnType() == int.class) { + return 0; + } + return null; + }); + } + + /** Recorded calls in order, each formatted by {@link #call}. */ + final List<String> calls = new ArrayList<>(); + + /** The resolver-valued hook this factory reports; the wrapper's floor or whatever a test installs. */ + Object resolverHook; + + /** The allocator last installed via {@code setEventAllocator}. */ + XMLEventAllocator allocator; + + /** The reporter last installed via {@code setXMLReporter}. */ + XMLReporter reporter; + + /** The answer {@code isPropertySupported} gives; {@code true} by default. */ + boolean supported = true; + + @Override + public XMLEventReader createFilteredReader(final XMLEventReader reader, final EventFilter filter) { + record("createFilteredReader", reader, filter); + return EVENT_SENTINEL; + } + + @Override + public XMLStreamReader createFilteredReader(final XMLStreamReader reader, final StreamFilter filter) { + record("createFilteredReader", reader, filter); + return STREAM_SENTINEL; + } + + @Override + public XMLEventReader createXMLEventReader(final InputStream stream) { + record("createXMLEventReader", stream); + return EVENT_SENTINEL; + } + + @Override + public XMLEventReader createXMLEventReader(final InputStream stream, final String encoding) { + record("createXMLEventReader", stream, encoding); + return EVENT_SENTINEL; + } + + @Override + public XMLEventReader createXMLEventReader(final Reader reader) { + record("createXMLEventReader", reader); + return EVENT_SENTINEL; + } + + @Override + public XMLEventReader createXMLEventReader(final Source source) { + record("createXMLEventReader", source); + return EVENT_SENTINEL; + } + + @Override + public XMLEventReader createXMLEventReader(final String systemId, final InputStream stream) { + record("createXMLEventReader", systemId, stream); + return EVENT_SENTINEL; + } + + @Override + public XMLEventReader createXMLEventReader(final String systemId, final Reader reader) { + record("createXMLEventReader", systemId, reader); + return EVENT_SENTINEL; + } + + @Override + public XMLEventReader createXMLEventReader(final XMLStreamReader reader) { + record("createXMLEventReader", reader); + return EVENT_SENTINEL; + } + + @Override + public XMLStreamReader createXMLStreamReader(final InputStream stream) { + record("createXMLStreamReader", stream); + return STREAM_SENTINEL; + } + + @Override + public XMLStreamReader createXMLStreamReader(final InputStream stream, final String encoding) { + record("createXMLStreamReader", stream, encoding); + return STREAM_SENTINEL; + } + + @Override + public XMLStreamReader createXMLStreamReader(final Reader reader) { + record("createXMLStreamReader", reader); + return STREAM_SENTINEL; + } + + @Override + public XMLStreamReader createXMLStreamReader(final Source source) { + record("createXMLStreamReader", source); + return STREAM_SENTINEL; + } + + @Override + public XMLStreamReader createXMLStreamReader(final String systemId, final InputStream stream) { + record("createXMLStreamReader", systemId, stream); + return STREAM_SENTINEL; + } + + @Override + public XMLStreamReader createXMLStreamReader(final String systemId, final Reader reader) { + record("createXMLStreamReader", systemId, reader); + return STREAM_SENTINEL; + } + + @Override + public XMLEventAllocator getEventAllocator() { + record("getEventAllocator"); + return allocator; + } + + @Override + public Object getProperty(final String name) { + record("getProperty", name); + return resolverHook; + } + + @Override + public XMLReporter getXMLReporter() { + record("getXMLReporter"); + return reporter; + } + + @Override + public XMLResolver getXMLResolver() { + record("getXMLResolver"); + return (XMLResolver) resolverHook; + } + + @Override + public boolean isPropertySupported(final String name) { + record("isPropertySupported", name); + return supported; + } + + /** + * Records a call with the runtime class and identity hash code of each argument, the format {@link #calls} entries use. + */ + private void record(final String method, final Object... args) { + calls.add(call(method, args)); + } + + @Override + public void setEventAllocator(final XMLEventAllocator eventAllocator) { + record("setEventAllocator", eventAllocator); + allocator = eventAllocator; + } + + @Override + public void setProperty(final String name, final Object value) { + record("setProperty", name, value); + if (value == null || value instanceof XMLResolver) { + resolverHook = value; + } + } + + @Override + public void setXMLReporter(final XMLReporter xmlReporter) { + record("setXMLReporter", xmlReporter); + reporter = xmlReporter; + } + + @Override + public void setXMLResolver(final XMLResolver resolver) { + record("setXMLResolver", resolver); + resolverHook = resolver; + } + } + + private static final String BENIGN_XML = "<?xml version=\"1.0\"?>\n<root><child>hello</child></root>\n"; + + private static final String SYSTEM_ID = "http://example.invalid/document.xml";; + + /** + * Drains every event from the event reader and returns the accumulated character and CDATA data. + */ + private static String drainEvents(final XMLEventReader reader) throws XMLStreamException { + final StringBuilder text = new StringBuilder(); + try { + while (reader.hasNext()) { + final XMLEvent event = reader.nextEvent(); + if (event.isCharacters() || event.getEventType() == XMLStreamConstants.CDATA) { + text.append(event.asCharacters().getData()); + } + } + } finally { + reader.close(); + } + return text.toString(); + } + + /** + * Drains every event from the stream reader and returns the accumulated character data. + */ + private static String drainStream(final XMLStreamReader reader) throws XMLStreamException { + final StringBuilder text = new StringBuilder(); + try { + while (reader.hasNext()) { + if (reader.next() == XMLStreamConstants.CHARACTERS) { + text.append(reader.getText()); + } + } + } finally { + reader.close(); + } + return text.toString(); + } + + @Test + void getPropertyReportsForeignResolverUnchanged() { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + final XMLResolver foreign = (publicID, systemID, baseURI, namespace) -> "foreign"; + fake.setProperty(XMLInputFactory.RESOLVER, foreign); + assertSame(foreign, secure.getProperty(XMLInputFactory.RESOLVER), "a non-floor resolver must be reported unchanged"); + assertSame(foreign, secure.getXMLResolver(), "getXMLResolver must report a non-floor resolver unchanged"); + } + + @Test + void getPropertyReportsNullResolver() { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + fake.setProperty(XMLInputFactory.RESOLVER, null); + assertNull(secure.getProperty(XMLInputFactory.RESOLVER), "an empty hook must report no resolver"); + assertNull(secure.getXMLResolver(), "an empty hook must report no resolver"); + } + + @Test + void getXMLResolverInitiallyNull() { + assertNull(SecureXMLInputFactory.newInstance().getXMLResolver(), "a fresh secure factory must report no caller resolver"); + assertNull(SecureXMLInputFactory.newDefaultFactory().getXMLResolver(), "a fresh secure factory must report no caller resolver"); + } + + @Test + void newDefaultFactoryParsesBenignDocument() throws Exception { + final XMLInputFactory factory = SecureXMLInputFactory.newDefaultFactory(); + assertEquals(Boolean.TRUE, factory.getProperty(XMLInputFactory.SUPPORT_DTD), "a secure factory must keep the implementation's DTD default"); + assertTrue(drainStream(factory.createXMLStreamReader(new StringReader(BENIGN_XML))).contains("hello"), "stream reader must parse the document"); + assertTrue(drainEvents(factory.createXMLEventReader(new StringReader(BENIGN_XML))).contains("hello"), "event reader must parse the document"); + } + + @Test + void newFactoryNullFactoryIdThrows() { + assertThrows(NullPointerException.class, () -> SecureXMLInputFactory.newFactory(null, null), "a null factory id must be rejected"); + } + + @Test + void newFactoryParsesBenignDocument() throws Exception { + final XMLInputFactory factory = SecureXMLInputFactory.newFactory(); + assertTrue(drainStream(factory.createXMLStreamReader(new StringReader(BENIGN_XML))).contains("hello"), "stream reader must parse the document"); + assertTrue(drainEvents(factory.createXMLEventReader(new StringReader(BENIGN_XML))).contains("hello"), "event reader must parse the document"); + } + + @Test + void newFactoryWithFactoryIdReturnsUsableSecureFactory() throws Exception { + final String factoryId = "org.apache.commons.xml.test.inputFactory"; + System.setProperty(factoryId, XMLInputFactory.newInstance().getClass().getName()); + try { + final XMLInputFactory factory = SecureXMLInputFactory.newFactory(factoryId, getClass().getClassLoader()); + assertNull(factory.getXMLResolver(), "a fresh secure factory must report no caller resolver"); + assertTrue(drainStream(factory.createXMLStreamReader(new StringReader(BENIGN_XML))).contains("hello"), "factory must parse the document"); + } finally { + System.clearProperty(factoryId); + } + } + + @Test + void newInstanceParsesBenignDocument() throws Exception { + final XMLInputFactory factory = SecureXMLInputFactory.newInstance(); + assertTrue(drainStream(factory.createXMLStreamReader(new StringReader(BENIGN_XML))).contains("hello"), "stream reader must parse the document"); + assertTrue(drainEvents(factory.createXMLEventReader(new StringReader(BENIGN_XML))).contains("hello"), "event reader must parse the document"); + } + + @Test + void privateConstructorIsInvokable() throws Exception { + final Constructor<SecureXMLInputFactory> constructor = SecureXMLInputFactory.class.getDeclaredConstructor(); + constructor.setAccessible(true); + assertNotNull(constructor.newInstance(), "the private constructor must exist and be invokable"); + } + + @Test + void secureNullDelegateThrows() { + assertThrows(NullPointerException.class, () -> SecureXMLInputFactory.secure(null), "a null delegate must be rejected"); + } + + @Test + void setPropertyCallerFloorTakesControl() { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + final XMLResolver caller = (publicID, systemID, baseURI, namespace) -> "resolved"; + final FallbackIgnoreXMLResolver ownFloor = new FallbackIgnoreXMLResolver(caller); + secure.setProperty(XMLInputFactory.RESOLVER, ownFloor); + assertSame(ownFloor, fake.resolverHook, "the caller's own floor must be handed to the delegate as-is"); + assertSame(caller, secure.getXMLResolver(), "getXMLResolver must report the delegate of the caller's floor"); + assertSame(caller, secure.getProperty(XMLInputFactory.RESOLVER), "getProperty must report the delegate of the caller's floor"); + } + + @Test + void setPropertyNullResolverClearsCallerDelegate() { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + final FallbackIgnoreXMLResolver floor = (FallbackIgnoreXMLResolver) fake.resolverHook; + final XMLResolver caller = (publicID, systemID, baseURI, namespace) -> null; + secure.setXMLResolver(caller); + secure.setProperty(XMLInputFactory.RESOLVER, null); + assertNull(floor.getDelegate(), "a null resolver property must clear the floor's delegate"); + assertNull(secure.getXMLResolver(), "getXMLResolver must report no caller resolver"); + } + + @Test + void setPropertyRoutesEveryResolverHookUniformly() { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + final XMLResolver caller = (publicID, systemID, baseURI, namespace) -> "resolved"; + for (final String hook : new String[] { XMLInputFactory.RESOLVER, SecureXMLInputFactory.WSTX_DTD_RESOLVER, SecureXMLInputFactory.WSTX_ENTITY_RESOLVER, + SecureXMLInputFactory.WSTX_UNDECLARED_ENTITY_RESOLVER }) { + fake.setProperty(hook, null); + secure.setProperty(hook, caller); + assertSame(caller, secure.getProperty(hook), "the caller's resolver must be reported unwrapped on " + hook); + } + } + + @Test + void setPropertyWrapsCallerWhenHookIsNotAFloor() { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + final XMLResolver caller = (publicID, systemID, baseURI, namespace) -> "resolved"; + for (final Object foreign : new Object[] { null, (XMLResolver) (publicID, systemID, baseURI, namespace) -> "foreign" }) { + fake.setProperty(XMLInputFactory.RESOLVER, foreign); + secure.setProperty(XMLInputFactory.RESOLVER, caller); + assertTrue(fake.resolverHook instanceof FallbackIgnoreXMLResolver, "a caller resolver must land behind a floor"); + assertSame(caller, ((FallbackIgnoreXMLResolver) fake.resolverHook).getDelegate(), "the floor must delegate to the caller's resolver"); + assertSame(caller, secure.getXMLResolver(), "getXMLResolver must report the caller's resolver unwrapped"); + } + } + + @Test + void setPropertyWrongTypeForResolverHookReachesDelegate() { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + final Object wrongType = "not a resolver"; + secure.setProperty(XMLInputFactory.RESOLVER, wrongType); + assertTrue(fake.calls.contains(RecordingXMLInputFactory.call("setProperty", XMLInputFactory.RESOLVER, wrongType)), + "a wrong-typed value must reach the delegate so it can reject it"); + } + + @Test + void setPropertyWrongTypeForResolverHookSurfacesDelegateException() { + final XMLInputFactory factory = SecureXMLInputFactory.newInstance(); + assertThrows(ClassCastException.class, () -> factory.setProperty(XMLInputFactory.RESOLVER, "not a resolver"), + "the delegate must surface its own rejection of a wrong-typed resolver"); + } + + @Test + void setXMLResolverNullClearsCallerDelegate() { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + final FallbackIgnoreXMLResolver floor = (FallbackIgnoreXMLResolver) fake.resolverHook; + final XMLResolver caller = (publicID, systemID, baseURI, namespace) -> null; + secure.setXMLResolver(caller); + secure.setXMLResolver(null); + assertNull(floor.getDelegate(), "a null caller resolver must clear the floor's delegate"); + assertNull(secure.getXMLResolver(), "getXMLResolver must report no caller resolver"); + } + + @Test + void setXMLResolverRoutesCallerBehindInstalledFloor() { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + final FallbackIgnoreXMLResolver floor = (FallbackIgnoreXMLResolver) fake.resolverHook; + final XMLResolver caller = (publicID, systemID, baseURI, namespace) -> null; + secure.setXMLResolver(caller); + assertSame(caller, floor.getDelegate(), "the caller's resolver must become the delegate of the installed floor"); + assertSame(caller, secure.getXMLResolver(), "getXMLResolver must report the caller's resolver unwrapped"); + assertSame(caller, secure.getProperty(XMLInputFactory.RESOLVER), "getProperty must report the caller's resolver unwrapped"); + assertFalse(fake.calls.stream().anyMatch(c -> c.startsWith("setProperty(" + XMLInputFactory.RESOLVER)), + "a caller resolver must not replace the floor on the delegate's hook"); + } + + @Test + void unsupportedResolverHookSurfacesDelegateError() { + final XMLInputFactory factory = SecureXMLInputFactory.newInstance(); + Assumptions.assumeFalse(factory.isPropertySupported(SecureXMLInputFactory.WSTX_DTD_RESOLVER), "requires an implementation without the Woodstox hooks"); + final XMLResolver caller = (publicID, systemID, baseURI, namespace) -> null; + assertThrows(IllegalArgumentException.class, () -> factory.setProperty(SecureXMLInputFactory.WSTX_DTD_RESOLVER, caller), + "the delegate must surface its own rejection of an unknown resolver hook"); + assertThrows(IllegalArgumentException.class, () -> factory.getProperty(SecureXMLInputFactory.WSTX_DTD_RESOLVER), + "the delegate must surface its own rejection of an unknown resolver hook"); + } + + @Test + void woodstoxDtdHookRoutesBehindInstalledFloor() { + final XMLInputFactory factory = SecureXMLInputFactory.newInstance(); + Assumptions.assumeTrue(factory.isPropertySupported(SecureXMLInputFactory.WSTX_DTD_RESOLVER), "requires the Woodstox DTD resolver hook"); + final XMLResolver first = (publicID, systemID, baseURI, namespace) -> null; + factory.setProperty(SecureXMLInputFactory.WSTX_DTD_RESOLVER, first); + assertSame(first, factory.getProperty(SecureXMLInputFactory.WSTX_DTD_RESOLVER), "the Woodstox hook must report the caller's resolver unwrapped"); + final XMLResolver second = (publicID, systemID, baseURI, namespace) -> "resolved"; + factory.setProperty(SecureXMLInputFactory.WSTX_DTD_RESOLVER, second); + assertSame(second, factory.getProperty(SecureXMLInputFactory.WSTX_DTD_RESOLVER), "a second caller resolver must replace the first behind the floor"); + } + + @Test + void woodstoxUndeclaredEntityHookWrapsCallerResolver() { + final XMLInputFactory factory = SecureXMLInputFactory.newInstance(); + Assumptions.assumeTrue(factory.isPropertySupported(SecureXMLInputFactory.WSTX_UNDECLARED_ENTITY_RESOLVER), + "requires the Woodstox undeclared-entity resolver hook"); + final XMLResolver caller = (publicID, systemID, baseURI, namespace) -> null; + factory.setProperty(SecureXMLInputFactory.WSTX_UNDECLARED_ENTITY_RESOLVER, caller); + assertSame(caller, factory.getProperty(SecureXMLInputFactory.WSTX_UNDECLARED_ENTITY_RESOLVER), + "the Woodstox hook must report the caller's resolver unwrapped"); + } + + @Test + void wrapperDelegatesAllocatorAndReporter() { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + final XMLEventAllocator allocator = RecordingXMLInputFactory.ALLOCATOR_SENTINEL; + secure.setEventAllocator(allocator); + assertSame(allocator, secure.getEventAllocator(), "the allocator must round-trip through the delegate"); + assertTrue(fake.calls.contains(RecordingXMLInputFactory.call("setEventAllocator", allocator)), "the exact allocator must be forwarded"); + final XMLReporter reporter = RecordingXMLInputFactory.REPORTER_SENTINEL; + secure.setXMLReporter(reporter); + assertSame(reporter, secure.getXMLReporter(), "the reporter must round-trip through the delegate"); + assertTrue(fake.calls.contains(RecordingXMLInputFactory.call("setXMLReporter", reporter)), "the exact reporter must be forwarded"); + } + + @Test + void wrapperDelegatesIsPropertySupported() { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + assertTrue(secure.isPropertySupported(XMLInputFactory.SUPPORT_DTD), "the delegate's answer must be reported"); + fake.supported = false; + assertFalse(secure.isPropertySupported(XMLInputFactory.SUPPORT_DTD), "the delegate's answer must be reported"); + assertTrue(fake.calls.contains(RecordingXMLInputFactory.call("isPropertySupported", XMLInputFactory.SUPPORT_DTD)), + "the exact property name must be forwarded"); + } + + @Test + void wrapperDelegatesNonResolverProperties() { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + secure.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.TRUE); + assertTrue(fake.calls.contains(RecordingXMLInputFactory.call("setProperty", XMLInputFactory.SUPPORT_DTD, Boolean.TRUE)), + "a non-resolver property must reach the delegate unmodified"); + fake.resolverHook = Boolean.TRUE; + assertEquals(Boolean.TRUE, secure.getProperty(XMLInputFactory.SUPPORT_DTD), "a non-resolver property must be reported unmodified"); + } + + @Test + void wrapperDelegatesReaderCreationToDelegate() throws Exception { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + final XMLStreamReader streamSentinel = RecordingXMLInputFactory.STREAM_SENTINEL; + final XMLEventReader eventSentinel = RecordingXMLInputFactory.EVENT_SENTINEL; + final EventFilter eventFilter = RecordingXMLInputFactory.EVENT_FILTER_SENTINEL; + final StreamFilter streamFilter = RecordingXMLInputFactory.STREAM_FILTER_SENTINEL; + final InputStream stream = new ByteArrayInputStream(BENIGN_XML.getBytes(StandardCharsets.UTF_8)); + final StringReader reader = new StringReader(BENIGN_XML); + final Source source = new StreamSource(new StringReader(BENIGN_XML)); + assertSame(streamSentinel, secure.createXMLStreamReader(stream)); + assertSame(streamSentinel, secure.createXMLStreamReader(stream, "UTF-8")); + assertSame(streamSentinel, secure.createXMLStreamReader(reader)); + assertSame(streamSentinel, secure.createXMLStreamReader(source)); + assertSame(streamSentinel, secure.createXMLStreamReader(SYSTEM_ID, stream)); + assertSame(streamSentinel, secure.createXMLStreamReader(SYSTEM_ID, reader)); + assertSame(eventSentinel, secure.createXMLEventReader(stream)); + assertSame(eventSentinel, secure.createXMLEventReader(stream, "UTF-8")); + assertSame(eventSentinel, secure.createXMLEventReader(reader)); + assertSame(eventSentinel, secure.createXMLEventReader(source)); + assertSame(eventSentinel, secure.createXMLEventReader(SYSTEM_ID, stream)); + assertSame(eventSentinel, secure.createXMLEventReader(SYSTEM_ID, reader)); + assertSame(eventSentinel, secure.createXMLEventReader(streamSentinel)); + assertSame(eventSentinel, secure.createFilteredReader(eventSentinel, eventFilter)); + assertSame(streamSentinel, secure.createFilteredReader(streamSentinel, streamFilter)); + assertTrue(fake.calls.contains(RecordingXMLInputFactory.call("createXMLStreamReader", stream)), "the exact stream must be forwarded"); + assertTrue(fake.calls.contains(RecordingXMLInputFactory.call("createXMLStreamReader", reader)), "the exact reader must be forwarded"); + assertTrue(fake.calls.contains(RecordingXMLInputFactory.call("createXMLStreamReader", source)), "the exact source must be forwarded"); + assertTrue(fake.calls.contains(RecordingXMLInputFactory.call("createXMLStreamReader", SYSTEM_ID, stream)), "the exact system id must be forwarded"); + assertTrue(fake.calls.contains(RecordingXMLInputFactory.call("createXMLEventReader", stream, "UTF-8")), "the exact encoding must be forwarded"); + assertTrue(fake.calls.contains(RecordingXMLInputFactory.call("createFilteredReader", eventSentinel, eventFilter)), + "the exact event filter must be forwarded"); + assertTrue(fake.calls.contains(RecordingXMLInputFactory.call("createFilteredReader", streamSentinel, streamFilter)), + "the exact stream filter must be forwarded"); + } + + @Test + void wrapperInstallsFloorOnDelegateHook() { + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + assertNotNull(secure); + assertTrue(fake.calls.contains(RecordingXMLInputFactory.call("setXMLResolver", fake.resolverHook)), + "the floor must be installed through the delegate's setXMLResolver"); + assertTrue(fake.resolverHook instanceof FallbackIgnoreXMLResolver, "the constructor must install the ignore-all floor on the delegate's resolver hook"); + assertNull(((FallbackIgnoreXMLResolver) fake.resolverHook).getDelegate(), "the installed floor must have no caller delegate"); + } +} diff --git a/src/test/java/org/apache/commons/xml/XMLFilterTest.java b/src/test/java/org/apache/commons/xml/XMLFilterTest.java index 7230e57..cee17de 100644 --- a/src/test/java/org/apache/commons/xml/XMLFilterTest.java +++ b/src/test/java/org/apache/commons/xml/XMLFilterTest.java @@ -184,14 +184,6 @@ public Object getProperty(final String name) { return null; } - @Override - public void setFeature(final String name, final boolean value) { - } - - @Override - public void setProperty(final String name, final Object value) { - } - @Override public void parse(final InputSource input) throws SAXException { // Minimal well-formed document for the transformation to consume; no real parser behind this parent. @@ -219,6 +211,14 @@ public void setErrorHandler(final ErrorHandler handler) { wired.add(handler); super.setErrorHandler(handler); } + + @Override + public void setFeature(final String name, final boolean value) { + } + + @Override + public void setProperty(final String name, final Object value) { + } }; filter.setParent(parent); assertEquals("", filterAndCapture(filter, "<ignored/>"));
