Skip to content

HADOOP-18469: centralise XML parser creation in XMLUtils #4940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Oct 6, 2022
Merged

HADOOP-18469: centralise XML parser creation in XMLUtils #4940

merged 6 commits into from
Oct 6, 2022

Conversation

pjfanning
Copy link
Contributor

@pjfanning pjfanning commented Sep 27, 2022 •
edited
Loading

Description of PR

How was this patch tested?

For code changes:

  • Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
  • Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

steveloughran
steveloughran reviewed Sep 27, 2022
View reviewed changes
Copy link
Contributor

@steveloughran steveloughran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i like this

  1. the hdfs/yarn/mr changes need their own isolated changes and JIRAs for their branches
  2. we should add an enforcer rule to block all new use of DocumentBuilderFactory.newInstance();

@pjfanning
Copy link
Contributor Author

@steveloughran I can rework this to just have the hadoop-common changes and can do other PRs to uptake this in yarn, hdfs, etc.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 39s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 1s No case conflicting files found.
+0 🆗 codespell 0m 1s codespell was not available.
+0 🆗 detsecrets 0m 1s detect-secrets was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 33 new or modified test files.
_ trunk Compile Tests _
+0 🆗 mvndep 15m 45s Maven dependency ordering for branch
+1 💚 mvninstall 25m 46s trunk passed
+1 💚 compile 23m 26s trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 compile 20m 51s trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 checkstyle 4m 3s trunk passed
+1 💚 mvnsite 13m 26s trunk passed
+1 💚 javadoc 10m 58s trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 10m 20s trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 19m 58s trunk passed
+1 💚 shadedclient 21m 26s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 30s Maven dependency ordering for patch
+1 💚 mvninstall 7m 27s the patch passed
+1 💚 compile 23m 59s the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javac 23m 59s the patch passed
+1 💚 compile 21m 40s the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 javac 21m 40s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
-0 ⚠️ checkstyle 5m 38s /results-checkstyle-root.txt root: The patch generated 9 new + 328 unchanged - 1 fixed = 337 total (was 329)
+1 💚 mvnsite 12m 55s the patch passed
+1 💚 javadoc 9m 29s the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 10m 19s the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 21m 32s the patch passed
+1 💚 shadedclient 24m 3s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 19m 41s hadoop-common in the patch passed.
+1 💚 unit 3m 4s hadoop-hdfs-client in the patch passed.
-1 ❌ unit 251m 55s /patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt hadoop-hdfs in the patch passed.
+1 💚 unit 99m 14s hadoop-yarn-server-resourcemanager in the patch passed.
+1 💚 unit 24m 27s hadoop-yarn-server-nodemanager in the patch passed.
+1 💚 unit 7m 40s hadoop-mapreduce-client-core in the patch passed.
+1 💚 unit 9m 26s hadoop-mapreduce-client-app in the patch passed.
+1 💚 unit 5m 12s hadoop-mapreduce-client-hs in the patch passed.
+1 💚 unit 1m 39s hadoop-rumen in the patch passed.
+1 💚 asflicense 1m 36s The patch does not generate ASF License warnings.
734m 23s
Reason Tests
Failed junit tests hadoop.hdfs.TestRollingUpgrade
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/5/artifact/out/Dockerfile
GITHUB PR #4940
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname Linux ef0fe259e6fa 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / 2cd117a
Default Java Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/5/testReport/
Max. process+thread count 3159 (vs. ulimit of 5500)
modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs hadoop-tools/hadoop-rumen U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/5/console
versions git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 50s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 1s No case conflicting files found.
+0 🆗 codespell 0m 2s codespell was not available.
+0 🆗 detsecrets 0m 2s detect-secrets was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 33 new or modified test files.
_ trunk Compile Tests _
+0 🆗 mvndep 15m 56s Maven dependency ordering for branch
+1 💚 mvninstall 25m 57s trunk passed
+1 💚 compile 24m 28s trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 compile 25m 17s trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 checkstyle 5m 31s trunk passed
+1 💚 mvnsite 15m 35s trunk passed
+1 💚 javadoc 12m 48s trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 10m 56s trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
-1 ❌ spotbugs 2m 7s /branch-spotbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt hadoop-yarn-server-nodemanager in trunk failed.
-1 ❌ spotbugs 1m 6s /branch-spotbugs-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt hadoop-mapreduce-client-core in trunk failed.
-1 ❌ spotbugs 1m 6s /branch-spotbugs-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-app.txt hadoop-mapreduce-client-app in trunk failed.
-1 ❌ spotbugs 1m 8s /branch-spotbugs-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-hs.txt hadoop-mapreduce-client-hs in trunk failed.
+1 💚 shadedclient 27m 26s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 30s Maven dependency ordering for patch
+1 💚 mvninstall 7m 41s the patch passed
+1 💚 compile 22m 39s the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javac 22m 39s the patch passed
+1 💚 compile 21m 8s the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 javac 21m 9s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
-0 ⚠️ checkstyle 4m 34s /results-checkstyle-root.txt root: The patch generated 27 new + 436 unchanged - 1 fixed = 463 total (was 437)
+1 💚 mvnsite 13m 22s the patch passed
+1 💚 javadoc 11m 11s the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 10m 55s the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 21m 55s the patch passed
+1 💚 shadedclient 21m 49s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 18m 45s hadoop-common in the patch passed.
+1 💚 unit 3m 5s hadoop-hdfs-client in the patch passed.
+1 💚 unit 240m 7s hadoop-hdfs in the patch passed.
+1 💚 unit 99m 25s hadoop-yarn-server-resourcemanager in the patch passed.
+1 💚 unit 24m 43s hadoop-yarn-server-nodemanager in the patch passed.
+1 💚 unit 7m 44s hadoop-mapreduce-client-core in the patch passed.
+1 💚 unit 9m 26s hadoop-mapreduce-client-app in the patch passed.
+1 💚 unit 5m 12s hadoop-mapreduce-client-hs in the patch passed.
+1 💚 unit 1m 28s hadoop-rumen in the patch passed.
+1 💚 asflicense 1m 43s The patch does not generate ASF License warnings.
739m 54s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/6/artifact/out/Dockerfile
GITHUB PR #4940
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname Linux 5cac9e38c6e3 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / d8dd8eb
Default Java Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/6/testReport/
Max. process+thread count 3127 (vs. ulimit of 5500)
modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs hadoop-tools/hadoop-rumen U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/6/console
versions git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 49s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 1s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell was not available.
+0 🆗 detsecrets 0m 1s detect-secrets was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 33 new or modified test files.
_ trunk Compile Tests _
+0 🆗 mvndep 15m 50s Maven dependency ordering for branch
+1 💚 mvninstall 25m 55s trunk passed
+1 💚 compile 23m 20s trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 compile 20m 59s trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 checkstyle 4m 20s trunk passed
+1 💚 mvnsite 13m 19s trunk passed
+1 💚 javadoc 11m 7s trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 10m 34s trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 19m 53s trunk passed
+1 💚 shadedclient 21m 31s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 28s Maven dependency ordering for patch
+1 💚 mvninstall 7m 44s the patch passed
+1 💚 compile 22m 43s the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javac 22m 43s the patch passed
+1 💚 compile 20m 45s the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 javac 20m 45s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 4m 0s root: The patch generated 0 new + 436 unchanged - 1 fixed = 436 total (was 437)
+1 💚 mvnsite 13m 12s the patch passed
+1 💚 javadoc 10m 44s the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 10m 45s the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 22m 9s the patch passed
+1 💚 shadedclient 21m 38s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 18m 37s hadoop-common in the patch passed.
+1 💚 unit 3m 7s hadoop-hdfs-client in the patch passed.
-1 ❌ unit 309m 41s /patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt hadoop-hdfs in the patch passed.
+1 💚 unit 99m 21s hadoop-yarn-server-resourcemanager in the patch passed.
+1 💚 unit 24m 34s hadoop-yarn-server-nodemanager in the patch passed.
+1 💚 unit 7m 39s hadoop-mapreduce-client-core in the patch passed.
+1 💚 unit 9m 22s hadoop-mapreduce-client-app in the patch passed.
+1 💚 unit 5m 33s hadoop-mapreduce-client-hs in the patch passed.
+1 💚 unit 1m 23s hadoop-rumen in the patch passed.
+1 💚 asflicense 1m 37s The patch does not generate ASF License warnings.
789m 6s
Reason Tests
Failed junit tests hadoop.hdfs.server.datanode.TestBatchIbr
hadoop.hdfs.server.blockmanagement.TestBlockTokenWithDFSStriped
hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting
hadoop.hdfs.server.datanode.TestDataNodeErasureCodingMetrics
hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
hadoop.hdfs.server.datanode.TestDataNodeVolumeMetrics
hadoop.hdfs.server.balancer.TestBalancerRPCDelay
hadoop.hdfs.server.blockmanagement.TestReconstructStripedBlocksWithRackAwareness
hadoop.hdfs.server.datanode.TestDiskError
hadoop.hdfs.server.blockmanagement.TestBlocksWithNotEnoughRacks
hadoop.hdfs.server.mover.TestMover
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/7/artifact/out/Dockerfile
GITHUB PR #4940
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname Linux aff3310d094d 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / e62c468
Default Java Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/7/testReport/
Max. process+thread count 3142 (vs. ulimit of 5500)
modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs hadoop-tools/hadoop-rumen U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/7/console
versions git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@apache apache deleted a comment from hadoop-yetus Sep 30, 2022
@apache apache deleted a comment from hadoop-yetus Sep 30, 2022
@apache apache deleted a comment from hadoop-yetus Sep 30, 2022
@apache apache deleted a comment from hadoop-yetus Sep 30, 2022
steveloughran
steveloughran requested changes Sep 30, 2022
View reviewed changes
Copy link
Contributor

@steveloughran steveloughran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

core code looks good, some minor details related to javadocs, constants etc.

tests: see comments.

your IDE's import settings don't match what we like. i've tried to explain what's expected on new files. But: leave old files alone; it makes backporting a nightmare otherwise

@pjfanning
HADOOP-18469: centralise XML parser creation in XMLUtils
b05445f 
undo hdfs and yarn changes

remove mapreduce changes

add tests

secure transformer factory

indent issue

some review items
@pjfanning pjfanning force-pushed the HADOOP-18469-xml-utils branch from e62c468 to b05445f Compare September 30, 2022 19:14
@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 39s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 1s codespell was not available.
+0 🆗 detsecrets 0m 1s detect-secrets was not available.
+0 🆗 xmllint 0m 1s xmllint was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 4 new or modified test files.
_ trunk Compile Tests _
+0 🆗 mvndep 15m 50s Maven dependency ordering for branch
+1 💚 mvninstall 26m 6s trunk passed
+1 💚 compile 23m 20s trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 compile 21m 5s trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 checkstyle 4m 8s trunk passed
+1 💚 mvnsite 2m 55s trunk passed
+1 💚 javadoc 2m 39s trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 2m 12s trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 4m 37s trunk passed
+1 💚 shadedclient 21m 53s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 33s Maven dependency ordering for patch
+1 💚 mvninstall 1m 33s the patch passed
+1 💚 compile 22m 48s the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javac 22m 48s the patch passed
+1 💚 compile 20m 46s the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 javac 20m 46s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
-0 ⚠️ checkstyle 4m 5s /results-checkstyle-root.txt root: The patch generated 11 new + 182 unchanged - 1 fixed = 193 total (was 183)
+1 💚 mvnsite 2m 55s the patch passed
+1 💚 javadoc 2m 24s the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 1m 59s the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 4m 16s the patch passed
+1 💚 shadedclient 21m 24s patch has no errors when building and testing our client artifacts.
_ Other Tests _
-1 ❌ unit 18m 44s /patch-unit-hadoop-common-project_hadoop-common.txt hadoop-common in the patch passed.
+1 💚 unit 1m 5s hadoop-rumen in the patch passed.
-1 ❌ asflicense 1m 25s /results-asflicense.txt The patch generated 1 ASF License warnings.
234m 30s
Reason Tests
Failed junit tests hadoop.util.TestXMLUtils
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/8/artifact/out/Dockerfile
GITHUB PR #4940
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets xmllint
uname Linux 45925de3a47c 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / b05445f
Default Java Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/8/testReport/
Max. process+thread count 1740 (vs. ulimit of 5500)
modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-rumen U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/8/console
versions git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@hadoop-yetus
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 43s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 1s codespell was not available.
+0 🆗 detsecrets 0m 1s detect-secrets was not available.
+0 🆗 xmllint 0m 1s xmllint was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 5 new or modified test files.
_ trunk Compile Tests _
+0 🆗 mvndep 15m 46s Maven dependency ordering for branch
+1 💚 mvninstall 26m 19s trunk passed
+1 💚 compile 23m 27s trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 compile 20m 45s trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 checkstyle 4m 12s trunk passed
+1 💚 mvnsite 3m 3s trunk passed
+1 💚 javadoc 2m 36s trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 2m 20s trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 4m 30s trunk passed
+1 💚 shadedclient 21m 20s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 31s Maven dependency ordering for patch
+1 💚 mvninstall 1m 34s the patch passed
+1 💚 compile 22m 35s the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javac 22m 35s the patch passed
+1 💚 compile 20m 45s the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 javac 20m 45s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 4m 6s root: The patch generated 0 new + 182 unchanged - 1 fixed = 182 total (was 183)
+1 💚 mvnsite 2m 55s the patch passed
+1 💚 javadoc 2m 16s the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 2m 23s the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 4m 21s the patch passed
+1 💚 shadedclient 21m 21s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 18m 48s hadoop-common in the patch passed.
+1 💚 unit 1m 3s hadoop-rumen in the patch passed.
+1 💚 asflicense 1m 20s The patch does not generate ASF License warnings.
234m 26s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/12/artifact/out/Dockerfile
GITHUB PR #4940
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets xmllint
uname Linux 29cf89afc73a 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / 93a7556
Default Java Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/12/testReport/
Max. process+thread count 3159 (vs. ulimit of 5500)
modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-rumen U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/12/console
versions git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@steveloughran
Copy link
Contributor

should those tests be removed, or simply modified to expect the different failure?

what does happen now when something invalid is tried? an exception raised or simply the result ignored.

(FWIW, this is why I prefer LambdaTestUtils.intercept(); that will print the toString value of the callable invoked if the exception isn't raised, so you can design tests to be meaningful on failures.)

@pjfanning
Copy link
Contributor Author

@steveloughran the transformer tests that I added and then removed - they do not fail. The transformer in the PR as is does not seem to stop DTD entities altogether (it does for DOM and SAX parsing). The transformers are not as widely used in the hadoop code base as DOM parsing and often the input is already parsed (via DOM or SAX parsing) before it is transformed.

@apache apache deleted a comment from hadoop-yetus Oct 6, 2022
@apache apache deleted a comment from hadoop-yetus Oct 6, 2022
@apache apache deleted a comment from hadoop-yetus Oct 6, 2022
steveloughran
steveloughran approved these changes Oct 6, 2022
View reviewed changes
Copy link
Contributor

@steveloughran steveloughran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, happy with the explanation.

I'm +1 for the change. there is one suggestion, use AbstractHadoopTestBase, but its not a blocker for this patch. if you don't want to do that, say so and i will merge as is.

@hadoop-yetus
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 49s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 1s codespell was not available.
+0 🆗 detsecrets 0m 1s detect-secrets was not available.
+0 🆗 xmllint 0m 1s xmllint was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 5 new or modified test files.
_ trunk Compile Tests _
+0 🆗 mvndep 15m 25s Maven dependency ordering for branch
+1 💚 mvninstall 25m 50s trunk passed
+1 💚 compile 23m 16s trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 compile 20m 42s trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 checkstyle 4m 7s trunk passed
+1 💚 mvnsite 3m 5s trunk passed
+1 💚 javadoc 2m 26s trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 2m 9s trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 4m 16s trunk passed
+1 💚 shadedclient 21m 17s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 33s Maven dependency ordering for patch
+1 💚 mvninstall 1m 35s the patch passed
+1 💚 compile 22m 46s the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javac 22m 46s the patch passed
+1 💚 compile 20m 44s the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 javac 20m 44s the patch passed
+1 💚 blanks 0m 1s The patch has no blanks issues.
+1 💚 checkstyle 4m 57s root: The patch generated 0 new + 182 unchanged - 1 fixed = 182 total (was 183)
+1 💚 mvnsite 3m 13s the patch passed
+1 💚 javadoc 2m 22s the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 2m 6s the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 4m 36s the patch passed
+1 💚 shadedclient 22m 28s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 18m 36s hadoop-common in the patch passed.
+1 💚 unit 1m 12s hadoop-rumen in the patch passed.
+1 💚 asflicense 1m 22s The patch does not generate ASF License warnings.
237m 47s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/13/artifact/out/Dockerfile
GITHUB PR #4940
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets xmllint
uname Linux 40d53db007e2 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / e44488d
Default Java Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/13/testReport/
Max. process+thread count 1301 (vs. ulimit of 5500)
modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-rumen U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4940/13/console
versions git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@pjfanning
Copy link
Contributor Author

thanks, happy with the explanation.

I'm +1 for the change. there is one suggestion, use AbstractHadoopTestBase, but its not a blocker for this patch. if you don't want to do that, say so and i will merge as is.

@steveloughran I fixed the formatting issue and the build passed

@steveloughran steveloughran merged commit 8336b91 into apache:trunk Oct 6, 2022
@steveloughran
Copy link
Contributor

merged to trunk; will see the build in 3.3. changed the title of the commit as (a) the centralisation hasn't taken place (b) didn't want to have to choose between us and non us spellings of centralise

steveloughran pushed a commit to steveloughran/hadoop that referenced this pull request Oct 6, 2022
@pjfanning @steveloughran
HADOOP-18469. Add secure XML parser factories to XMLUtils (apache#4940)
df47f3e 
Add to XMLUtils a set of methods to create secure XML Parsers/transformers,
locking down DTD, schema, XXE exposure.

Use these wherever XML parsers are created.

Contributed by PJ Fanning

Change-Id: I15057a369c2f44b0a03e8660f934f38e34d2979e
@steveloughran steveloughran mentioned this pull request Oct 6, 2022
Merged
4 tasks
@pjfanning pjfanning deleted the HADOOP-18469-xml-utils branch October 6, 2022 20:10
steveloughran added a commit that referenced this pull request Oct 7, 2022
@steveloughran
HADOOP-18469. Add secure XML parser factories to XMLUtils (#4940)
8078130 
Add to XMLUtils a set of methods to create secure XML Parsers/transformers,
locking down DTD, schema, XXE exposure.

Use these wherever XML parsers are created.

Contributed by PJ Fanning
asfgit pushed a commit that referenced this pull request Oct 7, 2022
@steveloughran
HADOOP-18469. Add secure XML parser factories to XMLUtils (#4940)
8fb3a2b 
Add to XMLUtils a set of methods to create secure XML Parsers/transformers,
locking down DTD, schema, XXE exposure.

Use these wherever XML parsers are created.

Contributed by PJ Fanning
HarshitGupta11 pushed a commit to HarshitGupta11/hadoop that referenced this pull request Nov 28, 2022
@pjfanning
HADOOP-18469. Add secure XML parser factories to XMLUtils (apache#4940)
2d8c158 
Add to XMLUtils a set of methods to create secure XML Parsers/transformers, locking down DTD, schema, XXE exposure.

Use these wherever XML parsers are created.

Contributed by PJ Fanning
steveloughran pushed a commit that referenced this pull request Dec 18, 2022
@pjfanning
HADOOP-18575. Make XML transformer factory more lenient (#5224)
6a07b5d 
Due diligence followup to
HADOOP-18469. Add secure XML parser factories to XMLUtils (#4940)

Contributed by P J Fanning
asfgit pushed a commit that referenced this pull request Dec 18, 2022
@pjfanning @steveloughran
HADOOP-18575. Make XML transformer factory more lenient (#5224)
29b6df5 
Due diligence followup to
HADOOP-18469. Add secure XML parser factories to XMLUtils (#4940)

Contributed by P J Fanning
asfgit pushed a commit that referenced this pull request Dec 18, 2022
@pjfanning @steveloughran
HADOOP-18575. Make XML transformer factory more lenient (#5224)
b67c950 
Due diligence followup to
HADOOP-18469. Add secure XML parser factories to XMLUtils (#4940)

Contributed by P J Fanning
slfan1989 pushed a commit to slfan1989/hadoop that referenced this pull request Dec 20, 2022
@pjfanning
HADOOP-18575. Make XML transformer factory more lenient (apache#5224)
a023375 
Due diligence followup to
HADOOP-18469. Add secure XML parser factories to XMLUtils (apache#4940)

Contributed by P J Fanning
jojochuang pushed a commit to jojochuang/hadoop that referenced this pull request May 23, 2023
@steveloughran @rohit-kb
CDPD-46151: HADOOP-18469. Add secure XML parser factories to XMLUtils…
3be1486 
… + followups

This change is a squash of below three patches from upstream:

1. HADOOP-18469. Add secure XML parser factories to XMLUtils (apache#4940)

Add to XMLUtils a set of methods to create secure XML Parsers/transformers,
locking down DTD, schema, XXE exposure.

Use these wherever XML parsers are created.

Contributed by PJ Fanning
(cherry-picked from 8336b91)


2. HADOOP-18575. Make XML transformer factory more lenient (apache#5224)

Due diligence followup to
HADOOP-18469. Add secure XML parser factories to XMLUtils (apache#4940)

Contributed by P J Fanning
(cherry-picked from 6a07b5d)


3. HADOOP-18575: followup: try to avoid repeatedly hitting exceptions when transformer factories do not support attributes (apache#5253)

Part of HADOOP-18469 and the hardening of XML/XSL parsers.
Followup to the main HADOOP-18575 patch, to improve performance when
working with xml/xsl engines which don't support the relevant attributes.

Include this change when backporting.

Contributed by PJ Fanning.
(cherry-picked from d81d983)

Change-Id: Ic519987c1f07d286fb3811b961a406bd280f039a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steveloughran steveloughran steveloughran approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pjfanning @hadoop-yetus @steveloughran