Skip to content

[mdns] allow resolving IPv4 address for host or service with mDNSResponder - #3557

Open
librasungirl wants to merge 1 commit into
openthread:mainfrom
librasungirl:mdns-allow-ipv4-resolve
Open

[mdns] allow resolving IPv4 address for host or service with mDNSResponder#3557
librasungirl wants to merge 1 commit into
openthread:mainfrom
librasungirl:mdns-allow-ipv4-resolve

Conversation

@librasungirl

Copy link
Copy Markdown
Contributor

In mdns_mdnssd.cpp, DNSServiceGetAddrInfo is called with kDNSServiceProtocol_IPv6 | kDNSServiceProtocol_IPv4 for both service instance resolution and host subscription resolution. This indicates that either an IPv6 or an IPv4 address is explicitly expected and allowed from the mDNS responder.

However, the callback handlers HandleGetAddrInfoResult and HandleResolveResult previously discarded all non-IPv6 addresses with VerifyOrExit(aAddress->sa_family == AF_INET6). This was a bug that caused hosts and services advertising only IPv4 A records to be ignored rather than resolved.

This change fixes the bug by:

  • Accepting both AF_INET and AF_INET6 address families in PublisherMDnsSd::ServiceInstanceResolution::HandleGetAddrInfoResult and PublisherMDnsSd::HostSubscription::HandleResolveResult.
  • Extending Ip6Address with CopyFrom(const struct sockaddr &), CopyFrom(const struct sockaddr_in &) and CopyFrom(const struct in_addr &) to convert resolved IPv4 addresses into IPv4-mapped IPv6 addresses (::ffff:a.b.c.d), consistent with OpenThread's DNS-SD / discovery proxy specification.
  • Adding unit test for Ip6Address covering IPv4-mapped addresses.

@librasungirl
librasungirl requested review from bukepo and jwhui August 26, 2026 12:03

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for IPv4-mapped IPv6 addresses in the Ip6Address class, adding helper methods to copy addresses from sockaddr, sockaddr_in, and in_addr structures, along with corresponding unit tests. It also updates the mDNS publisher to handle these address types. The review feedback highlights a potential null pointer dereference of aAddress in src/mdns/mdns_mdnssd.cpp and suggests avoiding union type punning in src/common/types.cpp to prevent undefined behavior and endianness issues.

Comment thread src/mdns/mdns_mdnssd.cpp
Comment thread src/mdns/mdns_mdnssd.cpp
Comment thread src/common/types.cpp
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.97%. Comparing base (2b41187) to head (0b0c5ee).
⚠️ Report is 1644 commits behind head on main.

Files with missing lines Patch % Lines
tests/gtest/test_common_types.cpp 36.17% 30 Missing ⚠️
src/mdns/mdns_mdnssd.cpp 42.10% 9 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3557      +/-   ##
==========================================
- Coverage   55.77%   47.97%   -7.80%     
==========================================
  Files          87      149      +62     
  Lines        6890    18514   +11624     
  Branches        0     1496    +1496     
==========================================
+ Hits         3843     8883    +5040     
- Misses       3047     9015    +5968     
- Partials        0      616     +616     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/common/types.hpp Outdated
Comment thread src/mdns/mdns_mdnssd.cpp Outdated
Comment thread src/mdns/mdns_mdnssd.cpp Outdated
Comment thread tests/gtest/test_common_types.cpp
@librasungirl
librasungirl force-pushed the mdns-allow-ipv4-resolve branch from b34941a to a8513df Compare September 3, 2026 08:47
…onder

In mdns_mdnssd.cpp, DNSServiceGetAddrInfo is called with
`kDNSServiceProtocol_IPv6 | kDNSServiceProtocol_IPv4` for both service
instance resolution and host subscription resolution. This indicates
that either an IPv6 or an IPv4 address is explicitly expected and
allowed from the mDNS responder.

However, the callback handlers `HandleGetAddrInfoResult` and
`HandleResolveResult` previously discarded all non-IPv6 addresses with
`VerifyOrExit(aAddress->sa_family == AF_INET6)`. This was a bug that
caused hosts and services advertising only IPv4 A records to be
ignored rather than resolved.

This change fixes the bug by:
- Accepting both AF_INET and AF_INET6 address families in
  `PublisherMDnsSd::ServiceInstanceResolution::HandleGetAddrInfoResult`
  and `PublisherMDnsSd::HostSubscription::HandleResolveResult`.
- Extending `Ip6Address` with `CopyFrom(const struct sockaddr &)`,
  `CopyFrom(const struct sockaddr_in &)` and
  `CopyFrom(const struct in_addr &)` to convert resolved IPv4 addresses
  into IPv4-mapped IPv6 addresses (`::ffff:a.b.c.d`), consistent with
  OpenThread's DNS-SD / discovery proxy specification.
- Adding unit test for `Ip6Address` covering IPv4-mapped addresses.
@librasungirl
librasungirl force-pushed the mdns-allow-ipv4-resolve branch from a8513df to 0b0c5ee Compare September 3, 2026 08:49
@librasungirl
librasungirl requested a review from jwhui September 3, 2026 08:58
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