DNS: Prepare nsHostResolver and nsDNSService to use different record types

NEW
Unassigned

Status

()

Core
Networking: DNS
P3
normal
6 years ago
4 months ago

People

(Reporter: sworkman, Unassigned)

Tracking

(Blocks: 3 bugs)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [necko-backlog])

Attachments

(2 attachments)

Created attachment 606078 [details] [diff] [review]
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.
(Reporter)

Updated

6 years ago
Attachment #606078 - Attachment is patch: true
(Reporter)

Comment 1

6 years ago
Created attachment 606079 [details] [diff] [review]
WIP v1.0 changes to nsIDNSService etc.
(Reporter)

Comment 2

6 years ago
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).
(Reporter)

Updated

6 years ago
Blocks: 545866
(Reporter)

Updated

6 years ago
Blocks: 14328

Comment 3

6 years ago
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
(Reporter)

Comment 4

6 years ago
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.
Assignee: nobody → sworkman

Comment 5

6 years ago
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

6 years ago
Steve,

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

thanks!
(Reporter)

Comment 7

6 years ago
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

6 years ago
Steve,

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

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

Updated

5 years ago
Blocks: 861622
Assignee: sworkman → nobody
Whiteboard: [necko-backlog]
You need to log in before you can comment on or make changes to this bug.