[2976] Implemented unit tests for D2UpdateMessage setters.

src/bin/d2/d2_update_message.cc

@@ -25,8 +25,12 @@ using namespace isc::dns;
 
 D2UpdateMessage::D2UpdateMessage(const bool parse)
     : message_(parse ? dns::Message::PARSE : dns::Message::RENDER) {
+    // If this object is to create an outgoing message, we have to
+    // set the proper Opcode field and QR flag here.
     if (!parse) {
         message_.setOpcode(Opcode(Opcode::UPDATE_CODE));
+        message_.setHeaderFlag(dns::Message::HEADERFLAG_QR, false);
+
     }
 }
 
@@ -36,20 +40,14 @@ D2UpdateMessage::getQRFlag() const {
             RESPONSE : REQUEST);
 }
 
-void
-D2UpdateMessage::setQRFlag(const QRFlag flag) {
-    bool on = (flag == RESPONSE ? true : false);
-    message_.setHeaderFlag(dns::Message::HEADERFLAG_QR, on);
-}
-
 uint16_t
-D2UpdateMessage::getQid() const {
+D2UpdateMessage::getId() const {
     return (message_.getQid());
 }
 
 void
-D2UpdateMessage::setQid(const uint16_t qid) {
-    message_.setQid(qid);
+D2UpdateMessage::setId(const uint16_t id) {
+    message_.setQid(id);
 }
 
 
@@ -80,12 +78,16 @@ D2UpdateMessage::endSection(const UpdateMsgSection section) const {
 
 void
 D2UpdateMessage::setZone(const Name& zone, const RRClass& rrclass) {
+    // The Zone data is kept in the underlying Question class. If there
+    // is a record stored there already, we need to remove it, because
+    // we may have at most one Zone record in the DNS Update message.
     if (message_.getRRCount(dns::Message::SECTION_QUESTION) > 0) {
         message_.clearSection(dns::Message::SECTION_QUESTION);
     }
-
+    // Add the new record...
     Question question(zone, rrclass, RRType::SOA());
     message_.addQuestion(question);
+    // ... and update the local class member holding the D2Zone object.
     zone_.reset(new D2Zone(question.getName(), question.getClass()));
 }
 
@@ -98,22 +100,19 @@ void
 D2UpdateMessage::addRRset(const UpdateMsgSection section,
                           const dns::RRsetPtr& rrset) {
     message_.addRRset(ddnsToDnsSection(section), rrset);
-
-}
-
-bool
-D2UpdateMessage::hasRRset(const UpdateMsgSection section, const dns::Name& name,
-                          const dns::RRClass& rrclass, const dns::RRType& rrtype) {
-    return (message_.hasRRset(ddnsToDnsSection(section), name, rrclass, rrtype));
-}
-
-bool
-D2UpdateMessage::hasRRset(const UpdateMsgSection section, const dns::RRsetPtr &rrset) {
-    return (message_.hasRRset(ddnsToDnsSection(section), rrset));
 }
 
 void
 D2UpdateMessage::toWire(AbstractMessageRenderer& renderer) {
+    // We are preparing the wire format of the message, meaning
+    // that this message will be sent as a request to the DNS.
+    // Therefore, we expect that this message is a REQUEST.
+    if (getQRFlag() != REQUEST) {
+        isc_throw(InvalidQRFlag, "QR flag must be cleared for the outgoing"
+                  " DNS Update message");
+    }
+    // According to RFC2136, the ZONE section may contain exactly one
+    // record.
     if (getRRCount(SECTION_ZONE) != 1) {
         isc_throw(InvalidZoneSection, "Zone section of the DNS Update message"
                   " must comprise exactly one record (RFC2136, section 2.3)");
@@ -123,16 +122,36 @@ D2UpdateMessage::toWire(AbstractMessageRenderer& renderer) {
 
 void
 D2UpdateMessage::fromWire(isc::util::InputBuffer& buffer) {
+    // First, use the underlying dns::Message implementation to get the
+    // contents of the DNS response message. Note that it may or may
+    // not be the message that we are interested in, but needs to be
+    // parsed so as we can check its ID, Opcode etc.
     message_.fromWire(buffer);
+    // This class implements exposes the getZone() function. This function
+    // will return pointer to the D2Zone object if non-empty Zone
+    // section exists in the received message. It will return NULL pointer
+    // if it doesn't exist. The pointer is held in the D2UpdateMessage class
+    // member. We need to update this pointer every time we parse the
+    // message.
     if (getRRCount(D2UpdateMessage::SECTION_ZONE) > 0) {
+        // There is a Zone section in the received message. Replace
+        // Zone pointer with the new value.
         QuestionPtr question = *message_.beginQuestion();
         assert(question);
         zone_.reset(new D2Zone(question->getName(), question->getClass()));
 
     } else {
+        // Zone section doesn't hold any pointers, so set the pointer to NULL.
         zone_.reset();
 
     }
+    // Check that the content of the received message is sane.
+    // One of the basic checks to do is to verify that we have
+    // received the DNS update message. If not, it can be dropped
+    // or an error message can be printed. Other than that, we
+    // will check that there is at most one Zone record and QR flag
+    // is set.
+    validate();
 }
 
 dns::Message::Section
@@ -153,6 +172,33 @@ D2UpdateMessage::ddnsToDnsSection(const UpdateMsgSection section) {
               "unknown message section " << section);
 }
 
+void
+D2UpdateMessage::validate() const {
+    // Verify that we are dealing with the DNS Update message. According to
+    // RFC 2136, section 3.8 server will copy the Opcode from the query.
+    // If we are dealing with a different type of message, we may simply
+    // stop further processing, because it is likely that the message was
+    // directed to someone else.
+    if (message_.getOpcode() != Opcode::UPDATE()) {
+        isc_throw(NotUpdateMessage, "received message is not a DDNS update, received"
+                  " message code is " << message_.getOpcode().getCode());
+    }
+    // Received message should have QR flag set, which indicates that it is
+    // a RESPONSE.
+    if (getQRFlag() == REQUEST) {
+        isc_throw(NotUpdateMessage, "received message should should have QR flag set,"
+                  " to indicate that it is a RESPONSE message, the QR flag is unset");
+    }
+    // DNS server may copy a Zone record from the query message. Since query must
+    // comprise exactly one Zone record (RFC 2136, section 2.3), the response message
+    // may contain 1 record at most. It may also contain no records if a server
+    // chooses not to copy Zone section.
+    if (getRRCount(SECTION_ZONE) > 1) {
+        isc_throw(InvalidZoneSection, "received message contains " << getRRCount(SECTION_ZONE)
+                  << " Zone records, it should contain at most 1 record");
+    }
+}
+
 } // namespace d2
 } // namespace isc
 

src/bin/d2/d2_update_message.h

@@ -39,6 +39,30 @@ public:
         isc::Exception(file, line, what) {}
 };
 
+/// @brief Exception indicating that QR flag has invalid value.
+///
+/// This exception is thrown when QR flag has invalid value for
+/// the operation performed on the particular message. For instance,
+/// the QR flag must be set to indicate that the given message is
+/// a RESPONSE when @c D2UpdateMessage::fromWire is performed.
+/// The QR flag must be cleared when @c D2UpdateMessage::toWire
+/// is executed.
+class InvalidQRFlag : public Exception {
+public:
+    InvalidQRFlag(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
+/// @brief Exception indicating that the parsed message is not DNS Update.
+///
+/// This exception is thrown when decoding the DNS message which is not
+/// a DNS Update.
+class NotUpdateMessage : public Exception {
+public:
+    NotUpdateMessage(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
 
 /// @brief The @c D2UpdateMessage encapsulates a DNS Update message.
 ///
@@ -80,11 +104,9 @@ public:
 
     QRFlag getQRFlag() const;
 
-    void setQRFlag(const QRFlag flag);
+    uint16_t getId() const;
 
-    uint16_t getQid() const;
-
-    void setQid(const uint16_t qid);
+    void setId(const uint16_t qid);
 
     const dns::Rcode& getRcode() const;
 
@@ -102,15 +124,6 @@ public:
 
     void addRRset(const UpdateMsgSection section, const dns::RRsetPtr& rrset);
 
-    bool hasRRset(const UpdateMsgSection section, const dns::Name& name,
-                  const dns::RRClass& rrclass, const dns::RRType& rrtype);
-
-    bool hasRRset(const UpdateMsgSection section, const dns::RRsetPtr &rrset);
-
-    void clearSection(const UpdateMsgSection section);
-
-    void clear(const bool parse_mode);
-
     void toWire(dns::AbstractMessageRenderer& renderer);
 
     void fromWire(isc::util::InputBuffer& buffer);
@@ -119,6 +132,7 @@ public:
 private:
 
     static dns::Message::Section ddnsToDnsSection(const UpdateMsgSection section);
+    void validate() const;
 
     dns::Message message_;
     D2ZonePtr zone_;

src/bin/d2/tests/d2_update_message_unittests.cc

@@ -76,6 +76,45 @@ public:
     }
 };
 
+// This test verifies that DNS Update message ID can be set using
+// setId function.
+TEST_F(D2UpdateMessageTest, setId) {
+    D2UpdateMessage msg;
+    EXPECT_EQ(0, msg.getId());
+    msg.setId(0x1234);
+    EXPECT_EQ(0x1234, msg.getId());
+}
+
+// This test verifies that the DNS Update message RCODE can be set
+// using setRcode function.
+TEST_F(D2UpdateMessageTest, setRcode) {
+    D2UpdateMessage msg;
+    msg.setRcode(Rcode::NOERROR());
+    EXPECT_EQ(Rcode::NOERROR().getCode(), msg.getRcode().getCode());
+
+    msg.setRcode(Rcode::NOTIMP());
+    EXPECT_EQ(Rcode::NOTIMP().getCode(), msg.getRcode().getCode());
+}
+
+// This test verifies that the Zone section in the DNS Update message
+// can be set.
+TEST_F(D2UpdateMessageTest, setZone) {
+    D2UpdateMessage msg;
+    D2ZonePtr zone = msg.getZone();
+    EXPECT_FALSE(zone);
+    msg.setZone(Name("example.com"), RRClass::ANY());
+    zone = msg.getZone();
+    EXPECT_TRUE(zone);
+    EXPECT_EQ("example.com.", zone->getName().toText());
+    EXPECT_EQ(RRClass::ANY().getCode(), zone->getClass().getCode());
+
+    msg.setZone(Name("foo.example.com"), RRClass::NONE());
+    zone = msg.getZone();
+    EXPECT_TRUE(zone);
+    EXPECT_EQ("foo.example.com.", zone->getName().toText());
+    EXPECT_EQ(RRClass::NONE().getCode(), zone->getClass().getCode());
+}
+
 // This test verifies that the DNS message is properly decoded from the
 // wire format.
 TEST_F(D2UpdateMessageTest, fromWire) {
@@ -158,7 +197,7 @@ TEST_F(D2UpdateMessageTest, fromWire) {
     ASSERT_NO_THROW(msg.fromWire(buf));
 
     // Check that the message header is valid.
-    EXPECT_EQ(0x05AF, msg.getQid());
+    EXPECT_EQ(0x05AF, msg.getId());
     EXPECT_EQ(D2UpdateMessage::RESPONSE, msg.getQRFlag());
     EXPECT_EQ(Rcode::YXDOMAIN_CODE, msg.getRcode().getCode());
 
@@ -225,9 +264,7 @@ TEST_F(D2UpdateMessageTest, fromWire) {
 TEST_F(D2UpdateMessageTest, toWire) {
     D2UpdateMessage msg;
     // Set message ID.
-    msg.setQid(0x1234);
-    // Make it a Request message by setting the QR flag to 0.
-    msg.setQRFlag(D2UpdateMessage::REQUEST);
+    msg.setId(0x1234);
     // Rcode to NOERROR.
     msg.setRcode(Rcode(Rcode::NOERROR_CODE));