BIND 10 #2905: buggy zone should result in SERVFAIL, not REFUSED

[BIND 10 Development]

#2905: buggy zone should result in SERVFAIL, not REFUSED
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |  jinmei
            Priority:  medium        |                       Status:
           Component:  data source   |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130528
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  1.32          |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei
 * totalhours:  0 => 1.32


Comment:

 Hello

 Replying to [comment:9 jinmei]:
 > Hmm, not sure exactly which part of th code you meant, but I've tried
 > to make some clarification in zone_table.h.

 Yes, I think it was this one.

 > > The exception text „Zone content must not be NULL or empty“ is
 understandable, but only with some thinking about what it means. Should it
 better be „nor“?
 >
 > (Readability aside) in my understanding "not A or B" is the
 > grammatically correct form while "not A nor B" isn't.  But in any case
 > I've revised it more fundamentally.  Hopefully that one is clearer.

 But I believe such would translate to `(!A) || B`, which is wrong. It
 should AFAIK be „Neither A nor B“.

 Anyway, the revised version looks good.

 > It checks that by confirming finder_ is NULL and `dsrc_client_` isn't.
 > I considered introducing some more explicitly interface that indicates
 > the zone is empty/broken, but adding another boolean didn't seem to be
 > a good idea because that one and finder_ and dsrc_client_ are related
 > and we'll need to be careful not to break the integrity.  Adding a
 > method like `isZoneEmpty()` (return true iff both finder_ and
 > dsrc_client_ are non NULL) would be a possible idea, but for the user
 > like auth query.cc it didn't seem to be very useful anyway.

 Hmm. I just got the impression that something like 5 different places got
 the flags, so this variable must have gotten them too. But I was wrong,
 obviously. In such case, this is OK.

 > It's at least implicitly tested as NULL is the default parameter
 > value.  I clarified that point in a comment.  We might test the case
 > of passing NULL explicitly too, but for now I've only made the comment
 > clarification, preferring conciseness.

 Ah, OK then.

 It looks good to be merged.

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