[FlyWeb] Always set attributes in nsDNSServicediscovery when resolving service.

RESOLVED FIXED

Status

()

Core
Networking
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: djvj, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

a year ago
Created attachment 8751834 [details] [diff] [review]
always-set-nsdnsserviceinfo-attributes-on-osx.patch

nsDNSServiceDiscovery doesn't set nsDNSServiceInfo::attributes if there are no TXT records in the metadata.

It should set the attribute to the empty hash if that's the case, instead of leaving it nullptr.  Leaving it null breaks semantics: the nsDNSServiceInfo is supposed to be fully resolved and have all fields defined by the time it gets set to the onServiceResolved callback.
Attachment #8751834 - Flags: review?(jdarcangelo)
Comment on attachment 8751834 [details] [diff] [review]
always-set-nsdnsserviceinfo-attributes-on-osx.patch

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

LGTM
Attachment #8751834 - Flags: review?(jdarcangelo) → review+
(Reporter)

Comment 2

a year ago
https://hg.mozilla.org/projects/larch/rev/2362715386da
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Comment on attachment 8751834 [details] [diff] [review]
always-set-nsdnsserviceinfo-attributes-on-osx.patch

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

::: netwerk/dns/mdns/libmdns/MDNSResponderOperator.cpp
@@ +631,5 @@
>    // Resolve TXT record
>    int count = TXTRecordGetCount(aTxtLen, aTxtRecord);
>    LOG_I("resolve: txt count = %d, len = %d", count, aTxtLen);
> +  nsCOMPtr<nsIWritablePropertyBag2> attributes = new nsHashPropertyBag();
> +  if (NS_WARN_IF(!attributes)) { return; }

This nullcheck is a no-op these days. Our default |operator new| will exit() if the allocation fails, so we'll never get to the if-statement with a null |attributes|.
You need to log in before you can comment on or make changes to this bug.