[svn] commit: r3040 - in /branches/trac351/src/lib/dns: message.cc message.h python/message_python.cc python/tests/message_python_test.py tests/message_unittest.cc

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Sep 28 02:16:38 UTC 2010


Author: jinmei
Date: Tue Sep 28 02:16:37 2010
New Revision: 3040

Log:
tighten validation with exceptions, and add more documentation

Modified:
    branches/trac351/src/lib/dns/message.cc
    branches/trac351/src/lib/dns/message.h
    branches/trac351/src/lib/dns/python/message_python.cc
    branches/trac351/src/lib/dns/python/tests/message_python_test.py
    branches/trac351/src/lib/dns/tests/message_unittest.cc

Modified: branches/trac351/src/lib/dns/message.cc
==============================================================================
--- branches/trac351/src/lib/dns/message.cc (original)
+++ branches/trac351/src/lib/dns/message.cc Tue Sep 28 02:16:37 2010
@@ -259,6 +259,9 @@
 
 const Rcode&
 Message::getRcode() const {
+    if (impl_->rcode_ == NULL) {
+        isc_throw(InvalidMessageOperation, "getRcode attempted before set");
+    }
     return (*impl_->rcode_);
 }
 
@@ -273,6 +276,9 @@
 
 const Opcode&
 Message::getOpcode() const {
+    if (impl_->opcode_ == NULL) {
+        isc_throw(InvalidMessageOperation, "getOpcode attempted before set");
+    }
     return (*impl_->opcode_);
 }
 
@@ -424,11 +430,11 @@
     }
     if (impl_->rcode_ == NULL) {
         isc_throw(InvalidMessageOperation,
-                  "Message rendering attempted without Rcode");
+                  "Message rendering attempted without Rcode set");
     }
     if (impl_->opcode_ == NULL) {
         isc_throw(InvalidMessageOperation,
-                  "Message rendering attempted without Opcode");
+                  "Message rendering attempted without Opcode set");
     }
 
     // reserve room for the header
@@ -690,6 +696,15 @@
 
 string
 Message::toText() const {
+    if (impl_->rcode_ == NULL) {
+        isc_throw(InvalidMessageOperation,
+                  "Message::toText() attempted without Rcode set");
+    }
+    if (impl_->opcode_ == NULL) {
+        isc_throw(InvalidMessageOperation,
+                  "Message::toText() attempted without Opcode set");
+    }
+
     string s;
 
     s += ";; ->>HEADER<<- opcode: " + impl_->opcode_->toText();

Modified: branches/trac351/src/lib/dns/message.h
==============================================================================
--- branches/trac351/src/lib/dns/message.h (original)
+++ branches/trac351/src/lib/dns/message.h Tue Sep 28 02:16:37 2010
@@ -378,6 +378,11 @@
     /// included).  In the \c PARSE mode, if the received message contains
     /// an EDNS OPT RR, the corresponding extended code is identified and
     /// returned.
+    ///
+    /// The message must have been properly parsed (in the case of the
+    /// \c PARSE mode) or an \c Rcode has been set (in the case of the
+    /// \c RENDER mode) beforehand.  Otherwise, an exception of class
+    /// \c InvalidMessageOperation will be thrown.
     const Rcode& getRcode() const;
 
     /// \brief Return the Response Code of the message.
@@ -388,6 +393,11 @@
     void setRcode(const Rcode& rcode);
 
     /// \brief Return the OPCODE given in the header section of the message.
+    ///
+    /// The message must have been properly parsed (in the case of the
+    /// \c PARSE mode) or an \c Opcode has been set (in the case of the
+    /// \c RENDER mode) beforehand.  Otherwise, an exception of class
+    /// \c InvalidMessageOperation will be thrown.
     const Opcode& getOpcode() const;
 
     /// \brief Set the OPCODE of the header section of the message.
@@ -466,10 +476,19 @@
     void makeResponse();
 
     /// \brief Convert the Message to a string.
+    ///
+    /// At least \c Opcode and \c Rcode must be validly set in the \c Message
+    /// (as a result of parse in the \c PARSE mode or by explicitly setting
+    /// in the \c RENDER mode);  otherwise, an exception of
+    /// class \c InvalidMessageOperation will be thrown.
     std::string toText() const;
 
     /// \brief Render the message in wire formant into a \c MessageRenderer
     /// object.
+    ///
+    /// This \c Message must be in the \c RENDER mode and both \c Opcode and
+    /// \c Rcode must have been set beforehand; otherwise, an exception of
+    /// class \c InvalidMessageOperation will be thrown.
     void toWire(MessageRenderer& renderer);
 
     /// \brief Parse the header section of the \c Message.

Modified: branches/trac351/src/lib/dns/python/message_python.cc
==============================================================================
--- branches/trac351/src/lib/dns/python/message_python.cc (original)
+++ branches/trac351/src/lib/dns/python/message_python.cc Tue Sep 28 02:16:37 2010
@@ -745,12 +745,18 @@
 
     rcode = static_cast<s_Rcode*>(rcode_type.tp_alloc(&rcode_type, 0));
     if (rcode != NULL) {
-        rcode->rcode = new Rcode(self->message->getRcode());
-        if (rcode->rcode == NULL)
-          {
+        rcode->rcode = NULL;
+        try {
+            rcode->rcode = new Rcode(self->message->getRcode());
+        } catch (const InvalidMessageOperation& imo) {
+            PyErr_SetString(po_InvalidMessageOperation, imo.what());
+        } catch (...) {
+            PyErr_SetString(po_IscException, "Unexpected exception");
+        }
+        if (rcode->rcode == NULL) {
             Py_DECREF(rcode);
             return (NULL);
-          }
+        }
     }
 
     return (rcode);
@@ -777,7 +783,14 @@
 
     opcode = static_cast<s_Opcode*>(opcode_type.tp_alloc(&opcode_type, 0));
     if (opcode != NULL) {
-        opcode->opcode = new Opcode(self->message->getOpcode());
+        opcode->opcode = NULL;
+        try {
+            opcode->opcode = new Opcode(self->message->getOpcode());
+        } catch (const InvalidMessageOperation& imo) {
+            PyErr_SetString(po_InvalidMessageOperation, imo.what());
+        } catch (...) {
+            PyErr_SetString(po_IscException, "Unexpected exception");
+        }
         if (opcode->opcode == NULL) {
             Py_DECREF(opcode);
             return (NULL);
@@ -937,7 +950,16 @@
 static PyObject*
 Message_toText(s_Message* self) {
     // Py_BuildValue makes python objects from native data
-    return (Py_BuildValue("s", self->message->toText().c_str()));
+    try {
+        return (Py_BuildValue("s", self->message->toText().c_str()));
+    } catch (const InvalidMessageOperation& imo) {
+        PyErr_Clear();
+        PyErr_SetString(po_InvalidMessageOperation, imo.what());
+        return (NULL);
+    } catch (...) {
+        PyErr_SetString(po_IscException, "Unexpected exception");
+        return (NULL);
+    }
 }
 
 static PyObject*

Modified: branches/trac351/src/lib/dns/python/tests/message_python_test.py
==============================================================================
--- branches/trac351/src/lib/dns/python/tests/message_python_test.py (original)
+++ branches/trac351/src/lib/dns/python/tests/message_python_test.py Tue Sep 28 02:16:37 2010
@@ -198,6 +198,7 @@
         self.assertRaises(InvalidMessageOperation,
                           self.p.set_rcode, rcode)
         
+        self.assertRaises(InvalidMessageOperation, self.p.get_rcode)
 
     def test_set_opcode(self):
         self.assertRaises(TypeError, self.r.set_opcode, "wrong")
@@ -208,6 +209,8 @@
 
         self.assertRaises(InvalidMessageOperation,
                           self.p.set_opcode, opcode)
+
+        self.assertRaises(InvalidMessageOperation, self.p.get_opcode)
 
     def test_get_section(self):
         self.assertRaises(TypeError, self.r.get_section, "wrong")
@@ -273,6 +276,16 @@
         self.assertEqual(b'\x105\x85\x00\x00\x01\x00\x02\x00\x00\x00\x00\x04test\x07example\x03com\x00\x00\x01\x00\x01\xc0\x0c\x00\x01\x00\x01\x00\x00\x0e\x10\x00\x04\xc0\x00\x02\x01\xc0\x0c\x00\x01\x00\x01\x00\x00\x0e\x10\x00\x04\xc0\x00\x02\x02',
                          renderer.get_data())
 
+    def test_to_wire_without_opcode(self):
+        self.r.set_rcode(Rcode.NOERROR())
+        self.assertRaises(InvalidMessageOperation, self.r.to_wire,
+                          MessageRenderer())
+
+    def test_to_wire_without_rcode(self):
+        self.r.set_opcode(Opcode.QUERY())
+        self.assertRaises(InvalidMessageOperation, self.r.to_wire,
+                          MessageRenderer())
+
     def test_to_text(self):
         message_render = create_message()
         
@@ -289,6 +302,14 @@
 """
         self.assertEqual(msg_str, message_render.to_text())
         self.assertEqual(msg_str, str(message_render))
+
+    def test_to_text_without_opcode(self):
+        self.r.set_rcode(Rcode.NOERROR())
+        self.assertRaises(InvalidMessageOperation, self.r.to_text)
+
+    def test_to_text_without_rcode(self):
+        self.r.set_opcode(Opcode.QUERY())
+        self.assertRaises(InvalidMessageOperation, self.r.to_text)
 
     def test_from_wire(self):
         self.assertRaises(TypeError, self.r.from_wire, 1)

Modified: branches/trac351/src/lib/dns/tests/message_unittest.cc
==============================================================================
--- branches/trac351/src/lib/dns/tests/message_unittest.cc (original)
+++ branches/trac351/src/lib/dns/tests/message_unittest.cc Tue Sep 28 02:16:37 2010
@@ -111,6 +111,22 @@
     EXPECT_TRUE(it->isLast());
 }
 
+TEST_F(MessageTest, opcode) {    // for get/setOpcode
+    EXPECT_THROW(message_parse.setOpcode(Opcode::NOTIFY()),
+                 InvalidMessageOperation);
+    message_render.setOpcode(Opcode::UPDATE());
+    EXPECT_EQ(Opcode::UPDATE(), message_render.getOpcode());
+    EXPECT_THROW(message_parse.getOpcode(), InvalidMessageOperation);
+}
+
+TEST_F(MessageTest, rcode) {    // for get/setRcode
+    EXPECT_THROW(message_parse.setRcode(Rcode::BADVERS()),
+                 InvalidMessageOperation);
+    message_render.setRcode(Rcode::BADVERS());
+    EXPECT_EQ(Rcode::BADVERS(), message_render.getRcode());
+    EXPECT_THROW(message_parse.getRcode(), InvalidMessageOperation);
+}
+
 TEST_F(MessageTest, GetEDNS0DOBit) {
     // Without EDNS0, DNSSEC is considered to be unsupported.
     factoryFromFile(message_parse, "message_fromWire1");
@@ -246,4 +262,29 @@
     EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, obuffer.getData(),
                         obuffer.getLength(), &data[0], data.size());
 }
-}
+
+TEST_F(MessageTest, toWireInParseMode) {
+    // toWire() isn't allowed in the parse mode.
+    EXPECT_THROW(message_parse.toWire(renderer), InvalidMessageOperation);
+}
+
+TEST_F(MessageTest, toWireWithoutOpcode) {
+    message_render.setRcode(Rcode::NOERROR());
+    EXPECT_THROW(message_render.toWire(renderer), InvalidMessageOperation);
+}
+
+TEST_F(MessageTest, toWireWithoutRcode) {
+    message_render.setOpcode(Opcode::QUERY());
+    EXPECT_THROW(message_render.toWire(renderer), InvalidMessageOperation);
+}
+
+TEST_F(MessageTest, toTextWithoutOpcode) {
+    message_render.setRcode(Rcode::NOERROR());
+    EXPECT_THROW(message_render.toText(), InvalidMessageOperation);
+}
+
+TEST_F(MessageTest, toTextWithoutRcode) {
+    message_render.setOpcode(Opcode::QUERY());
+    EXPECT_THROW(message_render.toText(), InvalidMessageOperation);
+}
+}




More information about the bind10-changes mailing list