BIND 10 #2390: support generic version of rdata::createRdata(text) in NS, MX, PTR RDATA

[BIND 10 Development]

#2390: support generic version of rdata::createRdata(text) in  NS, MX, PTR RDATA
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  UnAssigned
            Priority:  medium        |                       Status:
           Component:  libdns++      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130205
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  4             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  loadzone-ng
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:5 muks]:

 Just took a quick look at the branch mainly for answering the
 questions (please make it unassigned when you address the following -
 at this point I'm not sure I'll be the eventual full reviewer):

 > I'm putting this one to review. But please read the comments below as I
 have questions:
 >
 > * The description asks to update tests in
 `src/bin/loadzone/tests/correct/`, but upon reading it, I wonder why these
 were not enabled before. Would it not have worked with the "old" method
 that used the string constructor?

 They don't work unless we handle omitted origins in name construction
 for NS RDATA.  (I wonder you tried enabling it to see the failure
 yourself...I believe it's pretty easy).  And, this leads to one
 critical point of the branch itself: we need to use the origin
 parameter for these types of RDATA.  Your implementation still misses
 it.

 I suspect the same point applies to #2656 (I've not looked).

 > * Currently, any exceptions in the lexer are propagated. Would these
 have to be caught and changed to `InvalidRdataText` exceptions? I have to
 ask because the meta ticket and this ticket's description are very terse
 and don't explain much.

 Propagating them is okay, and that's actually the intended way of
 handling them.  These exceptions are expected to be caught in
 `createRdata()` (taking lexer).  See rdata.cc.

 I realized the intention isn't clear in the documentation...I suggest
 updating the documentation of `RRParamRegistry::createRdata()` as part
 of this task.

 > * Does this require a `ChangeLog` entry?

 Probably, as omitting the origin for the domain name RDATA fields of
 some of these RR types would be common (particularly for NS), and it
 was previously rejected.  But maybe we should provide a single unified
 entry for both this one and #2656.

 A few more things:
 - I reckon it wasn't clear from the ticket description, but we should
   also update the `std::string` version of constructors.  See how SOA
   does that.  Also, consider using the `createNameFromLexer()` utility
   function.  It's defined dns/rdata/generic/detail/lexer_util.h.
 - some of these older RDATA implementations largely miss tests.
   Depending on the amount of the work for the main subject of this
   ticket, it's a good idea to provide some missing test cases,
   especially for methods that aren't tested at all.
 - some variables seem to have to be constified

-- 
Ticket URL: <http://bind10.isc.org/ticket/2390#comment:7>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development