BIND 10 #1389: xfrout should allow a message with size of 65535

[BIND 10 Development]

#1389: xfrout should allow a message with size of 65535
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:         |             Milestone:
  defect                             |  Sprint-20111206
                   Priority:  major  |            Resolution:
                  Component:         |             Sensitive:  0
  xfrout                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  2
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:4 jinmei]:
 > On closer look, I found that the code has (at least potentially)
 > more substantial bugs: it didn't take into account the size of
 > the question section, and TSIG len could be counted twice in some
 > cases.  Still, it's less likely to have caused real harm due to
 > name compression, but there was possibility to create some bogus
 > responses.  (I believe) I've fixed all these problems as well as
 > the original issue of 'off-by-one' bug.
 >
 > I also made a couple of unrelated small fixes/cleanups (the first and
 > last commits).

 From a design point of view, I believe this is still suboptimal, due to
 the name compression. If we compress the names, more RRs could fit in. If
 we repeatedly tried to add an RR and render the message until it
 overflows, we would get the biggest one possible, but it doesn't look very
 optimal. Maybe if there was some kind of iterative message rendering.
 Anyway, that is out of the scope of this ticket. Should I create a new one
 or did we already talked about this?

 The code looks sane, but the tests don't pass for me. But I'm not sure if
 it is related to this ticket:
 {{{
 ======================================================================
 FAIL: test_axfr_normal_session (__main__.TestXfroutSessionWithSQLite3)
 ----------------------------------------------------------------------
 Traceback (most recent call last):
   File "/home/vorner/work/bind10/src/bin/xfrout/tests/xfrout_test.py",
 line 1061, in test_axfr_normal_session
     self.check_axfr_stream(response)
   File "/home/vorner/work/bind10/src/bin/xfrout/tests/xfrout_test.py",
 line 1055, in check_axfr_stream
     self.assertTrue(rrsets_equal(expected_rr, actual_rr))
 AssertionError: False is not true

 ======================================================================
 FAIL: test_ixfr_to_axfr (__main__.TestXfroutSessionWithSQLite3)
 ----------------------------------------------------------------------
 Traceback (most recent call last):
   File "/home/vorner/work/bind10/src/bin/xfrout/tests/xfrout_test.py",
 line 1072, in test_ixfr_to_axfr
     self.check_axfr_stream(response)
   File "/home/vorner/work/bind10/src/bin/xfrout/tests/xfrout_test.py",
 line 1055, in check_axfr_stream
     self.assertTrue(rrsets_equal(expected_rr, actual_rr))
 AssertionError: False is not true

 ----------------------------------------------------------------------
 }}}

 > {{{
 > 333.  [bug]           jinmei
 >       b10-xfrout could potentially create an overflow response message
 >       (exceeding the 64KB max) or could create unnecessarily small
 >       messages.  The former was actually unlikely to happen due to the
 >       effect of name compression, and the latter was marginal and at
 least
 >       shouldn't cause an interoperability problem, but these were still
 >       potential problems and should be fixed.
 >       (Trac #1389, git TBD)
 > }}}

 Shouldn't it be „potential problems and were fixed“? Because this would
 look like as the problem is still there and we are asking someone to fix
 it for us.

 Thanks

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