[master] Fix overflow catching in number lexer state

src/lib/dns/master_lexer.cc

@@ -19,6 +19,7 @@
 #include <dns/master_lexer_state.h>
 
 #include <boost/shared_ptr.hpp>
+#include <boost/lexical_cast.hpp>
 
 #include <bitset>
 #include <cassert>
@@ -210,7 +211,8 @@ const char* const error_text[] = {
     "unbalanced parentheses",   // UNBALANCED_PAREN
     "unexpected end of input",  // UNEXPECTED_END
     "unbalanced quotes",        // UNBALANCED_QUOTES
-    "no token produced"         // NO_TOKEN_PRODUCED
+    "no token produced",        // NO_TOKEN_PRODUCED
+    "number out of range"       // NUMBER_RANGE
 };
 const size_t error_text_max_count = sizeof(error_text) / sizeof(error_text[0]);
 } // end unnamed namespace
@@ -449,8 +451,6 @@ QString::handle(MasterLexer& lexer) const {
 void
 Number::handle(MasterLexer& lexer) const {
     MasterLexer::Token& token = getLexerImpl(lexer)->token_;
-    // Do we want to support octal and/or hex here?
-    const int base = 10;
 
     // It may yet turn out to be a string, so we first
     // collect all the data
@@ -465,10 +465,17 @@ Number::handle(MasterLexer& lexer) const {
         if (getLexerImpl(lexer)->isTokenEnd(c, escaped)) {
             getLexerImpl(lexer)->source_->ungetChar();
             if (digits_only) {
-                // Close the string for strtoul
+                // Close the string for lexical_cast
                 data.push_back('\0');
-                token = MasterLexer::Token(strtoul(&data.at(0),
-                                                   NULL, base));
+                try {
+                    const uint32_t number32 =
+                        boost::lexical_cast<uint32_t, const char*>(&data.at(0));
+                    token = MasterLexer::Token(number32);
+                } catch (const boost::bad_lexical_cast&) {
+                    // Since we already know we have only digits,
+                    // range should be the only possible problem.
+                    token = Token(Token::NUMBER_RANGE);
+                }
             } else {
                 token = MasterLexer::Token(&data.at(0),
                                            data.size());

src/lib/dns/master_lexer.h

@@ -292,6 +292,7 @@ public:
         UNBALANCED_QUOTES,      ///< Unbalanced quotations detected
         NO_TOKEN_PRODUCED, ///< No token was produced. This means programmer
                            /// error and should never get out of the lexer.
+        NUMBER_RANGE,    ///< Number was out of range
         MAX_ERROR_CODE ///< Max integer corresponding to valid error codes.
                        /// (excluding this one). Mainly for internal use.
     };

src/lib/dns/tests/master_lexer_state_unittest.cc

@@ -457,12 +457,9 @@ TEST_F(MasterLexerStateTest, basicNumbers) {
     ss << "1 ";
     ss << "12345 ";
     ss << "4294967295 "; // 2^32-1
-    ss << "4294967296 "; // 2^32 (this overflows to 0, we
-                         // can consider failing on it, but
-                         // this is what bind9 does as well)
-    ss << "4294967297 "; // 2^32+1 (this overflows to 1, see
-                         // above)
-    ss << "1000000000000000000 "; // overflows to 2808348672
+    ss << "4294967296 "; // Out of range
+    ss << "340282366920938463463374607431768211456 ";
+                         // Very much out of range (2^128)
     ss << "005 ";        // Leading zeroes are ignored
     ss << "42;asdf\n";   // Number with comment
     ss << "37";          // Simple number again, here to make
@@ -484,19 +481,17 @@ TEST_F(MasterLexerStateTest, basicNumbers) {
 
     EXPECT_EQ(&s_number, State::start(lexer, common_options));
     s_number.handle(lexer);
-    EXPECT_EQ(4294967295u, s_number.getToken(lexer).getNumber());
+    EXPECT_EQ(4294967295, s_number.getToken(lexer).getNumber());
 
     EXPECT_EQ(&s_number, State::start(lexer, common_options));
     s_number.handle(lexer);
-    EXPECT_EQ(0, s_number.getToken(lexer).getNumber());
-
-    EXPECT_EQ(&s_number, State::start(lexer, common_options));
-    s_number.handle(lexer);
-    EXPECT_EQ(1, s_number.getToken(lexer).getNumber());
+    EXPECT_EQ(Token::NUMBER_RANGE,
+              s_number.getToken(lexer).getErrorCode());
 
     EXPECT_EQ(&s_number, State::start(lexer, common_options));
     s_number.handle(lexer);
-    EXPECT_EQ(2808348672u, s_number.getToken(lexer).getNumber());
+    EXPECT_EQ(Token::NUMBER_RANGE,
+              s_number.getToken(lexer).getErrorCode());
 
     EXPECT_EQ(&s_number, State::start(lexer, common_options));
     s_number.handle(lexer);

src/lib/dns/tests/master_lexer_token_unittest.cc

@@ -145,15 +145,18 @@ TEST_F(MasterLexerTokenTest, errors) {
     EXPECT_EQ("no token produced",
               MasterLexer::Token(MasterLexer::Token::NO_TOKEN_PRODUCED).
               getErrorText());
+    EXPECT_EQ("number out of range",
+              MasterLexer::Token(MasterLexer::Token::NUMBER_RANGE).
+              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 '5' (max code
+    // Only the pre-defined error code is accepted.  Hardcoding '6' (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(MasterLexer::Token(MasterLexer::Token::ErrorCode(5)),
+    EXPECT_THROW(MasterLexer::Token(MasterLexer::Token::ErrorCode(6)),
                  isc::InvalidParameter);
 
     // Check the coexistence of "from number" and "from error-code"