BIND 10 #2522: support generic version of rdata::createRdata(text) in RP, MINFO, TSIG RDATA

BIND 10 Development do-not-reply at isc.org
Thu May 23 22:15:37 UTC 2013


#2522: support generic version of rdata::createRdata(text) in RP, MINFO, TSIG
RDATA
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  libdns++      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130528
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 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:22 pselkirk]:

 > > '''rdata_tsig_unittest.cc'''
 > >
 > > - multi line case doesn't seem to be tested either (also for others)
 >
 > I've only been adding multi-line tests for rdata types with terminal
 key-data fields where spaces are allowed in the key data (dhcid, rrsig,
 sshfp), in order to test multi-line keys.
 >
 > RFC2845 doesn't specify a presentation format, so we've defined the data
 fields as base-64 with no spaces (or line breaks, as I tested it out). It
 appears that BIND 9 also doesn't allow whitespace of any sort in the MAC
 and Other Data fields.
 >
 > In any case, I added a test for multi-line ordinary data, which could be
 done for any rdata type with multiple fields. Is this what you mean by
 "also for others" - that all rdata unit tests should have a multi-line
 test?

 In general, I'll try to cover all general cases for completeness, but
 in this particular case I could live with skipping this check, because
 TSIG won't appear in a zone file and it'd be very unlikely to see
 multi-line input in practice.  I guess this was more about the review
 process protocol: if you are silent about a point, it's not clear to
 me whether you overlooked it or decided to not address it.  Sometimes
 it might be obvious, but in general it would be better to be explicit.

 As for "also for others"...I don't remember what I meant:-) but
 it probably meant other Rdata classes updated in this branch (and I
 guess I then added the same comments for them, which might make it
 confusing).  In any case, I don't think it's an open issue anymore.

 > > '''sshfp_44.cc'''
 > > - [if you switch to this type of pimpl, you'll need to define
 > >   `operator=()`; otherwise it would eventually cause duplicate free.]
 > >   (check that by writing a test)
 > >
 > > For the latter, maybe I was not clear but I intended to suggest adding
 > > a unit test case for assignment.  See, e.g., `Rdata_HINFO_Test.copy`.
 >
 > Added. My oversight.

 The added test actually uses the copy constructor, not `operator=`.
 I've updated the test with a suggested test that uses the latter.

 With this change (assuming it's okay), please merge.

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


More information about the bind10-tickets mailing list