Skip to content

Commit b67c950

Browse files
pjfanningsteveloughran
authored andcommitted
HADOOP-18575. Make XML transformer factory more lenient (#5224)
Due diligence followup to HADOOP-18469. Add secure XML parser factories to XMLUtils (#4940) Contributed by P J Fanning
1 parent 2d26f31 commit b67c950

File tree

2 files changed

+42
-4
lines changed

2 files changed

+42
-4
lines changed

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/XMLUtils.java

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import org.apache.hadoop.classification.InterfaceAudience;
3030
import org.apache.hadoop.classification.InterfaceStability;
3131

32+
import org.slf4j.Logger;
33+
import org.slf4j.LoggerFactory;
3234
import org.xml.sax.SAXException;
3335

3436
import java.io.*;
@@ -41,6 +43,9 @@
4143
@InterfaceStability.Unstable
4244
public class XMLUtils {
4345

46+
private static final Logger LOG =
47+
LoggerFactory.getLogger(XMLUtils.class);
48+
4449
public static final String DISALLOW_DOCTYPE_DECL =
4550
"http://apache.org/xml/features/disallow-doctype-decl";
4651
public static final String LOAD_EXTERNAL_DECL =
@@ -138,8 +143,8 @@ public static TransformerFactory newSecureTransformerFactory()
138143
throws TransformerConfigurationException {
139144
TransformerFactory trfactory = TransformerFactory.newInstance();
140145
trfactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
141-
trfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
142-
trfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
146+
bestEffortSetAttribute(trfactory, XMLConstants.ACCESS_EXTERNAL_DTD, "");
147+
bestEffortSetAttribute(trfactory, XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
143148
return trfactory;
144149
}
145150

@@ -156,8 +161,29 @@ public static SAXTransformerFactory newSecureSAXTransformerFactory()
156161
throws TransformerConfigurationException {
157162
SAXTransformerFactory trfactory = (SAXTransformerFactory) SAXTransformerFactory.newInstance();
158163
trfactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
159-
trfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
160-
trfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
164+
bestEffortSetAttribute(trfactory, XMLConstants.ACCESS_EXTERNAL_DTD, "");
165+
bestEffortSetAttribute(trfactory, XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
161166
return trfactory;
162167
}
168+
169+
/**
170+
* Set an attribute value on a {@link TransformerFactory}. If the TransformerFactory
171+
* does not support the attribute, the method just returns <code>false</code> and
172+
* logs the issue at debug level.
173+
*
174+
* @param transformerFactory to update
175+
* @param name of the attribute to set
176+
* @param value to set on the attribute
177+
* @return whether the attribute was successfully set
178+
*/
179+
static boolean bestEffortSetAttribute(TransformerFactory transformerFactory,
180+
String name, Object value) {
181+
try {
182+
transformerFactory.setAttribute(name, value);
183+
return true;
184+
} catch (Throwable t) {
185+
LOG.debug("Issue setting TransformerFactory attribute {}: {}", name, t.toString());
186+
}
187+
return false;
188+
}
163189
}

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestXMLUtils.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,20 @@
2020
import java.io.InputStream;
2121
import java.io.StringReader;
2222
import java.io.StringWriter;
23+
import javax.xml.XMLConstants;
2324
import javax.xml.parsers.DocumentBuilder;
2425
import javax.xml.parsers.SAXParser;
2526
import javax.xml.transform.Transformer;
2627
import javax.xml.transform.TransformerException;
28+
import javax.xml.transform.TransformerFactory;
2729
import javax.xml.transform.dom.DOMSource;
2830
import javax.xml.transform.stream.StreamResult;
2931
import javax.xml.transform.stream.StreamSource;
3032

3133
import org.apache.hadoop.test.AbstractHadoopTestBase;
3234

3335
import org.assertj.core.api.Assertions;
36+
import org.junit.Assert;
3437
import org.junit.Test;
3538
import org.w3c.dom.Document;
3639
import org.xml.sax.InputSource;
@@ -128,6 +131,15 @@ public void testExternalDtdWithSecureSAXTransformerFactory() throws Exception {
128131
}
129132
}
130133

134+
@Test
135+
public void testBestEffortSetAttribute() throws Exception {
136+
TransformerFactory factory = TransformerFactory.newInstance();
137+
Assert.assertFalse("unexpected attribute results in return of false",
138+
XMLUtils.bestEffortSetAttribute(factory, "unsupportedAttribute false", "abc"));
139+
Assert.assertTrue("expected attribute results in return of false",
140+
XMLUtils.bestEffortSetAttribute(factory, XMLConstants.ACCESS_EXTERNAL_DTD, ""));
141+
}
142+
131143
private static InputStream getResourceStream(final String filename) {
132144
return TestXMLUtils.class.getResourceAsStream(filename);
133145
}

0 commit comments

Comments
 (0)