[trac910] revised Question::toWire() so that it handles truncation case.

This is not directly related to the subject of this ticket, but
would be necessary to complete the TSIG + TC bit support.

src/lib/dns/python/tests/question_python_test.py

@@ -74,7 +74,6 @@ class QuestionTest(unittest.TestCase):
         self.assertEqual("foo.example.com. IN NS\n", str(self.test_question1))
         self.assertEqual("bar.example.com. CH A\n", self.test_question2.to_text())
     
-    
     def test_to_wire_buffer(self):
         obuffer = bytes()
         obuffer = self.test_question1.to_wire(obuffer)
@@ -82,7 +81,6 @@ class QuestionTest(unittest.TestCase):
         wiredata = read_wire_data("question_toWire1")
         self.assertEqual(obuffer, wiredata)
     
-    
     def test_to_wire_renderer(self):
         renderer = MessageRenderer()
         self.test_question1.to_wire(renderer)
@@ -91,5 +89,13 @@ class QuestionTest(unittest.TestCase):
         self.assertEqual(renderer.get_data(), wiredata)
         self.assertRaises(TypeError, self.test_question1.to_wire, 1)
 
+    def test_to_wire_truncated(self):
+        renderer = MessageRenderer()
+        renderer.set_length_limit(self.example_name1.get_length())
+        self.assertFalse(renderer.is_truncated())
+        self.test_question1.to_wire(renderer)
+        self.assertTrue(renderer.is_truncated())
+        self.assertEqual(0, renderer.get_length())
+
 if __name__ == '__main__':
     unittest.main()

src/lib/dns/question.cc

@@ -57,10 +57,19 @@ Question::toWire(OutputBuffer& buffer) const {
 
 unsigned int
 Question::toWire(AbstractMessageRenderer& renderer) const {
+    const size_t pos0 = renderer.getLength();
+
     renderer.writeName(name_);
     rrtype_.toWire(renderer);
     rrclass_.toWire(renderer);
 
+    // Make sure the renderer has a room for the question
+    if (renderer.getLength() > renderer.getLengthLimit()) {
+        renderer.trim(renderer.getLength() - pos0);
+        renderer.setTruncated();
+        return (0);
+    }
+
     return (1);                 // number of "entries"
 }
 

src/lib/dns/question.h

@@ -201,23 +201,23 @@ public:
     /// class description).
     ///
     /// The owner name will be compressed if possible, although it's an
-    /// unlikely event in practice because the %Question section a DNS
+    /// unlikely event in practice because the Question section a DNS
     /// message normally doesn't contain multiple question entries and
     /// it's located right after the Header section.
     /// Nevertheless, \c renderer records the information of the owner name
     /// so that it can be pointed by other RRs in other sections (which is
     /// more likely to happen).
     ///
-    /// In theory, an attempt to render a Question may cause truncation
-    /// (when the Question section contains a large number of entries),
-    /// but this implementation doesn't catch that situation.
-    /// It would make the code unnecessarily complicated (though perhaps
-    /// slightly) for almost impossible case in practice.
-    /// An upper layer will handle the pathological case as a general error.
+    /// It could be possible, though very rare in practice, that
+    /// an attempt to render a Question may cause truncation
+    /// (when the Question section contains a large number of entries).
+    /// In such a case this method avoid the rendering and indicate the
+    /// truncation in the \c renderer.  This method returns 0 in this case.
     ///
     /// \param renderer DNS message rendering context that encapsulates the
     /// output buffer and name compression information.
-    /// \return 1
+    ///
+    /// \return 1 on success; 0 if it causes truncation
     unsigned int toWire(AbstractMessageRenderer& renderer) const;
 
     /// \brief Render the Question in the wire format without name compression.

src/lib/dns/tests/question_unittest.cc

@@ -106,6 +106,22 @@ TEST_F(QuestionTest, toWireRenderer) {
                         obuffer.getLength(), &wiredata[0], wiredata.size());
 }
 
+TEST_F(QuestionTest, toWireTruncated) {
+    // If the available length in the renderer is too small, it would require
+    // truncation.  This won't happen in normal cases, but protocol wise it
+    // could still happen if and when we support some (possibly future) opcode
+    // that allows multiple questions.
+
+    // Set the length limit to the qname length so that the whole question
+    // would request truncated
+    renderer.setLengthLimit(example_name1.getLength());
+
+    EXPECT_FALSE(renderer.isTruncated()); // check pre-render condition
+    EXPECT_EQ(0, test_question1.toWire(renderer));
+    EXPECT_TRUE(renderer.isTruncated());
+    EXPECT_EQ(0, renderer.getLength()); // renderer shouldn't have any data
+}
+
 // test operator<<.  We simply confirm it appends the result of toText().
 TEST_F(QuestionTest, LeftShiftOperator) {
     ostringstream oss;