735967 - DNS: Prepare nsHostResolver and nsDNSService to use different record types

Status: NEW

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

Description

[Steve Workman [:sworkman] (INACTIVE)]

Attachment: WIP v1.0 changes to nsHostRecord & nsHostResolver

Hostname resolver currently supports A and AAAA resource record types only. Bug 14328 and bug 545866 have patches to support SRV, MX, NAPTR and TXT. However, an underlying framework which uses the existing threadpool and cache is desired.

Comment 1

[Steve Workman [:sworkman] (INACTIVE)]

Attachment: WIP v1.0 changes to nsIDNSService etc.

Comment 2

[Steve Workman [:sworkman] (INACTIVE)]

I've uploaded a couple of WIP patches for feedback on the general approach taken.  The nsHostResolver one is more complete, but definitely needs polish.  Both together should provide a framework with which we can use code from Bug 14328 and Bug 545866 to get those extra DNS resource record types, while using the current threadpool and DNS cache.

The main changes in nsHostResolver is that the nsHostRecord sub-classes now take ownership of the resolver function. This means that in nsHostResolver::ThreadFunc, a call is made to nsHostRecord::Lookup, which is a virtual function implemented by nsHostRecordTypeA|TypeSRV|TypeMX|TypeNAPTR sub-classes.

I'm thinking that this might be suitable for DNSSec queries later, i.e. having something like nsHostRecordTypeA making a standard query and nsHostRecordTypeASec making a DNSSec query. Motive at the moment would just be to separate out the functionality, possibly for a phased introduction of DNSSec (or whatever) in our code.

Re the nsDNSService patch, I've included IDL interfaces nsIDNSSRV|MX|NAPTRRecord, but they might not be needed - the nsIDNSTypeRecord interface may be sufficient along with nsIDNSSRV|MX|NAPTRRecordEntry interfaces.  This patch is very high level at present, but hopefully it should be enough to cast a vision of the framework.

(Note: I'm still not working on this as top priority, but I wanted to develop the idea in parallel to my other work).

Comment 3

[Mike Taylor [:bear]]

Steve,

Thanks for working on this!  I've been getting behind-the-scenes pings from many of the XMPP folk who are working in XMPP related js code that would love to see this class of problem be fixed in Mozilla Core

Please do let me know if there is anything I can do to help

Comment 4

[Steve Workman [:sworkman] (INACTIVE)]

Thanks, Mike.

Someone information to add to the proposed approach. We needed to confirm that res_query could be exposed on Android, which it seems like it can. We package some of the Android libc code, specifically an implementation of getaddrinfo, in order to get threadsafe functionality on pre-Honeycomb/v3.0 devices. res_query is not normally exposed in the Android NDK, but since we package it, that is under our control.

We can also restrict res_query to be called only for non-A/AAAA types.  Currently, we restrict the packaged getaddrinfo to pre-Honeycomb calls only, and we can maintain that: i.e. A/AAAA requests would use the current method. But, we'd have to expose and use res_query on all Android versions to get access to rr types on those devices.

Not a huge issues per se, but important to point out that it needs to work across all of our supported platforms.

Comment 5

[Ben Bucksch (:BenB)]

Steve, thanks for working on this!

FWIW, my patch in bug 545866 was based on the patch in bug 14328. I see you didn't make any IDL changes yet. If you could take a look and use the IDL changes I made (and the 5 code changes that are made necessary by that), that'd be great. I actually only cared that the IDL interface is clean and generic for all types (including A), because that's what apps have to use.

Comment 6

[Mike Taylor [:bear]]

Steve,

With the merge happening next week is there anything we can do to hit that window? or start preparing for the next?

thanks!

Comment 7

[Steve Workman [:sworkman] (INACTIVE)]

Hi Mike,
I'm sorry but I don't think hitting next week is likely. My hands are pretty full at the moment with DASH, and (unfortunately for this bug) that's my top priority. I think aiming for the next release is a more achievable goal.

Comment 8

[Mike Taylor [:bear]]

Steve,

thanks for the reply, I will try again next merge week :)

Comment 9

[Florian Quèze [:florian]]

Steve,

Is there any hope of seeing things move forward here in the foreseeable future?

Comment 10

[Firefox Bug Husbandry Bot]

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

Comment 11

[Firefox Bug Husbandry Bot]

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