[2521] Rearrange RRSIG parser error handling, per Jinmei's review.

Also handle space-separated base-64 in RRSIG signature field.
Also add more RRSIG unittest cases.

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

@@ -77,43 +77,48 @@ struct RRSIGImpl {
 // helper function for string and lexer constructors
 void
 RRSIG::createFromLexer(MasterLexer& lexer, const Name* origin) {
-    const string covered_txt =
-        lexer.getNextToken(MasterToken::STRING).getString();
-    const uint32_t algorithm = lexer.getNextToken(MasterToken::NUMBER).
-        getNumber();
-    const uint32_t labels = lexer.getNextToken(MasterToken::NUMBER).
-        getNumber();
-    const uint32_t originalttl =
-        RRTTL(lexer.getNextToken(MasterToken::STRING).getString()).getValue();
-    const string expire_txt =
-        lexer.getNextToken(MasterToken::STRING).getString();
-    const string inception_txt =
-        lexer.getNextToken(MasterToken::STRING).getString();
-    const uint32_t tag =
+    const RRType covered(lexer.getNextToken(MasterToken::STRING).getString());
+    const uint32_t algorithm =
         lexer.getNextToken(MasterToken::NUMBER).getNumber();
-    const Name signer = createNameFromLexer(lexer, origin);
-    const string signature_txt =
-        lexer.getNextToken(MasterToken::STRING).getString();
-
     if (algorithm > 0xff) {
         isc_throw(InvalidRdataText, "RRSIG algorithm out of range");
     }
+    const uint32_t labels =
+        lexer.getNextToken(MasterToken::NUMBER).getNumber();
     if (labels > 0xff) {
         isc_throw(InvalidRdataText, "RRSIG labels out of range");
     }
+    const uint32_t originalttl =
+        RRTTL(lexer.getNextToken(MasterToken::STRING).getString()).getValue();
+    const uint32_t timeexpire =
+        timeFromText32(lexer.getNextToken(MasterToken::STRING).getString());
+    const uint32_t timeinception =
+        timeFromText32(lexer.getNextToken(MasterToken::STRING).getString());
+    const uint32_t tag =
+        lexer.getNextToken(MasterToken::NUMBER).getNumber();
     if (tag > 0xffff) {
         isc_throw(InvalidRdataText, "RRSIG key tag out of range");
     }
-
-    const uint32_t timeexpire = timeFromText32(expire_txt);
-    const uint32_t timeinception = timeFromText32(inception_txt);
+    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.
+    while (true) {
+        const MasterToken& token = lexer.getNextToken();
+        if (token.getType() != MasterToken::STRING) {
+            break;
+        }
+        signature_txt.append(token.getString());
+    }
+    lexer.ungetToken();
 
     vector<uint8_t> signature;
     decodeBase64(signature_txt, signature);
 
-    impl_ = new RRSIGImpl(RRType(covered_txt), algorithm, labels,
-			  originalttl, timeexpire, timeinception,
-			  static_cast<uint16_t>(tag), signer, signature);
+    impl_ = new RRSIGImpl(covered, algorithm, labels,
+                          originalttl, timeexpire, timeinception,
+                          static_cast<uint16_t>(tag), signer, signature);
 }
 
 /// \brief Constructor from string.
@@ -161,7 +166,8 @@ RRSIG::RRSIG(const std::string& rrsig_str) :
 ///
 /// The Original TTL field can be either a valid decimal representation of an
 /// unsigned 32-bit integer or other valid textual representation of \c RRTTL
-/// such as "1H" (which means 3600).
+/// such as "1H" (which means 3600). Note that this differs from BIND 9,
+/// which only allows the Original TTL field to be expressed in seconds.
 ///
 /// \throw MasterLexer::LexerError General parsing error such as missing field.
 /// \throw Other Exceptions from the Name and RRTTL constructors if

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

@@ -32,6 +32,12 @@
 
 struct RRSIGImpl;
 
+/// \brief \c rdata::RRSIG class represents the RRSIG RDATA as defined %in
+/// RFC4034.
+///
+/// This class implements the basic interfaces inherited from the abstract
+/// \c rdata::Rdata class, and provides trivial accessors specific to the
+/// RRSIG RDATA.
 class RRSIG : public Rdata {
 public:
     // BEGIN_COMMON_MEMBERS

src/lib/dns/tests/rdata_rrsig_unittest.cc

@@ -55,9 +55,43 @@ TEST_F(Rdata_RRSIG_Test, fromText) {
     EXPECT_EQ(isc::dns::RRType::A(), rdata_rrsig.typeCovered());
 }
 
+TEST_F(Rdata_RRSIG_Test, spaceSeparatedBase64) {
+    const generic::RRSIG sig(
+	      "A 5 4 43200 20100223214617 20100222214617 8496 isc.org. "
+              "evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz "
+              "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/ "
+              "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU "
+              "f49t+sXKPzbipN9g+s1ZPiIyofc=");
+    EXPECT_EQ(rrsig_txt, sig.toText());
+}
+
+TEST_F(Rdata_RRSIG_Test, multiLineBase64) {
+    const generic::RRSIG sig(
+	      "A 5 4 43200 20100223214617 20100222214617 8496 isc.org. "
+              "( evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz\n"
+              "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/\n"
+              "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU\n"
+              "f49t+sXKPzbipN9g+s1ZPiIyofc= )");
+    EXPECT_EQ(rrsig_txt, sig.toText());
+}
+
 TEST_F(Rdata_RRSIG_Test, badText) {
     // missing fields
-    EXPECT_THROW(const generic::RRSIG sig("SPORK"), InvalidRdataText);
+    EXPECT_THROW(const generic::RRSIG sig("SPORK"), InvalidRRType);
+    EXPECT_THROW(const generic::RRSIG sig("A"), InvalidRdataText);
+    EXPECT_THROW(const generic::RRSIG sig("A 5"), InvalidRdataText);
+    EXPECT_THROW(const generic::RRSIG sig("A 5 4"), InvalidRdataText);
+    EXPECT_THROW(const generic::RRSIG sig("A 5 4 43200"), InvalidRdataText);
+    EXPECT_THROW(const generic::RRSIG sig("A 5 4 43200 20100223214617"),
+                 InvalidRdataText);
+    EXPECT_THROW(const generic::RRSIG sig("A 5 4 43200 20100223214617 "
+                                          "20100222214617"), InvalidRdataText);
+    EXPECT_THROW(const generic::RRSIG sig("A 5 4 43200 20100223214617 "
+                                          "20100222214617 8496"),
+                 InvalidRdataText);
+    EXPECT_THROW(const generic::RRSIG sig("A 5 4 43200 20100223214617 "
+                                          "20100222214617 8496 isc.org."),
+                 InvalidRdataText);
     // bad algorithm
     EXPECT_THROW(const generic::RRSIG sig("A 555 4 43200 "
                      "20100223214617 20100222214617 8496 isc.org. "
@@ -79,9 +113,16 @@ TEST_F(Rdata_RRSIG_Test, badText) {
                      "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/"
                      "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU"
                      "f49t+sXKPzbipN9g+s1ZPiIyofc="), InvalidRRTTL);
-    // bad signature expiration, inception
+    // bad signature expiration
+    EXPECT_THROW(const generic::RRSIG sig("A 5 4 43200 "
+                     "20100223 20100222214617 8496 isc.org. "
+                     "evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz"
+                     "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/"
+                     "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU"
+                     "f49t+sXKPzbipN9g+s1ZPiIyofc="), InvalidTime);
+    // bad signature inception
     EXPECT_THROW(const generic::RRSIG sig("A 5 4 43200 "
-                     "20100223 20100227 8496 isc.org. "
+                     "20100223214617 20100227 8496 isc.org. "
                      "evxhlGx13mpKLVkKsjpGzycS5twtIoxOmlN14w9t5AgzGBmz"
                      "diGdLIrFabqr72af2rUq+UDBKMWXujwZTZUTws32sVldDPk/"
                      "NbuacJM25fQXfv5mO3Af7TOoow3AjMaVG9icjCW0V55WcWQU"