1218660 - Only accept IPv4 address in DNSServiceGetAddrInfoReply callback

Status: RESOLVED WONTFIX

(Core :: Networking: DNS, defect, P5)

Description

[Kershaw Chang [:kershaw]]

DNSServiceGetAddrInfoReply[1] will be invoked multiple times for getting both IPv4 and IPv6 addresses, but we didn't handle it well. We decide to only handle IPv4 currently.


[1] https://dxr.mozilla.org/mozilla-central/source/netwerk/dns/mdns/libmdns/MDNSResponderOperator.cpp#738

Comment 1

[Kershaw Chang [:kershaw]]

Attachment: [WIP] Only get IPv4 address

SC, Gary,

Could you please take a look first?

Comment 2

[Gary Chen [:xeonchen]]

Comment on attachment 8679292 [details] [diff] [review]
[WIP] Only get IPv4 address

Review of attachment 8679292 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/mdns/MulticastDNSManager.java
@@ +532,5 @@
>              return;
>          }
>  
> +        // NOTE: We only accept IPv4 address currently.
> +        if (serviceInfo.getHost() instanceof Inet6Address) {

Based on the comment, I'll prefer to check Inet4Address.

::: netwerk/dns/mdns/libmdns/MDNSResponderOperator.cpp
@@ -745,5 @@
>  {
>    MOZ_ASSERT(GetThread() == NS_GetCurrentThread());
>  
> -  mDeleteProtector = nullptr;
> -

This should be moved to somewhere else rather than being deleted, right?

@@ +754,5 @@
>  
>    if (!mListener) { return; }
>  
> +  // If the address is valid, kDNSServiceFlagsAdd must be set. Also
> +  // check kDNSServiceFlagsMoreComing is not set to get the last address.

I'm not sure the reason here that only the last address is got.

Comment 3

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8679292 [details] [diff] [review]
[WIP] Only get IPv4 address

Review of attachment 8679292 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry I got a second thoughts after our discussion. MDNS is designed to be a general XPCOM, not only for Presentation API. Making Presentation API works for multiple callback or reporting multiple address in one callback will be the long term solution.

Comment 4

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 5

[Kershaw Chang [:kershaw]]

I think MDNS is not used now, so close this.