BIND 10 #69: review: data source hangs with CNAME loops

[BIND 10 Development]

#69: review: data source hangs with CNAME loops
-------------------------+--------------------------------------------------
 Reporter:  jinmei       |        Owner:  each                                          
     Type:  defect       |       Status:  reviewing                                     
 Priority:  minor        |    Milestone:  05. 3rd Incremental Release: Serious Secondary
Component:  data source  |   Resolution:                                                
 Keywords:               |    Sensitive:  0                                             
-------------------------+--------------------------------------------------

Comment(by each):

 Replying to [comment:11 stephen]:
 > Note: One test is disabled (synthesisedCnameTooLong).  When re-enabled,
 the test fails because the exception !TooLongName is thrown.  Isn't this
 expected?

 Exceptions are expensive; we'd prefer not to use them unless we have to.
 In this case particularly, a caller could force the server to throw an
 exception just by sending a long qname, which has potential as a DoS
 vector.  It's better to check the qname length before doing the Name
 concatenation.

 > Remark: addToMessage() returns no indication is returned as to whether
 the addition of the RRset to the message succeeded or failed because of an
 already-existing one.  Is there an advantage to returning a bool and
 having callers test the return value as an internal consistency check?

 IMHO, no.  The caller doesn't care, it just wants to be know the message
 got in.

 > Remark: The method addRRset() has a comment "Note: should check
 duplicate (TBD)".  Should the check be done here or (as it now is) in
 addToMessage()?

 We discussed this in comment:3 and comment:4, above.  I saw that comment
 in Jinmei's code in message.cc, and thought it made sense to do the check
 there.  Jinmei, however, had apparently reconsidered the point and thought
 it better for the caller to take care of this.  After I looked at BIND 9
 again I realized that the dns_message module doesn't do duplication
 checking; the caller does.  So I just took the same approach.  I'll remove
 that comment.

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