Merge branch 'trac2534'

src/lib/dns/master_lexer.cc

@@ -60,6 +60,7 @@ struct MasterLexer::MasterLexerImpl {
         separators_.set('\t');
         separators_.set('(');
         separators_.set(')');
+        separators_.set('"');
         esc_separators_.set('\r');
         esc_separators_.set('\n');
     }
@@ -325,7 +326,8 @@ const char* const error_text[] = {
     "unbalanced quotes",        // UNBALANCED_QUOTES
     "no token produced",        // NO_TOKEN_PRODUCED
     "number out of range",      // NUMBER_OUT_OF_RANGE
-    "not a valid number"        // BAD_NUMBER
+    "not a valid number",       // BAD_NUMBER
+    "unexpected quotes"         // UNEXPECTED_QUOTES
 };
 const size_t error_text_max_count = sizeof(error_text) / sizeof(error_text[0]);
 } // end unnamed namespace
@@ -477,9 +479,14 @@ State::start(MasterLexer& lexer, MasterLexer::Options options) {
             if (paren_count == 0) { // check if we are in () (see above)
                 return (&CRLF_STATE);
             }
-        } else if (c == '"' && (options & MasterLexer::QSTRING) != 0) {
-            lexerimpl.last_was_eol_ = false;
-            return (&QSTRING_STATE);
+        } else if (c == '"') {
+            if ((options & MasterLexer::QSTRING) != 0) {
+                lexerimpl.last_was_eol_ = false;
+                return (&QSTRING_STATE);
+            } else {
+                lexerimpl.token_ = MasterToken(MasterToken::UNEXPECTED_QUOTES);
+                return (NULL);
+            }
         } else if (c == '(') {
             lexerimpl.last_was_eol_ = false;
             ++paren_count;

src/lib/dns/master_lexer.h

@@ -78,6 +78,7 @@ public:
                            /// error and should never get out of the lexer.
         NUMBER_OUT_OF_RANGE, ///< Number was out of range
         BAD_NUMBER,    ///< Number is expected but not recognized
+        UNEXPECTED_QUOTES, ///< Unexpected quotes character detected
         MAX_ERROR_CODE ///< Max integer corresponding to valid error codes.
                        /// (excluding this one). Mainly for internal use.
     };
@@ -587,6 +588,9 @@ public:
     ///
     /// - If the expected type is MasterToken::QSTRING, both quoted and
     ///   unquoted strings are recognized and returned.
+    /// - A string with quotation marks is not recognized as a
+    /// - MasterToken::STRING. You have to get it as a
+    /// - MasterToken::QSTRING.
     /// - If the optional \c eol_ok parameter is \c true (very rare case),
     ///   MasterToken::END_OF_LINE and MasterToken::END_OF_FILE are recognized
     ///   and returned if they are found instead of the expected type of

src/lib/dns/tests/master_lexer_state_unittest.cc

@@ -384,10 +384,13 @@ TEST_F(MasterLexerStateTest, quotedString) {
     ss << "\"no;comment\"";
     lexer.pushSource(ss);
 
-    // by default, '"' doesn't have any special meaning and part of string
-    EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    s_string.handle(lexer); // recognize str, see \n
-    stringTokenCheck("\"ignore-quotes\"", s_string.getToken(lexer));
+    // by default, '"' is unexpected (when QSTRING is not specified),
+    // and it returns MasterToken::UNEXPECTED_QUOTES.
+    EXPECT_EQ(s_null, State::start(lexer, common_options));
+    EXPECT_EQ(Token::UNEXPECTED_QUOTES, s_string.getToken(lexer).getErrorCode());
+    // Read it as a QSTRING.
+    s_qstring.handle(lexer); // recognize quoted str, see \n
+    stringTokenCheck("ignore-quotes", s_qstring.getToken(lexer), true);
     EXPECT_EQ(s_null, State::start(lexer, common_options)); // skip \n after it
     EXPECT_TRUE(s_string.wasLastEOL(lexer));
 

src/lib/dns/tests/master_lexer_token_unittest.cc

@@ -143,15 +143,20 @@ TEST_F(MasterLexerTokenTest, errors) {
               getErrorText());
     EXPECT_EQ("not a valid number",
               MasterToken(MasterToken::BAD_NUMBER).getErrorText());
+    EXPECT_EQ("unexpected quotes",
+              MasterToken(MasterToken::UNEXPECTED_QUOTES).getErrorText());
 
     // getErrorCode/Text() isn't allowed for non number types
     EXPECT_THROW(token_num.getErrorCode(), isc::InvalidOperation);
     EXPECT_THROW(token_num.getErrorText(), isc::InvalidOperation);
 
-    // Only the pre-defined error code is accepted.  Hardcoding '7' (max code
+    // Only the pre-defined error code is accepted.  Hardcoding '8' (max code
     // + 1) is intentional; it'd be actually better if we notice it when we
     // update the enum list (which shouldn't happen too often).
-    EXPECT_THROW(MasterToken(MasterToken::ErrorCode(7)),
+    //
+    // Note: if you fix this testcase, you probably want to update the
+    // getErrorText() tests above too.
+    EXPECT_THROW(MasterToken(MasterToken::ErrorCode(8)),
                  isc::InvalidParameter);
 
     // Check the coexistence of "from number" and "from error-code"

src/lib/dns/tests/master_lexer_unittest.cc

@@ -246,7 +246,7 @@ TEST_F(MasterLexerTest, eof) {
 // Check we properly return error when there's an opened parentheses and no
 // closing one
 TEST_F(MasterLexerTest, getUnbalancedParen) {
-    ss << "(\"string\"";
+    ss << "(string";
     lexer.pushSource(ss);
 
     // The string gets out first

src/lib/dns/tests/master_loader_unittest.cc

@@ -966,7 +966,7 @@ struct ErrorCase {
       "$TTL with extra token" },
     { "$TTL", "unexpected end of input", "missing TTL" },
     { "$TTL No-ttl", "Unknown unit used: N in: No-ttl", "bad TTL" },
-    { "$TTL \"100\"", "invalid TTL: \"100\"", "bad TTL, quoted" },
+    { "$TTL \"100\"", "unexpected quotes", "bad TTL, quoted" },
     { "$TT 100", "Unknown directive 'TT'", "bad directive, too short" },
     { "$TTLLIKE 100", "Unknown directive 'TTLLIKE'", "bad directive, extra" },
     { NULL, NULL, NULL }

src/lib/dns/tests/rdata_caa_unittest.cc

@@ -127,9 +127,10 @@ TEST_F(Rdata_CAA_Test, fields) {
     EXPECT_THROW(const generic::CAA rdata_caa2("256 issue \"ca.example.net\""),
                  InvalidRdataText);
 
-    // Missing tag actually passes because it parses the value as tag
-    // and assumes that the value is empty instead.
-    EXPECT_NO_THROW(const generic::CAA rdata_caa2("0 \"ca.example.net\""));
+    // Missing tag causes the value to be parsed as the tag field. As
+    // the tag field does not allow quoted strings, this throws.
+    EXPECT_THROW(const generic::CAA rdata_caa2("0 \"ca.example.net\""),
+                 InvalidRdataText);
 
     // Tag is too long
     const std::string tag(256, 'a');

src/lib/dns/tests/rdata_soa_unittest.cc

@@ -98,11 +98,10 @@ TEST_F(Rdata_SOA_Test, createFromText) {
     checkFromTextSOA<EmptyLabel, EmptyLabel>(
         ". bad..example. 2010012601 1H 5M 1000H 20M");
 
-    // Names shouldn't be quoted. (Note: on completion of #2534, the resulting
-    // exception will be different).
-    checkFromTextSOA<MissingNameOrigin, MissingNameOrigin>(
+    // Names shouldn't be quoted.
+    checkFromTextSOA<InvalidRdataText, MasterLexer::LexerError>(
         "\".\" . 0 0 0 0 0");
-    checkFromTextSOA<MissingNameOrigin, MissingNameOrigin>(
+    checkFromTextSOA<InvalidRdataText, MasterLexer::LexerError>(
         ". \".\" 0 0 0 0 0");
 
     // Missing MAME or RNAME: for the string version, the serial would be

src/lib/dns/tests/rdata_txt_like_unittest.cc

@@ -187,8 +187,7 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createMultiStringsFromText) {
     texts.push_back("\"Test-String\" Test-String");  // no '"' for one
     texts.push_back("\"Test-String\"Test-String"); // and no space either
     texts.push_back("Test-String \"Test-String\""); // no '"' for the other
-    // This one currently doesn't work
-    //texts.push_back("Test-String\"Test-String\""); // and no space either
+    texts.push_back("Test-String\"Test-String\""); // and no space either
 
     std::stringstream ss;
     for (std::vector<std::string >::const_iterator it = texts.begin();
@@ -331,7 +330,7 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, toText) {
 
     // Check escape behavior
     const TypeParam double_quotes("Test-String\"Test-String\"");
-    EXPECT_EQ("\"Test-String\\\"Test-String\\\"\"", double_quotes.toText());
+    EXPECT_EQ("\"Test-String\" \"Test-String\"", double_quotes.toText());
     const TypeParam semicolon("Test-String\\;Test-String");
     EXPECT_EQ("\"Test-String\\;Test-String\"", semicolon.toText());
     const TypeParam backslash("Test-String\\\\Test-String");