Browse Source

[1258] added support for the PRESERVE_ORDER option for Message::fromWire().

JINMEI Tatuya 13 years ago
parent
commit
c943619d22

+ 24 - 19
src/lib/dns/message.cc

@@ -124,10 +124,12 @@ public:
     void setOpcode(const Opcode& opcode);
     void setRcode(const Rcode& rcode);
     int parseQuestion(InputBuffer& buffer);
-    int parseSection(const Message::Section section, InputBuffer& buffer);
+    int parseSection(const Message::Section section, InputBuffer& buffer,
+                     Message::ParseOptions options);
     void addRR(Message::Section section, const Name& name,
                const RRClass& rrclass, const RRType& rrtype,
-               const RRTTL& ttl, ConstRdataPtr rdata);
+               const RRTTL& ttl, ConstRdataPtr rdata,
+               Message::ParseOptions options);
     void addEDNS(Message::Section section, const Name& name,
                  const RRClass& rrclass, const RRType& rrtype,
                  const RRTTL& ttl, const Rdata& rdata);
@@ -614,7 +616,7 @@ Message::parseHeader(InputBuffer& buffer) {
 }
 
 void
-Message::fromWire(InputBuffer& buffer) {
+Message::fromWire(InputBuffer& buffer, ParseOptions options) {
     if (impl_->mode_ != Message::PARSE) {
         isc_throw(InvalidMessageOperation,
                   "Message parse attempted in non parse mode");
@@ -626,11 +628,11 @@ Message::fromWire(InputBuffer& buffer) {
 
     impl_->counts_[SECTION_QUESTION] = impl_->parseQuestion(buffer);
     impl_->counts_[SECTION_ANSWER] =
-        impl_->parseSection(SECTION_ANSWER, buffer);
+        impl_->parseSection(SECTION_ANSWER, buffer, options);
     impl_->counts_[SECTION_AUTHORITY] =
-        impl_->parseSection(SECTION_AUTHORITY, buffer);
+        impl_->parseSection(SECTION_AUTHORITY, buffer, options);
     impl_->counts_[SECTION_ADDITIONAL] =
-        impl_->parseSection(SECTION_ADDITIONAL, buffer);
+        impl_->parseSection(SECTION_ADDITIONAL, buffer, options);
 }
 
 int
@@ -706,7 +708,7 @@ struct MatchRR : public unary_function<RRsetPtr, bool> {
 // is hardcoded here.
 int
 MessageImpl::parseSection(const Message::Section section,
-                          InputBuffer& buffer)
+                          InputBuffer& buffer, Message::ParseOptions options)
 {
     assert(section < MessageImpl::NUM_SECTIONS);
 
@@ -738,7 +740,7 @@ MessageImpl::parseSection(const Message::Section section,
             addTSIG(section, count, buffer, start_position, name, rrclass, ttl,
                     *rdata);
         } else {
-            addRR(section, name, rrclass, rrtype, ttl, rdata);
+            addRR(section, name, rrclass, rrtype, ttl, rdata, options);
             ++added;
         }
     }
@@ -749,19 +751,22 @@ MessageImpl::parseSection(const Message::Section section,
 void
 MessageImpl::addRR(Message::Section section, const Name& name,
                    const RRClass& rrclass, const RRType& rrtype,
-                   const RRTTL& ttl, ConstRdataPtr rdata)
+                   const RRTTL& ttl, ConstRdataPtr rdata,
+                   Message::ParseOptions options)
 {
-    vector<RRsetPtr>::iterator it =
-        find_if(rrsets_[section].begin(), rrsets_[section].end(),
-                MatchRR(name, rrtype, rrclass));
-    if (it != rrsets_[section].end()) {
-        (*it)->setTTL(min((*it)->getTTL(), ttl));
-        (*it)->addRdata(rdata);
-    } else {
-        RRsetPtr rrset(new RRset(name, rrclass, rrtype, ttl));
-        rrset->addRdata(rdata);
-        rrsets_[section].push_back(rrset);
+    if ((options & Message::PRESERVE_ORDER) == 0) {
+        vector<RRsetPtr>::iterator it =
+            find_if(rrsets_[section].begin(), rrsets_[section].end(),
+                    MatchRR(name, rrtype, rrclass));
+        if (it != rrsets_[section].end()) {
+            (*it)->setTTL(min((*it)->getTTL(), ttl));
+            (*it)->addRdata(rdata);
+            return;
+        }
     }
+    RRsetPtr rrset(new RRset(name, rrclass, rrtype, ttl));
+    rrset->addRdata(rdata);
+    rrsets_[section].push_back(rrset);
 }
 
 void

+ 16 - 1
src/lib/dns/message.h

@@ -581,11 +581,26 @@ public:
     /// message
     void toWire(AbstractMessageRenderer& renderer, TSIGContext& tsig_ctx);
 
+    /// Parse options.
+    ///
+    /// describe PRESERVE_ORDER: note doesn't affect EDNS or TSIG.
+    ///
+    /// The option values are used as a parameter for \c fromWire().
+    /// These are values of a bitmask type.  Bitwise operations can be
+    /// performed on these values to express compound options.
+    enum ParseOptions {
+        PARSE_DEFAULT = 0,       ///< The default options
+        PRESERVE_ORDER = 1       ///< Preserve RR order and don't combining
+    };
+
     /// \brief Parse the header section of the \c Message.
     void parseHeader(isc::util::InputBuffer& buffer);
 
     /// \brief Parse the \c Message.
-    void fromWire(isc::util::InputBuffer& buffer);
+    ///
+    /// \param buffer
+    void fromWire(isc::util::InputBuffer& buffer, ParseOptions options
+        = PARSE_DEFAULT);
 
     ///
     /// \name Protocol constants

+ 58 - 0
src/lib/dns/tests/message_unittest.cc

@@ -622,6 +622,64 @@ TEST_F(MessageTest, fromWireCombineRRs) {
     EXPECT_EQ(1, (*it)->getRdataCount());
 }
 
+// A helper function for a test pattern commonly used in several tests below.
+void
+preserveRRCheck(const Message& message, Message::Section section) {
+    RRsetIterator it = message.beginSection(section);
+    RRsetIterator it_end = message.endSection(section);
+    ASSERT_TRUE(it != it_end);
+    EXPECT_EQ(RRType::A(), (*it)->getType());
+    EXPECT_EQ(1, (*it)->getRdataCount());
+    EXPECT_EQ("192.0.2.1", (*it)->getRdataIterator()->getCurrent().toText());
+
+    ++it;
+    ASSERT_TRUE(it != it_end);
+    EXPECT_EQ(RRType::AAAA(), (*it)->getType());
+    EXPECT_EQ(1, (*it)->getRdataCount());
+    EXPECT_EQ("2001:db8::1", (*it)->getRdataIterator()->getCurrent().toText());
+
+    ++it;
+    ASSERT_TRUE(it != it_end);
+    EXPECT_EQ(RRType::A(), (*it)->getType());
+    EXPECT_EQ(1, (*it)->getRdataCount());
+    EXPECT_EQ("192.0.2.2", (*it)->getRdataIterator()->getCurrent().toText());
+}
+
+TEST_F(MessageTest, fromWirePreserveAnswer) {
+    // Using the same data as the previous test, but specify the PRESERVE_ORDER
+    // option.  The received order of RRs should be preserved, and each RR
+    // should be stored in a single RRset.
+    UnitTestUtil::readWireData("message_fromWire19.wire", received_data);
+    InputBuffer buffer(&received_data[0], received_data.size());
+    message_parse.fromWire(buffer, Message::PRESERVE_ORDER);
+    {
+        SCOPED_TRACE("preserve answer RRs");
+        preserveRRCheck(message_parse, Message::SECTION_ANSWER);
+    }
+}
+
+TEST_F(MessageTest, fromWirePreserveAuthority) {
+    // Same for the previous test, but for the authority section.
+    UnitTestUtil::readWireData("message_fromWire20.wire", received_data);
+    InputBuffer buffer(&received_data[0], received_data.size());
+    message_parse.fromWire(buffer, Message::PRESERVE_ORDER);
+    {
+        SCOPED_TRACE("preserve authority RRs");
+        preserveRRCheck(message_parse, Message::SECTION_AUTHORITY);
+    }
+}
+
+TEST_F(MessageTest, fromWirePreserveAdditional) {
+    // Same for the previous test, but for the additional section.
+    UnitTestUtil::readWireData("message_fromWire21.wire", received_data);
+    InputBuffer buffer(&received_data[0], received_data.size());
+    message_parse.fromWire(buffer, Message::PRESERVE_ORDER);
+    {
+        SCOPED_TRACE("preserve additional RRs");
+        preserveRRCheck(message_parse, Message::SECTION_ADDITIONAL);
+    }
+}
+
 TEST_F(MessageTest, EDNS0ExtRcode) {
     // Extended Rcode = BADVERS
     factoryFromFile(message_parse, "message_fromWire10.wire");

+ 4 - 1
src/lib/dns/tests/testdata/Makefile.am

@@ -7,6 +7,8 @@ BUILT_SOURCES += message_fromWire12.wire message_fromWire13.wire
 BUILT_SOURCES += message_fromWire14.wire message_fromWire15.wire
 BUILT_SOURCES += message_fromWire16.wire message_fromWire17.wire
 BUILT_SOURCES += message_fromWire18.wire message_fromWire19.wire
+BUILT_SOURCES += message_fromWire19.wire message_fromWire20.wire
+BUILT_SOURCES += message_fromWire21.wire
 BUILT_SOURCES += message_toWire2.wire message_toWire3.wire
 BUILT_SOURCES += message_toWire4.wire message_toWire5.wire
 BUILT_SOURCES += message_toText1.wire message_toText2.wire
@@ -71,7 +73,8 @@ EXTRA_DIST += message_fromWire11.spec message_fromWire12.spec
 EXTRA_DIST += message_fromWire13.spec message_fromWire14.spec
 EXTRA_DIST += message_fromWire15.spec message_fromWire16.spec
 EXTRA_DIST += message_fromWire17.spec message_fromWire18.spec
-EXTRA_DIST += message_fromWire19.spec
+EXTRA_DIST += message_fromWire19.spec message_fromWire20.spec
+EXTRA_DIST += message_fromWire21.spec
 EXTRA_DIST += message_toWire1 message_toWire2.spec message_toWire3.spec
 EXTRA_DIST += message_toWire4.spec message_toWire5.spec
 EXTRA_DIST += message_toText1.txt message_toText1.spec

+ 20 - 0
src/lib/dns/tests/testdata/message_fromWire19.spec

@@ -0,0 +1,20 @@
+#
+# A non realistic DNS response message containing mixed types of RRs in the
+# answer section in a mixed order.
+#
+
+[custom]
+sections: header:question:a/1:aaaa:a/2
+[header]
+qr: 1
+ancount: 3
+[question]
+name: www.example.com
+rrtype: A
+[a/1]
+as_rr: True
+[aaaa]
+as_rr: True
+[a/2]
+as_rr: True
+address: 192.0.2.2

+ 20 - 0
src/lib/dns/tests/testdata/message_fromWire20.spec

@@ -0,0 +1,20 @@
+#
+# A non realistic DNS response message containing mixed types of RRs in the
+# authority section in a mixed order.
+#
+
+[custom]
+sections: header:question:a/1:aaaa:a/2
+[header]
+qr: 1
+nscount: 3
+[question]
+name: www.example.com
+rrtype: A
+[a/1]
+as_rr: True
+[aaaa]
+as_rr: True
+[a/2]
+as_rr: True
+address: 192.0.2.2

+ 20 - 0
src/lib/dns/tests/testdata/message_fromWire21.spec

@@ -0,0 +1,20 @@
+#
+# A non realistic DNS response message containing mixed types of RRs in the
+# additional section in a mixed order.
+#
+
+[custom]
+sections: header:question:a/1:aaaa:a/2
+[header]
+qr: 1
+arcount: 3
+[question]
+name: www.example.com
+rrtype: A
+[a/1]
+as_rr: True
+[aaaa]
+as_rr: True
+[a/2]
+as_rr: True
+address: 192.0.2.2