BIND 10 #1612: b10-auth should catch exceptions in response building

[BIND 10 Development]

#1612: b10-auth should catch exceptions in response building
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jelte
  jinmei                             |                Status:  reviewing
                       Type:         |             Milestone:
  defect                             |  Sprint-20120306
                   Priority:  major  |            Resolution:
                  Component:         |             Sensitive:  0
  b10-auth                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  3
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jelte


Comment:

 Hello

 I have few comments.

 First, should there be a changelog entry? This could change an external
 behaviour.

 '''Style issues'''

 Indentation.
 {{{#!c++
     virtual FindResult
     findAll(const isc::dns::Name& name,
                                std::vector<isc::dns::ConstRRsetPtr>
 &target,
                                const FindOptions options = FIND_DEFAULT)
     {
         checkThrow(throw_at_find_all, throw_when_, isc_exception_);
         return (real_zone_finder_->findAll(name, target, options));
     };
 }}}

 Space after comma.
 {{{#!c++
 findNSEC3(const isc::dns::Name& name,bool recursive) {
 }}}

 Typo:
 {{{#!c++
 /// if this instance was constructed with throw_shen set to find_zone,
 }}}

 How deep recursion do we allow in tests? ;-)
 {{{
 // Test for the tests
 }}}

 Maybe the `ThrowWhen` members should be all in capitals?

 '''Code brevity vs. repetititiveness'''
 There's a bunch of tests with really similar body:
 {{{#!c++
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
     setupThrow(&server, CONFIG_INMEMORY_EXAMPLE, throw_at_get_origin,
 true);
     processAndCheckSERVFAIL();
 }}}

 (and one test type in many instances as well). Would it be possible to
 make it shorter by using a for cycle over the places where it should fail,
 something like:
 {{{#!c++
 ThrowWhen[] throws = {
         throw_at_get_origin,
         ...
         throw_never
 };
 for (ThrowWhen* when(throws); *when != throw_never; ++ when) {
         // The body
 }
 }}}

 '''Correctness'''
 The lettuce tests fail for me.

 {{{
  The last query response should have adcount 0
 # features/terrain/querying.py:258
     Traceback (most recent call last):
       File "/home/vorner/.local/lib64/python2.7/site-
 packages/lettuce/core.py", line 117, in __call__
         ret = self.function(self.step, *args, **kw)
       File
 "/home/vorner/work/bind10/tests/lettuce/features/terrain/querying.py",
 line 273, in check_last_query
         "Got: " + str(lq_val) + ", expected: " + str(value)
     AssertionError: Got: 1, expected: 0
     The answer section of the last query response should be
 # features/terrain/querying.py:276
       | """                                               |
       | www.example.org.   3600    IN    A      192.0.2.1 |
       | """                                               |
     A query for example.org type NS should have rcode NOERROR
 # features/terrain/querying.py:204
     The answer section of the last query response should be
 # features/terrain/querying.py:276
       | """                                      |
       | example.org. 3600 IN NS ns1.example.org. |
       | example.org. 3600 IN NS ns2.example.org. |
       | example.org. 3600 IN NS ns3.example.org. |
       | """                                      |
     The SOA serial for example.org should be 1234
 # features/terrain/querying.py:238
     A query for doesnotexist.example.org should have rcode NXDOMAIN
 # features/terrain/querying.py:204
     The last query response should have qdcount 1
 # features/terrain/querying.py:258
     The last query response should have ancount 0
 # features/terrain/querying.py:258
     The last query response should have nscount 1
 # features/terrain/querying.py:258
     The last query response should have adcount 0
 # features/terrain/querying.py:258
     The last query response should have flags qr aa rd
 # features/terrain/querying.py:258
     A query for www.example.org type TXT should have rcode NOERROR
 # features/terrain/querying.py:204
     The last query response should have ancount 0
 # features/terrain/querying.py:258
     A query for www.example.org class CH should have rcode REFUSED
 # features/terrain/querying.py:204
     A query for www.example.org to 127.0.0.1 should have rcode NOERROR
 # features/terrain/querying.py:204
     A query for www.example.org to 127.0.0.1:47806 should have rcode
 NOERROR                       # features/terrain/querying.py:204
     A query for www.example.org type A class IN to 127.0.0.1:47806 should
 have rcode NOERROR       # features/terrain/querying.py:204
 }}}

 Does it fail for you too? How can it be related?

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