Browse Source

[2382] handle other generic isc::Exception

JINMEI Tatuya 12 years ago
parent
commit
0ad163bb37
2 changed files with 88 additions and 12 deletions
  1. 51 12
      src/lib/dns/rdata.cc
  2. 37 0
      src/lib/dns/tests/rdata_unittest.cc

+ 51 - 12
src/lib/dns/rdata.cc

@@ -84,15 +84,33 @@ createRdata(const RRType& rrtype, const RRClass& rrclass, const Rdata& source)
 
 
 namespace {
 namespace {
 void
 void
-fromtextError(const MasterLexer& lexer, MasterLoaderCallbacks& callbacks,
-              const MasterToken& token, const char* reason)
+fromtextError(bool& error_issued, const MasterLexer& lexer,
+              MasterLoaderCallbacks& callbacks,
+              const MasterToken* token, const char* reason)
 {
 {
-    switch (token.getType()) {
+    // Don't be too noisy if there are many issues for single RDATA
+    if (error_issued) {
+        return;
+    }
+    error_issued = true;
+
+    if (token == NULL) {
+        callbacks.error(lexer.getSourceName(), lexer.getSourceLine(),
+                        "createRdata from text failed: " + string(reason));
+        return;
+    }
+
+    switch (token->getType()) {
     case MasterToken::STRING:
     case MasterToken::STRING:
     case MasterToken::QSTRING:
     case MasterToken::QSTRING:
         callbacks.error(lexer.getSourceName(), lexer.getSourceLine(),
         callbacks.error(lexer.getSourceName(), lexer.getSourceLine(),
                         "createRdata from text failed near '" +
                         "createRdata from text failed near '" +
-                        token.getString() + "': " + string(reason));
+                        token->getString() + "': " + string(reason));
+        break;
+    case MasterToken::ERROR:
+        callbacks.error(lexer.getSourceName(), lexer.getSourceLine(),
+                        "createRdata from text failed: " +
+                        token->getErrorText());
         break;
         break;
     default:
     default:
         assert(false);
         assert(false);
@@ -106,18 +124,39 @@ createRdata(const RRType& rrtype, const RRClass& rrclass,
             MasterLoader::Options options,
             MasterLoader::Options options,
             MasterLoaderCallbacks& callbacks)
             MasterLoaderCallbacks& callbacks)
 {
 {
-    const RdataPtr rdata = RRParamRegistry::getRegistry().createRdata(
-        rrtype, rrclass, lexer, origin, options, callbacks);
+    RdataPtr rdata;
+
+    bool error_issued = false;
+    try {
+        rdata = RRParamRegistry::getRegistry().createRdata(
+            rrtype, rrclass, lexer, origin, options, callbacks);
+    } catch (const MasterLexer::LexerError& error) {
+        fromtextError(error_issued, lexer, callbacks, &error.token_, "");
+    } catch (const Exception& ex) {
+        // Catching all isc::Exception is too broad, and right now we don't
+        // have better granularity.  When we complete #2518 we can make this
+        // finer.
+        fromtextError(error_issued, lexer, callbacks, NULL, ex.what());
+    }
+    // Other exceptions mean a serious implementation bug; it doesn't make
+    // sense to catch and try to recover from them here.  Just propagate.
 
 
     // Consume to end of line / file.
     // Consume to end of line / file.
     // If not at end of line initially set error code.
     // If not at end of line initially set error code.
     // Call callback via fromtextError once if there was an error.
     // Call callback via fromtextError once if there was an error.
-    const MasterToken& token = lexer.getNextToken();
-    if (token.getType() != MasterToken::END_OF_LINE &&
-        token.getType() != MasterToken::END_OF_FILE) {
-        fromtextError(lexer, callbacks, token, "extra input text");
-        return (RdataPtr());
-    }
+    do {
+        const MasterToken& token = lexer.getNextToken();
+        if (token.getType() != MasterToken::END_OF_LINE &&
+            token.getType() != MasterToken::END_OF_FILE) {
+            rdata.reset();      // we'll return NULL
+            if (!error_issued) {
+                fromtextError(error_issued, lexer, callbacks, &token,
+                              "extra input text");
+            }
+        } else {                // reached EOL or EOF
+            break;
+        }
+    } while (true);
 
 
     return (rdata);
     return (rdata);
 }
 }

+ 37 - 0
src/lib/dns/tests/rdata_unittest.cc

@@ -105,8 +105,12 @@ public:
         reason_txt_.clear();
         reason_txt_.clear();
     }
     }
 
 
+    // Return if callback is called since the previous call to clear().
+    bool isCalled() const { return (type_ != NONE); }
+
     void check(const string& expected_srcname, size_t expected_line,
     void check(const string& expected_srcname, size_t expected_line,
                CallbackType expected_type, const string& expected_reason)
                CallbackType expected_type, const string& expected_reason)
+        const
     {
     {
         EXPECT_EQ(expected_srcname, source_);
         EXPECT_EQ(expected_srcname, source_);
         EXPECT_EQ(expected_line, line_);
         EXPECT_EQ(expected_line, line_);
@@ -129,6 +133,9 @@ TEST_F(RdataTest, createRdataWithLexer) {
     const string src_name = "stream-" + boost::lexical_cast<string>(&ss);
     const string src_name = "stream-" + boost::lexical_cast<string>(&ss);
     ss << aaaa_rdata.toText() << "\n"; // valid case
     ss << aaaa_rdata.toText() << "\n"; // valid case
     ss << aaaa_rdata.toText() << " extra-token\n"; // extra token
     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
+    ss << "192.0.2.1\n"; // semantics error: IPv4 address is given for AAAA
     lexer.pushSource(ss);
     lexer.pushSource(ss);
 
 
     CreateRdataCallback callback;
     CreateRdataCallback callback;
@@ -137,17 +144,47 @@ TEST_F(RdataTest, createRdataWithLexer) {
                     CreateRdataCallback::ERROR, _1, _2, _3),
                     CreateRdataCallback::ERROR, _1, _2, _3),
         boost::bind(&CreateRdataCallback::callback, &callback,
         boost::bind(&CreateRdataCallback::callback, &callback,
                     CreateRdataCallback::WARN,  _1, _2, _3));
                     CreateRdataCallback::WARN,  _1, _2, _3));
+
+    // Valid case.
     ConstRdataPtr rdata = createRdata(RRType::AAAA(), RRClass::IN(), lexer,
     ConstRdataPtr rdata = createRdata(RRType::AAAA(), RRClass::IN(), lexer,
                                       NULL, MasterLoader::MANY_ERRORS,
                                       NULL, MasterLoader::MANY_ERRORS,
                                       callbacks);
                                       callbacks);
     EXPECT_EQ(0, aaaa_rdata.compare(*rdata));
     EXPECT_EQ(0, aaaa_rdata.compare(*rdata));
+    EXPECT_FALSE(callback.isCalled());
 
 
+    // Broken RDATA text: extra token.  createRdata() returns NULL, error
+    // callback is called.
     callback.clear();
     callback.clear();
     EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL,
     EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL,
                              MasterLoader::MANY_ERRORS, callbacks));
                              MasterLoader::MANY_ERRORS, callbacks));
     callback.check(src_name, 2, CreateRdataCallback::ERROR,
     callback.check(src_name, 2, CreateRdataCallback::ERROR,
                    "createRdata from text failed near 'extra-token': "
                    "createRdata from text failed near 'extra-token': "
                    "extra input text");
                    "extra input text");
+
+    // Similar to the previous case, but only the first extra token triggers
+    // callback.
+    callback.clear();
+    EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL,
+                             MasterLoader::MANY_ERRORS, callbacks));
+    callback.check(src_name, 3, CreateRdataCallback::ERROR,
+                   "createRdata from text failed near 'extra': "
+                   "extra input text");
+
+    // Lexer error will happen, corresponding error callback will be triggered.
+    callback.clear();
+    EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL,
+                             MasterLoader::MANY_ERRORS, callbacks));
+    callback.check(src_name, 4, CreateRdataCallback::ERROR,
+                   "createRdata from text failed: unbalanced parentheses");
+
+    // Semantics level error will happen, corresponding error callback will be
+    // triggered.
+    callback.clear();
+    EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL,
+                             MasterLoader::MANY_ERRORS, callbacks));
+    callback.check(src_name, 5, CreateRdataCallback::ERROR,
+                   "createRdata from text failed: Failed to convert "
+                   "'192.0.2.1' to IN/AAAA RDATA");
 }
 }
 
 
 }
 }