adoroszlai commented on code in PR #9951:
URL: https://github.com/apache/ozone/pull/9951#discussion_r2964315437
##########
hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigFileAppender.java:
##########
@@ -48,7 +49,7 @@ public class ConfigFileAppender {
public ConfigFileAppender() {
try {
- DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+ DocumentBuilderFactory factory =
XMLUtils.newSecureDocumentBuilderFactory();
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
Review Comment:
`FEATURE_SECURE_PROCESSING` can be removed.
```suggestion
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/MessageUnmarshaller.java:
##########
@@ -51,7 +52,7 @@ public MessageUnmarshaller(Class<T> cls) {
try {
context = JAXBContext.newInstance(cls);
- saxParserFactory = SAXParserFactory.newInstance();
+ saxParserFactory = XMLUtils.newSecureSAXParserFactory();
saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING,
true);
Review Comment:
```suggestion
```
##########
hadoop-hdds/config/pom.xml:
##########
@@ -26,6 +26,10 @@
<description>Apache Ozone Distributed Data Store Config Tools</description>
<dependencies>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-common</artifactId>
Review Comment:
> I’m concerned about adding `hadoop-common` as a dependency to
`hadoop-hdds/config`. By pulling in `hadoop-common` just to use `XMLUtils` in
`ConfigFileAppender.java`
>
> Can we look into an alternative?
Thanks @sarvekshayr for raising the concern. Since `XMLUtils` is annotated
as `@InterfaceAudience.Private`, we may be better off by copying it to Ozone.
We can do that as a follow-up, and add exclusions for transitive dependencies
now.
```suggestion
<artifactId>hadoop-common</artifactId>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
```
##########
hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigFileAppender.java:
##########
@@ -113,7 +114,7 @@ private void addXmlElement(Element parentElement, String
tagValue,
*/
public void write(Writer writer) {
try {
- TransformerFactory factory = TransformerFactory.newInstance();
+ TransformerFactory factory = XMLUtils.newSecureTransformerFactory();
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
Review Comment:
`FEATURE_SECURE_PROCESSING` can be removed.
```suggestion
```
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/net/NodeSchemaLoader.java:
##########
@@ -174,7 +175,7 @@ private NodeSchemaLoadResult loadSchema(InputStream
inputStream) throws
ParserConfigurationException, SAXException, IOException {
LOG.info("Loading network topology layer schema file");
// Read and parse the schema file.
- DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
+ DocumentBuilderFactory dbf = XMLUtils.newSecureDocumentBuilderFactory();
dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
Review Comment:
```suggestion
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
