Repository navigation
HADOOP-20020. DNS.reverseDns throws NullPointerException when the answer has no PTR record - #8820
Merged
Merged
Conversation
…wer has no PTR record Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open findings
ptr.get()can still fail if the PTR attribute exists but contains zero values (e.g., empty… · NewIOExceptionhere and treating it as a normal shutdown can mask unexpected failures… · New
What changed in this PR
Fixes DNS.reverseDns to avoid NullPointerException when DNS responds with NOERROR but no PTR (NODATA), and adds a regression test to validate the new behavior.
Changes:
- Treat missing PTR attribute in reverse DNS responses as
NameNotFoundException(instead of triggering an NPE). - Add a UDP-based test DNS responder to simulate NOERROR-without-records for both IPv4 and IPv6.
| File | Description |
|---|---|
| hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java | Adds null-check for PTR attribute and throws NameNotFoundException for NODATA reverse lookups. |
| hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestDNS.java | Adds regression test and minimal DNS responder that returns NOERROR with no records. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot stopped reviewing on behalf of
smengcl due to an error
October 14, 2026 11:28
This comment was marked as outdated.
This comment was marked as outdated.
|
🎊 +1 overall
This message was automatically generated. |
slfan1989
approved these changes
Oct 14, 2026
slfan1989
left a comment
slfan1989
left a comment
Contributor
There was a problem hiding this comment.
LGTM.
The fix correctly routes missing PTR responses through the existing NamingException fallback handling.
I verified that the new regression test passes with this change and fails with the original NullPointerException without it.
The latest CI checks are green. +1.
Contributor
Author
|
Thanks @slfan1989 for the review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Generated-by: Claude Code (Opus 5.5)
Description of PR
DNS.reverseDnsreadsattribute.get("PTR").get()without a null check. When a DNS server answers the reverse query with NOERROR but no PTR record (a NODATA answer), JNDI returns noPTRattribute andreverseDnsthrowsNullPointerException. Its callers,DNS.getHostsandDNSDomainNameResolver.getHostnameByIP, only catchNamingException, so the exception escapes instead of falling back. ThroughgetHostnameByIPit reachesSecurityUtil.getServerPrincipal(String, InetAddress)andServiceAuthorizationManager.authorize.This PR throws
NameNotFoundExceptionin that case, the exception JNDI already throws when the name does not exist (NXDOMAIN). Answers that carry a PTR record are handled as before. IPv4 and IPv6 addresses are affected alike.How was this patch tested?
Added
TestDNS#testReverseDnsWithoutPtrRecord. It starts a small DNS responder on a loopback UDP port that answers every query with NOERROR and no records, and checks that the reverse lookup of an IPv4 and an IPv6 address fails withNameNotFoundException. On trunk the test fails with theNullPointerException.mvn test -Dtest='TestDNS,TestSecurityUtil,TestNetUtils'inhadoop-commonwith JDK 17: 91 tests run, 0 failures, 0 errors, 0 skipped.For code changes:
declared according to the connector-specific documentation? Note: Automated CI
testing doesn't cover all cases so manual testing with cloud storage is still
required.
LICENSE,LICENSE-binary,NOTICE-binaryfiles?AI Tooling
Contains content generated by Claude Code.
where is the name of the AI tool used.
https://www.apache.org/legal/generative-tooling.html
🤖 Generated with Claude Code