BIND 10 #955: xfrin should check TSIG before other part of incoming message

[BIND 10 Development]

#955: xfrin should check TSIG before other part of incoming message
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  UnAssigned
  jinmei                             |                Status:  reviewing
                       Type:         |             Milestone:
  defect                             |  Sprint-20110614
                   Priority:  major  |            Resolution:
                  Component:  xfrin  |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  0.0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:8 zzchen_pku]:

 > > Unless I miss something, there's no test for this.
 > Because I just reversed the checking order, both of the checking will
 raise XfrinException and discard bogus responses, so the function of the
 interface has not been changed, the original test should be enough to
 handle it.

 Not really.  We can still check whether the exception error message is
 the expected one (and I think we should).  In general, when we wanted
 to say "we don't need a test for this case", we should re-think at
 least three times if it's really, really true.  IMO in many cases it's
 not (and, often, embarrassingly, there's a bug that could be
 identifiable via tests).

 And, this remind me of another issue I've been having with xfrin and
 xfrout: they use the same single exception in so many places,
 obscuring the accuracy of test results.  When we do
 self.assertRaises(XfrinException, xxx), we cannot really be sure if
 the exception is really triggered at where it should be triggered.  We
 could check the exception error message as I said above, but since the
 error message tends to be modified it will be unstable.  I guess we'll
 need a higher granularity of exceptions (or sub types of them) for
 these apps.  But I understand that's beyond the scope of this ticket.

 > > Also, before picking up a new development task (after finishing one),
 > > could you first consider purging open review requests?  If everyone
 > > only takes on new development tasks and keeps asking for review, the
 > > size of the review queue will be monotonically increased and will
 > > never be empty.
 > oh, currently, almost all review tickets have been assigned to somebody,
 except #767 and #755.
 > (#767 require discussion  and #755 was done after I picked this task.)

 When this ticket was picked up, #957 was already in the review queue
 (though I'm not sure if there are others).  Maybe you checked the
 queue first and simply overlooked it and then picked up this one.  If
 so, that's fine; I've been a bit concerned about the size of review
 queue for a while, and just wanted to make sure reviews are generally
 given a higher priority.

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