BIND 10 #3099: Bogus error message in b10-xfrin if no tsig key used

[BIND 10 Development]

#3099: Bogus error message in b10-xfrin if no tsig key used
-------------------------------------+-------------------------------------
            Reporter:  shane         |                        Owner:
                Type:  defect        |  vorner
            Priority:  medium        |                       Status:
           Component:  xfrin         |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130903
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  1             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:
                                     |  Medium
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by shane):

 * owner:  shane => vorner


Comment:

 Replying to [comment:6 vorner]:
 > **Translations**
 >
 > By hardcoding a text string inside the code, if we ever translated the
 messages, the result would be bilingual. Maybe something like „TSIG: %s“
 in the message and using „-“ in case none is available would be solution.

 Ah good point!

 Actually I was thinking it makes sense to simply leave the TSIG out...
 after all we're talking about an error with the TCP connection which
 occurs independent of TSIG. I'm not really sure why the TSIG was included
 in the message to begin with.

 I've updated the trac3099 branch to remove the TSIG key name to address
 this point.

 > **Deeper problems**
 >
 > The fact that there's no `tsig_key_name` indicates there's something
 rotten deeper inside. Reading the code, and this function in general:
 > {{{#!python
 >     def set_tsig_key_name(self, tsig_key_str):
 >         """Set the name of the tsig_key for this zone. If tsig_key_str
 >            is None, no TSIG key will be used. This name is used to
 >            find the TSIG key to use for transfers in the global TSIG
 >            key ring.
 >            Raises XfrinZoneInfoException if tsig_key_str is not a valid
 >            (dns) name."""
 >         if tsig_key_str is None:
 >             self.tsig_key_name = None
 >         else:
 >             # can throw a number of exceptions but it is just one
 >             # call, so Exception should be OK here
 >             try:
 >                 self.tsig_key_name = Name(tsig_key_str)
 >             except Exception as exc:
 >                 raise XfrinZoneInfoException("Bad TSIG key name: " +
 str(exc))
 > }}}
 >
 > The function is called from the `__init__`. And, it should set the
 `tsig_key_name` to either `None` or the name (and `None` wouldn't give
 that error message). The only case when it might not set it is when the
 key is set and the `Name` constructor throws, but I don't think the object
 is used after that.
 >
 > May I suggest we investigate more into the issue instead of blindly
 applying the fix? Do you have time for that, or should I try looking at
 it?

 The set_tsig_key_name() method is operating on a !ZoneInfo object, and the
 reference that I've removed is in the !XfrinConnection class.

 Surely there are also deeper issues, but we know the XFR code needs to be
 refactored so I'm not sure we need to dig into it too much...

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