Skip to content

Filter local interface DNS results by address family#128705

Open
simonrozsival wants to merge 2 commits into
mainfrom
dev/simonrozsival/android-dns-localhost-subdomain-128371
Open

Filter local interface DNS results by address family#128705
simonrozsival wants to merge 2 commits into
mainfrom
dev/simonrozsival/android-dns-localhost-subdomain-128371

Conversation

@simonrozsival
Copy link
Copy Markdown
Member

@simonrozsival simonrozsival commented May 28, 2026

Note

This PR description was generated with GitHub Copilot.

Fixes #128371

SystemNative_GetHostEntryForName already passes the requested address family to getaddrinfo through hint.ai_family. However, when the queried name matches the value returned by gethostname(), the PAL augments those resolver results with local interface addresses from getifaddrs. That augmentation path was not applying the same address-family filter, so an IPv4-only lookup could still append IPv6 interface addresses, and vice versa.

This became observable on Android after the minimum API level was raised from 21 to 24. Android API 24 exposes Bionic getifaddrs, so HAVE_GETIFADDRS is now true for Android native builds and this local-interface augmentation can run there. The failing CI pattern points specifically at Windows-hosted physical Android device lanes: those devices appear to enter the local-hostname augmentation path for localhost, while Ubuntu emulator lanes and a local Samsung physical device did not.

The fix filters getifaddrs entries by the requested platform family in both the counting and population passes. Requests with AF_UNSPEC are intentionally left unchanged, so default/unspecified lookups can continue returning both IPv4 and IPv6 addresses as before.

I considered making this Android-specific or filtering in managed code, but the mismatch is in the native PAL contract: callers pass an address family into SystemNative_GetHostEntryForName, getaddrinfo honors it, and only the native getifaddrs augmentation ignores it. Keeping the fix in native makes the whole returned HostEntry internally consistent across platforms. The behavioral impact on non-Android Unix platforms should be limited to local-hostname lookups that explicitly request IPv4 or IPv6; those calls should not receive opposite-family interface addresses.

When SystemNative_GetHostEntryForName augments local hostname results with getifaddrs entries, keep the interface address family consistent with the requested getaddrinfo family. This preserves AF_UNSPEC behavior while preventing IPv6 interface addresses from leaking into IPv4-only lookups.

Co-authored-by: Copilot <[email protected]>
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @karelz, @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fix for issue #128371: when SystemNative_GetHostEntryForName augments getaddrinfo results with addresses from getifaddrs for local hostname lookups, it now filters interface addresses by the requested address family, matching the filter that getaddrinfo already applies via hint.ai_family. This prevents IPv6 interface addresses from leaking into IPv4-only lookups (and vice versa) on platforms like Android.

Changes:

  • Skip getifaddrs entries whose family doesn't match platformFamily (unless AF_UNSPEC) in both the counting and the copy loops.
  • Cache ifa->ifa_addr->sa_family in a local interfaceFamily to avoid redundant dereferencing.

@simonrozsival
Copy link
Copy Markdown
Member Author

/azp run runtime-android

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@github-actions

This comment has been minimized.

@rzikm
Copy link
Copy Markdown
Member

rzikm commented May 28, 2026

/azp run runtime-extra-platforms

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

This was referenced May 28, 2026
Copy link
Copy Markdown
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

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

LGTM, this looks like it will address all recent problems with new DNS tests on Android

@rzikm
Copy link
Copy Markdown
Member

rzikm commented May 29, 2026

Will this also fix test disabled by #127742?

@simonrozsival
Copy link
Copy Markdown
Member Author

Will this also fix test disabled by #127742?

@rzikm it looks like it should address those too. I'll give it a try.

@simonrozsival
Copy link
Copy Markdown
Member Author

@rzikm those tests were already re-enabled in #128068, so they already run and pass. The other PR relaxed some of the assertions for Android and Apple mobile platforms, so I tried reverting those relaxations for Android — but the tests failed afterwards, so I suggest keeping them.

@simonrozsival simonrozsival marked this pull request as ready for review May 29, 2026 10:00
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #128705

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: The PR fixes a real bug where getifaddrs-based local interface augmentation in SystemNative_GetHostEntryForName did not honor the caller’s requested address family, causing IPv6 addresses to leak into IPv4-only lookups (observable on Android API 24+). The linked issue #128371 and CI failures confirm the problem is genuine.

Approach: The fix applies the same platformFamily filter that getaddrinfo already uses via hint.ai_family to both getifaddrs iteration loops (counting and population). This is the correct layer — it keeps the native PAL contract internally consistent rather than pushing filtering to managed code.

Summary: ✅ LGTM. The change is minimal, correct, and directly addresses the reported inconsistency. No blocking issues found.


Detailed Findings

✅ Correctness — Filter applied consistently in both loops

Both the counting pass (line ~456) and the population pass (line ~521) apply the identical filter:

sa_family_t interfaceFamily = ifa->ifa_addr->sa_family;
if (platformFamily != AF_UNSPEC && interfaceFamily != platformFamily)
{
    continue;
}

This ensures IPAddressCount matches the actual addresses written, preventing buffer overrun or under-fill.

✅ Backward compatibility — AF_UNSPEC guard preserves existing behavior

When callers pass AddressFamily.Unspecified, platformFamily is AF_UNSPEC and the filter is skipped, so default lookups continue returning both IPv4 and IPv6 addresses as before.

✅ Code quality — Local variable eliminates repeated dereferences

Extracting interfaceFamily from ifa->ifa_addr->sa_family is a clean refactor that improves readability and avoids repeated pointer chasing.

💡 Observation — Loopback tracking variables for the filtered-out family

When platformFamily == AF_INET, the includeIPv6Loopback variable stays at its default true value since no IPv6 interfaces pass the filter. The corresponding loopback-skip branch in the second loop becomes unreachable for IPv6, which is harmless but slightly unnecessary. This is a pre-existing design pattern and not worth changing in this PR.


Models consulted: claude-opus-4.6

Generated by Code Review for issue #128705 · ● 1.3M ·

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.

[ci-scan] Test failure: DnsGetHostAddresses_LocalhostSubdomain_RespectsAddressFamily on Android

3 participants