Copilot commented on code in PR #18: URL: https://github.com/apache/commons-xml/pull/18#discussion_r3537021148
########## src/main/java/org/apache/commons/xml/DocumentBuilderHardener.java: ########## @@ -24,7 +24,6 @@ import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; import org.xml.sax.EntityResolver; Review Comment: DocumentBuilderHardener has an unused import (org.xml.sax.EntityResolver), which will fail compilation. Since EntityResolver is only referenced from Javadoc here, either remove the import or qualify the Javadoc link. ########## src/main/java/org/apache/commons/xml/HardeningXMLInputFactory.java: ########## @@ -0,0 +1,113 @@ +/* + * 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 javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLResolver; + +/** + * {@link XMLInputFactory} wrapper that keeps the {@link Resolvers.FallbackDenyXMLResolver} floors {@link StaxHardener} installs on the entity-resolution hooks + * non-removable by the caller. + * + * <p>Every resolver-valued entry point ({@link #setXMLResolver(XMLResolver)}, {@code setProperty(XMLInputFactory.RESOLVER, ...)} and the Woodstox + * {@code com.ctc.wstx.*Resolver} keys) is routed uniformly: a caller who supplies their own {@link Resolvers.FallbackDenyXMLResolver} takes control and it is + * passed straight to the delegate; otherwise the current resolver on that hook is read, and if it is one of our floors the caller's resolver is set as its + * {@link Resolvers.FallbackDenyXMLResolver#setDelegate delegate} (an opt-in the floor cannot be removed by), or, if the hook is empty, the caller's resolver is + * wrapped in a fresh floor. This matters because Woodstox does not chain resolvers: when a resolver returns {@code null}, {@code DefaultInputResolver} falls + * through to fetching the systemId URL itself, so a caller-set resolver that returns {@code null} must still land behind the floor. {@link #getXMLResolver()} and + * {@code getProperty} report the caller's resolver unwrapped.</p> + */ +final class HardeningXMLInputFactory extends DelegatingXMLInputFactory { + + HardeningXMLInputFactory(final XMLInputFactory delegate) { + super(delegate); + } + + @Override + public void setXMLResolver(final XMLResolver resolver) { + if (resolver instanceof Resolvers.FallbackDenyXMLResolver) { + // The caller supplies their own floor: hand it to the delegate as-is. + super.setXMLResolver(resolver); + } else { + final XMLResolver current = super.getXMLResolver(); + if (current instanceof Resolvers.FallbackDenyXMLResolver) { + ((Resolvers.FallbackDenyXMLResolver) current).setDelegate(resolver); + } else { + super.setXMLResolver(new Resolvers.FallbackDenyXMLResolver(resolver)); + } + } + } + + @Override + public XMLResolver getXMLResolver() { + return unwrap(super.getXMLResolver()); + } + + @Override + public void setProperty(final String name, final Object value) { + if (XMLInputFactory.RESOLVER.equals(name)) { + setXMLResolver((XMLResolver) value); + } else if (isWstxResolverProperty(name) && (value instanceof XMLResolver || value == null)) { + setResolverProperty(name, (XMLResolver) value); + } else { + super.setProperty(name, value); + } + } Review Comment: HardeningXMLInputFactory#setProperty casts the value for XMLInputFactory.RESOLVER to XMLResolver unconditionally. If a caller passes a non-XMLResolver value, this will throw ClassCastException instead of preserving the underlying factory’s usual IllegalArgumentException/behavior. Please type-check and delegate for non-XMLResolver values (as is already done for the Woodstox resolver properties). ########## src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java: ########## @@ -0,0 +1,43 @@ +/* + * 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 javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; + +import org.xml.sax.EntityResolver; + +/** + * {@link DocumentBuilderFactory} wrapper that keeps a deny-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 + * 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> + */ Review Comment: HardeningDocumentBuilderFactory has an unused import (org.xml.sax.EntityResolver), which will fail compilation. Since it’s only referenced in Javadoc, remove the import and qualify the Javadoc link instead. -- 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]
