[2521] further RRSIG review updates

- fixed multi-line signature aggregator
- allow empty signature
- fixed some unit tests
- added unit test for unterminated multi-line base64

src/lib/dns/rdata/generic/rrsig_46.cc

@@ -74,8 +74,8 @@ struct RRSIGImpl {
 };
 
 // helper function for string and lexer constructors
-void
-RRSIG::createFromLexer(MasterLexer& lexer, const Name* origin) {
+RRSIGImpl *
+RRSIG::constructFromLexer(MasterLexer& lexer, const Name* origin) {
     const RRType covered(lexer.getNextToken(MasterToken::STRING).getString());
     const uint32_t algorithm =
         lexer.getNextToken(MasterToken::NUMBER).getNumber();
@@ -99,25 +99,31 @@ RRSIG::createFromLexer(MasterLexer& lexer, const Name* origin) {
         isc_throw(InvalidRdataText, "RRSIG key tag out of range");
     }
     const Name signer = createNameFromLexer(lexer, origin);
-    string signature_txt =
-        lexer.getNextToken(MasterToken::STRING).getString();
-    // RFC4034 says "Whitespace is allowed within the Base64 text."
-    // So read to the end of input.
+
+    string signature_txt;
+    string signature_part;
+    // Whitespace is allowed within base64 text, so read to the end of input.
     while (true) {
-        const MasterToken& token = lexer.getNextToken();
-        if (token.getType() != MasterToken::STRING) {
+        const MasterToken& token =
+            lexer.getNextToken(MasterToken::STRING, true);
+        if ((token.getType() == MasterToken::END_OF_FILE) ||
+            (token.getType() == MasterToken::END_OF_LINE)) {
             break;
         }
-        signature_txt.append(token.getString());
+        token.getString(signature_part);
+        signature_txt.append(signature_part);
     }
     lexer.ungetToken();
 
     vector<uint8_t> signature;
-    decodeBase64(signature_txt, signature);
+    // missing signature is okay
+    if (signature_txt.size() > 0) {
+        decodeBase64(signature_txt, signature);
+    }
 
-    impl_ = new RRSIGImpl(covered, algorithm, labels,
+    return (new RRSIGImpl(covered, algorithm, labels,
                           originalttl, timeexpire, timeinception,
-                          static_cast<uint16_t>(tag), signer, signature);
+                          static_cast<uint16_t>(tag), signer, signature));
 }
 
 /// \brief Constructor from string.
@@ -139,12 +145,17 @@ RRSIG::createFromLexer(MasterLexer& lexer, const Name* origin) {
 RRSIG::RRSIG(const std::string& rrsig_str) :
     impl_(NULL)
 {
+    // We use auto_ptr here because if there is an exception in this
+    // constructor, the destructor is not called and there could be a
+    // leak of the RRSIGImpl that constructFromLexer() returns.
+    std::auto_ptr<RRSIGImpl> impl_ptr(NULL);
+
     try {
-        istringstream iss(rrsig_str);
+        std::istringstream iss(rrsig_str);
         MasterLexer lexer;
         lexer.pushSource(iss);
 
-        createFromLexer(lexer, NULL);
+        impl_ptr.reset(constructFromLexer(lexer, NULL));
 
         if (lexer.getNextToken().getType() != MasterToken::END_OF_FILE) {
             isc_throw(InvalidRdataText, "extra input text for RRSIG: "
@@ -154,6 +165,8 @@ RRSIG::RRSIG(const std::string& rrsig_str) :
         isc_throw(InvalidRdataText, "Failed to construct RRSIG from '" <<
                   rrsig_str << "': " << ex.what());
     }
+
+    impl_ = impl_ptr.release();
 }
 
 /// \brief Constructor with a context of MasterLexer.
@@ -163,9 +176,9 @@ RRSIG::RRSIG(const std::string& rrsig_str) :
 /// origin is non NULL, in which case \c origin is used to make it absolute.
 /// This must not be represented as a quoted string.
 ///
-/// The Original TTL field is a valid decimal representation of an
-/// unsigned 32-bit integer. Note that RFC4034 does not allow alternate
-/// textual representations of \c RRTTL such as "1H" for 3600 seconds.
+/// The Original TTL field is a valid decimal representation of an unsigned
+/// 32-bit integer. Note that alternate textual representations of \c RRTTL,
+/// such as "1H" for 3600 seconds, are not allowed here.
 ///
 /// \throw MasterLexer::LexerError General parsing error such as missing field.
 /// \throw Other Exceptions from the Name constructor if
@@ -179,7 +192,7 @@ RRSIG::RRSIG(MasterLexer& lexer, const Name* origin,
              MasterLoader::Options, MasterLoaderCallbacks&) :
     impl_(NULL)
 {
-    createFromLexer(lexer, origin);
+    impl_ = constructFromLexer(lexer, origin);
 }
 
 RRSIG::RRSIG(InputBuffer& buffer, size_t rdata_len) {

src/lib/dns/rdata/generic/rrsig_46.h

@@ -48,7 +48,7 @@ public:
     const RRType& typeCovered() const;
 private:
     // helper function for string and lexer constructors
-    void createFromLexer(MasterLexer& lexer, const Name* origin);
+    RRSIGImpl* constructFromLexer(MasterLexer& lexer, const Name* origin);
 
     RRSIGImpl* impl_;
 };

src/lib/dns/tests/rdata_rrsig_unittest.cc

@@ -97,9 +97,13 @@ TEST_F(Rdata_RRSIG_Test, fromText) {
     EXPECT_EQ(rrsig_txt, rdata_rrsig.toText());
     EXPECT_EQ(isc::dns::RRType::A(), rdata_rrsig.typeCovered());
 
+    // Missing signature is OK
+    EXPECT_NO_THROW(const generic::RRSIG sig(
+              "A 5 4 43200 20100223214617 20100222214617 8496 isc.org."));
+
     // Space in signature data is OK
     checkFromText_None(
-	      "A 5 4 43200 20100223214617 20100222214617 8496 isc.org. "
+              "A 5 4 43200 20100223214617 20100222214617 8496 isc.org. "
               "evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz "
               "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/ "
               "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU "
@@ -107,7 +111,7 @@ TEST_F(Rdata_RRSIG_Test, fromText) {
 
     // Multi-line signature data is OK, if enclosed in parentheses
     checkFromText_None(
-	      "A 5 4 43200 20100223214617 20100222214617 8496 isc.org. "
+              "A 5 4 43200 20100223214617 20100222214617 8496 isc.org. "
               "( evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz\n"
               "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/\n"
               "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU\n"
@@ -117,15 +121,15 @@ TEST_F(Rdata_RRSIG_Test, fromText) {
     // to fail, but the lexer constructor must be able to continue
     // parsing from it.
     checkFromText_BadString(
-	      "A 5 4 43200 20100223214617 20100222214617 8496 isc.org. "
-              "evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz "
-              "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/ "
-              "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU "
+              "A 5 4 43200 20100223214617 20100222214617 8496 isc.org. "
+              "evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz"
+              "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/"
+              "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU"
               "f49t+sXKPzbipN9g+s1ZPiIyofc= ; comment\n"
-	      "A 5 4 43200 20100223214617 20100222214617 8496 isc.org. "
-              "evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz "
-              "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/ "
-              "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU "
+              "A 5 4 43200 20100223214617 20100222214617 8496 isc.org. "
+              "evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz"
+              "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/"
+              "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU"
               "f49t+sXKPzbipN9g+s1ZPiIyofc=");
 }
 
@@ -139,10 +143,7 @@ TEST_F(Rdata_RRSIG_Test, badText) {
     checkFromText_LexerError("A 5 4 43200 20100223214617");
     checkFromText_LexerError("A 5 4 43200 20100223214617 20100222214617");
     checkFromText_LexerError("A 5 4 43200 20100223214617 20100222214617 "
-			      "8496");
-    checkFromText_LexerError("A 5 4 43200 20100223214617 20100222214617 "
-			      "8496 isc.org.");
-
+                              "8496");
     // bad algorithm
     checkFromText_InvalidText(
                      "A 555 4 43200 "
@@ -158,6 +159,7 @@ TEST_F(Rdata_RRSIG_Test, badText) {
                      "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/"
                      "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU"
                      "f49t+sXKPzbipN9g+s1ZPiIyofc=");
+
     // bad labels
     checkFromText_InvalidText(
                      "A 5 4444 43200 "
@@ -173,6 +175,7 @@ TEST_F(Rdata_RRSIG_Test, badText) {
                      "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/"
                      "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU"
                      "f49t+sXKPzbipN9g+s1ZPiIyofc=");
+
     // bad original ttl
     checkFromText_LexerError(
                      "A 5 4 999999999999 "
@@ -188,13 +191,15 @@ TEST_F(Rdata_RRSIG_Test, badText) {
                      "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/"
                      "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU"
                      "f49t+sXKPzbipN9g+s1ZPiIyofc=");
-    // Alternate form of TTL is not okay
+
+    // alternate form of TTL is not okay
     checkFromText_LexerError(
-	      "A 5 4 12H 20100223214617 20100222214617 8496 isc.org. "
+              "A 5 4 12H 20100223214617 20100222214617 8496 isc.org. "
               "evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz "
               "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/ "
               "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU "
               "f49t+sXKPzbipN9g+s1ZPiIyofc=");
+
     // bad signature expiration
     checkFromText_InvalidTime(
                      "A 5 4 43200 "
@@ -202,7 +207,7 @@ TEST_F(Rdata_RRSIG_Test, badText) {
                      "evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz"
                      "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/"
                      "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU"
-                     "f49t+sXKPzbipN9g+s1ZPiIyofc=");
+                     "f49t+sXKPzbipN9g+s1ZPiIyofc="); 
     checkFromText_InvalidTime(
                      "A 5 4 43200 "
                      "EXPIRATION 20100222214617 8496 isc.org. "
@@ -210,6 +215,7 @@ TEST_F(Rdata_RRSIG_Test, badText) {
                      "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/"
                      "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU"
                      "f49t+sXKPzbipN9g+s1ZPiIyofc=");
+
    // bad signature inception
     checkFromText_InvalidTime(
                      "A 5 4 43200 "
@@ -225,6 +231,7 @@ TEST_F(Rdata_RRSIG_Test, badText) {
                      "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/"
                      "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU"
                      "f49t+sXKPzbipN9g+s1ZPiIyofc=");
+
     // bad key tag
     checkFromText_InvalidText(
                      "A 5 4 43200 "
@@ -240,23 +247,34 @@ TEST_F(Rdata_RRSIG_Test, badText) {
                      "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/"
                      "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU"
                      "f49t+sXKPzbipN9g+s1ZPiIyofc=");
+
     // bad signer name
     checkFromText_MissingOrigin(
                      "A 5 4 43200 "
-                     "20100223214617 20100222214617 8496 isc.org"
+                     "20100223214617 20100222214617 8496 isc.org "
                      "evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz"
                      "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/"
                      "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU"
                      "f49t+sXKPzbipN9g+s1ZPiIyofc=");
+
     // bad signature
     checkFromText_BadValue(
                      "A 5 4 43200 "
                      "20100223214617 20100222214617 8496 isc.org. "
                      "EEeeeeeeEEEeeeeeeGaaahAAAAAAAAHHHHHHHHHHH!=");
+
     // no space between the tag and signer
     checkFromText_LexerError(
                      "A 5 4 43200 20100223214617 20100222214617 "
                      "8496isc.org. ofc=");
+
+    // unterminated multi-line base64
+    checkFromText_LexerError(
+              "A 5 4 43200 20100223214617 20100222214617 8496 isc.org. "
+              "( evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz\n"
+              "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/\n"
+              "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU\n"
+              "f49t+sXKPzbipN9g+s1ZPiIyofc=");
 }
 
 TEST_F(Rdata_RRSIG_Test, createFromLexer) {