BIND 10 trac2382, updated. 816eacf47e4168d336c2f64b1b621f13f8ca6cb4 [2382] throw unexpected instead of assert on unexpected token in createRdata.

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Dec 4 17:28:37 UTC 2012


The branch, trac2382 has been updated
       via  816eacf47e4168d336c2f64b1b621f13f8ca6cb4 (commit)
       via  09d70952ca89ef967f8652046ed6fe869625b42e (commit)
       via  6944e7758d326b69482bcfd7d3f501df5113b394 (commit)
       via  970ee33b88d3268780fc4a7d8950ba2a4a8202fd (commit)
      from  5d239b6aed71f987330add0bbcfc7baed670e804 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 816eacf47e4168d336c2f64b1b621f13f8ca6cb4
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue Dec 4 09:27:49 2012 -0800

    [2382] throw unexpected instead of assert on unexpected token in createRdata.
    
    assert() should be okay, but that depends on details of other classes, so
    exception is probably a bit safer.

commit 09d70952ca89ef967f8652046ed6fe869625b42e
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue Dec 4 09:22:44 2012 -0800

    [2382] added one suggested test: a case where RDATA is followed by comment

commit 6944e7758d326b69482bcfd7d3f501df5113b394
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue Dec 4 09:18:03 2012 -0800

    [2382] unify pushing '\0' for number-like data in Number::handle()

commit 970ee33b88d3268780fc4a7d8950ba2a4a8202fd
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue Dec 4 09:10:56 2012 -0800

    [2382] corrected typo in doc for createRdata()

-----------------------------------------------------------------------

Summary of changes:
 src/lib/dns/master_lexer.cc         |    6 +++---
 src/lib/dns/rdata.cc                |   31 +++++++++++++++++++------------
 src/lib/dns/rdata.h                 |    2 +-
 src/lib/dns/tests/rdata_unittest.cc |   28 +++++++++++++++++++++++-----
 4 files changed, 46 insertions(+), 21 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc
index 4593b48..b3b78c0 100644
--- a/src/lib/dns/master_lexer.cc
+++ b/src/lib/dns/master_lexer.cc
@@ -522,9 +522,10 @@ Number::handle(MasterLexer& lexer) const {
             getLexerImpl(lexer)->source_->getChar(), escaped);
         if (getLexerImpl(lexer)->isTokenEnd(c, escaped)) {
             getLexerImpl(lexer)->source_->ungetChar();
+            // We need to close the string whether it's digits-only (for
+            // lexical_cast) or not (see String::handle()).
+            data.push_back('\0');
             if (digits_only) {
-                // Close the string for lexical_cast
-                data.push_back('\0');
                 try {
                     const uint32_t number32 =
                         boost::lexical_cast<uint32_t, const char*>(&data[0]);
@@ -535,7 +536,6 @@ Number::handle(MasterLexer& lexer) const {
                     token = MasterToken(MasterToken::NUMBER_OUT_OF_RANGE);
                 }
             } else {
-                data.push_back('\0'); // see String::handle()
                 token = MasterToken(&data.at(0), data.size() - 1);
             }
             return;
diff --git a/src/lib/dns/rdata.cc b/src/lib/dns/rdata.cc
index 9e96378..081f855 100644
--- a/src/lib/dns/rdata.cc
+++ b/src/lib/dns/rdata.cc
@@ -12,6 +12,20 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <exceptions/exceptions.h>
+
+#include <util/buffer.h>
+
+#include <dns/name.h>
+#include <dns/messagerenderer.h>
+#include <dns/master_lexer.h>
+#include <dns/rdata.h>
+#include <dns/rrparamregistry.h>
+#include <dns/rrtype.h>
+
+#include <boost/lexical_cast.hpp>
+#include <boost/shared_ptr.hpp>
+
 #include <algorithm>
 #include <cctype>
 #include <string>
@@ -24,17 +38,6 @@
 #include <stdint.h>
 #include <string.h>
 
-#include <boost/lexical_cast.hpp>
-#include <boost/shared_ptr.hpp>
-
-#include <util/buffer.h>
-#include <dns/name.h>
-#include <dns/messagerenderer.h>
-#include <dns/master_lexer.h>
-#include <dns/rdata.h>
-#include <dns/rrparamregistry.h>
-#include <dns/rrtype.h>
-
 using namespace std;
 using boost::lexical_cast;
 using namespace isc::util;
@@ -113,7 +116,11 @@ fromtextError(bool& error_issued, const MasterLexer& lexer,
                         token->getErrorText());
         break;
     default:
-        assert(false);
+        // This case shouldn't happen based on how we use MasterLexer in
+        // createRdata(), so we could assert() that here.  But since it
+        // depends on detailed behavior of other classes, we treat the case
+        // in a bit less harsh way.
+        isc_throw(Unexpected, "bug: createRdata() saw unexpected token type");
     }
 }
 }
diff --git a/src/lib/dns/rdata.h b/src/lib/dns/rdata.h
index 186ace2..4cd63cc 100644
--- a/src/lib/dns/rdata.h
+++ b/src/lib/dns/rdata.h
@@ -502,7 +502,7 @@ RdataPtr createRdata(const RRType& rrtype, const RRClass& rrclass,
 /// most of syntax and semantics errors of the input (reported as exceptions),
 /// calls the corresponding callback specified by the \c callbacks parameters,
 /// and returns a NULL smart pointer.  If the caller rather wants to get
-/// an exception in these cases, it can use pass a callback that internally
+/// an exception in these cases, it can pass a callback that internally
 /// throws on error.  Some critical exceptions such as \c std::bad_alloc are
 /// still propagated to the upper layer as it doesn't make sense to try
 /// recovery from such a situation within this function.
diff --git a/src/lib/dns/tests/rdata_unittest.cc b/src/lib/dns/tests/rdata_unittest.cc
index bb39d05..7f0dd65 100644
--- a/src/lib/dns/tests/rdata_unittest.cc
+++ b/src/lib/dns/tests/rdata_unittest.cc
@@ -132,6 +132,7 @@ TEST_F(RdataTest, createRdataWithLexer) {
     stringstream ss;
     const string src_name = "stream-" + boost::lexical_cast<string>(&ss);
     ss << aaaa_rdata.toText() << "\n"; // valid case
+    ss << aaaa_rdata.toText() << "; comment, should be ignored\n";
     ss << aaaa_rdata.toText() << " extra-token\n"; // extra token
     ss << aaaa_rdata.toText() << " extra token\n"; // 2 extra tokens
     ss << ")\n"; // causing lexer error in parsing the RDATA text
@@ -146,54 +147,71 @@ TEST_F(RdataTest, createRdataWithLexer) {
         boost::bind(&CreateRdataCallback::callback, &callback,
                     CreateRdataCallback::WARN,  _1, _2, _3));
 
+    size_t line = 0;
+
     // Valid case.
+    ++line;
     ConstRdataPtr rdata = createRdata(RRType::AAAA(), RRClass::IN(), lexer,
                                       NULL, MasterLoader::MANY_ERRORS,
                                       callbacks);
     EXPECT_EQ(0, aaaa_rdata.compare(*rdata));
     EXPECT_FALSE(callback.isCalled());
 
+    // Similar to the previous case, but RDATA is followed by a comment.
+    // It should cause any confusion.
+    ++line;
+    callback.clear();
+    rdata = createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL,
+                        MasterLoader::MANY_ERRORS, callbacks);
+    EXPECT_EQ(0, aaaa_rdata.compare(*rdata));
+    EXPECT_FALSE(callback.isCalled());
+
     // Broken RDATA text: extra token.  createRdata() returns NULL, error
     // callback is called.
+    ++line;
     callback.clear();
     EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL,
                              MasterLoader::MANY_ERRORS, callbacks));
-    callback.check(src_name, 2, CreateRdataCallback::ERROR,
+    callback.check(src_name, line, CreateRdataCallback::ERROR,
                    "createRdata from text failed near 'extra-token': "
                    "extra input text");
 
     // Similar to the previous case, but only the first extra token triggers
     // callback.
+    ++line;
     callback.clear();
     EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL,
                              MasterLoader::MANY_ERRORS, callbacks));
-    callback.check(src_name, 3, CreateRdataCallback::ERROR,
+    callback.check(src_name, line, CreateRdataCallback::ERROR,
                    "createRdata from text failed near 'extra': "
                    "extra input text");
 
     // Lexer error will happen, corresponding error callback will be triggered.
+    ++line;
     callback.clear();
     EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL,
                              MasterLoader::MANY_ERRORS, callbacks));
-    callback.check(src_name, 4, CreateRdataCallback::ERROR,
+    callback.check(src_name, line, CreateRdataCallback::ERROR,
                    "createRdata from text failed: unbalanced parentheses");
 
     // Semantics level error will happen, corresponding error callback will be
     // triggered.
+    ++line;
     callback.clear();
     EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL,
                              MasterLoader::MANY_ERRORS, callbacks));
-    callback.check(src_name, 5, CreateRdataCallback::ERROR,
+    callback.check(src_name, line, CreateRdataCallback::ERROR,
                    "createRdata from text failed: Failed to convert "
                    "'192.0.2.1' to IN/AAAA RDATA");
 
     // Input is valid and parse will succeed, but with a warning that the
     // file is not ended with a newline.
+    ++line;
     callback.clear();
     rdata = createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL,
                         MasterLoader::MANY_ERRORS, callbacks);
     EXPECT_EQ(0, aaaa_rdata.compare(*rdata));
-    callback.check(src_name, 6, CreateRdataCallback::WARN,
+    callback.check(src_name, line, CreateRdataCallback::WARN,
                    "file does not end with newline");
 }
 



More information about the bind10-changes mailing list