Browse Source

[1357] Clear the renderer on Message::toWire()

Because of the jumping back and forward, the renderer needs to be cleared at the start of toWire(). Length Limit and Compression mode that were set are kept.

This fixes an uninitialized memory error.
Jelte Jansen 12 years ago
parent
commit
c0d2f860c8

+ 9 - 0
src/lib/dns/message.cc

@@ -254,6 +254,13 @@ MessageImpl::toWire(AbstractMessageRenderer& renderer, TSIGContext* tsig_ctx) {
     const size_t orig_msg_len_limit = renderer.getLengthLimit();
     const AbstractMessageRenderer::CompressMode orig_compress_mode =
         renderer.getCompressMode();
+
+    // We are going to skip soon, so we need to clear the renderer
+    // But we'll leave the length limit  and the compress mode intact
+    // (or shortened in case of TSIG)
+    renderer.clear();
+    renderer.setCompressMode(orig_compress_mode);
+
     if (tsig_len > 0) {
         if (tsig_len > orig_msg_len_limit) {
             isc_throw(InvalidParameter, "Failed to render DNS message: "
@@ -261,6 +268,8 @@ MessageImpl::toWire(AbstractMessageRenderer& renderer, TSIGContext* tsig_ctx) {
                       orig_msg_len_limit << ")");
         }
         renderer.setLengthLimit(orig_msg_len_limit - tsig_len);
+    } else {
+        renderer.setLengthLimit(orig_msg_len_limit);
     }
 
     // reserve room for the header

+ 8 - 0
src/lib/dns/message.h

@@ -556,6 +556,10 @@ public:
     /// \c Rcode must have been set beforehand; otherwise, an exception of
     /// class \c InvalidMessageOperation will be thrown.
     ///
+    /// \note The renderer's internal buffers and data are automatically
+    /// cleared, keeping the length limit and the compression mode intact.
+    /// In case truncation is triggered, the renderer is cleared completely.
+    ///
     /// \param renderer DNS message rendering context that encapsulates the
     /// output buffer and name compression information.
     void toWire(AbstractMessageRenderer& renderer);
@@ -581,6 +585,10 @@ public:
     /// it should mean a bug either in the TSIG context or in the renderer
     /// implementation.
     ///
+    /// \note The renderer's internal buffers and data are automatically
+    /// cleared, keeping the length limit and the compression mode intact.
+    /// In case truncation is triggered, the renderer is cleared completely.
+    ///
     /// \param renderer See the other version
     /// \param tsig_ctx A TSIG context that is to be used for signing the
     /// message

+ 7 - 7
src/lib/dns/python/tests/message_python_test.py

@@ -453,7 +453,7 @@ class MessageTest(unittest.TestCase):
 
     def test_to_text(self):
         message_render = create_message()
-        
+
         msg_str =\
 """;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 4149
 ;; flags: qr aa rd; QUERY: 1, ANSWER: 2, AUTHORITY: 0, ADDITIONAL: 0
@@ -484,7 +484,7 @@ test.example.com. 3600 IN A 192.0.2.2
                           Message.from_wire, self.p, bytes())
 
         test_name = Name("test.example.com");
-        
+
         message_parse = Message(0)
         factoryFromFile(message_parse, "message_fromWire1")
         self.assertEqual(0x1035, message_parse.get_qid())
@@ -493,7 +493,7 @@ test.example.com. 3600 IN A 192.0.2.2
         self.assertTrue(message_parse.get_header_flag(Message.HEADERFLAG_QR))
         self.assertTrue(message_parse.get_header_flag(Message.HEADERFLAG_RD))
         self.assertTrue(message_parse.get_header_flag(Message.HEADERFLAG_AA))
-    
+
         #QuestionPtr q = *message_parse.beginQuestion()
         q = message_parse.get_question()[0]
         self.assertEqual(test_name, q.get_name())
@@ -503,7 +503,7 @@ test.example.com. 3600 IN A 192.0.2.2
         self.assertEqual(2, message_parse.get_rr_count(Message.SECTION_ANSWER))
         self.assertEqual(0, message_parse.get_rr_count(Message.SECTION_AUTHORITY))
         self.assertEqual(0, message_parse.get_rr_count(Message.SECTION_ADDITIONAL))
-    
+
         #RRsetPtr rrset = *message_parse.beginSection(Message.SECTION_ANSWER)
         rrset = message_parse.get_section(Message.SECTION_ANSWER)[0]
         self.assertEqual(test_name, rrset.get_name())
@@ -569,12 +569,12 @@ test.example.com. 3600 IN A 192.0.2.2
         message_parse = Message(Message.PARSE)
         factoryFromFile(message_parse, "message_fromWire10.wire")
         self.assertEqual(Rcode.BADVERS(), message_parse.get_rcode())
-    
+
         # Maximum extended Rcode
         message_parse.clear(Message.PARSE)
         factoryFromFile(message_parse, "message_fromWire11.wire")
         self.assertEqual(0xfff, message_parse.get_rcode().get_code())
-    
+
     def test_BadEDNS0(self):
         message_parse = Message(Message.PARSE)
         # OPT RR in the answer section
@@ -596,7 +596,7 @@ test.example.com. 3600 IN A 192.0.2.2
                           factoryFromFile,
                           message_parse,
                           "message_fromWire6")
-                          
+
         # Compressed owner name of OPT RR points to a root name.
         # Not necessarily bogus, but very unusual and mostly pathological.
         # We accept it, but is it okay?

+ 2 - 2
src/lib/dns/tests/message_unittest.cc

@@ -534,7 +534,7 @@ TEST_F(MessageTest, appendSection) {
         RRClass::IN(), RRType::A()));
     EXPECT_TRUE(target.hasRRset(Message::SECTION_ANSWER, test_name,
         RRClass::IN(), RRType::AAAA()));
-    
+
 }
 
 TEST_F(MessageTest, parseHeader) {
@@ -1091,7 +1091,7 @@ TEST_F(MessageTest, toWireWithoutRcode) {
 TEST_F(MessageTest, toText) {
     // Check toText() output for a typical DNS response with records in
     // all sections
-    
+
     factoryFromFile(message_parse, "message_toText1.wire");
     {
         SCOPED_TRACE("Message toText test (basic case)");