Skip to content

HADOOP-19930. ObjectWritable to validate class on load - #8640

Merged
steveloughran merged 4 commits into
apache:trunkfrom
steveloughran:pr/HADOOP-19864-writable-load-validation
Sep 8, 2026
Merged

steveloughran merged 4 commits into
apache:trunkfrom
steveloughran:pr/HADOOP-19864-writable-load-validation

Conversation

@steveloughran

@steveloughran steveloughran commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Validate class instantiation through ObjectWritable and elsewhere to make sure
the class is of the requested type before the class is instantiated

New reflection validation in ReflectionUtils
-Pulls up class type checking to before actual class load; fails fast -Add check of array length.
-Limit recursive depth to 100.

This is #8569 on a new branch to get yetus to pick it up

Contains content generated by Claude.ai

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?

AI Tooling

If an AI tool was used:

@hadoop-yetus

Copy link
Copy Markdown

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 33s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell was not available.
+0 🆗 detsecrets 0m 0s 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 6 new or modified test files.
_ trunk Compile Tests _
+0 🆗 mvndep 2m 0s Maven dependency ordering for branch
+1 💚 mvninstall 43m 22s trunk passed
+1 💚 compile 16m 7s trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 compile 16m 47s trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 checkstyle 5m 55s trunk passed
+1 💚 mvnsite 4m 11s trunk passed
+1 💚 javadoc 3m 19s trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 javadoc 3m 14s trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
-1 ❌ spotbugs 1m 7s /branch-spotbugs-hadoop-tools_hadoop-federation-balance-warnings.html hadoop-tools/hadoop-federation-balance in trunk has 12 extant spotbugs warnings.
+1 💚 shadedclient 28m 56s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 26s Maven dependency ordering for patch
+1 💚 mvninstall 2m 18s the patch passed
+1 💚 compile 15m 26s the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 javac 15m 26s the patch passed
+1 💚 compile 16m 38s the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 javac 16m 38s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 5m 34s root: The patch generated 0 new + 431 unchanged - 1 fixed = 431 total (was 432)
+1 💚 mvnsite 4m 10s the patch passed
+1 💚 javadoc 3m 18s the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 javadoc 3m 15s the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 spotbugs 6m 51s the patch passed
+1 💚 shadedclient 29m 5s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 23m 38s hadoop-common in the patch passed.
+1 💚 unit 8m 46s hadoop-mapreduce-client-core in the patch passed.
+1 💚 unit 2m 57s hadoop-federation-balance in the patch passed.
+1 💚 asflicense 1m 11s The patch does not generate ASF License warnings.
262m 48s
Subsystem Report/Notes
Docker ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8640/1/artifact/out/Dockerfile
GITHUB PR #8640
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname Linux bde632a7b1d2 5.15.0-181-generic #191-Ubuntu SMP Fri May 22 19:09:02 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / df7fca6
Default Java Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
Multi-JDK versions /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.11+10-1-24.04.2-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8640/1/testReport/
Max. process+thread count 1577 (vs. ulimit of 10000)
modules C: hadoop-common-project/hadoop-common hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-tools/hadoop-federation-balance U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8640/1/console
versions git=2.43.0 maven=3.9.15 spotbugs=4.9.7
Powered by Apache Yetus 0.14.1 https://yetus.apache.org

This message was automatically generated.

@steveloughran

Copy link
Copy Markdown
Contributor Author

conflict in TFile to resolve

New reflection validation in ReflectionUtils
-Pulls up class type checking to before actual class load; fails fast
-Add check of array length.
-Limit recursive depth to 100.

+Sequence file logic.

Conf loading does force classloading, so move sequence
file logic out of it.

This does remove it from the conf file weak hash map of classes, but
the jvm does its own class map here. The real feature of the conf
map is the caching of cache misses, which would actually surface
as errors on reading files, so you aren't saving anything performance-wise.

Contains content generated by Claude.ai

Contributed by Steve Loughran.
@steveloughran
steveloughran force-pushed the pr/HADOOP-19864-writable-load-validation branch from df7fca6 to 0cbe6cf Compare August 24, 2026 18:14
@github-actions github-actions Bot added the Infra label Aug 24, 2026
@hadoop-yetus

Copy link
Copy Markdown

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 6m 36s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell was not available.
+0 🆗 detsecrets 0m 0s detect-secrets was not available.
+0 🆗 yamllint 0m 0s yamllint was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 6 new or modified test files.
_ trunk Compile Tests _
+0 🆗 mvndep 2m 3s Maven dependency ordering for branch
+1 💚 mvninstall 26m 33s trunk passed
+1 💚 compile 9m 49s trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 compile 10m 23s trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 checkstyle 3m 41s trunk passed
+1 💚 mvnsite 12m 39s trunk passed
+1 💚 javadoc 6m 29s trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 javadoc 6m 28s trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
-1 ❌ spotbugs 22m 18s /branch-spotbugs-root-warnings.html root in trunk has 93 extant spotbugs warnings.
-1 ❌ spotbugs 0m 29s /branch-spotbugs-hadoop-tools_hadoop-federation-balance-warnings.html hadoop-tools/hadoop-federation-balance in trunk has 12 extant spotbugs warnings.
+1 💚 shadedclient 15m 9s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 16s Maven dependency ordering for patch
+1 💚 mvninstall 20m 53s the patch passed
+1 💚 compile 8m 43s the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 javac 8m 43s the patch passed
+1 💚 compile 8m 53s the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 javac 8m 53s the patch passed
+1 💚 blanks 0m 1s The patch has no blanks issues.
+1 💚 checkstyle 2m 59s root: The patch generated 0 new + 431 unchanged - 1 fixed = 431 total (was 432)
+1 💚 mvnsite 9m 42s the patch passed
+1 💚 javadoc 6m 3s the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 javadoc 5m 56s the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 spotbugs 22m 43s the patch passed
+1 💚 shadedclient 34m 10s patch has no errors when building and testing our client artifacts.
_ Other Tests _
-1 ❌ unit 625m 32s /patch-unit-root.txt root in the patch failed.
-1 ❌ asflicense 0m 59s /results-asflicense.txt The patch generated 1 ASF License warnings.
857m 35s
Reason Tests
Failed junit tests hadoop.yarn.server.router.webapp.TestFederationWebApp
Subsystem Report/Notes
Docker ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8640/2/artifact/out/Dockerfile
GITHUB PR #8640
Optional Tests dupname asflicense codespell detsecrets yamllint compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle
uname Linux 168d17042016 5.15.0-181-generic #191-Ubuntu SMP Fri May 22 19:09:02 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / f7854dc
Default Java Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
Multi-JDK versions /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.11+10-1-24.04.2-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8640/2/testReport/
Max. process+thread count 4803 (vs. ulimit of 10000)
modules C: hadoop-common-project/hadoop-common hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-tools/hadoop-federation-balance . U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8640/2/console
versions git=2.43.0 maven=3.9.15 spotbugs=4.9.7
Powered by Apache Yetus 0.14.1 https://yetus.apache.org

This message was automatically generated.

@ajfabbri ajfabbri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM pending removal of unrelated change. Other comments are just nits.

update:
name: "Update Build Status"
runs-on: ubuntu-slim
if: github.repository == 'apache/hadoop'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i believe this was already included in #8702

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah

in.readFully(bytes, 0, length);
return Text.decode(bytes);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: superfluous whitespace change

public void testDeepNestingRejected() throws Exception {
DataOutputBuffer out = new DataOutputBuffer();
String arrayClass = Object[].class.getName();
for (int i = 0; i < 110; i++) { // each level: an Object[] of length 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not a blocking comment, but a constant for 100 depth limit and using it here (X + 10) would be clearer to future generations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

there is a constant; made package private and referenced it

@hadoop-yetus

Copy link
Copy Markdown

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 0s Docker mode activated.
-1 ❌ patch 0m 23s #8640 does not apply to trunk. Rebase required? Wrong Branch? See https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute for help.
Subsystem Report/Notes
GITHUB PR #8640
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8640/4/console
versions git=2.34.1
Powered by Apache Yetus 0.14.1 https://yetus.apache.org

This message was automatically generated.

@steveloughran

Copy link
Copy Markdown
Contributor Author

I'm doing the merge with the approval. The build/test is failing in the debian java 25 setup now I've merged in trunk, which I'm going to assert is unrelated. Bad, needs fixing, but unrelated.

@steveloughran
steveloughran merged commit 40433c5 into apache:trunk Sep 8, 2026
2 of 4 checks passed
steveloughran added a commit that referenced this pull request Sep 9, 2026
New reflection validation in ReflectionUtils
-Pulls up class type checking to before actual class load; fails fast
-Add check of array length.
-Limit recursive depth to 100.

This is used in SequenceFile, InputSplits and elsewhere that
Writable subclasses instantiate classes.

Contributed by Steve Loughran
Assisted-by: Claude.ai

"This is Ripley, last survivor of the Nostromo, signing off."
9589683f-369f-4a30-8c93-e9f5d04ceb5a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants