From 610720a5bef22ba7193c9ad6e3cbe318eebdbf14 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 28 Nov 2012 14:37:23 -0800 Subject: [PATCH 01/69] [2506] removed meaningless comment this is remaining open point from #2375. The comment was added at commit 33a3143, and it's been the same since the introduction. Aside from the fact that the sentence is incomplete, IMO the comment doesn't provide any meaningful information; it's obvious that "the source is not available" if source_ is NULL. Even if we complete the sentence by adding, e.g, "(then) throw InvalidOperation", it still doesn't provide any additional information than the code itself. So my decision is to to simply clean it up. --- src/lib/dns/master_lexer.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc index 2bf02547a9..d223d7a71c 100644 --- a/src/lib/dns/master_lexer.cc +++ b/src/lib/dns/master_lexer.cc @@ -167,7 +167,6 @@ MasterLexer::getSourceLine() const { const MasterLexer::Token& MasterLexer::getNextToken(Options options) { - // If the source is not available if (impl_->source_ == NULL) { isc_throw(isc::InvalidOperation, "No source to read tokens from"); } From ebea1d79bcd55536f89e250ce1be13a8369171f7 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 28 Nov 2012 14:49:19 -0800 Subject: [PATCH 02/69] [2506] updated part of lexer state doc to match the latest changes. --- src/lib/dns/master_lexer_state.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib/dns/master_lexer_state.h b/src/lib/dns/master_lexer_state.h index 2a64c9d227..9ddd68a598 100644 --- a/src/lib/dns/master_lexer_state.h +++ b/src/lib/dns/master_lexer_state.h @@ -43,10 +43,10 @@ namespace master_lexer_internal { /// state, so it makes more sense to separate the interface for the transition /// from the initial state. /// -/// When an object of a specific state class completes the session, it -/// normally sets the identified token in the lexer, and returns NULL; -/// if more transition is necessary, it returns a pointer to the next state -/// object. +/// If the whole lexer transition is completed within start(), it sets the +/// identified token and returns NULL; otherwise it returns a pointer to +/// an object of a specific state class that completes the session +/// on the call of handle(). /// /// As is usual in the state design pattern, the \c State class is made /// a friend class of \c MasterLexer and can refer to its internal details. From c6c7cdee4088d6be3728ffaf07a25b920b0e8b5c Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 28 Nov 2012 15:12:28 -0800 Subject: [PATCH 03/69] [2506] rename: MasterLexer::Token to MasterToken. it's a kind of hack for the convenience of the planned change of this branch, but the revised name doesn't seem really bad anyway. --- src/lib/dns/master_lexer.cc | 49 +- src/lib/dns/master_lexer.h | 462 +++++++++--------- src/lib/dns/master_lexer_state.h | 2 +- .../dns/tests/master_lexer_state_unittest.cc | 4 +- .../dns/tests/master_lexer_token_unittest.cc | 86 ++-- src/lib/dns/tests/master_lexer_unittest.cc | 54 +- 6 files changed, 322 insertions(+), 335 deletions(-) diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc index d223d7a71c..4db2bf8527 100644 --- a/src/lib/dns/master_lexer.cc +++ b/src/lib/dns/master_lexer.cc @@ -36,7 +36,7 @@ using namespace master_lexer_internal; struct MasterLexer::MasterLexerImpl { - MasterLexerImpl() : source_(NULL), token_(Token::NOT_STARTED), + MasterLexerImpl() : source_(NULL), token_(MasterToken::NOT_STARTED), paren_count_(0), last_was_eol_(false), has_previous_(false), previous_paren_count_(0), @@ -82,7 +82,7 @@ struct MasterLexer::MasterLexerImpl { std::vector sources_; InputSource* source_; // current source (NULL if sources_ is empty) - Token token_; // currently recognized token (set by a state) + MasterToken token_; // currently recognized token (set by a state) std::vector data_; // placeholder for string data // These are used in states, and defined here only as a placeholder. @@ -165,7 +165,7 @@ MasterLexer::getSourceLine() const { return (impl_->sources_.back()->getCurrentLine()); } -const MasterLexer::Token& +const MasterToken& MasterLexer::getNextToken(Options options) { if (impl_->source_ == NULL) { isc_throw(isc::InvalidOperation, "No source to read tokens from"); @@ -177,7 +177,7 @@ MasterLexer::getNextToken(Options options) { impl_->has_previous_ = true; // Reset the token now. This is to check a token was actually produced. // This is debugging aid. - impl_->token_ = Token(Token::NO_TOKEN_PRODUCED); + impl_->token_ = MasterToken(MasterToken::NO_TOKEN_PRODUCED); // And get the token // This actually handles EOF internally too. @@ -187,8 +187,8 @@ MasterLexer::getNextToken(Options options) { } // Make sure a token was produced. Since this Can Not Happen, we assert // here instead of throwing. - assert(impl_->token_.getType() != Token::ERROR || - impl_->token_.getErrorCode() != Token::NO_TOKEN_PRODUCED); + assert(impl_->token_.getType() != MasterToken::ERROR || + impl_->token_.getErrorCode() != MasterToken::NO_TOKEN_PRODUCED); return (impl_->token_); } @@ -217,10 +217,10 @@ const size_t error_text_max_count = sizeof(error_text) / sizeof(error_text[0]); } // end unnamed namespace std::string -MasterLexer::Token::getErrorText() const { +MasterToken::getErrorText() const { if (type_ != ERROR) { isc_throw(InvalidOperation, - "Token::getErrorText() for non error type"); + "MasterToken::getErrorText() for non error type"); } // The class integrity ensures the following: @@ -233,14 +233,12 @@ namespace master_lexer_internal { // Note that these need to be defined here so that they can refer to // the details of MasterLexerImpl. -typedef MasterLexer::Token Token; // convenience shortcut - bool State::wasLastEOL(const MasterLexer& lexer) const { return (lexer.impl_->last_was_eol_); } -const MasterLexer::Token& +const MasterToken& State::getToken(const MasterLexer& lexer) const { return (lexer.impl_->token_); } @@ -270,7 +268,7 @@ public: if (c != '\n') { getLexerImpl(lexer)->source_->ungetChar(); } - getLexerImpl(lexer)->token_ = Token(Token::END_OF_LINE); + getLexerImpl(lexer)->token_ = MasterToken(MasterToken::END_OF_LINE); getLexerImpl(lexer)->last_was_eol_ = true; } }; @@ -341,24 +339,24 @@ State::start(MasterLexer& lexer, MasterLexer::Options options) { if (c == InputSource::END_OF_STREAM) { lexerimpl.last_was_eol_ = false; if (paren_count != 0) { - lexerimpl.token_ = Token(Token::UNBALANCED_PAREN); + lexerimpl.token_ = MasterToken(MasterToken::UNBALANCED_PAREN); paren_count = 0; // reset to 0; this helps in lenient mode. return (NULL); } - lexerimpl.token_ = Token(Token::END_OF_FILE); + lexerimpl.token_ = MasterToken(MasterToken::END_OF_FILE); return (NULL); } else if (c == ' ' || c == '\t') { // If requested and we are not in (), recognize the initial space. if (lexerimpl.last_was_eol_ && paren_count == 0 && (options & MasterLexer::INITIAL_WS) != 0) { lexerimpl.last_was_eol_ = false; - lexerimpl.token_ = Token(Token::INITIAL_WS); + lexerimpl.token_ = MasterToken(MasterToken::INITIAL_WS); return (NULL); } } else if (c == '\n') { lexerimpl.last_was_eol_ = true; if (paren_count == 0) { // we don't recognize EOL if we are in () - lexerimpl.token_ = Token(Token::END_OF_LINE); + lexerimpl.token_ = MasterToken(MasterToken::END_OF_LINE); return (NULL); } } else if (c == '\r') { @@ -374,7 +372,7 @@ State::start(MasterLexer& lexer, MasterLexer::Options options) { } else if (c == ')') { lexerimpl.last_was_eol_ = false; if (paren_count == 0) { - lexerimpl.token_ = Token(Token::UNBALANCED_PAREN); + lexerimpl.token_ = MasterToken(MasterToken::UNBALANCED_PAREN); return (NULL); } --paren_count; @@ -406,7 +404,7 @@ String::handle(MasterLexer& lexer) const { if (getLexerImpl(lexer)->isTokenEnd(c, escaped)) { getLexerImpl(lexer)->source_->ungetChar(); getLexerImpl(lexer)->token_ = - MasterLexer::Token(&data.at(0), data.size()); + MasterToken(&data.at(0), data.size()); return; } escaped = (c == '\\' && !escaped); @@ -416,7 +414,7 @@ String::handle(MasterLexer& lexer) const { void QString::handle(MasterLexer& lexer) const { - MasterLexer::Token& token = getLexerImpl(lexer)->token_; + MasterToken& token = getLexerImpl(lexer)->token_; std::vector& data = getLexerImpl(lexer)->data_; data.clear(); @@ -424,7 +422,7 @@ QString::handle(MasterLexer& lexer) const { while (true) { const int c = getLexerImpl(lexer)->source_->getChar(); if (c == InputSource::END_OF_STREAM) { - token = Token(Token::UNEXPECTED_END); + token = MasterToken(MasterToken::UNEXPECTED_END); return; } else if (c == '"') { if (escaped) { @@ -433,12 +431,12 @@ QString::handle(MasterLexer& lexer) const { escaped = false; data.back() = '"'; } else { - token = MasterLexer::Token(&data.at(0), data.size(), true); + token = MasterToken(&data.at(0), data.size(), true); return; } } else if (c == '\n' && !escaped) { getLexerImpl(lexer)->source_->ungetChar(); - token = Token(Token::UNBALANCED_QUOTES); + token = MasterToken(MasterToken::UNBALANCED_QUOTES); return; } else { escaped = (c == '\\' && !escaped); @@ -449,7 +447,7 @@ QString::handle(MasterLexer& lexer) const { void Number::handle(MasterLexer& lexer) const { - MasterLexer::Token& token = getLexerImpl(lexer)->token_; + MasterToken& token = getLexerImpl(lexer)->token_; // It may yet turn out to be a string, so we first // collect all the data @@ -473,11 +471,10 @@ Number::handle(MasterLexer& lexer) const { } 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_OUT_OF_RANGE); + token = Token(MasterToken::NUMBER_OUT_OF_RANGE); } } else { - token = MasterLexer::Token(&data.at(0), - data.size()); + token = MasterToken(&data.at(0), data.size()); } return; } diff --git a/src/lib/dns/master_lexer.h b/src/lib/dns/master_lexer.h index 4a861fc704..13205be14f 100644 --- a/src/lib/dns/master_lexer.h +++ b/src/lib/dns/master_lexer.h @@ -28,6 +28,235 @@ namespace master_lexer_internal { class State; } +/// \brief Tokens for \c MasterLexer +/// +/// This is a simple value-class encapsulating a type of a lexer token and +/// (if it has a value) its value. Essentially, the class provides +/// constructors corresponding to different types of tokens, and corresponding +/// getter methods. The type and value are fixed at the time of construction +/// and will never be modified throughout the lifetime of the object. +/// The getter methods are still provided to maximize the safety; an +/// application cannot refer to a value that is invalid for the type of token. +/// +/// This class is intentionally implemented as copyable and assignable +/// (using the default version of copy constructor and assignment operator), +/// but it's mainly for internal implementation convenience. Applications will +/// simply refer to Token object as a reference via the \c MasterLexer class. +class MasterToken { +public: + /// \brief Enumeration for token types + /// + /// \note At the time of initial implementation, all numeric tokens + /// that would be extracted from \c MasterLexer should be represented + /// as an unsigned 32-bit integer. If we see the need for larger integers + /// or negative numbers, we can then extend the token types. + enum Type { + END_OF_LINE, ///< End of line detected + END_OF_FILE, ///< End of file detected + INITIAL_WS, ///< White spaces at the beginning of a line after an + ///< end of line (if asked for detecting it) + NOVALUE_TYPE_MAX = INITIAL_WS, ///< Max integer corresponding to + /// no-value (type only) types. + /// Mainly for internal use. + STRING, ///< A single string + QSTRING, ///< A single string quoted by double-quotes ("). + NUMBER, ///< A decimal number (unsigned 32-bit) + ERROR ///< Error detected in getting a token + }; + + /// \brief Enumeration for lexer error codes + enum ErrorCode { + NOT_STARTED, ///< The lexer is just initialized and has no token + UNBALANCED_PAREN, ///< Unbalanced parentheses detected + UNEXPECTED_END, ///< The lexer reaches the end of line or file + /// unexpectedly + 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_OUT_OF_RANGE, ///< Number was out of range + MAX_ERROR_CODE ///< Max integer corresponding to valid error codes. + /// (excluding this one). Mainly for internal use. + }; + + /// \brief A simple representation of a range of a string. + /// + /// This is a straightforward pair of the start pointer of a string + /// and its length. The \c STRING and \c QSTRING types of tokens + /// will be primarily represented in this form. + /// + /// Any character can be stored in the valid range of the region. + /// In particular, there can be a nul character (\0) in the middle of + /// the region. On the other hand, it is not ensured that the string + /// is nul-terminated. So the usual string manipulation API may not work + /// as expected. + struct StringRegion { + const char* beg; ///< The start address of the string + size_t len; ///< The length of the string in bytes + }; + + /// \brief Constructor for non-value type of token. + /// + /// \throw InvalidParameter A value type token is specified. + /// \param type The type of the token. It must indicate a non-value + /// type (not larger than \c NOVALUE_TYPE_MAX). + explicit MasterToken(Type type) : type_(type) { + if (type > NOVALUE_TYPE_MAX) { + isc_throw(InvalidParameter, "Token per-type constructor " + "called with invalid type: " << type); + } + } + + /// \brief Constructor for string and quoted-string types of token. + /// + /// The optional \c quoted parameter specifies whether it's a quoted or + /// non quoted string. + /// + /// The string is specified as a pair of a pointer to the start address + /// and its length. Any character can be contained in any position of + /// the valid range (see \c StringRegion). + /// + /// When it's a quoted string, the quotation marks must be excluded + /// from the specified range. + /// + /// \param str_beg The start address of the string + /// \param str_len The size of the string in bytes + /// \param quoted true if it's a quoted string; false otherwise. + MasterToken(const char* str_beg, size_t str_len, bool quoted = false) : + type_(quoted ? QSTRING : STRING) + { + val_.str_region_.beg = str_beg; + val_.str_region_.len = str_len; + } + + /// \brief Constructor for number type of token. + /// + /// \brief number An unsigned 32-bit integer corresponding to the token + /// value. + explicit MasterToken(uint32_t number) : type_(NUMBER) { + val_.number_ = number; + } + + /// \brief Constructor for error type of token. + /// + /// \throw InvalidParameter Invalid error code value is specified. + /// \brief error_code A pre-defined constant of \c ErrorCode. + explicit MasterToken(ErrorCode error_code) : type_(ERROR) { + if (!(error_code < MAX_ERROR_CODE)) { + isc_throw(InvalidParameter, "Invalid master lexer error code: " + << error_code); + } + val_.error_code_ = error_code; + } + + /// \brief Return the token type. + /// + /// \throw none + Type getType() const { return (type_); } + + /// \brief Return the value of a string-variant token. + /// + /// \throw InvalidOperation Called on a non string-variant types of token. + /// \return A reference to \c StringRegion corresponding to the string + /// token value. + const StringRegion& getStringRegion() const { + if (type_ != STRING && type_ != QSTRING) { + isc_throw(InvalidOperation, + "Token::getStringRegion() for non string-variant type"); + } + return (val_.str_region_); + } + + /// \brief Return the value of a string-variant token as a string object. + /// + /// Note that the underlying string may contain a nul (\0) character + /// in the middle. The returned string object will contain all characters + /// of the valid range of the underlying string. So some string + /// operations such as c_str() may not work as expected. + /// + /// \throw InvalidOperation Called on a non string-variant types of token. + /// \throw std::bad_alloc Resource allocation failure in constructing the + /// string object. + /// \return A std::string object corresponding to the string token value. + std::string getString() const { + std::string ret; + getString(ret); + return (ret); + } + + /// \brief Fill in a string with the value of a string-variant token. + /// + /// This is similar to the other version of \c getString(), but + /// the caller is supposed to pass a placeholder string object. + /// This will be more efficient if the caller uses the same + /// \c MasterLexer repeatedly and needs to get string token in the + /// form of a string object many times as this version could reuse + /// the existing internal storage of the passed string. + /// + /// Any existing content of the passed string will be removed. + /// + /// \throw InvalidOperation Called on a non string-variant types of token. + /// \throw std::bad_alloc Resource allocation failure in constructing the + /// string object. + /// + /// \param ret A string object to be filled with the token string. + void getString(std::string& ret) const { + if (type_ != STRING && type_ != QSTRING) { + isc_throw(InvalidOperation, + "Token::getString() for non string-variant type"); + } + ret.assign(val_.str_region_.beg, + val_.str_region_.beg + val_.str_region_.len); + } + + /// \brief Return the value of a string-variant token as a string object. + /// + /// \throw InvalidOperation Called on a non number type of token. + /// \return The integer corresponding to the number token value. + uint32_t getNumber() const { + if (type_ != NUMBER) { + isc_throw(InvalidOperation, + "Token::getNumber() for non number type"); + } + return (val_.number_); + } + + /// \brief Return the error code of a error type token. + /// + /// \throw InvalidOperation Called on a non error type of token. + /// \return The error code of the token. + ErrorCode getErrorCode() const { + if (type_ != ERROR) { + isc_throw(InvalidOperation, + "Token::getErrorCode() for non error type"); + } + return (val_.error_code_); + }; + + /// \brief Return a textual description of the error of a error type token. + /// + /// The returned string would be useful to produce a log message when + /// a zone file parser encounters an error. + /// + /// \throw InvalidOperation Called on a non error type of token. + /// \throw std::bad_alloc Resource allocation failure in constructing the + /// string object. + /// \return A string object that describes the meaning of the error. + std::string getErrorText() const; + +private: + Type type_; // this is not const so the class can be assignable + + // We use a union to represent different types of token values via the + // unified Token class. The class integrity should ensure valid operation + // on the union; getter methods should only refer to the member set at + // the construction. + union { + StringRegion str_region_; + uint32_t number_; + ErrorCode error_code_; + } val_; +}; + /// \brief Tokenizer for parsing DNS master files. /// /// The \c MasterLexer class provides tokenize interfaces for parsing DNS @@ -77,8 +306,6 @@ public: {} }; - class Token; // we define it separately for better readability - /// \brief Options for getNextToken. /// /// A compound option, indicating multiple options are set, can be @@ -213,7 +440,7 @@ public: /// source (eg. I/O error in the file on the disk). /// \throw std::bad_alloc in case allocation of some internal resources /// or the token fail. - const Token& getNextToken(Options options = NONE); + const MasterToken& getNextToken(Options options = NONE); /// \brief Return the last token back to the lexer. /// @@ -247,235 +474,6 @@ operator|(MasterLexer::Options o1, MasterLexer::Options o2) { static_cast(o1) | static_cast(o2))); } -/// \brief Tokens for \c MasterLexer -/// -/// This is a simple value-class encapsulating a type of a lexer token and -/// (if it has a value) its value. Essentially, the class provides -/// constructors corresponding to different types of tokens, and corresponding -/// getter methods. The type and value are fixed at the time of construction -/// and will never be modified throughout the lifetime of the object. -/// The getter methods are still provided to maximize the safety; an -/// application cannot refer to a value that is invalid for the type of token. -/// -/// This class is intentionally implemented as copyable and assignable -/// (using the default version of copy constructor and assignment operator), -/// but it's mainly for internal implementation convenience. Applications will -/// simply refer to Token object as a reference via the \c MasterLexer class. -class MasterLexer::Token { -public: - /// \brief Enumeration for token types - /// - /// \note At the time of initial implementation, all numeric tokens - /// that would be extracted from \c MasterLexer should be represented - /// as an unsigned 32-bit integer. If we see the need for larger integers - /// or negative numbers, we can then extend the token types. - enum Type { - END_OF_LINE, ///< End of line detected - END_OF_FILE, ///< End of file detected - INITIAL_WS, ///< White spaces at the beginning of a line after an - ///< end of line (if asked for detecting it) - NOVALUE_TYPE_MAX = INITIAL_WS, ///< Max integer corresponding to - /// no-value (type only) types. - /// Mainly for internal use. - STRING, ///< A single string - QSTRING, ///< A single string quoted by double-quotes ("). - NUMBER, ///< A decimal number (unsigned 32-bit) - ERROR ///< Error detected in getting a token - }; - - /// \brief Enumeration for lexer error codes - enum ErrorCode { - NOT_STARTED, ///< The lexer is just initialized and has no token - UNBALANCED_PAREN, ///< Unbalanced parentheses detected - UNEXPECTED_END, ///< The lexer reaches the end of line or file - /// unexpectedly - 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_OUT_OF_RANGE, ///< Number was out of range - MAX_ERROR_CODE ///< Max integer corresponding to valid error codes. - /// (excluding this one). Mainly for internal use. - }; - - /// \brief A simple representation of a range of a string. - /// - /// This is a straightforward pair of the start pointer of a string - /// and its length. The \c STRING and \c QSTRING types of tokens - /// will be primarily represented in this form. - /// - /// Any character can be stored in the valid range of the region. - /// In particular, there can be a nul character (\0) in the middle of - /// the region. On the other hand, it is not ensured that the string - /// is nul-terminated. So the usual string manipulation API may not work - /// as expected. - struct StringRegion { - const char* beg; ///< The start address of the string - size_t len; ///< The length of the string in bytes - }; - - /// \brief Constructor for non-value type of token. - /// - /// \throw InvalidParameter A value type token is specified. - /// \param type The type of the token. It must indicate a non-value - /// type (not larger than \c NOVALUE_TYPE_MAX). - explicit Token(Type type) : type_(type) { - if (type > NOVALUE_TYPE_MAX) { - isc_throw(InvalidParameter, "Token per-type constructor " - "called with invalid type: " << type); - } - } - - /// \brief Constructor for string and quoted-string types of token. - /// - /// The optional \c quoted parameter specifies whether it's a quoted or - /// non quoted string. - /// - /// The string is specified as a pair of a pointer to the start address - /// and its length. Any character can be contained in any position of - /// the valid range (see \c StringRegion). - /// - /// When it's a quoted string, the quotation marks must be excluded - /// from the specified range. - /// - /// \param str_beg The start address of the string - /// \param str_len The size of the string in bytes - /// \param quoted true if it's a quoted string; false otherwise. - Token(const char* str_beg, size_t str_len, bool quoted = false) : - type_(quoted ? QSTRING : STRING) - { - val_.str_region_.beg = str_beg; - val_.str_region_.len = str_len; - } - - /// \brief Constructor for number type of token. - /// - /// \brief number An unsigned 32-bit integer corresponding to the token - /// value. - explicit Token(uint32_t number) : type_(NUMBER) { - val_.number_ = number; - } - - /// \brief Constructor for error type of token. - /// - /// \throw InvalidParameter Invalid error code value is specified. - /// \brief error_code A pre-defined constant of \c ErrorCode. - explicit Token(ErrorCode error_code) : type_(ERROR) { - if (!(error_code < MAX_ERROR_CODE)) { - isc_throw(InvalidParameter, "Invalid master lexer error code: " - << error_code); - } - val_.error_code_ = error_code; - } - - /// \brief Return the token type. - /// - /// \throw none - Type getType() const { return (type_); } - - /// \brief Return the value of a string-variant token. - /// - /// \throw InvalidOperation Called on a non string-variant types of token. - /// \return A reference to \c StringRegion corresponding to the string - /// token value. - const StringRegion& getStringRegion() const { - if (type_ != STRING && type_ != QSTRING) { - isc_throw(InvalidOperation, - "Token::getStringRegion() for non string-variant type"); - } - return (val_.str_region_); - } - - /// \brief Return the value of a string-variant token as a string object. - /// - /// Note that the underlying string may contain a nul (\0) character - /// in the middle. The returned string object will contain all characters - /// of the valid range of the underlying string. So some string - /// operations such as c_str() may not work as expected. - /// - /// \throw InvalidOperation Called on a non string-variant types of token. - /// \throw std::bad_alloc Resource allocation failure in constructing the - /// string object. - /// \return A std::string object corresponding to the string token value. - std::string getString() const { - std::string ret; - getString(ret); - return (ret); - } - - /// \brief Fill in a string with the value of a string-variant token. - /// - /// This is similar to the other version of \c getString(), but - /// the caller is supposed to pass a placeholder string object. - /// This will be more efficient if the caller uses the same - /// \c MasterLexer repeatedly and needs to get string token in the - /// form of a string object many times as this version could reuse - /// the existing internal storage of the passed string. - /// - /// Any existing content of the passed string will be removed. - /// - /// \throw InvalidOperation Called on a non string-variant types of token. - /// \throw std::bad_alloc Resource allocation failure in constructing the - /// string object. - /// - /// \param ret A string object to be filled with the token string. - void getString(std::string& ret) const { - if (type_ != STRING && type_ != QSTRING) { - isc_throw(InvalidOperation, - "Token::getString() for non string-variant type"); - } - ret.assign(val_.str_region_.beg, - val_.str_region_.beg + val_.str_region_.len); - } - - /// \brief Return the value of a string-variant token as a string object. - /// - /// \throw InvalidOperation Called on a non number type of token. - /// \return The integer corresponding to the number token value. - uint32_t getNumber() const { - if (type_ != NUMBER) { - isc_throw(InvalidOperation, - "Token::getNumber() for non number type"); - } - return (val_.number_); - } - - /// \brief Return the error code of a error type token. - /// - /// \throw InvalidOperation Called on a non error type of token. - /// \return The error code of the token. - ErrorCode getErrorCode() const { - if (type_ != ERROR) { - isc_throw(InvalidOperation, - "Token::getErrorCode() for non error type"); - } - return (val_.error_code_); - }; - - /// \brief Return a textual description of the error of a error type token. - /// - /// The returned string would be useful to produce a log message when - /// a zone file parser encounters an error. - /// - /// \throw InvalidOperation Called on a non error type of token. - /// \throw std::bad_alloc Resource allocation failure in constructing the - /// string object. - /// \return A string object that describes the meaning of the error. - std::string getErrorText() const; - -private: - Type type_; // this is not const so the class can be assignable - - // We use a union to represent different types of token values via the - // unified Token class. The class integrity should ensure valid operation - // on the union; getter methods should only refer to the member set at - // the construction. - union { - StringRegion str_region_; - uint32_t number_; - ErrorCode error_code_; - } val_; -}; - } // namespace dns } // namespace isc #endif // MASTER_LEXER_H diff --git a/src/lib/dns/master_lexer_state.h b/src/lib/dns/master_lexer_state.h index 9ddd68a598..f296c1c810 100644 --- a/src/lib/dns/master_lexer_state.h +++ b/src/lib/dns/master_lexer_state.h @@ -119,7 +119,7 @@ public: /// purposes. ///@{ bool wasLastEOL(const MasterLexer& lexer) const; - const MasterLexer::Token& getToken(const MasterLexer& lexer) const; + const MasterToken& getToken(const MasterLexer& lexer) const; size_t getParenCount(const MasterLexer& lexer) const; ///@} diff --git a/src/lib/dns/tests/master_lexer_state_unittest.cc b/src/lib/dns/tests/master_lexer_state_unittest.cc index d8a6b669b9..7645ede198 100644 --- a/src/lib/dns/tests/master_lexer_state_unittest.cc +++ b/src/lib/dns/tests/master_lexer_state_unittest.cc @@ -24,7 +24,7 @@ using namespace isc::dns; using namespace master_lexer_internal; namespace { -typedef MasterLexer::Token Token; // shortcut +typedef MasterToken Token; // shortcut class MasterLexerStateTest : public ::testing::Test { protected: @@ -260,7 +260,7 @@ TEST_F(MasterLexerStateTest, crlf) { // Commonly used check for string related test cases, checking if the given // token has expected values. void -stringTokenCheck(const std::string& expected, const MasterLexer::Token& token, +stringTokenCheck(const std::string& expected, const MasterToken& token, bool quoted = false) { EXPECT_EQ(quoted ? Token::QSTRING : Token::STRING, token.getType()); diff --git a/src/lib/dns/tests/master_lexer_token_unittest.cc b/src/lib/dns/tests/master_lexer_token_unittest.cc index 1f022dfc21..0fc7ff9519 100644 --- a/src/lib/dns/tests/master_lexer_token_unittest.cc +++ b/src/lib/dns/tests/master_lexer_token_unittest.cc @@ -31,27 +31,27 @@ const size_t TEST_STRING_LEN = sizeof(TEST_STRING) - 1; class MasterLexerTokenTest : public ::testing::Test { protected: MasterLexerTokenTest() : - token_eof(MasterLexer::Token::END_OF_FILE), + token_eof(MasterToken::END_OF_FILE), token_str(TEST_STRING, TEST_STRING_LEN), token_num(42), - token_err(MasterLexer::Token::UNEXPECTED_END) + token_err(MasterToken::UNEXPECTED_END) {} - const MasterLexer::Token token_eof; // an example of non-value type token - const MasterLexer::Token token_str; - const MasterLexer::Token token_num; - const MasterLexer::Token token_err; + const MasterToken token_eof; // an example of non-value type token + const MasterToken token_str; + const MasterToken token_num; + const MasterToken token_err; }; TEST_F(MasterLexerTokenTest, strings) { // basic construction and getter checks - EXPECT_EQ(MasterLexer::Token::STRING, token_str.getType()); + EXPECT_EQ(MasterToken::STRING, token_str.getType()); EXPECT_EQ(std::string("string token"), token_str.getString()); std::string strval = "dummy"; // this should be replaced token_str.getString(strval); EXPECT_EQ(std::string("string token"), strval); - const MasterLexer::Token::StringRegion str_region = + const MasterToken::StringRegion str_region = token_str.getStringRegion(); EXPECT_EQ(TEST_STRING, str_region.beg); EXPECT_EQ(TEST_STRING_LEN, str_region.len); @@ -62,17 +62,17 @@ TEST_F(MasterLexerTokenTest, strings) { std::string expected_str("string token"); expected_str.push_back('\0'); EXPECT_EQ(expected_str, - MasterLexer::Token(TEST_STRING, TEST_STRING_LEN + 1).getString()); - MasterLexer::Token(TEST_STRING, TEST_STRING_LEN + 1).getString(strval); + MasterToken(TEST_STRING, TEST_STRING_LEN + 1).getString()); + MasterToken(TEST_STRING, TEST_STRING_LEN + 1).getString(strval); EXPECT_EQ(expected_str, strval); // Construct type of qstring - EXPECT_EQ(MasterLexer::Token::QSTRING, - MasterLexer::Token(TEST_STRING, sizeof(TEST_STRING), true). + EXPECT_EQ(MasterToken::QSTRING, + MasterToken(TEST_STRING, sizeof(TEST_STRING), true). getType()); // if we explicitly set 'quoted' to false, it should be normal string - EXPECT_EQ(MasterLexer::Token::STRING, - MasterLexer::Token(TEST_STRING, sizeof(TEST_STRING), false). + EXPECT_EQ(MasterToken::STRING, + MasterToken(TEST_STRING, sizeof(TEST_STRING), false). getType()); // getString/StringRegion() aren't allowed for non string(-variant) types @@ -86,23 +86,23 @@ TEST_F(MasterLexerTokenTest, strings) { TEST_F(MasterLexerTokenTest, numbers) { EXPECT_EQ(42, token_num.getNumber()); - EXPECT_EQ(MasterLexer::Token::NUMBER, token_num.getType()); + EXPECT_EQ(MasterToken::NUMBER, token_num.getType()); // It's copyable and assignable. - MasterLexer::Token token(token_num); + MasterToken token(token_num); EXPECT_EQ(42, token.getNumber()); - EXPECT_EQ(MasterLexer::Token::NUMBER, token.getType()); + EXPECT_EQ(MasterToken::NUMBER, token.getType()); token = token_num; EXPECT_EQ(42, token.getNumber()); - EXPECT_EQ(MasterLexer::Token::NUMBER, token.getType()); + EXPECT_EQ(MasterToken::NUMBER, token.getType()); // it's okay to replace it with a different type of token token = token_eof; - EXPECT_EQ(MasterLexer::Token::END_OF_FILE, token.getType()); + EXPECT_EQ(MasterToken::END_OF_FILE, token.getType()); // Possible max value - token = MasterLexer::Token(0xffffffff); + token = MasterToken(0xffffffff); EXPECT_EQ(4294967295u, token.getNumber()); // getNumber() isn't allowed for non number types @@ -112,41 +112,34 @@ TEST_F(MasterLexerTokenTest, numbers) { TEST_F(MasterLexerTokenTest, novalues) { // Just checking we can construct them and getType() returns correct value. - EXPECT_EQ(MasterLexer::Token::END_OF_FILE, token_eof.getType()); - EXPECT_EQ(MasterLexer::Token::END_OF_LINE, - MasterLexer::Token(MasterLexer::Token::END_OF_LINE).getType()); - EXPECT_EQ(MasterLexer::Token::INITIAL_WS, - MasterLexer::Token(MasterLexer::Token::INITIAL_WS).getType()); + EXPECT_EQ(MasterToken::END_OF_FILE, token_eof.getType()); + EXPECT_EQ(MasterToken::END_OF_LINE, + MasterToken(MasterToken::END_OF_LINE).getType()); + EXPECT_EQ(MasterToken::INITIAL_WS, + MasterToken(MasterToken::INITIAL_WS).getType()); // Special types of tokens cannot have value-based types - EXPECT_THROW(MasterLexer::Token t(MasterLexer::Token::STRING), - isc::InvalidParameter); - EXPECT_THROW(MasterLexer::Token t(MasterLexer::Token::QSTRING), - isc::InvalidParameter); - EXPECT_THROW(MasterLexer::Token t(MasterLexer::Token::NUMBER), - isc::InvalidParameter); - EXPECT_THROW(MasterLexer::Token t(MasterLexer::Token::ERROR), - isc::InvalidParameter); + EXPECT_THROW(MasterToken t(MasterToken::STRING), isc::InvalidParameter); + EXPECT_THROW(MasterToken t(MasterToken::QSTRING), isc::InvalidParameter); + EXPECT_THROW(MasterToken t(MasterToken::NUMBER), isc::InvalidParameter); + EXPECT_THROW(MasterToken t(MasterToken::ERROR), isc::InvalidParameter); } TEST_F(MasterLexerTokenTest, errors) { - EXPECT_EQ(MasterLexer::Token::ERROR, token_err.getType()); - EXPECT_EQ(MasterLexer::Token::UNEXPECTED_END, token_err.getErrorCode()); + EXPECT_EQ(MasterToken::ERROR, token_err.getType()); + EXPECT_EQ(MasterToken::UNEXPECTED_END, token_err.getErrorCode()); EXPECT_EQ("unexpected end of input", token_err.getErrorText()); - EXPECT_EQ("lexer not started", - MasterLexer::Token(MasterLexer::Token::NOT_STARTED). + EXPECT_EQ("lexer not started", MasterToken(MasterToken::NOT_STARTED). getErrorText()); EXPECT_EQ("unbalanced parentheses", - MasterLexer::Token(MasterLexer::Token::UNBALANCED_PAREN). + MasterToken(MasterToken::UNBALANCED_PAREN). getErrorText()); - EXPECT_EQ("unbalanced quotes", - MasterLexer::Token(MasterLexer::Token::UNBALANCED_QUOTES). + EXPECT_EQ("unbalanced quotes", MasterToken(MasterToken::UNBALANCED_QUOTES). getErrorText()); - EXPECT_EQ("no token produced", - MasterLexer::Token(MasterLexer::Token::NO_TOKEN_PRODUCED). + EXPECT_EQ("no token produced", MasterToken(MasterToken::NO_TOKEN_PRODUCED). getErrorText()); EXPECT_EQ("number out of range", - MasterLexer::Token(MasterLexer::Token::NUMBER_OUT_OF_RANGE). + MasterLexer::Token(MasterToken::NUMBER_OUT_OF_RANGE). getErrorText()); // getErrorCode/Text() isn't allowed for non number types @@ -156,14 +149,13 @@ TEST_F(MasterLexerTokenTest, errors) { // 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(6)), + EXPECT_THROW(MasterToken(MasterToken::ErrorCode(6)), isc::InvalidParameter); // Check the coexistence of "from number" and "from error-code" // constructors won't cause confusion. - EXPECT_EQ(MasterLexer::Token::NUMBER, - MasterLexer::Token(static_cast( - MasterLexer::Token::NOT_STARTED)). + EXPECT_EQ(MasterToken::NUMBER, + MasterToken(static_cast(MasterToken::NOT_STARTED)). getType()); } } diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc index eca6a73663..7bec68f359 100644 --- a/src/lib/dns/tests/master_lexer_unittest.cc +++ b/src/lib/dns/tests/master_lexer_unittest.cc @@ -141,19 +141,19 @@ TEST_F(MasterLexerTest, getNextToken) { lexer.pushSource(ss); // First, the newline should get out. - EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); // Then the whitespace, if we specify the option. - EXPECT_EQ(MasterLexer::Token::INITIAL_WS, + EXPECT_EQ(MasterToken::INITIAL_WS, lexer.getNextToken(MasterLexer::INITIAL_WS).getType()); // The newline - EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); // The (quoted) string - EXPECT_EQ(MasterLexer::Token::QSTRING, + EXPECT_EQ(MasterToken::QSTRING, lexer.getNextToken(MasterLexer::QSTRING).getType()); // And the end of line and file - EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType()); - EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_FILE, lexer.getNextToken().getType()); } // Test we correctly find end of file. @@ -162,12 +162,12 @@ TEST_F(MasterLexerTest, eof) { lexer.pushSource(ss); // The first one is found to be EOF - EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_FILE, lexer.getNextToken().getType()); // And it stays on EOF for any following attempts - EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_FILE, lexer.getNextToken().getType()); // And we can step back one token, but that is the EOF too. lexer.ungetToken(); - EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_FILE, lexer.getNextToken().getType()); } // Check we properly return error when there's an opened parentheses and no @@ -177,12 +177,12 @@ TEST_F(MasterLexerTest, getUnbalancedParen) { lexer.pushSource(ss); // The string gets out first - EXPECT_EQ(MasterLexer::Token::STRING, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::STRING, lexer.getNextToken().getType()); // Then an unbalanced parenthesis - EXPECT_EQ(MasterLexer::Token::UNBALANCED_PAREN, + EXPECT_EQ(MasterToken::UNBALANCED_PAREN, lexer.getNextToken().getErrorCode()); // And then EOF - EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_FILE, lexer.getNextToken().getType()); } // Check we properly return error when there's an opened quoted string and no @@ -192,10 +192,10 @@ TEST_F(MasterLexerTest, getUnbalancedString) { lexer.pushSource(ss); // Then an unbalanced qstring (reported as an unexpected end) - EXPECT_EQ(MasterLexer::Token::UNEXPECTED_END, + EXPECT_EQ(MasterToken::UNEXPECTED_END, lexer.getNextToken(MasterLexer::QSTRING).getErrorCode()); // And then EOF - EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_FILE, lexer.getNextToken().getType()); } // Test ungetting tokens works @@ -204,28 +204,28 @@ TEST_F(MasterLexerTest, ungetToken) { lexer.pushSource(ss); // Try getting the newline - EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); // Return it and get again lexer.ungetToken(); - EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); // Get the string and return it back - EXPECT_EQ(MasterLexer::Token::QSTRING, + EXPECT_EQ(MasterToken::QSTRING, lexer.getNextToken(MasterLexer::QSTRING).getType()); lexer.ungetToken(); // But if we change the options, it honors them - EXPECT_EQ(MasterLexer::Token::INITIAL_WS, + EXPECT_EQ(MasterToken::INITIAL_WS, lexer.getNextToken(MasterLexer::QSTRING | MasterLexer::INITIAL_WS).getType()); // Get to the "more" string - EXPECT_EQ(MasterLexer::Token::QSTRING, + EXPECT_EQ(MasterToken::QSTRING, lexer.getNextToken(MasterLexer::QSTRING).getType()); - EXPECT_EQ(MasterLexer::Token::STRING, + EXPECT_EQ(MasterToken::STRING, lexer.getNextToken(MasterLexer::QSTRING).getType()); // Return it back. It should get inside the parentheses. // Upon next attempt to get it again, the newline inside the parentheses // should be still ignored. lexer.ungetToken(); - EXPECT_EQ(MasterLexer::Token::STRING, + EXPECT_EQ(MasterToken::STRING, lexer.getNextToken(MasterLexer::QSTRING).getType()); } @@ -235,16 +235,16 @@ TEST_F(MasterLexerTest, ungetRealOptions) { ss << "\n \n"; lexer.pushSource(ss); // Skip the first newline - EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); // If we call it the usual way, it skips up to the newline and returns // it - EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); // Now we return it. If we call it again, but with different options, // we get the initial whitespace. lexer.ungetToken(); - EXPECT_EQ(MasterLexer::Token::INITIAL_WS, + EXPECT_EQ(MasterToken::INITIAL_WS, lexer.getNextToken(MasterLexer::INITIAL_WS).getType()); } @@ -253,7 +253,7 @@ TEST_F(MasterLexerTest, ungetTwice) { ss << "\n"; lexer.pushSource(ss); - EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); // Unget the token. It can be done once lexer.ungetToken(); // But not twice @@ -271,14 +271,14 @@ TEST_F(MasterLexerTest, ungetBeforeGet) { TEST_F(MasterLexerTest, ungetAfterSwitch) { ss << "\n\n"; lexer.pushSource(ss); - EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); // Switch the source std::stringstream ss2; ss2 << "\n\n"; lexer.pushSource(ss2); EXPECT_THROW(lexer.ungetToken(), isc::InvalidOperation); // We can get from the new source - EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType()); + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); // And when we drop the current source, we can't unget again lexer.popSource(); EXPECT_THROW(lexer.ungetToken(), isc::InvalidOperation); From b991213a389213fe42ef0c87bde56e462c025229 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 29 Nov 2012 09:59:54 -0800 Subject: [PATCH 04/69] [2506] added another version of getNextToken, basic part --- src/lib/dns/master_lexer.cc | 28 +++++++++ src/lib/dns/master_lexer.h | 12 ++++ src/lib/dns/tests/master_lexer_unittest.cc | 72 ++++++++++++++++++++++ 3 files changed, 112 insertions(+) diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc index 4db2bf8527..74eb42ef98 100644 --- a/src/lib/dns/master_lexer.cc +++ b/src/lib/dns/master_lexer.cc @@ -192,6 +192,34 @@ MasterLexer::getNextToken(Options options) { return (impl_->token_); } +const MasterToken& +MasterLexer::getNextToken(MasterToken::Type expect, bool eol_ok) { + Options options = NONE; + if (expect == MasterToken::QSTRING) { + options = options | QSTRING; + } + + getNextToken(options); // the result should be set in impl_->token_ + + const bool is_eol_like = + (impl_->token_.getType() == MasterToken::END_OF_LINE || + impl_->token_.getType() == MasterToken::END_OF_FILE); + if (eol_ok && is_eol_like) { + return (impl_->token_); + } + if (impl_->token_.getType() == MasterToken::STRING && + expect == MasterToken::QSTRING) { + return (impl_->token_); + } + if (impl_->token_.getType() != expect) { + ungetToken(); + throw LexerError(__FILE__, __LINE__, + MasterToken(MasterToken::UNEXPECTED_END)); + } + + return (impl_->token_); +} + void MasterLexer::ungetToken() { if (impl_->has_previous_) { diff --git a/src/lib/dns/master_lexer.h b/src/lib/dns/master_lexer.h index 13205be14f..bee6b4690e 100644 --- a/src/lib/dns/master_lexer.h +++ b/src/lib/dns/master_lexer.h @@ -306,6 +306,15 @@ public: {} }; + class LexerError : public Exception { + public: + LexerError(const char* file, size_t line, MasterToken error_token) : + Exception(file, line, error_token.getErrorText().c_str()), + token_(error_token) + {} + const MasterToken token_; + }; + /// \brief Options for getNextToken. /// /// A compound option, indicating multiple options are set, can be @@ -442,6 +451,9 @@ public: /// or the token fail. const MasterToken& getNextToken(Options options = NONE); + const MasterToken& getNextToken(MasterToken::Type expect, + bool eol_ok = false); + /// \brief Return the last token back to the lexer. /// /// The method undoes the lasts call to getNextToken(). If you call the diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc index 7bec68f359..e8daf0886b 100644 --- a/src/lib/dns/tests/master_lexer_unittest.cc +++ b/src/lib/dns/tests/master_lexer_unittest.cc @@ -284,4 +284,76 @@ TEST_F(MasterLexerTest, ungetAfterSwitch) { EXPECT_THROW(lexer.ungetToken(), isc::InvalidOperation); } +// Common checks regarding expected/unexpected end-of-line +void +eolCheck(MasterLexer& lexer, MasterToken::Type expect) { + // If EOL is found and eol_ok is true, we get it. + EXPECT_EQ(MasterToken::END_OF_LINE, + lexer.getNextToken(expect, true).getType()); + // We'll see the second '\n'; by default it will fail. + EXPECT_THROW(lexer.getNextToken(expect), MasterLexer::LexerError); + // Same if eol_ok is explicitly set to false. This also checks the + // offending '\n' was "ungotten". + EXPECT_THROW(lexer.getNextToken(expect, false), MasterLexer::LexerError); + + // And also check the error token set in the exception object. + bool thrown = false; + try { + lexer.getNextToken(expect); + } catch (const MasterLexer::LexerError& error) { + EXPECT_EQ(MasterToken::UNEXPECTED_END, error.token_.getErrorCode()); + thrown = true; + } + EXPECT_TRUE(thrown); +} + +// Common checks regarding expected/unexpected end-of-file +void +eofCheck(MasterLexer& lexer, MasterToken::Type expect) { + EXPECT_EQ(MasterToken::END_OF_FILE, + lexer.getNextToken(expect, true).getType()); + EXPECT_THROW(lexer.getNextToken(expect), MasterLexer::LexerError); + EXPECT_THROW(lexer.getNextToken(expect, false), MasterLexer::LexerError); +} + +TEST_F(MasterLexerTest, getNextTokenString) { + ss << "normal-string\n"; + ss << "\n"; + ss << "another-string"; + lexer.pushSource(ss); + + // Normal successful case: Expecting a string and get one. + EXPECT_EQ("normal-string", + lexer.getNextToken(MasterToken::STRING).getString()); + eolCheck(lexer, MasterToken::STRING); + + // Skip the 2nd '\n' + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); + + // Same set of tests but for end-of-file + EXPECT_EQ("another-string", + lexer.getNextToken(MasterToken::STRING, true).getString()); + eofCheck(lexer, MasterToken::STRING); +} + +TEST_F(MasterLexerTest, getNextTokenQString) { + ss << "\"quoted-string\"\n"; + ss << "\n"; + ss << "normal-string"; + lexer.pushSource(ss); + + // Expecting a quoted string and get one. + EXPECT_EQ("quoted-string", + lexer.getNextToken(MasterToken::QSTRING).getString()); + eolCheck(lexer, MasterToken::QSTRING); + + // Skip the 2nd '\n' + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); + + // Expecting a quoted string but see a normal string. It's okay. + EXPECT_EQ("normal-string", + lexer.getNextToken(MasterToken::QSTRING).getString()); + eofCheck(lexer, MasterToken::QSTRING); +} + } From f70d131fd20085a472de32f86299a77647f18fd5 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 29 Nov 2012 10:06:01 -0800 Subject: [PATCH 05/69] [2506] resolved remaining conflicts on rebase --- src/lib/dns/master_lexer.cc | 4 ++-- src/lib/dns/tests/master_lexer_token_unittest.cc | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc index 74eb42ef98..5ea9ab7c2d 100644 --- a/src/lib/dns/master_lexer.cc +++ b/src/lib/dns/master_lexer.cc @@ -495,11 +495,11 @@ Number::handle(MasterLexer& lexer) const { try { const uint32_t number32 = boost::lexical_cast(&data[0]); - token = MasterLexer::Token(number32); + token = MasterToken(number32); } catch (const boost::bad_lexical_cast&) { // Since we already know we have only digits, // range should be the only possible problem. - token = Token(MasterToken::NUMBER_OUT_OF_RANGE); + token = MasterToken(MasterToken::NUMBER_OUT_OF_RANGE); } } else { token = MasterToken(&data.at(0), data.size()); diff --git a/src/lib/dns/tests/master_lexer_token_unittest.cc b/src/lib/dns/tests/master_lexer_token_unittest.cc index 0fc7ff9519..7cb83a5fa8 100644 --- a/src/lib/dns/tests/master_lexer_token_unittest.cc +++ b/src/lib/dns/tests/master_lexer_token_unittest.cc @@ -139,7 +139,7 @@ TEST_F(MasterLexerTokenTest, errors) { EXPECT_EQ("no token produced", MasterToken(MasterToken::NO_TOKEN_PRODUCED). getErrorText()); EXPECT_EQ("number out of range", - MasterLexer::Token(MasterToken::NUMBER_OUT_OF_RANGE). + MasterToken(MasterToken::NUMBER_OUT_OF_RANGE). getErrorText()); // getErrorCode/Text() isn't allowed for non number types From d52186367e1f8b49dcb5a3b4825841a95251412f Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 29 Nov 2012 10:14:01 -0800 Subject: [PATCH 06/69] [2506] supported NUMBER in getNextToken() with expected type --- src/lib/dns/master_lexer.cc | 24 ++++++++++++++++------ src/lib/dns/tests/master_lexer_unittest.cc | 21 +++++++++++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc index 5ea9ab7c2d..3127247141 100644 --- a/src/lib/dns/master_lexer.cc +++ b/src/lib/dns/master_lexer.cc @@ -192,14 +192,26 @@ MasterLexer::getNextToken(Options options) { return (impl_->token_); } +namespace { +inline MasterLexer::Options +optionsForTokenType(MasterToken::Type expect) { + switch (expect) { + case MasterToken::STRING: + return (MasterLexer::NONE); + case MasterToken::QSTRING: + return (MasterLexer::QSTRING); + case MasterToken::NUMBER: + return (MasterLexer::NUMBER); + default: + assert(false); + } +} +} + const MasterToken& MasterLexer::getNextToken(MasterToken::Type expect, bool eol_ok) { - Options options = NONE; - if (expect == MasterToken::QSTRING) { - options = options | QSTRING; - } - - getNextToken(options); // the result should be set in impl_->token_ + // the result should be set in impl_->token_ + getNextToken(optionsForTokenType(expect)); const bool is_eol_like = (impl_->token_.getType() == MasterToken::END_OF_LINE || diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc index e8daf0886b..ced9d31056 100644 --- a/src/lib/dns/tests/master_lexer_unittest.cc +++ b/src/lib/dns/tests/master_lexer_unittest.cc @@ -356,4 +356,25 @@ TEST_F(MasterLexerTest, getNextTokenQString) { eofCheck(lexer, MasterToken::QSTRING); } +TEST_F(MasterLexerTest, getNextTokenNumber) { + ss << "3600\n"; + ss << "\n"; + ss << "86400"; + lexer.pushSource(ss); + + // Expecting a number string and get one. + EXPECT_EQ(3600, + lexer.getNextToken(MasterToken::NUMBER).getNumber()); + eolCheck(lexer, MasterToken::NUMBER); + + // Skip the 2nd '\n' + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); + + // Unless we specify NUMBER, decimal number string should be recognized + // as a string. + EXPECT_EQ("86400", + lexer.getNextToken(MasterToken::STRING).getString()); + eofCheck(lexer, MasterToken::NUMBER); +} + } From ae3a1e5d17d5c8ab8c9dcf20cf60a68744f5cca0 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 29 Nov 2012 10:28:26 -0800 Subject: [PATCH 07/69] [2506] handled the case a number is expected but not found --- src/lib/dns/master_lexer.cc | 10 ++++++++-- src/lib/dns/master_lexer.h | 1 + src/lib/dns/tests/master_lexer_token_unittest.cc | 6 ++++-- src/lib/dns/tests/master_lexer_unittest.cc | 13 +++++++++++++ 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc index 3127247141..0b60d3b76c 100644 --- a/src/lib/dns/master_lexer.cc +++ b/src/lib/dns/master_lexer.cc @@ -225,8 +225,13 @@ MasterLexer::getNextToken(MasterToken::Type expect, bool eol_ok) { } if (impl_->token_.getType() != expect) { ungetToken(); + if (is_eol_like) { + throw LexerError(__FILE__, __LINE__, + MasterToken(MasterToken::UNEXPECTED_END)); + } + assert(expect == MasterToken::NUMBER); throw LexerError(__FILE__, __LINE__, - MasterToken(MasterToken::UNEXPECTED_END)); + MasterToken(MasterToken::BAD_NUMBER)); } return (impl_->token_); @@ -251,7 +256,8 @@ const char* const error_text[] = { "unexpected end of input", // UNEXPECTED_END "unbalanced quotes", // UNBALANCED_QUOTES "no token produced", // NO_TOKEN_PRODUCED - "number out of range" // NUMBER_OUT_OF_RANGE + "number out of range", // NUMBER_OUT_OF_RANGE + "not a valid number" // BAD_NUMBER }; const size_t error_text_max_count = sizeof(error_text) / sizeof(error_text[0]); } // end unnamed namespace diff --git a/src/lib/dns/master_lexer.h b/src/lib/dns/master_lexer.h index bee6b4690e..c7c9175898 100644 --- a/src/lib/dns/master_lexer.h +++ b/src/lib/dns/master_lexer.h @@ -74,6 +74,7 @@ public: NO_TOKEN_PRODUCED, ///< No token was produced. This means programmer /// 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 MAX_ERROR_CODE ///< Max integer corresponding to valid error codes. /// (excluding this one). Mainly for internal use. }; diff --git a/src/lib/dns/tests/master_lexer_token_unittest.cc b/src/lib/dns/tests/master_lexer_token_unittest.cc index 7cb83a5fa8..89a4f9c593 100644 --- a/src/lib/dns/tests/master_lexer_token_unittest.cc +++ b/src/lib/dns/tests/master_lexer_token_unittest.cc @@ -141,15 +141,17 @@ TEST_F(MasterLexerTokenTest, errors) { EXPECT_EQ("number out of range", MasterToken(MasterToken::NUMBER_OUT_OF_RANGE). getErrorText()); + EXPECT_EQ("not a valid number", + MasterToken(MasterToken::BAD_NUMBER).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 '6' (max code + // Only the pre-defined error code is accepted. Hardcoding '7' (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(6)), + EXPECT_THROW(MasterToken(MasterToken::ErrorCode(7)), isc::InvalidParameter); // Check the coexistence of "from number" and "from error-code" diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc index ced9d31056..a42bb19086 100644 --- a/src/lib/dns/tests/master_lexer_unittest.cc +++ b/src/lib/dns/tests/master_lexer_unittest.cc @@ -359,6 +359,7 @@ TEST_F(MasterLexerTest, getNextTokenQString) { TEST_F(MasterLexerTest, getNextTokenNumber) { ss << "3600\n"; ss << "\n"; + ss << "not-a-number "; ss << "86400"; lexer.pushSource(ss); @@ -370,6 +371,18 @@ TEST_F(MasterLexerTest, getNextTokenNumber) { // Skip the 2nd '\n' EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); + // Expecting a number, but see a string. + bool thrown = false; + try { + lexer.getNextToken(MasterToken::NUMBER); + } catch (const MasterLexer::LexerError& error) { + EXPECT_EQ(MasterToken::BAD_NUMBER, error.token_.getErrorCode()); + thrown = true; + } + EXPECT_TRUE(thrown); + // The unexpected string should have been "ungotten". Re-read and skip it. + EXPECT_EQ(MasterToken::STRING, lexer.getNextToken().getType()); + // Unless we specify NUMBER, decimal number string should be recognized // as a string. EXPECT_EQ("86400", From 597ed99f869f9d170c93467e8abcc2b03959f75f Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 29 Nov 2012 10:40:28 -0800 Subject: [PATCH 08/69] [2506] handled the case when getNextToken expects a number but it's too big. --- src/lib/dns/master_lexer.cc | 6 ++++ src/lib/dns/tests/master_lexer_unittest.cc | 40 +++++++++++++--------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc index 0b60d3b76c..920e99d2ec 100644 --- a/src/lib/dns/master_lexer.cc +++ b/src/lib/dns/master_lexer.cc @@ -213,6 +213,12 @@ MasterLexer::getNextToken(MasterToken::Type expect, bool eol_ok) { // the result should be set in impl_->token_ getNextToken(optionsForTokenType(expect)); + if (impl_->token_.getType() == MasterToken::ERROR) { + //if (impl_->token_.getErrorCode() == MasterToken::NUMBER_OUT_OF_RANGE) + ungetToken(); + throw LexerError(__FILE__, __LINE__, impl_->token_); + } + const bool is_eol_like = (impl_->token_.getType() == MasterToken::END_OF_LINE || impl_->token_.getType() == MasterToken::END_OF_FILE); diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc index a42bb19086..31e482f828 100644 --- a/src/lib/dns/tests/master_lexer_unittest.cc +++ b/src/lib/dns/tests/master_lexer_unittest.cc @@ -284,6 +284,21 @@ TEST_F(MasterLexerTest, ungetAfterSwitch) { EXPECT_THROW(lexer.ungetToken(), isc::InvalidOperation); } +// Common checks for the case when getNextToken() should result in LexerError +void +lexerErrorCheck(MasterLexer& lexer, MasterToken::Type expect, + MasterToken::ErrorCode expected_error) +{ + bool thrown = false; + try { + lexer.getNextToken(expect); + } catch (const MasterLexer::LexerError& error) { + EXPECT_EQ(expected_error, error.token_.getErrorCode()); + thrown = true; + } + EXPECT_TRUE(thrown); +} + // Common checks regarding expected/unexpected end-of-line void eolCheck(MasterLexer& lexer, MasterToken::Type expect) { @@ -297,14 +312,7 @@ eolCheck(MasterLexer& lexer, MasterToken::Type expect) { EXPECT_THROW(lexer.getNextToken(expect, false), MasterLexer::LexerError); // And also check the error token set in the exception object. - bool thrown = false; - try { - lexer.getNextToken(expect); - } catch (const MasterLexer::LexerError& error) { - EXPECT_EQ(MasterToken::UNEXPECTED_END, error.token_.getErrorCode()); - thrown = true; - } - EXPECT_TRUE(thrown); + lexerErrorCheck(lexer, expect, MasterToken::UNEXPECTED_END); } // Common checks regarding expected/unexpected end-of-file @@ -359,6 +367,7 @@ TEST_F(MasterLexerTest, getNextTokenQString) { TEST_F(MasterLexerTest, getNextTokenNumber) { ss << "3600\n"; ss << "\n"; + ss << "4294967296 "; // =2^32, out of range ss << "not-a-number "; ss << "86400"; lexer.pushSource(ss); @@ -371,15 +380,14 @@ TEST_F(MasterLexerTest, getNextTokenNumber) { // Skip the 2nd '\n' EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); + // Expecting a number, but it's too big for uint32. + lexerErrorCheck(lexer, MasterToken::NUMBER, + MasterToken::NUMBER_OUT_OF_RANGE); + // The token should have been "ungotten". Re-read and skip it. + EXPECT_EQ(MasterToken::STRING, lexer.getNextToken().getType()); + // Expecting a number, but see a string. - bool thrown = false; - try { - lexer.getNextToken(MasterToken::NUMBER); - } catch (const MasterLexer::LexerError& error) { - EXPECT_EQ(MasterToken::BAD_NUMBER, error.token_.getErrorCode()); - thrown = true; - } - EXPECT_TRUE(thrown); + lexerErrorCheck(lexer, MasterToken::NUMBER, MasterToken::BAD_NUMBER); // The unexpected string should have been "ungotten". Re-read and skip it. EXPECT_EQ(MasterToken::STRING, lexer.getNextToken().getType()); From 09cd4b9b32415144a2542171558b56226322bd7e Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 29 Nov 2012 10:54:54 -0800 Subject: [PATCH 09/69] [2506] handle other rare error cases --- src/lib/dns/master_lexer.cc | 11 +++++---- src/lib/dns/tests/master_lexer_unittest.cc | 26 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc index 920e99d2ec..f1f57493da 100644 --- a/src/lib/dns/master_lexer.cc +++ b/src/lib/dns/master_lexer.cc @@ -203,19 +203,22 @@ optionsForTokenType(MasterToken::Type expect) { case MasterToken::NUMBER: return (MasterLexer::NUMBER); default: - assert(false); + isc_throw(InvalidParameter, + "expected type for getNextToken not supported: " << expect); } } } const MasterToken& MasterLexer::getNextToken(MasterToken::Type expect, bool eol_ok) { - // the result should be set in impl_->token_ + // Get the next token, specifying an appropriate option corresponding to + // the expected type. The result should be set in impl_->token_. getNextToken(optionsForTokenType(expect)); if (impl_->token_.getType() == MasterToken::ERROR) { - //if (impl_->token_.getErrorCode() == MasterToken::NUMBER_OUT_OF_RANGE) - ungetToken(); + if (impl_->token_.getErrorCode() == MasterToken::NUMBER_OUT_OF_RANGE) { + ungetToken(); + } throw LexerError(__FILE__, __LINE__, impl_->token_); } diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc index 31e482f828..b751da84f3 100644 --- a/src/lib/dns/tests/master_lexer_unittest.cc +++ b/src/lib/dns/tests/master_lexer_unittest.cc @@ -398,4 +398,30 @@ TEST_F(MasterLexerTest, getNextTokenNumber) { eofCheck(lexer, MasterToken::NUMBER); } +TEST_F(MasterLexerTest, getNextTokenErrors) { + // Check miscellaneous error cases + + ss << ") "; // unbalanced parenthesis + ss << "string-after-error "; + lexer.pushSource(ss); + + // Only string/qstring/number can be "expected". + EXPECT_THROW(lexer.getNextToken(MasterToken::END_OF_LINE), + isc::InvalidParameter); + EXPECT_THROW(lexer.getNextToken(MasterToken::END_OF_FILE), + isc::InvalidParameter); + EXPECT_THROW(lexer.getNextToken(MasterToken::INITIAL_WS), + isc::InvalidParameter); + EXPECT_THROW(lexer.getNextToken(MasterToken::ERROR), + isc::InvalidParameter); + + // If it encounters a syntax error, it results in LexerError exception. + lexerErrorCheck(lexer, MasterToken::STRING, MasterToken::UNBALANCED_PAREN); + + // Unlike the NUMBER_OUT_OF_RANGE case, the error part has been skipped + // within getNextToken(). We should be able to get the next token. + EXPECT_EQ("string-after-error", + lexer.getNextToken(MasterToken::STRING).getString()); +} + } From d27729277c9e4c9bb4cf121700d67181d6c0aadc Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 29 Nov 2012 13:28:20 -0800 Subject: [PATCH 10/69] [2506] documentation update --- src/lib/dns/master_lexer.h | 74 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/src/lib/dns/master_lexer.h b/src/lib/dns/master_lexer.h index c7c9175898..7a8a1d1593 100644 --- a/src/lib/dns/master_lexer.h +++ b/src/lib/dns/master_lexer.h @@ -307,6 +307,13 @@ public: {} }; + /// \brief Exception thrown from a wrapper version of + /// \c MasterLexer::getNextToken() for non fatal errors. + /// + /// See the method description for more details. + /// + /// The \c token_ member variable (read-only) is set to a \c MasterToken + /// object of type ERROR indicating the reason for the error. class LexerError : public Exception { public: LexerError(const char* file, size_t line, MasterToken error_token) : @@ -452,6 +459,73 @@ public: /// or the token fail. const MasterToken& getNextToken(Options options = NONE); + /// \brief Parse the input for the expected type of token. + /// + /// This method is a wrapper of the other version, customized for the case + /// where a particular type of token is expected as the next one. + /// More specifically, it's intended to be used to get tokens for RDATA + /// fields. Since most RDATA types of fixed format, the token type is + /// often predictable and the method interface can be simplified. + /// + /// This method basically works as follows: it gets the type of the + /// expected token, calls the other version of \c getNextToken(Options), + /// and returns the token if it's of the expected type (due to the usage + /// assumption this should be normally the case). There are some non + /// trivial details though: + /// + /// - If the expected type is MasterToken::QSTRING, both quoted and + /// unquoted strings are recognized and returned. + /// - 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 + /// token. + /// - If the next token is not of the expected type (including the case + /// a number is expected but it's out of range), ungetToken() is + /// internally called so the caller can re-read that token. + /// - If other types or errors (such as unbalanced parentheses) are + /// detected, the erroneous part isn't "ungotten"; the caller can + /// continue parsing after that part. + /// + /// In some very rare cases where the RDATA has an optional trailing field, + /// the \c eol_ok parameter would be set to \c true. This way the caller + /// can handle both cases (the field does or does not exist) by a single + /// call to this method. In all other cases \c eol_ok should be set to + /// \c false, and that is the default and can be omitted. + /// + /// Unlike the other version of \c getNextToken(Options), this method + /// throws an exception of type \c LexerError for non fatal errors such as + /// broken syntax or encountering an unexpected type of token. This way + /// the caller can write RDATA parser code without bothering to handle + /// errors for each field. For example, pseudo parser code for MX RDATA + /// would look like this: + /// \code + /// const uint32_t pref = + /// lexer.getNextToken(MasterToken::NUMBER).getNumber(); + /// // check if pref is the uint16_t range; no other check is needed. + /// const Name mx(lexer.getNextToken(MasterToken::STRING).getString()); + /// \endcode + /// + /// In the case where \c LexerError exception is thrown, it's expected + /// to be handled comprehensively for the parser of the RDATA or at a + /// higher layer. The \c token_ member variable of the corresponding + /// \c LexerError exception object stores a token of type + /// \c MasterToken::ERROR that indicates the reason for the error. + /// + /// Due to the specific intended usage of this method, only a subset + /// of \c MasterToken::Type values are acceptable for the \c expect + /// parameter: \c MasterToken::STRING, \c MasterToken::QSTRING, and + /// \c MasterToken::NUMBER. Specifying other values will result in + /// an \c InvalidParameter exception. + /// + /// \throw InvalidParameter The expected token type is not allowed for + /// this method. + /// \throw LexerError The lexer finds non fatal error or it finds an + /// \throw other Anything the other version of getNextToken() can throw. + /// + /// \param expect Expected type of token. Must be either STRING, QSTRING, + /// or NUMBER. + /// \param eol_ok \c true iff END_OF_LINE or END_OF_FILE is acceptable. + /// \return The expected type of token. const MasterToken& getNextToken(MasterToken::Type expect, bool eol_ok = false); From d4b9c33282199c912e6f795f6bbfddde12510795 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 29 Nov 2012 13:29:02 -0800 Subject: [PATCH 11/69] [2506] (unrelated) cleanup: define ReadError as a class, not a struct. for consistency; it's awkward to see it as a struct while LexerError as a .class. The choice is basically a matter of taste, but I personally think it's better to be defined as a class as its base is defined as a class, and these exception structs are not just a trivial set of values. --- src/lib/dns/master_lexer.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/dns/master_lexer.h b/src/lib/dns/master_lexer.h index 7a8a1d1593..fdc69fcfa3 100644 --- a/src/lib/dns/master_lexer.h +++ b/src/lib/dns/master_lexer.h @@ -301,7 +301,8 @@ class MasterLexer { public: /// \brief Exception thrown when we fail to read from the input /// stream or file. - struct ReadError : public Unexpected { + class ReadError : public Unexpected { + public: ReadError(const char* file, size_t line, const char* what) : Unexpected(file, line, what) {} From 3e964a48bd208e8a7dc04ef1ff8de5cf1da5a514 Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Fri, 30 Nov 2012 17:42:39 +0530 Subject: [PATCH 12/69] [2497] Add tests for NSEC3, NSEC and TSIG rrtypes --- src/lib/dns/tests/rdata_nsec3_unittest.cc | 14 ++++++++++++++ src/lib/dns/tests/rdata_nsec_unittest.cc | 13 +++++++++++++ src/lib/dns/tests/rdata_tsig_unittest.cc | 12 ++++++++++++ 3 files changed, 39 insertions(+) diff --git a/src/lib/dns/tests/rdata_nsec3_unittest.cc b/src/lib/dns/tests/rdata_nsec3_unittest.cc index edd2d4b6e7..c7ca93e2bb 100644 --- a/src/lib/dns/tests/rdata_nsec3_unittest.cc +++ b/src/lib/dns/tests/rdata_nsec3_unittest.cc @@ -130,6 +130,20 @@ TEST_F(Rdata_NSEC3_Test, createFromWire) { } } +TEST_F(Rdata_NSEC3_Test, createFromLexer) { + const generic::NSEC3 rdata_nsec3(nsec3_txt); + EXPECT_EQ(0, rdata_nsec3.compare( + *test::createRdataUsingLexer(RRType::NSEC3(), RRClass::IN(), + nsec3_txt))); + + // Check that bad input throws as usual (next hash shouldn't be + // padded) + EXPECT_THROW({ + *test::createRdataUsingLexer(RRType::NSEC3(), RRClass::IN(), + "1 1 1 ADDAFEEE CPNMU=== A NS SOA"); + }, InvalidRdataText); +} + TEST_F(Rdata_NSEC3_Test, assign) { generic::NSEC3 rdata_nsec3(nsec3_txt); generic::NSEC3 other_nsec3 = rdata_nsec3; diff --git a/src/lib/dns/tests/rdata_nsec_unittest.cc b/src/lib/dns/tests/rdata_nsec_unittest.cc index 88c6201b65..5e5ac89a69 100644 --- a/src/lib/dns/tests/rdata_nsec_unittest.cc +++ b/src/lib/dns/tests/rdata_nsec_unittest.cc @@ -66,6 +66,19 @@ TEST_F(Rdata_NSEC_Test, createFromWire_NSEC) { // Invalid bitmap cases are tested in Rdata_NSECBITMAP_Test. } +TEST_F(Rdata_NSEC_Test, createFromLexer_NSEC) { + const generic::NSEC rdata_nsec(nsec_txt); + EXPECT_EQ(0, rdata_nsec.compare( + *test::createRdataUsingLexer(RRType::NSEC(), RRClass::IN(), + nsec_txt))); + + // Check that bad input throws as usual + EXPECT_THROW({ + *test::createRdataUsingLexer(RRType::NSEC(), RRClass::IN(), + "www.isc.org."); + }, InvalidRdataText); +} + TEST_F(Rdata_NSEC_Test, toWireRenderer_NSEC) { renderer.skip(2); const generic::NSEC rdata_nsec(nsec_txt); diff --git a/src/lib/dns/tests/rdata_tsig_unittest.cc b/src/lib/dns/tests/rdata_tsig_unittest.cc index c8ee8ac674..f562d15e8c 100644 --- a/src/lib/dns/tests/rdata_tsig_unittest.cc +++ b/src/lib/dns/tests/rdata_tsig_unittest.cc @@ -247,6 +247,18 @@ TEST_F(Rdata_TSIG_Test, createFromParams) { isc::InvalidParameter); } +TEST_F(Rdata_TSIG_Test, createFromLexer) { + EXPECT_EQ(0, rdata_tsig.compare( + *test::createRdataUsingLexer(RRType::TSIG(), RRClass::ANY(), + valid_text1))); + + // Check that bad input throws as usual + EXPECT_THROW({ + *test::createRdataUsingLexer(RRType::TSIG(), RRClass::ANY(), + "foo 0 0 0 0 BADKEY 0 0"); + }, InvalidRdataText); +} + TEST_F(Rdata_TSIG_Test, assignment) { any::TSIG copy((string(valid_text2))); copy = rdata_tsig; From a9a6007e022bdc61bcec8d01b51562f9f33f8fd3 Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Fri, 30 Nov 2012 18:05:48 +0530 Subject: [PATCH 13/69] [2497] Add tests for NSEC3PARAM, DS-like and TXT-like rrtypes --- src/lib/dns/tests/rdata_ds_like_unittest.cc | 12 ++++++++++++ src/lib/dns/tests/rdata_nsec3param_unittest.cc | 7 +++++++ src/lib/dns/tests/rdata_txt_like_unittest.cc | 12 ++++++++++++ 3 files changed, 31 insertions(+) diff --git a/src/lib/dns/tests/rdata_ds_like_unittest.cc b/src/lib/dns/tests/rdata_ds_like_unittest.cc index 6172431d77..7838a34980 100644 --- a/src/lib/dns/tests/rdata_ds_like_unittest.cc +++ b/src/lib/dns/tests/rdata_ds_like_unittest.cc @@ -85,6 +85,18 @@ TYPED_TEST(Rdata_DS_LIKE_Test, createFromWire_DS_LIKE) { "rdata_ds_fromWire"))); } +TYPED_TEST(Rdata_DS_LIKE_Test, createFromLexer_DS_LIKE) { + EXPECT_EQ(0, this->rdata_ds_like.compare( + *test::createRdataUsingLexer(RRTYPE(), RRClass::IN(), + ds_like_txt))); + + // Check that bad input throws as usual + EXPECT_THROW({ + *test::createRdataUsingLexer(RRTYPE(), RRClass::IN(), + "99999 5 2 BEEF"); + }, InvalidRdataText); +} + TYPED_TEST(Rdata_DS_LIKE_Test, assignment_DS_LIKE) { TypeParam copy((string(ds_like_txt))); copy = this->rdata_ds_like; diff --git a/src/lib/dns/tests/rdata_nsec3param_unittest.cc b/src/lib/dns/tests/rdata_nsec3param_unittest.cc index 7558b42a74..115d3d3f31 100644 --- a/src/lib/dns/tests/rdata_nsec3param_unittest.cc +++ b/src/lib/dns/tests/rdata_nsec3param_unittest.cc @@ -86,6 +86,13 @@ TEST_F(Rdata_NSEC3PARAM_Test, createFromWire) { } } +TEST_F(Rdata_NSEC3PARAM_Test, createFromLexer) { + const generic::NSEC3PARAM rdata_nsec3param(nsec3param_txt); + EXPECT_EQ(0, rdata_nsec3param.compare( + *test::createRdataUsingLexer(RRType::NSEC3PARAM(), RRClass::IN(), + nsec3param_txt))); +} + TEST_F(Rdata_NSEC3PARAM_Test, toWireRenderer) { renderer.skip(2); const generic::NSEC3PARAM rdata_nsec3param(nsec3param_txt); diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc index 981265e380..7ff9ac9a0d 100644 --- a/src/lib/dns/tests/rdata_txt_like_unittest.cc +++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc @@ -185,6 +185,18 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromWire) { DNSMessageFORMERR); } +TYPED_TEST(Rdata_TXT_LIKE_Test, createFromLexer) { + EXPECT_EQ(0, this->rdata_txt_like.compare( + *test::createRdataUsingLexer(RRTYPE(), RRClass::IN(), + "Test String"))); + EXPECT_EQ(0, this->rdata_txt_like_empty.compare( + *test::createRdataUsingLexer(RRTYPE(), RRClass::IN(), + ""))); + EXPECT_EQ(0, this->rdata_txt_like_quoted.compare( + *test::createRdataUsingLexer(RRTYPE(), RRClass::IN(), + "\"Test String\""))); +} + TYPED_TEST(Rdata_TXT_LIKE_Test, toWireBuffer) { this->rdata_txt_like.toWire(this->obuffer); EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, From cb52403a8ad934f8dc32c6404b5c97b65057d63d Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Fri, 30 Nov 2012 18:14:21 +0530 Subject: [PATCH 14/69] [2497] Add tests for NSEC3PARAM-like rrtypes --- src/lib/dns/tests/rdata_nsec3param_like_unittest.cc | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/lib/dns/tests/rdata_nsec3param_like_unittest.cc b/src/lib/dns/tests/rdata_nsec3param_like_unittest.cc index 51fef01bf7..04aa42bfde 100644 --- a/src/lib/dns/tests/rdata_nsec3param_like_unittest.cc +++ b/src/lib/dns/tests/rdata_nsec3param_like_unittest.cc @@ -208,6 +208,19 @@ TYPED_TEST(NSEC3PARAMLikeTest, createFromWire) { EXPECT_EQ(0, this->convert(*rdata).getSalt().size()); } +TYPED_TEST(NSEC3PARAMLikeTest, createFromLexer) { + EXPECT_EQ(0, this->fromText(this->salt_txt).compare( + *test::createRdataUsingLexer(this->getType(), RRClass::IN(), + this->salt_txt))); + + // Check that bad input throws as usual (too large algorithm) + EXPECT_THROW({ + *test::createRdataUsingLexer(this->getType(), RRClass::IN(), + "1000000 1 1 ADDAFEEE" + + this->getCommonText()); + }, InvalidRdataText); +} + template void toWireCheck(RRType rrtype, OUTPUT_TYPE& output, const string& data_file) { From 638a2d7fbb2a0548e002d57f662f51621175a3a4 Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Fri, 30 Nov 2012 20:01:49 +0530 Subject: [PATCH 15/69] [2497] Return NULL upon exception in rdata::createRdata() --- src/lib/dns/rdata.cc | 14 +++++++++++--- src/lib/dns/tests/rdata_afsdb_unittest.cc | 8 +++----- src/lib/dns/tests/rdata_dhcid_unittest.cc | 7 +++---- src/lib/dns/tests/rdata_dnskey_unittest.cc | 8 +++----- src/lib/dns/tests/rdata_ds_like_unittest.cc | 8 +++----- src/lib/dns/tests/rdata_hinfo_unittest.cc | 8 +++----- src/lib/dns/tests/rdata_mx_unittest.cc | 7 +++---- src/lib/dns/tests/rdata_naptr_unittest.cc | 10 ++++------ src/lib/dns/tests/rdata_ns_unittest.cc | 7 +++---- src/lib/dns/tests/rdata_nsec3_unittest.cc | 10 ++++------ .../dns/tests/rdata_nsec3param_like_unittest.cc | 10 ++++------ src/lib/dns/tests/rdata_nsec_unittest.cc | 8 +++----- src/lib/dns/tests/rdata_opt_unittest.cc | 9 ++++----- src/lib/dns/tests/rdata_rp_unittest.cc | 8 +++----- src/lib/dns/tests/rdata_rrsig_unittest.cc | 8 +++----- src/lib/dns/tests/rdata_srv_unittest.cc | 10 +++++----- src/lib/dns/tests/rdata_tsig_unittest.cc | 8 +++----- 17 files changed, 65 insertions(+), 83 deletions(-) diff --git a/src/lib/dns/rdata.cc b/src/lib/dns/rdata.cc index 40b6d79dbc..f8deec6d22 100644 --- a/src/lib/dns/rdata.cc +++ b/src/lib/dns/rdata.cc @@ -87,9 +87,17 @@ createRdata(const RRType& rrtype, const RRClass& rrclass, MasterLoader::Options options, MasterLoaderCallbacks& callbacks) { - return (RRParamRegistry::getRegistry().createRdata(rrtype, rrclass, - lexer, origin, - options, callbacks)); + RdataPtr ret; + + try { + ret = RRParamRegistry::getRegistry().createRdata(rrtype, rrclass, + lexer, origin, + options, callbacks); + } catch (...) { + // ret is NULL here. + } + + return (ret); } int diff --git a/src/lib/dns/tests/rdata_afsdb_unittest.cc b/src/lib/dns/tests/rdata_afsdb_unittest.cc index 7da2be4d08..9bb64b7229 100644 --- a/src/lib/dns/tests/rdata_afsdb_unittest.cc +++ b/src/lib/dns/tests/rdata_afsdb_unittest.cc @@ -119,11 +119,9 @@ TEST_F(Rdata_AFSDB_Test, createFromLexer) { *test::createRdataUsingLexer(RRType::AFSDB(), RRClass::IN(), afsdb_text))); - // Check that bad input throws as usual - EXPECT_THROW({ - *test::createRdataUsingLexer(RRType::AFSDB(), RRClass::IN(), - "1root.example.com."); - }, InvalidRdataText); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::AFSDB(), RRClass::IN(), + "1root.example.com.")); } TEST_F(Rdata_AFSDB_Test, toWireBuffer) { diff --git a/src/lib/dns/tests/rdata_dhcid_unittest.cc b/src/lib/dns/tests/rdata_dhcid_unittest.cc index 748af96716..8d56c0e7a3 100644 --- a/src/lib/dns/tests/rdata_dhcid_unittest.cc +++ b/src/lib/dns/tests/rdata_dhcid_unittest.cc @@ -68,10 +68,9 @@ TEST_F(Rdata_DHCID_Test, createFromLexer) { *test::createRdataUsingLexer(RRType::DHCID(), RRClass::IN(), string_dhcid))); - // Check that bad input throws as usual - EXPECT_THROW({ - *test::createRdataUsingLexer(RRType::DHCID(), RRClass::IN(), "00"); - }, isc::BadValue); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::DHCID(), RRClass::IN(), + "00")); } TEST_F(Rdata_DHCID_Test, toWireRenderer) { diff --git a/src/lib/dns/tests/rdata_dnskey_unittest.cc b/src/lib/dns/tests/rdata_dnskey_unittest.cc index 5481c05550..58d29bf501 100644 --- a/src/lib/dns/tests/rdata_dnskey_unittest.cc +++ b/src/lib/dns/tests/rdata_dnskey_unittest.cc @@ -88,11 +88,9 @@ TEST_F(Rdata_DNSKEY_Test, createFromLexer) { *test::createRdataUsingLexer(RRType::DNSKEY(), RRClass::IN(), dnskey_txt))); - // Check that bad input throws as usual - EXPECT_THROW({ - *test::createRdataUsingLexer(RRType::DNSKEY(), RRClass::IN(), - "257 3 5"); - }, InvalidRdataText); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::DNSKEY(), RRClass::IN(), + "257 3 5")); } TEST_F(Rdata_DNSKEY_Test, toWireRenderer) { diff --git a/src/lib/dns/tests/rdata_ds_like_unittest.cc b/src/lib/dns/tests/rdata_ds_like_unittest.cc index 7838a34980..28a2e174f8 100644 --- a/src/lib/dns/tests/rdata_ds_like_unittest.cc +++ b/src/lib/dns/tests/rdata_ds_like_unittest.cc @@ -90,11 +90,9 @@ TYPED_TEST(Rdata_DS_LIKE_Test, createFromLexer_DS_LIKE) { *test::createRdataUsingLexer(RRTYPE(), RRClass::IN(), ds_like_txt))); - // Check that bad input throws as usual - EXPECT_THROW({ - *test::createRdataUsingLexer(RRTYPE(), RRClass::IN(), - "99999 5 2 BEEF"); - }, InvalidRdataText); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRTYPE(), RRClass::IN(), + "99999 5 2 BEEF")); } TYPED_TEST(Rdata_DS_LIKE_Test, assignment_DS_LIKE) { diff --git a/src/lib/dns/tests/rdata_hinfo_unittest.cc b/src/lib/dns/tests/rdata_hinfo_unittest.cc index 597c43d4d5..746904751f 100644 --- a/src/lib/dns/tests/rdata_hinfo_unittest.cc +++ b/src/lib/dns/tests/rdata_hinfo_unittest.cc @@ -83,11 +83,9 @@ TEST_F(Rdata_HINFO_Test, createFromLexer) { *test::createRdataUsingLexer(RRType::HINFO(), RRClass::IN(), hinfo_str))); - // Check that bad input throws as usual - EXPECT_THROW({ - *test::createRdataUsingLexer(RRType::HINFO(), RRClass::IN(), - "\"Pentium\"\"Linux\""); - }, InvalidRdataText); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::HINFO(), RRClass::IN(), + "\"Pentium\"\"Linux\"")); } TEST_F(Rdata_HINFO_Test, toText) { diff --git a/src/lib/dns/tests/rdata_mx_unittest.cc b/src/lib/dns/tests/rdata_mx_unittest.cc index 4747138eae..6c6039aa60 100644 --- a/src/lib/dns/tests/rdata_mx_unittest.cc +++ b/src/lib/dns/tests/rdata_mx_unittest.cc @@ -67,10 +67,9 @@ TEST_F(Rdata_MX_Test, createFromLexer) { *test::createRdataUsingLexer(RRType::MX(), RRClass::IN(), "10 mx.example.com"))); - EXPECT_THROW({ - test::createRdataUsingLexer(RRType::MX(), RRClass::IN(), - "10 mx. example.com"); - }, InvalidRdataText); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::MX(), RRClass::IN(), + "10 mx. example.com")); } TEST_F(Rdata_MX_Test, toWireRenderer) { diff --git a/src/lib/dns/tests/rdata_naptr_unittest.cc b/src/lib/dns/tests/rdata_naptr_unittest.cc index 7a6df49627..e23fbcab15 100644 --- a/src/lib/dns/tests/rdata_naptr_unittest.cc +++ b/src/lib/dns/tests/rdata_naptr_unittest.cc @@ -135,12 +135,10 @@ TEST_F(Rdata_NAPTR_Test, createFromLexer) { *test::createRdataUsingLexer(RRType::NAPTR(), RRClass::IN(), naptr_str))); - // Check that bad input throws as usual (order > 65535) - EXPECT_THROW({ - *test::createRdataUsingLexer(RRType::NAPTR(), RRClass::IN(), - "65536 10 S SIP \"\" " - "_sip._udp.example.com."); - }, InvalidRdataText); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::NAPTR(), RRClass::IN(), + "65536 10 S SIP \"\" " + "_sip._udp.example.com.")); } TEST_F(Rdata_NAPTR_Test, toWire) { diff --git a/src/lib/dns/tests/rdata_ns_unittest.cc b/src/lib/dns/tests/rdata_ns_unittest.cc index 569e43135c..d536393083 100644 --- a/src/lib/dns/tests/rdata_ns_unittest.cc +++ b/src/lib/dns/tests/rdata_ns_unittest.cc @@ -91,10 +91,9 @@ TEST_F(Rdata_NS_Test, createFromLexer) { *test::createRdataUsingLexer(RRType::NS(), RRClass::IN(), "ns.example.com"))); - EXPECT_THROW({ - test::createRdataUsingLexer(RRType::NS(), RRClass::IN(), - ""); - }, IncompleteName); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::NS(), RRClass::IN(), + "")); } TEST_F(Rdata_NS_Test, toWireBuffer) { diff --git a/src/lib/dns/tests/rdata_nsec3_unittest.cc b/src/lib/dns/tests/rdata_nsec3_unittest.cc index c7ca93e2bb..0fec3ebb1a 100644 --- a/src/lib/dns/tests/rdata_nsec3_unittest.cc +++ b/src/lib/dns/tests/rdata_nsec3_unittest.cc @@ -136,12 +136,10 @@ TEST_F(Rdata_NSEC3_Test, createFromLexer) { *test::createRdataUsingLexer(RRType::NSEC3(), RRClass::IN(), nsec3_txt))); - // Check that bad input throws as usual (next hash shouldn't be - // padded) - EXPECT_THROW({ - *test::createRdataUsingLexer(RRType::NSEC3(), RRClass::IN(), - "1 1 1 ADDAFEEE CPNMU=== A NS SOA"); - }, InvalidRdataText); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::NSEC3(), RRClass::IN(), + "1 1 1 ADDAFEEE CPNMU=== " + "A NS SOA")); } TEST_F(Rdata_NSEC3_Test, assign) { diff --git a/src/lib/dns/tests/rdata_nsec3param_like_unittest.cc b/src/lib/dns/tests/rdata_nsec3param_like_unittest.cc index 04aa42bfde..23d6d0ed68 100644 --- a/src/lib/dns/tests/rdata_nsec3param_like_unittest.cc +++ b/src/lib/dns/tests/rdata_nsec3param_like_unittest.cc @@ -213,12 +213,10 @@ TYPED_TEST(NSEC3PARAMLikeTest, createFromLexer) { *test::createRdataUsingLexer(this->getType(), RRClass::IN(), this->salt_txt))); - // Check that bad input throws as usual (too large algorithm) - EXPECT_THROW({ - *test::createRdataUsingLexer(this->getType(), RRClass::IN(), - "1000000 1 1 ADDAFEEE" + - this->getCommonText()); - }, InvalidRdataText); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(this->getType(), RRClass::IN(), + "1000000 1 1 ADDAFEEE" + + this->getCommonText())); } template diff --git a/src/lib/dns/tests/rdata_nsec_unittest.cc b/src/lib/dns/tests/rdata_nsec_unittest.cc index 5e5ac89a69..4092c6dbb9 100644 --- a/src/lib/dns/tests/rdata_nsec_unittest.cc +++ b/src/lib/dns/tests/rdata_nsec_unittest.cc @@ -72,11 +72,9 @@ TEST_F(Rdata_NSEC_Test, createFromLexer_NSEC) { *test::createRdataUsingLexer(RRType::NSEC(), RRClass::IN(), nsec_txt))); - // Check that bad input throws as usual - EXPECT_THROW({ - *test::createRdataUsingLexer(RRType::NSEC(), RRClass::IN(), - "www.isc.org."); - }, InvalidRdataText); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::NSEC(), RRClass::IN(), + "www.isc.org.")); } TEST_F(Rdata_NSEC_Test, toWireRenderer_NSEC) { diff --git a/src/lib/dns/tests/rdata_opt_unittest.cc b/src/lib/dns/tests/rdata_opt_unittest.cc index 86bf90a2f6..5699259935 100644 --- a/src/lib/dns/tests/rdata_opt_unittest.cc +++ b/src/lib/dns/tests/rdata_opt_unittest.cc @@ -57,11 +57,10 @@ TEST_F(Rdata_OPT_Test, createFromWire) { } TEST_F(Rdata_OPT_Test, createFromLexer) { - // OPT RR cannot be created from text. - EXPECT_THROW({ - *test::createRdataUsingLexer(RRType::OPT(), RRClass::IN(), - "this does not matter"); - }, InvalidRdataText); + // OPT RR cannot be created from text. Exceptions cause NULL to be + // returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::OPT(), RRClass::IN(), + "this does not matter")); } TEST_F(Rdata_OPT_Test, toWireBuffer) { diff --git a/src/lib/dns/tests/rdata_rp_unittest.cc b/src/lib/dns/tests/rdata_rp_unittest.cc index 449a608212..5508d9cd2e 100644 --- a/src/lib/dns/tests/rdata_rp_unittest.cc +++ b/src/lib/dns/tests/rdata_rp_unittest.cc @@ -112,11 +112,9 @@ TEST_F(Rdata_RP_Test, createFromLexer) { "root.example.com. " "rp-text.example.com."))); - // Check that bad input throws as usual - EXPECT_THROW({ - *test::createRdataUsingLexer(RRType::RP(), RRClass::IN(), - "mailbox.example.com."); - }, InvalidRdataText); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::RP(), RRClass::IN(), + "mailbox.example.com.")); } TEST_F(Rdata_RP_Test, toWireBuffer) { diff --git a/src/lib/dns/tests/rdata_rrsig_unittest.cc b/src/lib/dns/tests/rdata_rrsig_unittest.cc index 04ec481707..726fc535df 100644 --- a/src/lib/dns/tests/rdata_rrsig_unittest.cc +++ b/src/lib/dns/tests/rdata_rrsig_unittest.cc @@ -106,11 +106,9 @@ TEST_F(Rdata_RRSIG_Test, createFromLexer) { *test::createRdataUsingLexer(RRType::RRSIG(), RRClass::IN(), rrsig_txt))); - // Check that bad input throws as usual - EXPECT_THROW({ - *test::createRdataUsingLexer(RRType::RRSIG(), RRClass::IN(), - "INVALIDINPUT"); - }, InvalidRdataText); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::RRSIG(), RRClass::IN(), + "INVALIDINPUT")); } TEST_F(Rdata_RRSIG_Test, toWireRenderer) { diff --git a/src/lib/dns/tests/rdata_srv_unittest.cc b/src/lib/dns/tests/rdata_srv_unittest.cc index 44ef94fe6d..066755f437 100644 --- a/src/lib/dns/tests/rdata_srv_unittest.cc +++ b/src/lib/dns/tests/rdata_srv_unittest.cc @@ -123,11 +123,11 @@ TEST_F(Rdata_SRV_Test, createFromLexer) { EXPECT_EQ(0, rdata_srv.compare( *test::createRdataUsingLexer(RRType::SRV(), RRClass::IN(), "1 5 1500 a.example.com."))); - // port is too large - EXPECT_THROW({ - *test::createRdataUsingLexer(RRType::SRV(), RRClass::IN(), - "1 5 281474976710656 a.example.com."); - }, InvalidRdataText); + + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::SRV(), RRClass::IN(), + "1 5 281474976710656 " + "a.example.com.")); } TEST_F(Rdata_SRV_Test, toWireBuffer) { diff --git a/src/lib/dns/tests/rdata_tsig_unittest.cc b/src/lib/dns/tests/rdata_tsig_unittest.cc index f562d15e8c..df35842233 100644 --- a/src/lib/dns/tests/rdata_tsig_unittest.cc +++ b/src/lib/dns/tests/rdata_tsig_unittest.cc @@ -252,11 +252,9 @@ TEST_F(Rdata_TSIG_Test, createFromLexer) { *test::createRdataUsingLexer(RRType::TSIG(), RRClass::ANY(), valid_text1))); - // Check that bad input throws as usual - EXPECT_THROW({ - *test::createRdataUsingLexer(RRType::TSIG(), RRClass::ANY(), - "foo 0 0 0 0 BADKEY 0 0"); - }, InvalidRdataText); + // Exceptions cause NULL to be returned. + EXPECT_FALSE(test::createRdataUsingLexer(RRType::TSIG(), RRClass::ANY(), + "foo 0 0 0 0 BADKEY 0 0")); } TEST_F(Rdata_TSIG_Test, assignment) { From b48ecd894a0ee52c132e041f686938bdb230d743 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Fri, 30 Nov 2012 13:41:01 -0800 Subject: [PATCH 16/69] [2497] constify --- src/lib/dns/tests/rdata_naptr_unittest.cc | 2 +- src/lib/dns/tests/rdata_rrsig_unittest.cc | 15 ++++++++------- src/lib/dns/tests/rdata_unittest.cc | 2 +- src/lib/dns/tests/rrparamregistry_unittest.cc | 2 +- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/lib/dns/tests/rdata_naptr_unittest.cc b/src/lib/dns/tests/rdata_naptr_unittest.cc index e23fbcab15..fed98cbda8 100644 --- a/src/lib/dns/tests/rdata_naptr_unittest.cc +++ b/src/lib/dns/tests/rdata_naptr_unittest.cc @@ -129,7 +129,7 @@ TEST_F(Rdata_NAPTR_Test, createFromWire) { } TEST_F(Rdata_NAPTR_Test, createFromLexer) { - NAPTR rdata_naptr(naptr_str); + const NAPTR rdata_naptr(naptr_str); EXPECT_EQ(0, rdata_naptr.compare( *test::createRdataUsingLexer(RRType::NAPTR(), RRClass::IN(), diff --git a/src/lib/dns/tests/rdata_rrsig_unittest.cc b/src/lib/dns/tests/rdata_rrsig_unittest.cc index 726fc535df..d758ff390b 100644 --- a/src/lib/dns/tests/rdata_rrsig_unittest.cc +++ b/src/lib/dns/tests/rdata_rrsig_unittest.cc @@ -46,8 +46,8 @@ public: rdata_rrsig(rrsig_txt) {} - string rrsig_txt; - generic::RRSIG rdata_rrsig; + const string rrsig_txt; + const generic::RRSIG rdata_rrsig; }; TEST_F(Rdata_RRSIG_Test, fromText) { @@ -122,14 +122,15 @@ TEST_F(Rdata_RRSIG_Test, toWireBuffer) { } TEST_F(Rdata_RRSIG_Test, createFromWire) { - string rrsig_txt2("A 5 2 43200 20100327070149 20100225070149 2658 isc.org. " - "HkJk/xZTvzePU8NENl/ley8bbUumhk1hXciyqhLnz1VQFzkDooej6neX" - "ZgWZzQKeTKPOYWrnYtdZW4PnPQFeUl3orgLev7F8J6FZlDn0y/J/ThR5" - "m36Mo2/Gdxjj8lJ/IjPVkdpKyBpcnYND8KEIma5MyNCNeyO1UkfPQZGHNSQ="); + const string rrsig_txt2( + "A 5 2 43200 20100327070149 20100225070149 2658 isc.org. " + "HkJk/xZTvzePU8NENl/ley8bbUumhk1hXciyqhLnz1VQFzkDooej6neX" + "ZgWZzQKeTKPOYWrnYtdZW4PnPQFeUl3orgLev7F8J6FZlDn0y/J/ThR5" + "m36Mo2/Gdxjj8lJ/IjPVkdpKyBpcnYND8KEIma5MyNCNeyO1UkfPQZGHNSQ="); EXPECT_EQ(rrsig_txt2, rdataFactoryFromFile(RRType("RRSIG"), RRClass("IN"), "rdata_rrsig_fromWire1")->toText()); - generic::RRSIG rdata_rrsig2(rrsig_txt2); + const generic::RRSIG rdata_rrsig2(rrsig_txt2); EXPECT_EQ(0, rdata_rrsig2.compare( *rdataFactoryFromFile(RRType("RRSIG"), RRClass("IN"), "rdata_rrsig_fromWire1"))); diff --git a/src/lib/dns/tests/rdata_unittest.cc b/src/lib/dns/tests/rdata_unittest.cc index 984289c773..bc91f7ad0d 100644 --- a/src/lib/dns/tests/rdata_unittest.cc +++ b/src/lib/dns/tests/rdata_unittest.cc @@ -74,7 +74,7 @@ createRdataUsingLexer(const RRType& rrtype, const RRClass& rrclass, const MasterLoaderCallbacks::IssueCallback callback (boost::bind(&dummyCallback, _1, _2, _3)); MasterLoaderCallbacks callbacks(callback, callback); - Name origin("example.org."); + const Name origin("example.org."); return (createRdata(rrtype, rrclass, lexer, &origin, MasterLoader::MANY_ERRORS, callbacks)); diff --git a/src/lib/dns/tests/rrparamregistry_unittest.cc b/src/lib/dns/tests/rrparamregistry_unittest.cc index b749d74db7..0ae0a762c6 100644 --- a/src/lib/dns/tests/rrparamregistry_unittest.cc +++ b/src/lib/dns/tests/rrparamregistry_unittest.cc @@ -172,7 +172,7 @@ createRdataHelper(const std::string& str) { const MasterLoaderCallbacks::IssueCallback callback (boost::bind(&dummyCallback, _1, _2, _3)); MasterLoaderCallbacks callbacks(callback, callback); - Name origin("example.org."); + const Name origin("example.org."); return (rdf->create(lexer, &origin, MasterLoader::MANY_ERRORS, From ca8fc9f4147a8f332280fde6cfad99b963ebc40a Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 29 Nov 2012 21:06:09 -0800 Subject: [PATCH 17/69] [2382] resolved remaining merge conflict on MasterLexer::Token. --- src/lib/dns/rdata.cc | 6 +++--- src/lib/dns/rrparamregistry-placeholder.cc | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib/dns/rdata.cc b/src/lib/dns/rdata.cc index f8deec6d22..46224c65b8 100644 --- a/src/lib/dns/rdata.cc +++ b/src/lib/dns/rdata.cc @@ -211,9 +211,9 @@ Generic::Generic(MasterLexer& lexer, const Name*, std::string s; while (true) { - const MasterLexer::Token& token = lexer.getNextToken(); - if ((token.getType() == MasterLexer::Token::END_OF_FILE) || - (token.getType() == MasterLexer::Token::END_OF_LINE)) { + const MasterToken& token = lexer.getNextToken(); + if ((token.getType() == MasterToken::END_OF_FILE) || + (token.getType() == MasterToken::END_OF_LINE)) { break; } diff --git a/src/lib/dns/rrparamregistry-placeholder.cc b/src/lib/dns/rrparamregistry-placeholder.cc index e35b2f4664..cea5b26a83 100644 --- a/src/lib/dns/rrparamregistry-placeholder.cc +++ b/src/lib/dns/rrparamregistry-placeholder.cc @@ -52,9 +52,9 @@ AbstractRdataFactory::create(MasterLexer& lexer, const Name*, std::string s; while (true) { - const MasterLexer::Token& token = lexer.getNextToken(); - if ((token.getType() == MasterLexer::Token::END_OF_FILE) || - (token.getType() == MasterLexer::Token::END_OF_LINE)) { + const MasterToken& token = lexer.getNextToken(); + if ((token.getType() == MasterToken::END_OF_FILE) || + (token.getType() == MasterToken::END_OF_LINE)) { break; } From f73f27474fe73abacded88f4dad74867331cb402 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Fri, 30 Nov 2012 13:51:33 -0800 Subject: [PATCH 18/69] [2382] unrelated fix to lexer: support empty qstring and nul termination. an empty qstring previously caused an exception, which is a clear bug and should be fixed. nul-terminating string regions is an extension, but I found it useful when implementing RDATA parsers. --- src/lib/dns/master_lexer.cc | 13 ++++++++++--- src/lib/dns/master_lexer.h | 7 +++++++ src/lib/dns/tests/master_lexer_state_unittest.cc | 10 ++++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc index f1f57493da..4593b48b5f 100644 --- a/src/lib/dns/master_lexer.cc +++ b/src/lib/dns/master_lexer.cc @@ -458,8 +458,11 @@ String::handle(MasterLexer& lexer) const { if (getLexerImpl(lexer)->isTokenEnd(c, escaped)) { getLexerImpl(lexer)->source_->ungetChar(); + // make sure it nul-terminated as a c-str (excluded from token + // data). + data.push_back('\0'); getLexerImpl(lexer)->token_ = - MasterToken(&data.at(0), data.size()); + MasterToken(&data.at(0), data.size() - 1); return; } escaped = (c == '\\' && !escaped); @@ -486,7 +489,10 @@ QString::handle(MasterLexer& lexer) const { escaped = false; data.back() = '"'; } else { - token = MasterToken(&data.at(0), data.size(), true); + // make sure it nul-terminated as a c-str (excluded from token + // data). This also simplifies the case of an empty string. + data.push_back('\0'); + token = MasterToken(&data.at(0), data.size() - 1, true); return; } } else if (c == '\n' && !escaped) { @@ -529,7 +535,8 @@ Number::handle(MasterLexer& lexer) const { token = MasterToken(MasterToken::NUMBER_OUT_OF_RANGE); } } else { - token = MasterToken(&data.at(0), data.size()); + data.push_back('\0'); // see String::handle() + token = MasterToken(&data.at(0), data.size() - 1); } return; } diff --git a/src/lib/dns/master_lexer.h b/src/lib/dns/master_lexer.h index fdc69fcfa3..35586feb0f 100644 --- a/src/lib/dns/master_lexer.h +++ b/src/lib/dns/master_lexer.h @@ -90,6 +90,13 @@ public: /// the region. On the other hand, it is not ensured that the string /// is nul-terminated. So the usual string manipulation API may not work /// as expected. + /// + /// The `MasterLexer` implementation ensures that there are at least + /// len + 1 bytes of valid memory region starting from beg, and that + /// beg[len] is \0. This means the application can use the bytes as a + /// validly nul-terminated C string if there is no intermediate nul + /// character. Note also that due to this property beg is always non + /// NULL; for an empty string len will be set to 0 and beg[0] is \0. struct StringRegion { const char* beg; ///< The start address of the string size_t len; ///< The length of the string in bytes diff --git a/src/lib/dns/tests/master_lexer_state_unittest.cc b/src/lib/dns/tests/master_lexer_state_unittest.cc index 7645ede198..846c4c2aeb 100644 --- a/src/lib/dns/tests/master_lexer_state_unittest.cc +++ b/src/lib/dns/tests/master_lexer_state_unittest.cc @@ -269,6 +269,10 @@ stringTokenCheck(const std::string& expected, const MasterToken& token, token.getStringRegion().beg + token.getStringRegion().len); EXPECT_EQ(expected, actual); + + // There should be "hidden" nul-terminator after the string data. + ASSERT_NE(static_cast(NULL), token.getStringRegion().beg); + EXPECT_EQ(0, *(token.getStringRegion().beg + token.getStringRegion().len)); } TEST_F(MasterLexerStateTest, string) { @@ -365,6 +369,7 @@ TEST_F(MasterLexerStateTest, stringEscape) { TEST_F(MasterLexerStateTest, quotedString) { ss << "\"ignore-quotes\"\n"; ss << "\"quoted string\" "; // space is part of the qstring + ss << "\"\" "; // empty quoted string // also check other separator characters. note that \r doesn't cause // UNBALANCED_QUOTES. Not sure if it's intentional, but that's how the // BIND 9 version works, so we follow it (it should be too minor to matter @@ -391,6 +396,11 @@ TEST_F(MasterLexerStateTest, quotedString) { s_qstring.handle(lexer); stringTokenCheck("quoted string", s_string.getToken(lexer), true); + // Empty string is okay as qstring + EXPECT_EQ(&s_qstring, State::start(lexer, options)); + s_qstring.handle(lexer); + stringTokenCheck("", s_string.getToken(lexer), true); + // Also checks other separator characters within a qstring EXPECT_EQ(&s_qstring, State::start(lexer, options)); s_qstring.handle(lexer); From 3ae13a89a2f238602d5dd7e3dc12da963de21ec8 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Fri, 30 Nov 2012 14:28:52 -0800 Subject: [PATCH 19/69] [2382] added from-lexer ctor for AAAA RDATA. not directly related to this task, but I found we need one real (non wrapper) example to test some of the feature of generic createRdata(). also updated RDATA template including with-lexer constructor. --- src/lib/dns/gen-rdatacode.py.in | 29 ++++++++++++++++++++++++----- src/lib/dns/rdata/in_1/aaaa_28.cc | 27 +++++++++++++++++++-------- src/lib/dns/rdata/template.cc | 5 +++++ 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/lib/dns/gen-rdatacode.py.in b/src/lib/dns/gen-rdatacode.py.in index 9f2258206a..6a9ff8aca5 100755 --- a/src/lib/dns/gen-rdatacode.py.in +++ b/src/lib/dns/gen-rdatacode.py.in @@ -28,7 +28,7 @@ re_typecode = re.compile('([\da-z]+)_(\d+)') classcode2txt = {} typecode2txt = {} typeandclass = [] -new_rdatafactory_users = [] +new_rdatafactory_users = ['aaaa'] generic_code = 65536 # something larger than any code value rdata_declarations = '' class_definitions = '' @@ -117,6 +117,9 @@ class AbstractMessageRenderer;\n\n''' explicit ''' + type_utxt + '''(const std::string& type_str); ''' + type_utxt + '''(isc::util::InputBuffer& buffer, size_t rdata_len); ''' + type_utxt + '''(const ''' + type_utxt + '''& other); + ''' + type_utxt + '''( + MasterLexer& lexer, const Name* name, + MasterLoader::Options options, MasterLoaderCallbacks& callbacks); virtual std::string toText() const; virtual void toWire(isc::util::OutputBuffer& buffer) const; virtual void toWire(AbstractMessageRenderer& renderer) const; @@ -204,17 +207,33 @@ def generate_rdatadef(file, basemtime): rdata_deffile.write(class_definitions) rdata_deffile.close() -def generate_rdatahdr(file, declarations, basemtime): +def generate_rdatahdr(file, heading, declarations, basemtime): if not need_generate(file, basemtime): print('skip generating ' + file); return + heading += ''' +#ifndef DNS_RDATACLASS_H +#define DNS_RDATACLASS_H 1 + +#include + +namespace isc { +namespace dns { +class Name; +class MasterLexer; +class MasterLoaderCallbacks; +} +} +''' declarations += ''' +#endif // DNS_RDATACLASS_H + // Local Variables: // mode: c++ // End: ''' rdata_header = open(file, 'w') - rdata_header.write(heading_txt) + rdata_header.write(heading) rdata_header.write(declarations) rdata_header.close() @@ -306,8 +325,8 @@ if __name__ == "__main__": try: import_definitions(classcode2txt, typecode2txt, typeandclass) generate_rdatadef('@builddir@/rdataclass.cc', rdatadef_mtime) - generate_rdatahdr('@builddir@/rdataclass.h', rdata_declarations, - rdatahdr_mtime) + generate_rdatahdr('@builddir@/rdataclass.h', heading_txt, + rdata_declarations, rdatahdr_mtime) generate_typeclasscode('rrtype', rdatahdr_mtime, typecode2txt, 'Type') generate_typeclasscode('rrclass', classdir_mtime, classcode2txt, 'Class') diff --git a/src/lib/dns/rdata/in_1/aaaa_28.cc b/src/lib/dns/rdata/in_1/aaaa_28.cc index ce49a04c51..0466f1a445 100644 --- a/src/lib/dns/rdata/in_1/aaaa_28.cc +++ b/src/lib/dns/rdata/in_1/aaaa_28.cc @@ -12,6 +12,15 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +#include +#include +#include +#include +#include +#include +#include +#include + #include #include @@ -20,14 +29,6 @@ #include // XXX: for inet_pton/ntop(), not exist in C++ standards #include // for AF_INET/AF_INET6 -#include - -#include -#include -#include -#include -#include - using namespace std; using namespace isc::util; @@ -42,6 +43,16 @@ AAAA::AAAA(const std::string& addrstr) { } } +AAAA::AAAA(MasterLexer& lexer, const Name*, + MasterLoader::Options, MasterLoaderCallbacks&) +{ + const MasterToken& token = lexer.getNextToken(MasterToken::STRING); + if (inet_pton(AF_INET6, token.getStringRegion().beg, &addr_) != 1) { + isc_throw(InvalidRdataText, "Failed to convert '" + << token.getString() << "' to IN/AAAA RDATA"); + } +} + AAAA::AAAA(InputBuffer& buffer, size_t rdata_len) { if (rdata_len != sizeof(addr_)) { isc_throw(DNSMessageFORMERR, diff --git a/src/lib/dns/rdata/template.cc b/src/lib/dns/rdata/template.cc index ee1097e7f6..6486e6a51a 100644 --- a/src/lib/dns/rdata/template.cc +++ b/src/lib/dns/rdata/template.cc @@ -34,6 +34,11 @@ using namespace isc::util; // If you added member functions specific to this derived class, you'll need // to implement them here, of course. +MyType::MyType(MasterLexer& lexer, const Name* origin, + MasterLoader::Options options, MasterLoaderCallbacks& callbacks) +{ +} + MyType::MyType(const string& type_str) { } From 03dc2386b14131c0eb35408952e3ddc8ca017831 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 29 Nov 2012 21:48:25 -0800 Subject: [PATCH 20/69] [2382] ungetToken EOL/EOF in the backend factories. --- src/lib/dns/rdata.cc | 1 + src/lib/dns/rrparamregistry-placeholder.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/src/lib/dns/rdata.cc b/src/lib/dns/rdata.cc index 46224c65b8..ddb4b77258 100644 --- a/src/lib/dns/rdata.cc +++ b/src/lib/dns/rdata.cc @@ -214,6 +214,7 @@ Generic::Generic(MasterLexer& lexer, const Name*, const MasterToken& token = lexer.getNextToken(); if ((token.getType() == MasterToken::END_OF_FILE) || (token.getType() == MasterToken::END_OF_LINE)) { + lexer.ungetToken(); // let the upper layer handle the end-of token break; } diff --git a/src/lib/dns/rrparamregistry-placeholder.cc b/src/lib/dns/rrparamregistry-placeholder.cc index cea5b26a83..2208d3f440 100644 --- a/src/lib/dns/rrparamregistry-placeholder.cc +++ b/src/lib/dns/rrparamregistry-placeholder.cc @@ -55,6 +55,7 @@ AbstractRdataFactory::create(MasterLexer& lexer, const Name*, const MasterToken& token = lexer.getNextToken(); if ((token.getType() == MasterToken::END_OF_FILE) || (token.getType() == MasterToken::END_OF_LINE)) { + lexer.ungetToken(); // let the upper layer handle the end-of token break; } From b9f1eefe7e7040db4841e75a38b6a9cd1511eb59 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 29 Nov 2012 22:30:56 -0800 Subject: [PATCH 21/69] [2382] Consume to end of line / file in createRdata(). --- src/lib/dns/rdata.cc | 19 ++++++++++--------- src/lib/dns/tests/rdata_unittest.cc | 24 ++++++++++++++++++++++++ src/lib/dns/tests/rdata_unittest.h | 2 ++ 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/lib/dns/rdata.cc b/src/lib/dns/rdata.cc index ddb4b77258..cc7c582743 100644 --- a/src/lib/dns/rdata.cc +++ b/src/lib/dns/rdata.cc @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -87,17 +88,17 @@ createRdata(const RRType& rrtype, const RRClass& rrclass, MasterLoader::Options options, MasterLoaderCallbacks& callbacks) { - RdataPtr ret; + const RdataPtr rdata = RRParamRegistry::getRegistry().createRdata( + rrtype, rrclass, lexer, origin, options, callbacks); - try { - ret = RRParamRegistry::getRegistry().createRdata(rrtype, rrclass, - lexer, origin, - options, callbacks); - } catch (...) { - // ret is NULL here. - } + // Consume to end of line / file. + // If not at end of line initially set error code. + // Call callback via fromtext_error once if there was an error. + const MasterToken& token = lexer.getNextToken(); + assert(token.getType() == MasterToken::END_OF_LINE || + token.getType() == MasterToken::END_OF_FILE); - return (ret); + return (rdata); } int diff --git a/src/lib/dns/tests/rdata_unittest.cc b/src/lib/dns/tests/rdata_unittest.cc index bc91f7ad0d..08d9628d19 100644 --- a/src/lib/dns/tests/rdata_unittest.cc +++ b/src/lib/dns/tests/rdata_unittest.cc @@ -82,6 +82,30 @@ createRdataUsingLexer(const RRType& rrtype, const RRClass& rrclass, } // end of namespace isc::dns::rdata::test +// Test class/type-independent behavior of createRdata(). +TEST_F(RdataTest, createRdataWithLexer) { + const generic::NS ns_rdata("ns.example.com."); + const in::AAAA aaaa_rdata("2001:db8::1"); + + stringstream ss; + ss << ns_rdata.toText() << "\n"; // valid case + ss << aaaa_rdata.toText() << " extra-token\n"; // extra token + lexer.pushSource(ss); + + const MasterLoaderCallbacks::IssueCallback callback + (boost::bind(&test::dummyCallback, _1, _2, _3)); + MasterLoaderCallbacks callbacks(callback, callback); + ConstRdataPtr rdata = createRdata(RRType::NS(), RRClass::IN(), lexer, NULL, + MasterLoader::MANY_ERRORS, callbacks); + EXPECT_EQ(0, ns_rdata.compare(*rdata)); + +#ifdef notyet + rdata = createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL, + MasterLoader::MANY_ERRORS, callbacks); + EXPECT_EQ(0, aaaa_rdata.compare(*rdata)); +#endif +} + } } } diff --git a/src/lib/dns/tests/rdata_unittest.h b/src/lib/dns/tests/rdata_unittest.h index 3efb5d8346..af19311794 100644 --- a/src/lib/dns/tests/rdata_unittest.h +++ b/src/lib/dns/tests/rdata_unittest.h @@ -20,6 +20,7 @@ #include #include #include +#include #include @@ -40,6 +41,7 @@ protected: /// This is an RDATA object of some "unknown" RR type so that it can be /// used to test the compare() method against a well-known RR type. RdataPtr rdata_nomatch; + MasterLexer lexer; }; namespace test { From f7f3060b97886bea4fbc4408a5c432e81096e3a9 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Fri, 30 Nov 2012 15:19:03 -0800 Subject: [PATCH 22/69] [2382] supported extra token case and callback calls on error. --- src/lib/dns/rdata.cc | 27 +++++++++-- src/lib/dns/tests/rdata_unittest.cc | 70 +++++++++++++++++++++++------ 2 files changed, 81 insertions(+), 16 deletions(-) diff --git a/src/lib/dns/rdata.cc b/src/lib/dns/rdata.cc index cc7c582743..ef031ace5e 100644 --- a/src/lib/dns/rdata.cc +++ b/src/lib/dns/rdata.cc @@ -82,6 +82,24 @@ createRdata(const RRType& rrtype, const RRClass& rrclass, const Rdata& source) source)); } +namespace { +void +fromtextError(const MasterLexer& lexer, MasterLoaderCallbacks& callbacks, + const MasterToken& token, const char* reason) +{ + switch (token.getType()) { + case MasterToken::STRING: + case MasterToken::QSTRING: + callbacks.error(lexer.getSourceName(), lexer.getSourceLine(), + "createRdata from text failed near '" + + token.getString() + "': " + string(reason)); + break; + default: + assert(false); + } +} +} + RdataPtr createRdata(const RRType& rrtype, const RRClass& rrclass, MasterLexer& lexer, const Name* origin, @@ -93,10 +111,13 @@ createRdata(const RRType& rrtype, const RRClass& rrclass, // Consume to end of line / file. // If not at end of line initially set error code. - // Call callback via fromtext_error once if there was an error. + // Call callback via fromtextError once if there was an error. const MasterToken& token = lexer.getNextToken(); - assert(token.getType() == MasterToken::END_OF_LINE || - token.getType() == MasterToken::END_OF_FILE); + if (token.getType() != MasterToken::END_OF_LINE && + token.getType() != MasterToken::END_OF_FILE) { + fromtextError(lexer, callbacks, token, "extra input text"); + return (RdataPtr()); + } return (rdata); } diff --git a/src/lib/dns/tests/rdata_unittest.cc b/src/lib/dns/tests/rdata_unittest.cc index 08d9628d19..e72b7b8176 100644 --- a/src/lib/dns/tests/rdata_unittest.cc +++ b/src/lib/dns/tests/rdata_unittest.cc @@ -29,6 +29,7 @@ #include #include +#include using isc::UnitTestUtil; using namespace std; @@ -82,28 +83,71 @@ createRdataUsingLexer(const RRType& rrtype, const RRClass& rrclass, } // end of namespace isc::dns::rdata::test +// A mock class to check parameters passed via loader callbacks. Its callback +// records the passed parameters, allowing the test to check them later via +// the check() method. +class CreateRdataCallback { +public: + enum CallbackType { NONE, ERROR, WARN }; + CreateRdataCallback() : type_(NONE), line_(0) {} + void callback(CallbackType type, const string& source, size_t line, + const string& reason_txt) { + type_ = type; + source_ = source; + line_ = line; + reason_txt_ = reason_txt; + } + + void clear() { + type_ = NONE; + source_.clear(); + line_ = 0; + reason_txt_.clear(); + } + + void check(const string& expected_srcname, size_t expected_line, + CallbackType expected_type, const string& expected_reason) + { + EXPECT_EQ(expected_srcname, source_); + EXPECT_EQ(expected_line, line_); + EXPECT_EQ(expected_type, type_); + EXPECT_EQ(expected_reason, reason_txt_); + } + +private: + CallbackType type_; + string source_; + size_t line_; + string reason_txt_; +}; + // Test class/type-independent behavior of createRdata(). TEST_F(RdataTest, createRdataWithLexer) { - const generic::NS ns_rdata("ns.example.com."); const in::AAAA aaaa_rdata("2001:db8::1"); stringstream ss; - ss << ns_rdata.toText() << "\n"; // valid case + const string src_name = "stream-" + boost::lexical_cast(&ss); + ss << aaaa_rdata.toText() << "\n"; // valid case ss << aaaa_rdata.toText() << " extra-token\n"; // extra token lexer.pushSource(ss); - const MasterLoaderCallbacks::IssueCallback callback - (boost::bind(&test::dummyCallback, _1, _2, _3)); - MasterLoaderCallbacks callbacks(callback, callback); - ConstRdataPtr rdata = createRdata(RRType::NS(), RRClass::IN(), lexer, NULL, - MasterLoader::MANY_ERRORS, callbacks); - EXPECT_EQ(0, ns_rdata.compare(*rdata)); - -#ifdef notyet - rdata = createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL, - MasterLoader::MANY_ERRORS, callbacks); + CreateRdataCallback callback; + MasterLoaderCallbacks callbacks( + boost::bind(&CreateRdataCallback::callback, &callback, + CreateRdataCallback::ERROR, _1, _2, _3), + boost::bind(&CreateRdataCallback::callback, &callback, + CreateRdataCallback::WARN, _1, _2, _3)); + ConstRdataPtr rdata = createRdata(RRType::AAAA(), RRClass::IN(), lexer, + NULL, MasterLoader::MANY_ERRORS, + callbacks); EXPECT_EQ(0, aaaa_rdata.compare(*rdata)); -#endif + + callback.clear(); + EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL, + MasterLoader::MANY_ERRORS, callbacks)); + callback.check(src_name, 2, CreateRdataCallback::ERROR, + "createRdata from text failed near 'extra-token': " + "extra input text"); } } From 0ad163bb379066067551f1f3623afe43cf5bd327 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Fri, 30 Nov 2012 19:04:33 -0800 Subject: [PATCH 23/69] [2382] handle other generic isc::Exception --- src/lib/dns/rdata.cc | 63 +++++++++++++++++++++++------ src/lib/dns/tests/rdata_unittest.cc | 37 +++++++++++++++++ 2 files changed, 88 insertions(+), 12 deletions(-) diff --git a/src/lib/dns/rdata.cc b/src/lib/dns/rdata.cc index ef031ace5e..962ff86a57 100644 --- a/src/lib/dns/rdata.cc +++ b/src/lib/dns/rdata.cc @@ -84,15 +84,33 @@ createRdata(const RRType& rrtype, const RRClass& rrclass, const Rdata& source) namespace { 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::QSTRING: callbacks.error(lexer.getSourceName(), lexer.getSourceLine(), "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; default: assert(false); @@ -106,18 +124,39 @@ createRdata(const RRType& rrtype, const RRClass& rrclass, MasterLoader::Options options, 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. // If not at end of line initially set error code. // 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); } diff --git a/src/lib/dns/tests/rdata_unittest.cc b/src/lib/dns/tests/rdata_unittest.cc index e72b7b8176..cf438276ab 100644 --- a/src/lib/dns/tests/rdata_unittest.cc +++ b/src/lib/dns/tests/rdata_unittest.cc @@ -105,8 +105,12 @@ public: 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, CallbackType expected_type, const string& expected_reason) + const { EXPECT_EQ(expected_srcname, source_); EXPECT_EQ(expected_line, line_); @@ -129,6 +133,9 @@ TEST_F(RdataTest, createRdataWithLexer) { const string src_name = "stream-" + boost::lexical_cast(&ss); ss << aaaa_rdata.toText() << "\n"; // valid case 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); CreateRdataCallback callback; @@ -137,17 +144,47 @@ TEST_F(RdataTest, createRdataWithLexer) { CreateRdataCallback::ERROR, _1, _2, _3), boost::bind(&CreateRdataCallback::callback, &callback, CreateRdataCallback::WARN, _1, _2, _3)); + + // Valid case. ConstRdataPtr rdata = createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL, MasterLoader::MANY_ERRORS, callbacks); 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(); EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL, MasterLoader::MANY_ERRORS, callbacks)); callback.check(src_name, 2, CreateRdataCallback::ERROR, "createRdata from text failed near 'extra-token': " "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"); } } From e441d6b05a022f3c0a7140ae43fe42d5fd12cce3 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Fri, 30 Nov 2012 19:37:28 -0800 Subject: [PATCH 24/69] [2382] warn if RDATA immediately followed by EOF --- src/lib/dns/rdata.cc | 28 +++++++++++++++++----------- src/lib/dns/tests/rdata_unittest.cc | 10 ++++++++++ 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/lib/dns/rdata.cc b/src/lib/dns/rdata.cc index 962ff86a57..54644b7756 100644 --- a/src/lib/dns/rdata.cc +++ b/src/lib/dns/rdata.cc @@ -138,27 +138,33 @@ createRdata(const RRType& rrtype, const RRClass& rrclass, // 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. + // Other exceptions mean a serious implementation bug or fatal system + // error; it doesn't make sense to catch and try to recover from them + // here. Just propagate. // Consume to end of line / file. // If not at end of line initially set error code. // Call callback via fromtextError once if there was an error. do { const MasterToken& token = lexer.getNextToken(); - if (token.getType() != MasterToken::END_OF_LINE && - token.getType() != MasterToken::END_OF_FILE) { + switch (token.getType()) { + case MasterToken::END_OF_LINE: + return (rdata); + case MasterToken::END_OF_FILE: + callbacks.warning(lexer.getSourceName(), lexer.getSourceLine(), + "file does not end with newline"); + return (rdata); + default: rdata.reset(); // we'll return NULL - if (!error_issued) { - fromtextError(error_issued, lexer, callbacks, &token, - "extra input text"); - } - } else { // reached EOL or EOF - break; + fromtextError(error_issued, lexer, callbacks, &token, + "extra input text"); + // Continue until we see EOL or EOF } } while (true); - return (rdata); + // We shouldn't reach here + assert(false); + return (RdataPtr()); // add explicit return to silence some compilers } int diff --git a/src/lib/dns/tests/rdata_unittest.cc b/src/lib/dns/tests/rdata_unittest.cc index cf438276ab..bb39d05fe0 100644 --- a/src/lib/dns/tests/rdata_unittest.cc +++ b/src/lib/dns/tests/rdata_unittest.cc @@ -136,6 +136,7 @@ TEST_F(RdataTest, createRdataWithLexer) { 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 + ss << aaaa_rdata.toText(); // valid, but end with EOF, not EOL lexer.pushSource(ss); CreateRdataCallback callback; @@ -185,6 +186,15 @@ TEST_F(RdataTest, createRdataWithLexer) { callback.check(src_name, 5, CreateRdataCallback::ERROR, "createRdata from text failed: Failed to convert " "'192.0.2.1' to IN/AAAA RDATA"); + + // Input is valid and parse will succeed, but with a warning that the + // file is not ended with a newline. + callback.clear(); + rdata = createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL, + MasterLoader::MANY_ERRORS, callbacks); + EXPECT_EQ(0, aaaa_rdata.compare(*rdata)); + callback.check(src_name, 6, CreateRdataCallback::WARN, + "file does not end with newline"); } } From dfd0dd105b4fc8054e3b9cfd1bc5547302f54578 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Fri, 30 Nov 2012 20:17:23 -0800 Subject: [PATCH 25/69] [2382] documentation updates --- src/lib/dns/rdata.h | 41 ++++++++++++++++++++++++++++++++++- src/lib/dns/rrparamregistry.h | 35 +++++++++++++++++++++++++----- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/src/lib/dns/rdata.h b/src/lib/dns/rdata.h index e7811c9510..186ace2cc3 100644 --- a/src/lib/dns/rdata.h +++ b/src/lib/dns/rdata.h @@ -485,8 +485,47 @@ RdataPtr createRdata(const RRType& rrtype, const RRClass& rrclass, RdataPtr createRdata(const RRType& rrtype, const RRClass& rrclass, const Rdata& source); -/// \brief Create RDATA of a given pair of RR type and class from the +/// \brief Create RDATA of a given pair of RR type and class using the /// master lexer. +/// +/// This is a more generic form of factory from textual RDATA, and is mainly +/// intended to be used internally by the master file parser (\c MasterLoader) +/// of this library. +/// +/// The \c lexer is expected to be at the beginning of textual RDATA of the +/// specified type and class. This function (and its underlying Rdata +/// implementations) extracts necessary tokens from the lexer and constructs +/// the RDATA from them. +/// +/// Due to the intended usage of this version, this function handles error +/// cases quite differently from other versions. It internally catches +/// most of syntax and semantics errors of the input (reported as exceptions), +/// calls the corresponding callback specified by the \c callbacks parameters, +/// and returns a NULL smart pointer. If the caller rather wants to get +/// an exception in these cases, it can use pass a callback that internally +/// throws on error. Some critical exceptions such as \c std::bad_alloc are +/// still propagated to the upper layer as it doesn't make sense to try +/// recovery from such a situation within this function. +/// +/// Whether or not the creation succeeds, this function updates the lexer +/// until it reaches either the end of line or file, starting from the end of +/// the RDATA text (or the point of failure if the parsing fails in the +/// middle of it). The caller can therefore assume it's ready for reading +/// the next data (which is normally a subsequent RR in the zone file) on +/// return, whether or not this function succeeds. +/// +/// \param rrtype An \c RRType object specifying the type/class pair. +/// \param rrclass An \c RRClass object specifying the type/class pair. +/// \param lexer A \c MasterLexer object parsing a master file for the +/// RDATA to be created +/// \param origin If non NULL, specifies the origin of any domain name fields +/// of the RDATA that are non absolute. +/// \param options Master loader options controlling how to deal with errors +/// or non critical issues in the parsed RDATA. +/// \param callbacks Callback to be called when an error or non critical issue +/// is found. +/// \return An \c RdataPtr object pointing to the created +/// \c Rdata object. Will be NULL if parsing fails. RdataPtr createRdata(const RRType& rrtype, const RRClass& rrclass, MasterLexer& lexer, const Name* origin, MasterLoader::Options options, diff --git a/src/lib/dns/rrparamregistry.h b/src/lib/dns/rrparamregistry.h index e156dc95b2..56ae981614 100644 --- a/src/lib/dns/rrparamregistry.h +++ b/src/lib/dns/rrparamregistry.h @@ -119,10 +119,22 @@ public: /// \return An \c RdataPtr object pointing to the created \c Rdata object. virtual RdataPtr create(const rdata::Rdata& source) const = 0; - /// \brief Create RDATA from MasterLexer - virtual RdataPtr create(MasterLexer& lexer, const Name*, - MasterLoader::Options, - MasterLoaderCallbacks&) const; + /// \brief Create RDATA using MasterLexer. + /// + /// This version of the method defines the entry point of factory + /// of a specific RR type and class for \c RRParamRegistry::createRdata() + /// that uses \c MasterLexer. See its description for the expected + /// behavior and meaning of the parameters. + /// + /// \note Right now this is not defined as a pure virtual method and + /// provides the default implementation. This is an intermediate + /// workaround until we implement the underlying constructor for all + /// supported \c Rdata classes; once it's completed the workaround + /// default implementation should be removed and this method should become + /// pure virtual. + virtual RdataPtr create(MasterLexer& lexer, const Name* origin, + MasterLoader::Options options, + MasterLoaderCallbacks& callbacks) const; //@} }; @@ -504,9 +516,20 @@ public: rdata::RdataPtr createRdata(const RRType& rrtype, const RRClass& rrclass, const rdata::Rdata& source); - /// \brief Create RDATA from MasterLexer + /// \brief Create RDATA using MasterLexer + /// + /// This method is expected to be used as the underlying implementation + /// of the same signature of \c rdata::createRdata(). One main + /// difference is that this method is only responsible for constructing + /// the Rdata; it doesn't update the lexer to reach the end of line or + /// file or doesn't care about whether there's an extra (garbage) token + /// after the textual RDATA representation. Another difference is that + /// this method can throw on error and never returns a NULL pointer. + /// + /// For other details and parameters, see the description of + /// \c rdata::createRdata(). rdata::RdataPtr createRdata(const RRType& rrtype, const RRClass& rrclass, - MasterLexer& lexer, const Name* name, + MasterLexer& lexer, const Name* origin, MasterLoader::Options options, MasterLoaderCallbacks& callbacks); //@} From 9440d65d7e71ec30e0d268f5f9f5b1980d288b98 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sat, 1 Dec 2012 08:28:37 -0800 Subject: [PATCH 26/69] [2382] comment wording fix --- src/lib/dns/rdata.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dns/rdata.cc b/src/lib/dns/rdata.cc index 54644b7756..3e1d0a4183 100644 --- a/src/lib/dns/rdata.cc +++ b/src/lib/dns/rdata.cc @@ -133,7 +133,7 @@ createRdata(const RRType& rrtype, const RRClass& rrclass, } 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 + // Catching all isc::Exception is too broad, but 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()); From 5d239b6aed71f987330add0bbcfc7baed670e804 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sat, 1 Dec 2012 08:32:06 -0800 Subject: [PATCH 27/69] [2382] more update on comment --- src/lib/dns/rdata.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/dns/rdata.cc b/src/lib/dns/rdata.cc index 3e1d0a4183..9e96378bc0 100644 --- a/src/lib/dns/rdata.cc +++ b/src/lib/dns/rdata.cc @@ -143,7 +143,6 @@ createRdata(const RRType& rrtype, const RRClass& rrclass, // here. Just propagate. // Consume to end of line / file. - // If not at end of line initially set error code. // Call callback via fromtextError once if there was an error. do { const MasterToken& token = lexer.getNextToken(); From 191d96a58702a5a3059993a1287136833ec94a4e Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Dec 2012 13:38:01 -0800 Subject: [PATCH 28/69] [2506] add some comments of how eolCheck() works, and unify skipping 2nd \n. --- src/lib/dns/tests/master_lexer_unittest.cc | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc index b751da84f3..2e7002f4aa 100644 --- a/src/lib/dns/tests/master_lexer_unittest.cc +++ b/src/lib/dns/tests/master_lexer_unittest.cc @@ -300,6 +300,11 @@ lexerErrorCheck(MasterLexer& lexer, MasterToken::Type expect, } // Common checks regarding expected/unexpected end-of-line +// +// The 'lexer' should be at a position before two consecutive '\n's. +// The first one will be recognized, and the second one will be considered an +// unexpected token. Then this helper consumes the second '\n', so the caller +// can continue the test after these '\n's. void eolCheck(MasterLexer& lexer, MasterToken::Type expect) { // If EOL is found and eol_ok is true, we get it. @@ -313,9 +318,14 @@ eolCheck(MasterLexer& lexer, MasterToken::Type expect) { // And also check the error token set in the exception object. lexerErrorCheck(lexer, expect, MasterToken::UNEXPECTED_END); + + // Then skip the 2nd '\n' + EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); } // Common checks regarding expected/unexpected end-of-file +// +// The 'lexer' should be at a position just before an end-of-file. void eofCheck(MasterLexer& lexer, MasterToken::Type expect) { EXPECT_EQ(MasterToken::END_OF_FILE, @@ -335,9 +345,6 @@ TEST_F(MasterLexerTest, getNextTokenString) { lexer.getNextToken(MasterToken::STRING).getString()); eolCheck(lexer, MasterToken::STRING); - // Skip the 2nd '\n' - EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); - // Same set of tests but for end-of-file EXPECT_EQ("another-string", lexer.getNextToken(MasterToken::STRING, true).getString()); @@ -355,9 +362,6 @@ TEST_F(MasterLexerTest, getNextTokenQString) { lexer.getNextToken(MasterToken::QSTRING).getString()); eolCheck(lexer, MasterToken::QSTRING); - // Skip the 2nd '\n' - EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); - // Expecting a quoted string but see a normal string. It's okay. EXPECT_EQ("normal-string", lexer.getNextToken(MasterToken::QSTRING).getString()); @@ -377,9 +381,6 @@ TEST_F(MasterLexerTest, getNextTokenNumber) { lexer.getNextToken(MasterToken::NUMBER).getNumber()); eolCheck(lexer, MasterToken::NUMBER); - // Skip the 2nd '\n' - EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType()); - // Expecting a number, but it's too big for uint32. lexerErrorCheck(lexer, MasterToken::NUMBER, MasterToken::NUMBER_OUT_OF_RANGE); From 2f554072bc7b82a18d898073802f545305a90110 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Dec 2012 13:41:33 -0800 Subject: [PATCH 29/69] [2506] add another test for getting numbers --- src/lib/dns/tests/master_lexer_unittest.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc index 2e7002f4aa..b2415dab86 100644 --- a/src/lib/dns/tests/master_lexer_unittest.cc +++ b/src/lib/dns/tests/master_lexer_unittest.cc @@ -373,6 +373,7 @@ TEST_F(MasterLexerTest, getNextTokenNumber) { ss << "\n"; ss << "4294967296 "; // =2^32, out of range ss << "not-a-number "; + ss << "123abc "; // starting with digits, but resulting in a string ss << "86400"; lexer.pushSource(ss); @@ -392,6 +393,11 @@ TEST_F(MasterLexerTest, getNextTokenNumber) { // The unexpected string should have been "ungotten". Re-read and skip it. EXPECT_EQ(MasterToken::STRING, lexer.getNextToken().getType()); + // Expecting a number, but see a string. + lexerErrorCheck(lexer, MasterToken::NUMBER, MasterToken::BAD_NUMBER); + // The unexpected string should have been "ungotten". Re-read and skip it. + EXPECT_EQ(MasterToken::STRING, lexer.getNextToken().getType()); + // Unless we specify NUMBER, decimal number string should be recognized // as a string. EXPECT_EQ("86400", From 2abaf1a0335fc470556c2506a9dc7ff002cca8d0 Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Mon, 3 Dec 2012 10:51:18 +0530 Subject: [PATCH 30/69] [2497] Make new_rdata_factory_users[] a list of tuples Also: * document new_rdata_factory_users[] with an example * Rename new_rdatafactory_users[] to new_rdata_factory_users[] --- src/lib/dns/gen-rdatacode.py.in | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/lib/dns/gen-rdatacode.py.in b/src/lib/dns/gen-rdatacode.py.in index 6a9ff8aca5..bdf5449ae5 100755 --- a/src/lib/dns/gen-rdatacode.py.in +++ b/src/lib/dns/gen-rdatacode.py.in @@ -24,11 +24,20 @@ from os.path import getmtime import re import sys +# new_rdata_factory_users[] is a list of tuples of the form (rrtype, +# rrclass). Items in the list use the (new) RdataFactory class, and +# items which are not in the list use OldRdataFactory class. +# Note: rrtype and rrclass must be specified in lowercase in +# new_rdata_factory_users. +# +# Example: +# new_rdata_factory_users = [('a', 'in'), ('a', 'ch')] +new_rdata_factory_users = [] + re_typecode = re.compile('([\da-z]+)_(\d+)') classcode2txt = {} typecode2txt = {} typeandclass = [] -new_rdatafactory_users = ['aaaa'] generic_code = 65536 # something larger than any code value rdata_declarations = '' class_definitions = '' @@ -294,8 +303,8 @@ def generate_rrparam(fileprefix, basemtime): # By default, we use OldRdataFactory (see bug #2497). If you # want to pick RdataFactory for a particular type, add it to - # new_rdatafactory_users. - if type_txt in new_rdatafactory_users: + # new_rdata_factory_users. + if (type_txt.lower(), class_txt.lower()) in new_rdata_factory_users: rdf_class = 'RdataFactory' else: rdf_class = 'OldRdataFactory' From 3666f50858de194cbca5f0e3e8d9fba747bb3250 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Dec 2012 11:09:00 +0530 Subject: [PATCH 31/69] [2497] Move repeated code to a function --- src/lib/dns/rrparamregistry-placeholder.cc | 77 +++++++++++----------- 1 file changed, 37 insertions(+), 40 deletions(-) diff --git a/src/lib/dns/rrparamregistry-placeholder.cc b/src/lib/dns/rrparamregistry-placeholder.cc index 2208d3f440..ded8a66c97 100644 --- a/src/lib/dns/rrparamregistry-placeholder.cc +++ b/src/lib/dns/rrparamregistry-placeholder.cc @@ -512,6 +512,27 @@ RRParamRegistry::codeToClassText(uint16_t code) const { impl_->code2classmap)); } +namespace { +inline const AbstractRdataFactory* +findRdataFactory(RRParamRegistryImpl* reg_impl, + const RRType& rrtype, const RRClass& rrclass) +{ + RdataFactoryMap::const_iterator found; + found = reg_impl->rdata_factories.find(RRTypeClass(rrtype, rrclass)); + if (found != reg_impl->rdata_factories.end()) { + return (found->second.get()); + } + + GenericRdataFactoryMap::const_iterator genfound = + reg_impl->genericrdata_factories.find(rrtype); + if (genfound != reg_impl->genericrdata_factories.end()) { + return (genfound->second.get()); + } + + return (NULL); +} +} + RdataPtr RRParamRegistry::createRdata(const RRType& rrtype, const RRClass& rrclass, const std::string& rdata_string) @@ -519,16 +540,10 @@ RRParamRegistry::createRdata(const RRType& rrtype, const RRClass& rrclass, // If the text indicates that it's rdata of an "unknown" type (beginning // with '\# n'), parse it that way. (TBD) - RdataFactoryMap::const_iterator found; - found = impl_->rdata_factories.find(RRTypeClass(rrtype, rrclass)); - if (found != impl_->rdata_factories.end()) { - return (found->second->create(rdata_string)); - } - - GenericRdataFactoryMap::const_iterator genfound = - impl_->genericrdata_factories.find(rrtype); - if (genfound != impl_->genericrdata_factories.end()) { - return (genfound->second->create(rdata_string)); + const AbstractRdataFactory* factory = + findRdataFactory(impl_, rrtype, rrclass); + if (factory != NULL) { + return (factory->create(rdata_string)); } return (RdataPtr(new generic::Generic(rdata_string))); @@ -538,16 +553,10 @@ RdataPtr RRParamRegistry::createRdata(const RRType& rrtype, const RRClass& rrclass, InputBuffer& buffer, size_t rdata_len) { - RdataFactoryMap::const_iterator found = - impl_->rdata_factories.find(RRTypeClass(rrtype, rrclass)); - if (found != impl_->rdata_factories.end()) { - return (found->second->create(buffer, rdata_len)); - } - - GenericRdataFactoryMap::const_iterator genfound = - impl_->genericrdata_factories.find(rrtype); - if (genfound != impl_->genericrdata_factories.end()) { - return (genfound->second->create(buffer, rdata_len)); + const AbstractRdataFactory* factory = + findRdataFactory(impl_, rrtype, rrclass); + if (factory != NULL) { + return (factory->create(buffer, rdata_len)); } return (RdataPtr(new generic::Generic(buffer, rdata_len))); @@ -557,16 +566,10 @@ RdataPtr RRParamRegistry::createRdata(const RRType& rrtype, const RRClass& rrclass, const Rdata& source) { - RdataFactoryMap::const_iterator found = - impl_->rdata_factories.find(RRTypeClass(rrtype, rrclass)); - if (found != impl_->rdata_factories.end()) { - return (found->second->create(source)); - } - - GenericRdataFactoryMap::const_iterator genfound = - impl_->genericrdata_factories.find(rrtype); - if (genfound != impl_->genericrdata_factories.end()) { - return (genfound->second->create(source)); + const AbstractRdataFactory* factory = + findRdataFactory(impl_, rrtype, rrclass); + if (factory != NULL) { + return (factory->create(source)); } return (RdataPtr(new rdata::generic::Generic( @@ -579,16 +582,10 @@ RRParamRegistry::createRdata(const RRType& rrtype, const RRClass& rrclass, MasterLoader::Options options, MasterLoaderCallbacks& callbacks) { - RdataFactoryMap::const_iterator found = - impl_->rdata_factories.find(RRTypeClass(rrtype, rrclass)); - if (found != impl_->rdata_factories.end()) { - return (found->second->create(lexer, name, options, callbacks)); - } - - GenericRdataFactoryMap::const_iterator genfound = - impl_->genericrdata_factories.find(rrtype); - if (genfound != impl_->genericrdata_factories.end()) { - return (genfound->second->create(lexer, name, options, callbacks)); + const AbstractRdataFactory* factory = + findRdataFactory(impl_, rrtype, rrclass); + if (factory != NULL) { + return (factory->create(lexer, name, options, callbacks)); } return (RdataPtr(new generic::Generic(lexer, name, options, callbacks))); From c0f25aade83aaf90bef0abd8bd7eff66cdaa7684 Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Mon, 3 Dec 2012 11:13:56 +0530 Subject: [PATCH 32/69] [2497] Remove a couple of txt tests --- src/lib/dns/tests/rdata_txt_like_unittest.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc index 7ff9ac9a0d..b894faebd6 100644 --- a/src/lib/dns/tests/rdata_txt_like_unittest.cc +++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc @@ -189,12 +189,6 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromLexer) { EXPECT_EQ(0, this->rdata_txt_like.compare( *test::createRdataUsingLexer(RRTYPE(), RRClass::IN(), "Test String"))); - EXPECT_EQ(0, this->rdata_txt_like_empty.compare( - *test::createRdataUsingLexer(RRTYPE(), RRClass::IN(), - ""))); - EXPECT_EQ(0, this->rdata_txt_like_quoted.compare( - *test::createRdataUsingLexer(RRTYPE(), RRClass::IN(), - "\"Test String\""))); } TYPED_TEST(Rdata_TXT_LIKE_Test, toWireBuffer) { From 3ee8c9cdab87054662af5a67993c9a3ced593256 Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Tue, 4 Dec 2012 04:13:57 +0530 Subject: [PATCH 33/69] [2497] Return whether the string is quoted in characterstr::getNextCharacterString() --- src/lib/dns/character_string.cc | 7 ++++++- src/lib/dns/character_string.h | 5 ++++- src/lib/dns/tests/character_string_unittest.cc | 7 ++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/lib/dns/character_string.cc b/src/lib/dns/character_string.cc index 3a289acd49..8b319483dc 100644 --- a/src/lib/dns/character_string.cc +++ b/src/lib/dns/character_string.cc @@ -29,7 +29,8 @@ bool isDigit(char c) { std::string characterstr::getNextCharacterString(const std::string& input_str, - std::string::const_iterator& input_iterator) + std::string::const_iterator& input_iterator, + bool* quoted) { string result; @@ -119,6 +120,10 @@ characterstr::getNextCharacterString(const std::string& input_str, isc_throw(InvalidRdataText, "The quotes are not paired"); } + if (quoted != NULL) { + *quoted = quotes_separated; + } + return (result); } diff --git a/src/lib/dns/character_string.h b/src/lib/dns/character_string.h index 2a68778c7b..0bfa38b8d5 100644 --- a/src/lib/dns/character_string.h +++ b/src/lib/dns/character_string.h @@ -39,9 +39,12 @@ namespace characterstr { /// \param input_iterator The iterator from which to start extracting, /// the iterator will be updated to new position after the function /// is returned + /// \param quoted If not \c NULL, returns \c true at this address if + /// the string is quoted, \cfalse otherwise /// \return A std::string that contains the extracted std::string getNextCharacterString(const std::string& input_str, - std::string::const_iterator& input_iterator); + std::string::const_iterator& input_iterator, + bool* quoted = NULL); /// Get a from a input buffer /// diff --git a/src/lib/dns/tests/character_string_unittest.cc b/src/lib/dns/tests/character_string_unittest.cc index 5fed9eb0a3..e8f58844a0 100644 --- a/src/lib/dns/tests/character_string_unittest.cc +++ b/src/lib/dns/tests/character_string_unittest.cc @@ -33,11 +33,13 @@ class CharacterString { public: CharacterString(const string& str){ string::const_iterator it = str.begin(); - characterStr_ = getNextCharacterString(str, it); + characterStr_ = getNextCharacterString(str, it, &is_quoted_); } const string& str() const { return characterStr_; } + bool quoted() const { return (is_quoted_); } private: string characterStr_; + bool is_quoted_; }; TEST(CharacterStringTest, testNormalCase) { @@ -47,14 +49,17 @@ TEST(CharacterStringTest, testNormalCase) { // Test that separated by space CharacterString cstr2("foo bar"); EXPECT_EQ(string("foo"), cstr2.str()); + EXPECT_FALSE(cstr2.quoted()); // Test that separated by quotes CharacterString cstr3("\"foo bar\""); EXPECT_EQ(string("foo bar"), cstr3.str()); + EXPECT_TRUE(cstr3.quoted()); // Test that not separate by quotes but ended with quotes CharacterString cstr4("foo\""); EXPECT_EQ(string("foo\""), cstr4.str()); + EXPECT_FALSE(cstr4.quoted()); } TEST(CharacterStringTest, testBadCase) { From 5f03da43d3b7cc5225b64f085320d501c8e1cfde Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Tue, 4 Dec 2012 04:23:36 +0530 Subject: [PATCH 34/69] [2497] Fix HINFO parsing in the string parser The lexer variant also uses the string parser eventually (for now). --- src/lib/dns/rdata/generic/hinfo_13.cc | 24 +++++++++++++++++------ src/lib/dns/rdata/generic/hinfo_13.h | 9 ++++++++- src/lib/dns/tests/rdata_hinfo_unittest.cc | 17 ++++++++++++---- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/lib/dns/rdata/generic/hinfo_13.cc b/src/lib/dns/rdata/generic/hinfo_13.cc index 12f034c138..77bfdfd21f 100644 --- a/src/lib/dns/rdata/generic/hinfo_13.cc +++ b/src/lib/dns/rdata/generic/hinfo_13.cc @@ -39,11 +39,22 @@ using namespace isc::dns::characterstr; HINFO::HINFO(const std::string& hinfo_str) { string::const_iterator input_iterator = hinfo_str.begin(); - cpu_ = getNextCharacterString(hinfo_str, input_iterator); - skipLeftSpaces(hinfo_str, input_iterator); + bool quoted; + cpu_ = getNextCharacterString(hinfo_str, input_iterator, "ed); + + skipLeftSpaces(hinfo_str, input_iterator, quoted); os_ = getNextCharacterString(hinfo_str, input_iterator); + + // Skip whitespace at the end. + while (input_iterator < hinfo_str.end() && isspace(*input_iterator)) { + ++input_iterator; + } + if (input_iterator < hinfo_str.end()) { + isc_throw(InvalidRdataText, + "Invalid HINFO text format: too many fields."); + } } HINFO::HINFO(InputBuffer& buffer, size_t rdata_len) { @@ -108,16 +119,17 @@ HINFO::getOS() const { void HINFO::skipLeftSpaces(const std::string& input_str, - std::string::const_iterator& input_iterator) + std::string::const_iterator& input_iterator, + bool optional) { if (input_iterator >= input_str.end()) { isc_throw(InvalidRdataText, - "Invalid HINFO text format, field is missing."); + "Invalid HINFO text format: field is missing."); } - if (!isspace(*input_iterator)) { + if (!isspace(*input_iterator) && !optional) { isc_throw(InvalidRdataText, - "Invalid HINFO text format, fields are not separated by space."); + "Invalid HINFO text format: fields are not separated by space."); } // Skip white spaces while (input_iterator < input_str.end() && isspace(*input_iterator)) { diff --git a/src/lib/dns/rdata/generic/hinfo_13.h b/src/lib/dns/rdata/generic/hinfo_13.h index 85134198b6..5534a7e794 100644 --- a/src/lib/dns/rdata/generic/hinfo_13.h +++ b/src/lib/dns/rdata/generic/hinfo_13.h @@ -46,10 +46,17 @@ public: private: /// Skip the left whitespaces of the input string /// + /// If \c optional argument is \c true and no spaces occur at the + /// current location, then nothing happens. If \c optional is + /// \c false and no spaces occur at the current location, then + /// the \c InvalidRdataText exception is thrown. + /// /// \param input_str The input string /// \param input_iterator From which the skipping started + /// \param optional If true, the spaces are optionally skipped. void skipLeftSpaces(const std::string& input_str, - std::string::const_iterator& input_iterator); + std::string::const_iterator& input_iterator, + bool optional); /// Helper template function for toWire() /// diff --git a/src/lib/dns/tests/rdata_hinfo_unittest.cc b/src/lib/dns/tests/rdata_hinfo_unittest.cc index 746904751f..005563935d 100644 --- a/src/lib/dns/tests/rdata_hinfo_unittest.cc +++ b/src/lib/dns/tests/rdata_hinfo_unittest.cc @@ -57,8 +57,11 @@ TEST_F(Rdata_HINFO_Test, createFromText) { } TEST_F(Rdata_HINFO_Test, badText) { - // Fields must be seperated by spaces - EXPECT_THROW(const HINFO hinfo("\"Pentium\"\"Linux\""), InvalidRdataText); + // Only 2 fields must exist + EXPECT_THROW(const HINFO hinfo("\"Pentium\"\"Linux\"\"Computer\""), + InvalidRdataText); + EXPECT_THROW(const HINFO hinfo("\"Pentium\" \"Linux\" \"Computer\""), + InvalidRdataText); // Field cannot be missing EXPECT_THROW(const HINFO hinfo("Pentium"), InvalidRdataText); // The cannot exceed 255 characters @@ -82,10 +85,16 @@ TEST_F(Rdata_HINFO_Test, createFromLexer) { EXPECT_EQ(0, rdata_hinfo.compare( *test::createRdataUsingLexer(RRType::HINFO(), RRClass::IN(), hinfo_str))); - + EXPECT_EQ(0, rdata_hinfo.compare( + *test::createRdataUsingLexer(RRType::HINFO(), RRClass::IN(), + "\"Pentium\"\"Linux\""))); // Exceptions cause NULL to be returned. EXPECT_FALSE(test::createRdataUsingLexer(RRType::HINFO(), RRClass::IN(), - "\"Pentium\"\"Linux\"")); + "\"Pentium\"\"Linux\"" + "\"Computer\"")); + EXPECT_FALSE(test::createRdataUsingLexer(RRType::HINFO(), RRClass::IN(), + "\"Pentium\" \"Linux\" " + "\"Computer\"")); } TEST_F(Rdata_HINFO_Test, toText) { From 5911431e8fcb1728b1a4765a6f3c1352c4879e8c Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Tue, 4 Dec 2012 04:44:34 +0530 Subject: [PATCH 35/69] [2497] Add a direct HINFO testcase for the string parser --- src/lib/dns/tests/rdata_hinfo_unittest.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lib/dns/tests/rdata_hinfo_unittest.cc b/src/lib/dns/tests/rdata_hinfo_unittest.cc index 005563935d..f592066b6f 100644 --- a/src/lib/dns/tests/rdata_hinfo_unittest.cc +++ b/src/lib/dns/tests/rdata_hinfo_unittest.cc @@ -41,6 +41,7 @@ static uint8_t hinfo_rdata[] = {0x07,0x50,0x65,0x6e,0x74,0x69,0x75,0x6d,0x05, static const char *hinfo_str = "\"Pentium\" \"Linux\""; static const char *hinfo_str1 = "\"Pen\\\"tium\" \"Linux\""; +static const char *hinfo_str_equal = "\"Pentium\"\"Linux\""; static const char *hinfo_str_small1 = "\"Lentium\" \"Linux\""; static const char *hinfo_str_small2 = "\"Pentium\" \"Kinux\""; static const char *hinfo_str_large1 = "\"Qentium\" \"Linux\""; @@ -127,6 +128,7 @@ TEST_F(Rdata_HINFO_Test, compare) { HINFO hinfo_large2(hinfo_str_large2); EXPECT_EQ(0, hinfo.compare(HINFO(hinfo_str))); + EXPECT_EQ(0, hinfo.compare(HINFO(hinfo_str_equal))); EXPECT_EQ(1, hinfo.compare(HINFO(hinfo_str_small1))); EXPECT_EQ(1, hinfo.compare(HINFO(hinfo_str_small2))); EXPECT_EQ(-1, hinfo.compare(HINFO(hinfo_str_large1))); From 8da8a38bcf022e5ad4d99a64be6cda1e557c5692 Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Tue, 4 Dec 2012 04:52:54 +0530 Subject: [PATCH 36/69] [2497] Apply generic rules to class IN too --- src/lib/dns/gen-rdatacode.py.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib/dns/gen-rdatacode.py.in b/src/lib/dns/gen-rdatacode.py.in index bdf5449ae5..1a2d6407f4 100755 --- a/src/lib/dns/gen-rdatacode.py.in +++ b/src/lib/dns/gen-rdatacode.py.in @@ -304,7 +304,9 @@ def generate_rrparam(fileprefix, basemtime): # By default, we use OldRdataFactory (see bug #2497). If you # want to pick RdataFactory for a particular type, add it to # new_rdata_factory_users. - if (type_txt.lower(), class_txt.lower()) in new_rdata_factory_users: + if ((type_txt.lower(), class_txt.lower()) in new_rdata_factory_users) or \ + ((class_txt.lower() == 'in') and \ + ((type_txt.lower(), 'generic') in new_rdata_factory_users)): rdf_class = 'RdataFactory' else: rdf_class = 'OldRdataFactory' From 2c9904e1543d72dac392f1592580d9f8351f5e51 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Dec 2012 18:29:37 -0800 Subject: [PATCH 37/69] [2497] some suggested editorial cleanups: long line, identation, better e.g. --- src/lib/dns/gen-rdatacode.py.in | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/lib/dns/gen-rdatacode.py.in b/src/lib/dns/gen-rdatacode.py.in index 1a2d6407f4..d0decfdf28 100755 --- a/src/lib/dns/gen-rdatacode.py.in +++ b/src/lib/dns/gen-rdatacode.py.in @@ -31,7 +31,7 @@ import sys # new_rdata_factory_users. # # Example: -# new_rdata_factory_users = [('a', 'in'), ('a', 'ch')] +# new_rdata_factory_users = [('a', 'in'), ('a', 'ch'), ('soa', 'generic')] new_rdata_factory_users = [] re_typecode = re.compile('([\da-z]+)_(\d+)') @@ -303,10 +303,13 @@ def generate_rrparam(fileprefix, basemtime): # By default, we use OldRdataFactory (see bug #2497). If you # want to pick RdataFactory for a particular type, add it to - # new_rdata_factory_users. - if ((type_txt.lower(), class_txt.lower()) in new_rdata_factory_users) or \ - ((class_txt.lower() == 'in') and \ - ((type_txt.lower(), 'generic') in new_rdata_factory_users)): + # new_rdata_factory_users. Note that we explicitly generate (for + # optimization) class-independent ("generic") factories for class IN + # for optimization. + if (((type_txt.lower(), class_txt.lower()) in + new_rdata_factory_users) or + ((class_txt.lower() == 'in') and + ((type_txt.lower(), 'generic') in new_rdata_factory_users))): rdf_class = 'RdataFactory' else: rdf_class = 'OldRdataFactory' From 31dd54a026b5427979a932f0735bacf8723ec7cb Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Fri, 30 Nov 2012 21:59:52 -0800 Subject: [PATCH 38/69] [2442] added minimum definition for strToCharString and tests --- src/lib/dns/Makefile.am | 4 ++ .../dns/rdata/generic/detail/char_string.cc | 45 ++++++++++++++++ .../dns/rdata/generic/detail/char_string.h | 47 ++++++++++++++++ src/lib/dns/tests/Makefile.am | 1 + .../dns/tests/rdata_char_string_unittest.cc | 53 +++++++++++++++++++ 5 files changed, 150 insertions(+) create mode 100644 src/lib/dns/rdata/generic/detail/char_string.cc create mode 100644 src/lib/dns/rdata/generic/detail/char_string.h create mode 100644 src/lib/dns/tests/rdata_char_string_unittest.cc diff --git a/src/lib/dns/Makefile.am b/src/lib/dns/Makefile.am index 9168e6bcaf..050860a612 100644 --- a/src/lib/dns/Makefile.am +++ b/src/lib/dns/Makefile.am @@ -21,6 +21,8 @@ EXTRA_DIST += rdata/ch_3/a_1.cc EXTRA_DIST += rdata/ch_3/a_1.h EXTRA_DIST += rdata/generic/cname_5.cc EXTRA_DIST += rdata/generic/cname_5.h +EXTRA_DIST += rdata/generic/detail/char_string.cc +EXTRA_DIST += rdata/generic/detail/char_string.h EXTRA_DIST += rdata/generic/detail/nsec_bitmap.cc EXTRA_DIST += rdata/generic/detail/nsec_bitmap.h EXTRA_DIST += rdata/generic/detail/nsec3param_common.cc @@ -121,6 +123,8 @@ libb10_dns___la_SOURCES += tsigrecord.h tsigrecord.cc libb10_dns___la_SOURCES += character_string.h character_string.cc libb10_dns___la_SOURCES += master_loader_callbacks.h libb10_dns___la_SOURCES += master_loader.h +libb10_dns___la_SOURCES += rdata/generic/detail/char_string.h +libb10_dns___la_SOURCES += rdata/generic/detail/char_string.cc libb10_dns___la_SOURCES += rdata/generic/detail/nsec_bitmap.h libb10_dns___la_SOURCES += rdata/generic/detail/nsec_bitmap.cc libb10_dns___la_SOURCES += rdata/generic/detail/nsec3param_common.cc diff --git a/src/lib/dns/rdata/generic/detail/char_string.cc b/src/lib/dns/rdata/generic/detail/char_string.cc new file mode 100644 index 0000000000..841945e41c --- /dev/null +++ b/src/lib/dns/rdata/generic/detail/char_string.cc @@ -0,0 +1,45 @@ +// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#include +#include + +#include +#include + +namespace isc { +namespace dns { +namespace rdata { +namespace generic { +namespace detail { + +void +strToCharString(const MasterToken::StringRegion& str_region, + CharString& result) +{ + result.push_back(0); + size_t n = str_region.len; + const char* s = str_region.beg; + while (n-- != 0) { + const int c = (*s++) & 0xff; + result.push_back(c); + } + result[0] = str_region.len; // FIXME: this is not always correct +} + +} // end of detail +} // end of generic +} // end of rdata +} // end of dns +} // end of isc diff --git a/src/lib/dns/rdata/generic/detail/char_string.h b/src/lib/dns/rdata/generic/detail/char_string.h new file mode 100644 index 0000000000..b5dc338e96 --- /dev/null +++ b/src/lib/dns/rdata/generic/detail/char_string.h @@ -0,0 +1,47 @@ +// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#ifndef DNS_RDATA_CHARSTRING_H +#define DNS_RDATA_CHARSTRING_H 1 + +#include + +#include +#include + +namespace isc { +namespace dns { +namespace rdata { +namespace generic { +namespace detail { + +/// \brief Type for DNS character string. +/// +/// A character string can contain any unsigned 8-bit value, so this cannot +/// be the bare char basis. +typedef std::vector CharString; + +void strToCharString(const MasterToken::StringRegion& str_region, + CharString& result); + +} // namespace detail +} // namespace generic +} // namespace rdata +} // namespace dns +} // namespace isc +#endif // DNS_RDATA_CHARSTRING_H + +// Local Variables: +// mode: c++ +// End: diff --git a/src/lib/dns/tests/Makefile.am b/src/lib/dns/tests/Makefile.am index 12f8f2fef4..d5690d4aae 100644 --- a/src/lib/dns/tests/Makefile.am +++ b/src/lib/dns/tests/Makefile.am @@ -36,6 +36,7 @@ run_unittests_SOURCES += opcode_unittest.cc run_unittests_SOURCES += rcode_unittest.cc run_unittests_SOURCES += rdata_unittest.h rdata_unittest.cc run_unittests_SOURCES += rdatafields_unittest.cc +run_unittests_SOURCES += rdata_char_string_unittest.cc run_unittests_SOURCES += rdata_in_a_unittest.cc rdata_in_aaaa_unittest.cc run_unittests_SOURCES += rdata_ns_unittest.cc rdata_soa_unittest.cc run_unittests_SOURCES += rdata_txt_like_unittest.cc diff --git a/src/lib/dns/tests/rdata_char_string_unittest.cc b/src/lib/dns/tests/rdata_char_string_unittest.cc new file mode 100644 index 0000000000..fc5346b131 --- /dev/null +++ b/src/lib/dns/tests/rdata_char_string_unittest.cc @@ -0,0 +1,53 @@ +// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#include +#include + +#include + +#include +#include + +using namespace isc::dns; +using namespace isc::dns::rdata; +using isc::dns::rdata::generic::detail::CharString; +using isc::dns::rdata::generic::detail::strToCharString; +using isc::util::unittests::matchWireData; + +namespace { +const uint8_t test_charstr[] = { + sizeof("Test String") - 1, + 'T', 'e', 's', 't', ' ', 'S', 't', 'r', 'i', 'n', 'g' +}; + +class CharStringTest : public ::testing::Test { +protected: + CharStringTest() : + test_str("Test String") + { + str_region.beg = &test_str[0]; + str_region.len = test_str.size(); + } + CharString chstr; // placeholder + const std::string test_str; + MasterToken::StringRegion str_region; +}; + +TEST_F(CharStringTest, test) { + strToCharString(str_region, chstr); + matchWireData(test_charstr, sizeof(test_charstr), &chstr[0], chstr.size()); +} + +} // unnamed namespace From 6edf83daa67ac95bb45c2975e0511a830e64ac59 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Fri, 30 Nov 2012 23:50:33 -0800 Subject: [PATCH 39/69] [2442] handled various corner cases --- .../dns/rdata/generic/detail/char_string.cc | 64 ++++++++++- .../dns/tests/rdata_char_string_unittest.cc | 100 +++++++++++++++++- 2 files changed, 157 insertions(+), 7 deletions(-) diff --git a/src/lib/dns/rdata/generic/detail/char_string.cc b/src/lib/dns/rdata/generic/detail/char_string.cc index 841945e41c..e9f900a49b 100644 --- a/src/lib/dns/rdata/generic/detail/char_string.cc +++ b/src/lib/dns/rdata/generic/detail/char_string.cc @@ -12,10 +12,19 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +#include + +#include #include #include +#include + +#include +#include +#include #include + #include namespace isc { @@ -24,18 +33,65 @@ namespace rdata { namespace generic { namespace detail { +namespace { +// Convert a DDD form to the corresponding integer +int +decimalToNumber(const char* s, const char* s_end) { + if (s_end - s < 3) { + isc_throw(InvalidRdataText, "Escaped digits too short"); + } + + // Make a nul-terminated copy of the 'DDD' for lexical_cast + char buf[4]; + std::memcpy(buf, s, 3); + buf[3] = 0; + + try { + const int i = boost::lexical_cast(buf); + if (i > 255) { + isc_throw(InvalidRdataText, "Escaped digits too large: " << buf); + } + return (i); + } catch (const boost::bad_lexical_cast&) { + isc_throw(InvalidRdataText, + "Invalid form for escaped digits: " << buf); + } +} +} + void strToCharString(const MasterToken::StringRegion& str_region, CharString& result) { + // make a space for the 1-byte length field; filled in at the end result.push_back(0); - size_t n = str_region.len; + + bool escape = false; const char* s = str_region.beg; - while (n-- != 0) { - const int c = (*s++) & 0xff; + const char* const s_end = str_region.beg + str_region.len; + + for (size_t n = str_region.len; n != 0; --n, ++s) { + int c = (*s & 0xff); + if (escape && std::isdigit(c) != 0) { + c = decimalToNumber(s, s_end); + assert(n >= 3); + n -= 2; + s += 2; + } else if (!escape && c == '\\') { + escape = true; + continue; + } + escape = false; result.push_back(c); } - result[0] = str_region.len; // FIXME: this is not always correct + if (escape) { // terminated by non-escaped '\' + isc_throw(InvalidRdataText, "character-string ends with '\\'"); + } + if (result.size() > MAX_CHARSTRING_LEN + 1) { // '+ 1' due to the len field + isc_throw(CharStringTooLong, "character-string is too long: " << + result.size() << " bytes"); + } + result[0] = result.size() - 1; } } // end of detail diff --git a/src/lib/dns/tests/rdata_char_string_unittest.cc b/src/lib/dns/tests/rdata_char_string_unittest.cc index fc5346b131..83f0591280 100644 --- a/src/lib/dns/tests/rdata_char_string_unittest.cc +++ b/src/lib/dns/tests/rdata_char_string_unittest.cc @@ -13,6 +13,8 @@ // PERFORMANCE OF THIS SOFTWARE. #include + +#include #include #include @@ -35,19 +37,111 @@ const uint8_t test_charstr[] = { class CharStringTest : public ::testing::Test { protected: CharStringTest() : - test_str("Test String") + // char-string representation for test data using two types of escape + // ('r' = 114) + test_str("Test\\ St\\114ing") { str_region.beg = &test_str[0]; str_region.len = test_str.size(); } - CharString chstr; // placeholder + CharString chstr; // place holder const std::string test_str; MasterToken::StringRegion str_region; }; -TEST_F(CharStringTest, test) { +MasterToken::StringRegion +createStringRegion(const std::string& str) { + MasterToken::StringRegion region; + region.beg = &str[0]; // note this works even if str is empty + region.len = str.size(); + return (region); +} + +TEST_F(CharStringTest, normalConversion) { + uint8_t tmp[3]; // placeholder for expected sequence + strToCharString(str_region, chstr); matchWireData(test_charstr, sizeof(test_charstr), &chstr[0], chstr.size()); + + // Empty string + chstr.clear(); + strToCharString(createStringRegion(""), chstr); + tmp[0] = 0; + matchWireData(tmp, 1, &chstr[0], chstr.size()); + + // Possible largest char string + chstr.clear(); + std::string long_str(255, 'x'); + strToCharString(createStringRegion(long_str), chstr); + std::vector expected; + expected.push_back(255); // len of char string + expected.insert(expected.end(), long_str.begin(), long_str.end()); + matchWireData(&expected[0], expected.size(), &chstr[0], chstr.size()); + + // Same data as the previous case, but the original string is longer than + // the max; this shouldn't be rejected + chstr.clear(); + long_str.at(254) = '\\'; // replace the last 'x' with '\' + long_str.append("120"); // 'x' = 120 + strToCharString(createStringRegion(long_str), chstr); + matchWireData(&expected[0], expected.size(), &chstr[0], chstr.size()); + + // Escaped '\' + chstr.clear(); + tmp[0] = 1; + tmp[1] = '\\'; + strToCharString(createStringRegion("\\\\"), chstr); + matchWireData(tmp, 2, &chstr[0], chstr.size()); + + // Boundary values for \DDD + chstr.clear(); + tmp[0] = 1; + tmp[1] = 0; + strToCharString(createStringRegion("\\000"), chstr); + matchWireData(tmp, 2, &chstr[0], chstr.size()); + + chstr.clear(); + strToCharString(createStringRegion("\\255"), chstr); + tmp[0] = 1; + tmp[1] = 255; + matchWireData(tmp, 2, &chstr[0], chstr.size()); + + // Another digit follows DDD; it shouldn't cause confusion + chstr.clear(); + strToCharString(createStringRegion("\\2550"), chstr); + tmp[0] = 2; // string len is now 2 + tmp[2] = '0'; + matchWireData(tmp, 3, &chstr[0], chstr.size()); +} + +TEST_F(CharStringTest, badConversion) { + // string cannot exceed 255 bytes + EXPECT_THROW(strToCharString(createStringRegion(std::string(256, 'a')), + chstr), + CharStringTooLong); + + // input string ending with (non escaped) '\' + chstr.clear(); + EXPECT_THROW(strToCharString(createStringRegion("foo\\"), chstr), + InvalidRdataText); +} + +TEST_F(CharStringTest, badDDD) { + // Check various type of bad form of \DDD + + // Not a number + EXPECT_THROW(strToCharString(createStringRegion("\\1a2"), chstr), + InvalidRdataText); + EXPECT_THROW(strToCharString(createStringRegion("\\12a"), chstr), + InvalidRdataText); + + // Not in the range of uint8_t + EXPECT_THROW(strToCharString(createStringRegion("\\256"), chstr), + InvalidRdataText); + + // Short buffer + EXPECT_THROW(strToCharString(createStringRegion("\\42"), chstr), + InvalidRdataText); } } // unnamed namespace From 5819758217d6e95348c722bafe0f79c1e28ee43a Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Dec 2012 14:34:23 -0800 Subject: [PATCH 40/69] [2442] (unrelated) cleanup: use propoer namespace and avoid 'using namespace' --- src/lib/dns/rdata/generic/detail/txt_like.h | 44 ++++++++++++--------- src/lib/dns/rdata/generic/spf_99.cc | 11 +++--- src/lib/dns/rdata/generic/spf_99.h | 4 +- src/lib/dns/rdata/generic/txt_16.cc | 3 +- src/lib/dns/rdata/generic/txt_16.h | 4 +- 5 files changed, 37 insertions(+), 29 deletions(-) diff --git a/src/lib/dns/rdata/generic/detail/txt_like.h b/src/lib/dns/rdata/generic/detail/txt_like.h index fdab6bf4e6..b5553fcb90 100644 --- a/src/lib/dns/rdata/generic/detail/txt_like.h +++ b/src/lib/dns/rdata/generic/detail/txt_like.h @@ -20,8 +20,11 @@ #include #include -using namespace std; -using namespace isc::util; +namespace isc { +namespace dns { +namespace rdata { +namespace generic { +namespace detail { /// \brief \c rdata::TXTLikeImpl class represents the TXT-like RDATA for TXT /// and SPF types. @@ -41,7 +44,7 @@ public: /// /// \c InvalidRdataLength is thrown if rdata_len exceeds the maximum. /// \c DNSMessageFORMERR is thrown if the RR is misformed. - TXTLikeImpl(InputBuffer& buffer, size_t rdata_len) { + TXTLikeImpl(util::InputBuffer& buffer, size_t rdata_len) { if (rdata_len > MAX_RDLENGTH) { isc_throw(InvalidRdataLength, "RDLENGTH too large: " << rdata_len); } @@ -59,7 +62,7 @@ public: " RDATA: character string length is too large: " << static_cast(len)); } - vector data(len + 1); + std::vector data(len + 1); data[0] = len; buffer.readData(&data[0] + 1, len); string_list_.push_back(data); @@ -102,7 +105,7 @@ public: txtstr); } - vector data; + std::vector data; data.reserve(length + 1); data.push_back(length); data.insert(data.end(), txtstr.begin() + pos_begin, @@ -122,9 +125,9 @@ public: /// /// \param buffer An output buffer to store the wire data. void - toWire(OutputBuffer& buffer) const { - for (vector >::const_iterator it = - string_list_.begin(); + toWire(util::OutputBuffer& buffer) const { + for (std::vector >::const_iterator it = + string_list_.begin(); it != string_list_.end(); ++it) { @@ -139,8 +142,8 @@ public: /// to. void toWire(AbstractMessageRenderer& renderer) const { - for (vector >::const_iterator it = - string_list_.begin(); + for (std::vector >::const_iterator it = + string_list_.begin(); it != string_list_.end(); ++it) { @@ -151,14 +154,14 @@ public: /// \brief Convert the TXT-like data to a string. /// /// \return A \c string object that represents the TXT-like data. - string + std::string toText() const { - string s; + std::string s; // XXX: this implementation is not entirely correct. for example, it // should escape double-quotes if they appear in the character string. - for (vector >::const_iterator it = - string_list_.begin(); + for (std::vector >::const_iterator it = + string_list_.begin(); it != string_list_.end(); ++it) { @@ -189,7 +192,7 @@ public: OutputBuffer this_buffer(0); toWire(this_buffer); uint8_t const* const this_data = (uint8_t const*)this_buffer.getData(); - size_t this_len = this_buffer.getLength(); + const size_t this_len = this_buffer.getLength(); OutputBuffer other_buffer(0); other.toWire(other_buffer); @@ -214,11 +217,14 @@ private: std::vector > string_list_; }; -// END_RDATA_NAMESPACE -// END_ISC_NAMESPACE +} // namespace detail +} // namespace generic +} // namespace rdata +} // namespace dns +} // namespace isc #endif // TXT_LIKE_H -// Local Variables: +// Local Variables: // mode: c++ -// End: +// End: diff --git a/src/lib/dns/rdata/generic/spf_99.cc b/src/lib/dns/rdata/generic/spf_99.cc index aa3e4a13a9..4d19a8b729 100644 --- a/src/lib/dns/rdata/generic/spf_99.cc +++ b/src/lib/dns/rdata/generic/spf_99.cc @@ -24,18 +24,17 @@ #include #include +/// This class implements the basic interfaces inherited from the abstract +/// \c rdata::Rdata class. The semantics of the class is provided by +/// a copy of instantiated TXTLikeImpl class common to both TXT and SPF. +#include + using namespace std; using namespace isc::util; // BEGIN_ISC_NAMESPACE // BEGIN_RDATA_NAMESPACE -/// This class implements the basic interfaces inherited from the abstract -/// \c rdata::Rdata class. The semantics of the class is provided by -/// a copy of instantiated TXTLikeImpl class common to both TXT and SPF. - -#include - /// \brief The assignment operator /// /// It internally allocates a resource, and if it fails a corresponding diff --git a/src/lib/dns/rdata/generic/spf_99.h b/src/lib/dns/rdata/generic/spf_99.h index 04ac99b08a..eece47be4a 100644 --- a/src/lib/dns/rdata/generic/spf_99.h +++ b/src/lib/dns/rdata/generic/spf_99.h @@ -28,7 +28,9 @@ // BEGIN_RDATA_NAMESPACE +namespace detail { template class TXTLikeImpl; +} /// \brief \c rdata::SPF class represents the SPF RDATA as defined %in /// RFC4408. @@ -65,7 +67,7 @@ public: const std::vector >& getString() const; private: - typedef TXTLikeImpl SPFImpl; + typedef isc::dns::rdata::generic::detail::TXTLikeImpl SPFImpl; SPFImpl* impl_; }; diff --git a/src/lib/dns/rdata/generic/txt_16.cc b/src/lib/dns/rdata/generic/txt_16.cc index 418bc05fbc..6573487362 100644 --- a/src/lib/dns/rdata/generic/txt_16.cc +++ b/src/lib/dns/rdata/generic/txt_16.cc @@ -23,6 +23,7 @@ #include #include #include +#include using namespace std; using namespace isc::util; @@ -30,8 +31,6 @@ using namespace isc::util; // BEGIN_ISC_NAMESPACE // BEGIN_RDATA_NAMESPACE -#include - TXT& TXT::operator=(const TXT& source) { if (impl_ == source.impl_) { diff --git a/src/lib/dns/rdata/generic/txt_16.h b/src/lib/dns/rdata/generic/txt_16.h index d99d69b75d..e434085812 100644 --- a/src/lib/dns/rdata/generic/txt_16.h +++ b/src/lib/dns/rdata/generic/txt_16.h @@ -28,7 +28,9 @@ // BEGIN_RDATA_NAMESPACE +namespace detail { template class TXTLikeImpl; +} class TXT : public Rdata { public: @@ -39,7 +41,7 @@ public: ~TXT(); private: - typedef TXTLikeImpl TXTImpl; + typedef isc::dns::rdata::generic::detail::TXTLikeImpl TXTImpl; TXTImpl* impl_; }; From 9f8aabb7951987f3e247ad4263e259d82211e5b3 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Dec 2012 20:01:45 -0800 Subject: [PATCH 41/69] [2442] updated some TXT-like test data without using intermediate space. when we migrate the complete lexer version, this will cause confusion: the lexer will consider the space as a separater. --- src/lib/dns/tests/rdata_txt_like_unittest.cc | 55 +++++++++---------- .../dns/tests/testdata/rdata_txt_fromWire1 | 6 +- src/lib/util/python/gen_wiredata.py.in | 2 +- 3 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc index b894faebd6..f59163f4d0 100644 --- a/src/lib/dns/tests/rdata_txt_like_unittest.cc +++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc @@ -17,17 +17,19 @@ #include #include #include -#include #include #include +#include + using isc::UnitTestUtil; using namespace std; using namespace isc::dns; using namespace isc::util; using namespace isc::dns::rdata; +namespace { template class RRTYPE : public RRType { @@ -38,37 +40,31 @@ public: template<> RRTYPE::RRTYPE() : RRType(RRType::TXT()) {} template<> RRTYPE::RRTYPE() : RRType(RRType::SPF()) {} -namespace { const uint8_t wiredata_txt_like[] = { - sizeof("Test String") - 1, - 'T', 'e', 's', 't', ' ', 'S', 't', 'r', 'i', 'n', 'g' + sizeof("Test-String") - 1, + 'T', 'e', 's', 't', '-', 'S', 't', 'r', 'i', 'n', 'g' }; const uint8_t wiredata_nulltxt[] = { 0 }; -vector wiredata_longesttxt(256, 'a'); template class Rdata_TXT_LIKE_Test : public RdataTest { protected: - Rdata_TXT_LIKE_Test() { + Rdata_TXT_LIKE_Test() : + wiredata_longesttxt(256, 'a'), + rdata_txt_like("Test-String"), + rdata_txt_like_empty("\"\""), + rdata_txt_like_quoted("\"Test-String\"") + { wiredata_longesttxt[0] = 255; // adjust length } - static const TXT_LIKE rdata_txt_like; - static const TXT_LIKE rdata_txt_like_empty; - static const TXT_LIKE rdata_txt_like_quoted; + vector wiredata_longesttxt; + const TXT_LIKE rdata_txt_like; + const TXT_LIKE rdata_txt_like_empty; + const TXT_LIKE rdata_txt_like_quoted; }; -template -const TXT_LIKE Rdata_TXT_LIKE_Test::rdata_txt_like("Test String"); - -template -const TXT_LIKE Rdata_TXT_LIKE_Test::rdata_txt_like_empty(""); - -template -const TXT_LIKE Rdata_TXT_LIKE_Test::rdata_txt_like_quoted - ("\"Test String\""); - // The list of types we want to test. typedef testing::Types Implementations; @@ -94,18 +90,19 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, this->obuffer.getData(), this->obuffer.getLength(), - &wiredata_longesttxt[0], wiredata_longesttxt.size()); + &this->wiredata_longesttxt[0], + this->wiredata_longesttxt.size()); // Too long text for a valid character-string. EXPECT_THROW(TypeParam(string(256, 'a')), CharStringTooLong); // The escape character makes the double quote a part of character-string, // so this is invalid input and should be rejected. - EXPECT_THROW(TypeParam("\"Test String\\\""), InvalidRdataText); + EXPECT_THROW(TypeParam("\"Test-String\\\""), InvalidRdataText); // Terminating double-quote is provided, so this is valid, but in this // version of implementation we reject escaped characters. - EXPECT_THROW(TypeParam("\"Test String\\\"\""), InvalidRdataText); + EXPECT_THROW(TypeParam("\"Test-String\\\"\""), InvalidRdataText); } void @@ -129,13 +126,15 @@ makeLargest(vector& data) { TYPED_TEST(Rdata_TXT_LIKE_Test, createFromWire) { EXPECT_EQ(0, this->rdata_txt_like.compare( - *this->rdataFactoryFromFile(RRTYPE(), RRClass("IN"), - "rdata_txt_fromWire1"))); + *this->rdataFactoryFromFile(RRTYPE(), + RRClass("IN"), + "rdata_txt_fromWire1"))); // Empty character string EXPECT_EQ(0, this->rdata_txt_like_empty.compare( - *this->rdataFactoryFromFile(RRTYPE(), RRClass("IN"), - "rdata_txt_fromWire2.wire"))); + *this->rdataFactoryFromFile(RRTYPE(), + RRClass("IN"), + "rdata_txt_fromWire2.wire"))); // Multiple character strings this->obuffer.clear(); @@ -188,7 +187,7 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromWire) { TYPED_TEST(Rdata_TXT_LIKE_Test, createFromLexer) { EXPECT_EQ(0, this->rdata_txt_like.compare( *test::createRdataUsingLexer(RRTYPE(), RRClass::IN(), - "Test String"))); + "Test-String"))); } TYPED_TEST(Rdata_TXT_LIKE_Test, toWireBuffer) { @@ -208,7 +207,7 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, toWireRenderer) { } TYPED_TEST(Rdata_TXT_LIKE_Test, toText) { - EXPECT_EQ("\"Test String\"", this->rdata_txt_like.toText()); + EXPECT_EQ("\"Test-String\"", this->rdata_txt_like.toText()); } TYPED_TEST(Rdata_TXT_LIKE_Test, assignment) { diff --git a/src/lib/dns/tests/testdata/rdata_txt_fromWire1 b/src/lib/dns/tests/testdata/rdata_txt_fromWire1 index 2c51efea24..95980dbfc9 100644 --- a/src/lib/dns/tests/testdata/rdata_txt_fromWire1 +++ b/src/lib/dns/tests/testdata/rdata_txt_fromWire1 @@ -1,9 +1,9 @@ # # various kinds of TXT RDATA stored in an input buffer # -# Valid RDATA for "Test String" +# Valid RDATA for "Test-String" # # RDLENGHT=12 bytes 00 0c -# T e s t S t r i n g - 0b 54 65 73 74 20 53 74 72 69 6e 67 +# T e s t - S t r i n g + 0b 54 65 73 74 2d 53 74 72 69 6e 67 diff --git a/src/lib/util/python/gen_wiredata.py.in b/src/lib/util/python/gen_wiredata.py.in index f997701540..55724c9f10 100755 --- a/src/lib/util/python/gen_wiredata.py.in +++ b/src/lib/util/python/gen_wiredata.py.in @@ -770,7 +770,7 @@ class TXT(RR): nstring = 1 stringlen = None - string = 'Test String' + string = 'Test-String' def dump(self, f): stringlen_list = [] From c0d55602e3304a95d7956b401ab1ce6c55d11dc1 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Dec 2012 20:59:06 -0800 Subject: [PATCH 42/69] [2442] supported lexer version of constructor for TXTLike RR types it's incomplete, but pass some tests --- src/lib/dns/gen-rdatacode.py.in | 3 +- src/lib/dns/rdata/generic/detail/txt_like.h | 22 +++++++++++++ src/lib/dns/rdata/generic/spf_99.cc | 7 ++++ src/lib/dns/rdata/generic/txt_16.cc | 5 +++ src/lib/dns/tests/rdata_txt_like_unittest.cc | 34 +++++++++++++++++++- 5 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/lib/dns/gen-rdatacode.py.in b/src/lib/dns/gen-rdatacode.py.in index d0decfdf28..2bf9de6f0f 100755 --- a/src/lib/dns/gen-rdatacode.py.in +++ b/src/lib/dns/gen-rdatacode.py.in @@ -32,7 +32,8 @@ import sys # # Example: # new_rdata_factory_users = [('a', 'in'), ('a', 'ch'), ('soa', 'generic')] -new_rdata_factory_users = [] +new_rdata_factory_users = [('aaaa', 'in'), ('txt', 'generic'), + ('spf', 'generic')] re_typecode = re.compile('([\da-z]+)_(\d+)') classcode2txt = {} diff --git a/src/lib/dns/rdata/generic/detail/txt_like.h b/src/lib/dns/rdata/generic/detail/txt_like.h index b5553fcb90..24a4e0fe9b 100644 --- a/src/lib/dns/rdata/generic/detail/txt_like.h +++ b/src/lib/dns/rdata/generic/detail/txt_like.h @@ -15,6 +15,11 @@ #ifndef TXT_LIKE_H #define TXT_LIKE_H 1 +#include +#include +#include +#include + #include #include @@ -113,6 +118,23 @@ public: string_list_.push_back(data); } + TXTLikeImpl(MasterLexer& lexer, const Name*, + MasterLoader::Options, + MasterLoaderCallbacks&) + { + while (true) { + const MasterToken& token = lexer.getNextToken( + MasterToken::QSTRING, true); + if (token.getType() != MasterToken::STRING && + token.getType() != MasterToken::QSTRING) { + break; + } + string_list_.push_back(std::vector()); + strToCharString(token.getStringRegion(), string_list_.back()); + } + lexer.ungetToken(); + } + /// \brief The copy constructor. /// /// Trivial for now, we could've used the default one. diff --git a/src/lib/dns/rdata/generic/spf_99.cc b/src/lib/dns/rdata/generic/spf_99.cc index 4d19a8b729..6be9ea5fde 100644 --- a/src/lib/dns/rdata/generic/spf_99.cc +++ b/src/lib/dns/rdata/generic/spf_99.cc @@ -66,6 +66,13 @@ SPF::SPF(InputBuffer& buffer, size_t rdata_len) : impl_(new SPFImpl(buffer, rdata_len)) {} + +/// \brief Constructor from TBD +SPF::SPF(MasterLexer& lexer, const Name* origin, + MasterLoader::Options options, MasterLoaderCallbacks& callbacks) : + impl_(new SPFImpl(lexer, origin, options, callbacks)) +{} + /// \brief Constructor from string. /// /// It internally allocates a resource, and if it fails a corresponding diff --git a/src/lib/dns/rdata/generic/txt_16.cc b/src/lib/dns/rdata/generic/txt_16.cc index 6573487362..6e054d2e70 100644 --- a/src/lib/dns/rdata/generic/txt_16.cc +++ b/src/lib/dns/rdata/generic/txt_16.cc @@ -52,6 +52,11 @@ TXT::TXT(InputBuffer& buffer, size_t rdata_len) : impl_(new TXTImpl(buffer, rdata_len)) {} +TXT::TXT(MasterLexer& lexer, const Name* origin, + MasterLoader::Options options, MasterLoaderCallbacks& callbacks) : + impl_(new TXTImpl(lexer, origin, options, callbacks)) +{} + TXT::TXT(const std::string& txtstr) : impl_(new TXTImpl(txtstr)) {} diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc index f59163f4d0..99bd2144f1 100644 --- a/src/lib/dns/tests/rdata_txt_like_unittest.cc +++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc @@ -23,6 +23,8 @@ #include +#include + using isc::UnitTestUtil; using namespace std; using namespace isc::dns; @@ -47,10 +49,17 @@ const uint8_t wiredata_txt_like[] = { const uint8_t wiredata_nulltxt[] = { 0 }; +// For lexer-based constructor +void +dummyCallback(const string&, size_t, const string&) { +} + template class Rdata_TXT_LIKE_Test : public RdataTest { protected: Rdata_TXT_LIKE_Test() : + callback(boost::bind(&dummyCallback, _1, _2, _3)), + loader_cb(callback, callback), wiredata_longesttxt(256, 'a'), rdata_txt_like("Test-String"), rdata_txt_like_empty("\"\""), @@ -59,10 +68,16 @@ protected: wiredata_longesttxt[0] = 255; // adjust length } +private: + const MasterLoaderCallbacks::IssueCallback callback; + +protected: + MasterLoaderCallbacks loader_cb; vector wiredata_longesttxt; const TXT_LIKE rdata_txt_like; const TXT_LIKE rdata_txt_like_empty; const TXT_LIKE rdata_txt_like_quoted; + ConstRdataPtr rdata_txt_like_fromwire; }; // The list of types we want to test. @@ -71,10 +86,27 @@ typedef testing::Types Implementations; TYPED_TEST_CASE(Rdata_TXT_LIKE_Test, Implementations); TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { + std::stringstream ss; + ss << "Test-String\n"; + ss << "\"Test-String\"\n"; // explicitly surrounded by '"'s + this->lexer.pushSource(ss); + // normal case is covered in toWireBuffer. + this->rdata_txt_like_fromwire = + this->rdataFactoryFromFile(RRTYPE(), + RRClass("IN"), "rdata_txt_fromWire1"); + EXPECT_EQ(0, this->rdata_txt_like.compare(*this->rdata_txt_like_fromwire)); + EXPECT_EQ(0, TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, + this->loader_cb).compare( + *this->rdata_txt_like_fromwire)); + EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); // surrounding double-quotes shouldn't change the result. - EXPECT_EQ(0, this->rdata_txt_like.compare(this->rdata_txt_like_quoted)); + EXPECT_EQ(0, this->rdata_txt_like_quoted.compare( + *this->rdata_txt_like_fromwire)); + EXPECT_EQ(0, TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, + this->loader_cb).compare( + *this->rdata_txt_like_fromwire)); // Null character-string. this->obuffer.clear(); From 7b148d962b1d211eab660ac79e6dcb37e1aa8831 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Dec 2012 21:51:39 -0800 Subject: [PATCH 43/69] [2442] tested various cases for the lexer verson of txt-like ctor --- src/lib/dns/tests/rdata_txt_like_unittest.cc | 41 +++++++++++++++++--- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc index 99bd2144f1..7ce0aec308 100644 --- a/src/lib/dns/tests/rdata_txt_like_unittest.cc +++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc @@ -89,12 +89,19 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { std::stringstream ss; ss << "Test-String\n"; ss << "\"Test-String\"\n"; // explicitly surrounded by '"'s + ss << "\"\"\n"; // empty string + ss << string(255, 'a') << "\n"; // Longest possible character-string. + ss << string(256, 'a') << "\n"; // char-string too long + ss << "\"Test-String\\\"\n"; // unbalanced quote + ss << "\"Test-String\\\"\"\n"; this->lexer.pushSource(ss); - // normal case is covered in toWireBuffer. + // Rdata to compare, created from wire this->rdata_txt_like_fromwire = this->rdataFactoryFromFile(RRTYPE(), RRClass("IN"), "rdata_txt_fromWire1"); + + // normal case is covered in toWireBuffer. EXPECT_EQ(0, this->rdata_txt_like.compare(*this->rdata_txt_like_fromwire)); EXPECT_EQ(0, TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, this->loader_cb).compare( @@ -107,34 +114,56 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { EXPECT_EQ(0, TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, this->loader_cb).compare( *this->rdata_txt_like_fromwire)); + EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); // Null character-string. this->obuffer.clear(); - TypeParam(string("")).toWire(this->obuffer); + TypeParam(string("\"\"")).toWire(this->obuffer); EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, - this->obuffer.getData(), - this->obuffer.getLength(), + this->obuffer.getData(), this->obuffer.getLength(), wiredata_nulltxt, sizeof(wiredata_nulltxt)); + this->obuffer.clear(); + TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, this->loader_cb). + toWire(this->obuffer); + EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, + this->obuffer.getData(), this->obuffer.getLength(), + wiredata_nulltxt, sizeof(wiredata_nulltxt)); + EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); // Longest possible character-string. this->obuffer.clear(); TypeParam(string(255, 'a')).toWire(this->obuffer); EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, - this->obuffer.getData(), - this->obuffer.getLength(), + this->obuffer.getData(), this->obuffer.getLength(), &this->wiredata_longesttxt[0], this->wiredata_longesttxt.size()); + this->obuffer.clear(); + TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, this->loader_cb). + toWire(this->obuffer); + EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, + this->obuffer.getData(), this->obuffer.getLength(), + &this->wiredata_longesttxt[0], + this->wiredata_longesttxt.size()); + EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); // Too long text for a valid character-string. EXPECT_THROW(TypeParam(string(256, 'a')), CharStringTooLong); + EXPECT_THROW(TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, + this->loader_cb), CharStringTooLong); + EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); // The escape character makes the double quote a part of character-string, // so this is invalid input and should be rejected. EXPECT_THROW(TypeParam("\"Test-String\\\""), InvalidRdataText); + EXPECT_THROW(TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, + this->loader_cb), MasterLexer::LexerError); + EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); // Terminating double-quote is provided, so this is valid, but in this // version of implementation we reject escaped characters. EXPECT_THROW(TypeParam("\"Test-String\\\"\""), InvalidRdataText); + TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, + this->loader_cb); } void From e4036de3cb6229d38270225eff7a00f5c07a4573 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Dec 2012 23:21:06 -0800 Subject: [PATCH 44/69] [2442] unified from std::string and with lexer ctro for text-like rdata. some tests needed to be adjusted: - ' ' * 256 (in python) now doesn't work as intended; they are considered space - in auth tests, make sure txt RDATA containing a space is explicitly double quoted - in Rdata_TXT_LIKE_Test::createFromText, removed the last test case because it's actually a valid representation as commented. now the implementation correctly accepts it. --- src/bin/auth/tests/auth_srv_unittest.cc | 2 +- src/lib/dns/python/tests/rdata_python_test.py | 2 +- src/lib/dns/rdata/generic/detail/txt_like.h | 45 +++++-------------- src/lib/dns/tests/rdata_txt_like_unittest.cc | 8 +--- 4 files changed, 15 insertions(+), 42 deletions(-) diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc index 8015043bf1..7f89fda23a 100644 --- a/src/bin/auth/tests/auth_srv_unittest.cc +++ b/src/bin/auth/tests/auth_srv_unittest.cc @@ -225,7 +225,7 @@ createBuiltinVersionResponse(const qid_t qid, vector& data) { message.setHeaderFlag(Message::HEADERFLAG_AA); RRsetPtr rrset_version = RRsetPtr(new RRset(version_name, RRClass::CH(), RRType::TXT(), RRTTL(0))); - rrset_version->addRdata(generic::TXT(PACKAGE_STRING)); + rrset_version->addRdata(generic::TXT("\"" PACKAGE_STRING "\"")); message.addRRset(Message::SECTION_ANSWER, rrset_version); RRsetPtr rrset_version_ns = RRsetPtr(new RRset(apex_name, RRClass::CH(), diff --git a/src/lib/dns/python/tests/rdata_python_test.py b/src/lib/dns/python/tests/rdata_python_test.py index 81dea5f6dc..3b8f9ad312 100644 --- a/src/lib/dns/python/tests/rdata_python_test.py +++ b/src/lib/dns/python/tests/rdata_python_test.py @@ -38,7 +38,7 @@ class RdataTest(unittest.TestCase): self.assertRaises(InvalidRdataText, Rdata, RRType("A"), RRClass("IN"), "Invalid Rdata Text") self.assertRaises(CharStringTooLong, Rdata, RRType("TXT"), - RRClass("IN"), ' ' * 256) + RRClass("IN"), 'x' * 256) self.assertRaises(InvalidRdataLength, Rdata, RRType("TXT"), RRClass("IN"), bytes(65536)) self.assertRaises(DNSMessageFORMERR, Rdata, RRType("TXT"), diff --git a/src/lib/dns/rdata/generic/detail/txt_like.h b/src/lib/dns/rdata/generic/detail/txt_like.h index 24a4e0fe9b..fecf1b0860 100644 --- a/src/lib/dns/rdata/generic/detail/txt_like.h +++ b/src/lib/dns/rdata/generic/detail/txt_like.h @@ -23,6 +23,7 @@ #include #include +#include #include namespace isc { @@ -85,43 +86,20 @@ public: /// \c InvalidRdataText is thrown if the method cannot process the /// parameter data. explicit TXTLikeImpl(const std::string& txtstr) { - // TBD: this is a simple, incomplete implementation that only supports - // a single character-string. - - size_t length = txtstr.size(); - size_t pos_begin = 0; - - if (length > 1 && txtstr[0] == '"' && txtstr[length - 1] == '"') { - pos_begin = 1; - length -= 2; - } - - if (length > MAX_CHARSTRING_LEN) { - isc_throw(CharStringTooLong, RRType(typeCode) << - " RDATA construction from text:" - " string length is too long: " << length); - } - - // TBD: right now, we don't support escaped characters - if (txtstr.find('\\') != string::npos) { - isc_throw(InvalidRdataText, RRType(typeCode) << - " RDATA from text:" - " escaped character is currently not supported: " << - txtstr); - } - - std::vector data; - data.reserve(length + 1); - data.push_back(length); - data.insert(data.end(), txtstr.begin() + pos_begin, - txtstr.begin() + pos_begin + length); - string_list_.push_back(data); + std::istringstream ss(txtstr); + MasterLexer lexer; + lexer.pushSource(ss); + buildFromTextHelper(lexer); } - TXTLikeImpl(MasterLexer& lexer, const Name*, - MasterLoader::Options, + TXTLikeImpl(MasterLexer& lexer, const Name*, MasterLoader::Options, MasterLoaderCallbacks&) { + buildFromTextHelper(lexer); + } + +private: + void buildFromTextHelper(MasterLexer& lexer) { while (true) { const MasterToken& token = lexer.getNextToken( MasterToken::QSTRING, true); @@ -135,6 +113,7 @@ public: lexer.ungetToken(); } +public: /// \brief The copy constructor. /// /// Trivial for now, we could've used the default one. diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc index 7ce0aec308..6c980f84a3 100644 --- a/src/lib/dns/tests/rdata_txt_like_unittest.cc +++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc @@ -154,16 +154,10 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { // The escape character makes the double quote a part of character-string, // so this is invalid input and should be rejected. - EXPECT_THROW(TypeParam("\"Test-String\\\""), InvalidRdataText); + EXPECT_THROW(TypeParam("\"Test-String\\\""), MasterLexer::LexerError); EXPECT_THROW(TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, this->loader_cb), MasterLexer::LexerError); EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); - - // Terminating double-quote is provided, so this is valid, but in this - // version of implementation we reject escaped characters. - EXPECT_THROW(TypeParam("\"Test-String\\\"\""), InvalidRdataText); - TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, - this->loader_cb); } void From 970ee33b88d3268780fc4a7d8950ba2a4a8202fd Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 09:10:56 -0800 Subject: [PATCH 45/69] [2382] corrected typo in doc for createRdata() --- src/lib/dns/rdata.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dns/rdata.h b/src/lib/dns/rdata.h index 186ace2cc3..4cd63cc4f9 100644 --- a/src/lib/dns/rdata.h +++ b/src/lib/dns/rdata.h @@ -502,7 +502,7 @@ RdataPtr createRdata(const RRType& rrtype, const RRClass& rrclass, /// most of syntax and semantics errors of the input (reported as exceptions), /// calls the corresponding callback specified by the \c callbacks parameters, /// and returns a NULL smart pointer. If the caller rather wants to get -/// an exception in these cases, it can use pass a callback that internally +/// an exception in these cases, it can pass a callback that internally /// throws on error. Some critical exceptions such as \c std::bad_alloc are /// still propagated to the upper layer as it doesn't make sense to try /// recovery from such a situation within this function. From 6944e7758d326b69482bcfd7d3f501df5113b394 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 09:18:03 -0800 Subject: [PATCH 46/69] [2382] unify pushing '\0' for number-like data in Number::handle() --- src/lib/dns/master_lexer.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc index 4593b48b5f..b3b78c0e68 100644 --- a/src/lib/dns/master_lexer.cc +++ b/src/lib/dns/master_lexer.cc @@ -522,9 +522,10 @@ Number::handle(MasterLexer& lexer) const { getLexerImpl(lexer)->source_->getChar(), escaped); if (getLexerImpl(lexer)->isTokenEnd(c, escaped)) { getLexerImpl(lexer)->source_->ungetChar(); + // We need to close the string whether it's digits-only (for + // lexical_cast) or not (see String::handle()). + data.push_back('\0'); if (digits_only) { - // Close the string for lexical_cast - data.push_back('\0'); try { const uint32_t number32 = boost::lexical_cast(&data[0]); @@ -535,7 +536,6 @@ Number::handle(MasterLexer& lexer) const { token = MasterToken(MasterToken::NUMBER_OUT_OF_RANGE); } } else { - data.push_back('\0'); // see String::handle() token = MasterToken(&data.at(0), data.size() - 1); } return; From 09d70952ca89ef967f8652046ed6fe869625b42e Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 09:22:44 -0800 Subject: [PATCH 47/69] [2382] added one suggested test: a case where RDATA is followed by comment --- src/lib/dns/tests/rdata_unittest.cc | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/lib/dns/tests/rdata_unittest.cc b/src/lib/dns/tests/rdata_unittest.cc index bb39d05fe0..7f0dd6580d 100644 --- a/src/lib/dns/tests/rdata_unittest.cc +++ b/src/lib/dns/tests/rdata_unittest.cc @@ -132,6 +132,7 @@ TEST_F(RdataTest, createRdataWithLexer) { stringstream ss; const string src_name = "stream-" + boost::lexical_cast(&ss); ss << aaaa_rdata.toText() << "\n"; // valid case + ss << aaaa_rdata.toText() << "; comment, should be ignored\n"; 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 @@ -146,54 +147,71 @@ TEST_F(RdataTest, createRdataWithLexer) { boost::bind(&CreateRdataCallback::callback, &callback, CreateRdataCallback::WARN, _1, _2, _3)); + size_t line = 0; + // Valid case. + ++line; ConstRdataPtr rdata = createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL, MasterLoader::MANY_ERRORS, callbacks); EXPECT_EQ(0, aaaa_rdata.compare(*rdata)); EXPECT_FALSE(callback.isCalled()); + // Similar to the previous case, but RDATA is followed by a comment. + // It should cause any confusion. + ++line; + callback.clear(); + rdata = createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL, + MasterLoader::MANY_ERRORS, callbacks); + EXPECT_EQ(0, aaaa_rdata.compare(*rdata)); + EXPECT_FALSE(callback.isCalled()); + // Broken RDATA text: extra token. createRdata() returns NULL, error // callback is called. + ++line; callback.clear(); EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL, MasterLoader::MANY_ERRORS, callbacks)); - callback.check(src_name, 2, CreateRdataCallback::ERROR, + callback.check(src_name, line, CreateRdataCallback::ERROR, "createRdata from text failed near 'extra-token': " "extra input text"); // Similar to the previous case, but only the first extra token triggers // callback. + ++line; callback.clear(); EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL, MasterLoader::MANY_ERRORS, callbacks)); - callback.check(src_name, 3, CreateRdataCallback::ERROR, + callback.check(src_name, line, CreateRdataCallback::ERROR, "createRdata from text failed near 'extra': " "extra input text"); // Lexer error will happen, corresponding error callback will be triggered. + ++line; callback.clear(); EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL, MasterLoader::MANY_ERRORS, callbacks)); - callback.check(src_name, 4, CreateRdataCallback::ERROR, + callback.check(src_name, line, CreateRdataCallback::ERROR, "createRdata from text failed: unbalanced parentheses"); // Semantics level error will happen, corresponding error callback will be // triggered. + ++line; callback.clear(); EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL, MasterLoader::MANY_ERRORS, callbacks)); - callback.check(src_name, 5, CreateRdataCallback::ERROR, + callback.check(src_name, line, CreateRdataCallback::ERROR, "createRdata from text failed: Failed to convert " "'192.0.2.1' to IN/AAAA RDATA"); // Input is valid and parse will succeed, but with a warning that the // file is not ended with a newline. + ++line; callback.clear(); rdata = createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL, MasterLoader::MANY_ERRORS, callbacks); EXPECT_EQ(0, aaaa_rdata.compare(*rdata)); - callback.check(src_name, 6, CreateRdataCallback::WARN, + callback.check(src_name, line, CreateRdataCallback::WARN, "file does not end with newline"); } From 1d4751ed52516da9036c61a16946ec74ea1f1ba5 Mon Sep 17 00:00:00 2001 From: Stephen Morris Date: Tue, 4 Dec 2012 17:27:27 +0000 Subject: [PATCH 48/69] [2513] Move libdhcpsrv.dox to the dhcpsrv directory --- src/lib/dhcp/Makefile.am | 2 +- src/lib/dhcpsrv/Makefile.am | 2 +- src/lib/{dhcp/libdhcsrv.dox => dhcpsrv/libdhcpsrv.dox} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/lib/{dhcp/libdhcsrv.dox => dhcpsrv/libdhcpsrv.dox} (100%) diff --git a/src/lib/dhcp/Makefile.am b/src/lib/dhcp/Makefile.am index e9333e2f76..85048b01cc 100644 --- a/src/lib/dhcp/Makefile.am +++ b/src/lib/dhcp/Makefile.am @@ -40,7 +40,7 @@ libb10_dhcp___la_LIBADD = $(top_builddir)/src/lib/asiolink/libb10-asiolink.la libb10_dhcp___la_LIBADD += $(top_builddir)/src/lib/util/libb10-util.la libb10_dhcp___la_LDFLAGS = -no-undefined -version-info 2:0:0 -EXTRA_DIST = README +EXTRA_DIST = README libdhcp++.dox if USE_CLANGPP # Disable unused parameter warning caused by some of the diff --git a/src/lib/dhcpsrv/Makefile.am b/src/lib/dhcpsrv/Makefile.am index e205aee932..6c08acdd15 100644 --- a/src/lib/dhcpsrv/Makefile.am +++ b/src/lib/dhcpsrv/Makefile.am @@ -48,5 +48,5 @@ libb10_dhcpsrv_la_CXXFLAGS += -Wno-unused-parameter endif # Distribute MySQL schema creation script and backend documentation -EXTRA_DIST = dhcpdb_create.mysql database_backends.dox +EXTRA_DIST = dhcpdb_create.mysql database_backends.dox libdhcpsrv.dox dist_pkgdata_DATA = dhcpdb_create.mysql diff --git a/src/lib/dhcp/libdhcsrv.dox b/src/lib/dhcpsrv/libdhcpsrv.dox similarity index 100% rename from src/lib/dhcp/libdhcsrv.dox rename to src/lib/dhcpsrv/libdhcpsrv.dox From 816eacf47e4168d336c2f64b1b621f13f8ca6cb4 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 09:27:49 -0800 Subject: [PATCH 49/69] [2382] throw unexpected instead of assert on unexpected token in createRdata. assert() should be okay, but that depends on details of other classes, so exception is probably a bit safer. --- src/lib/dns/rdata.cc | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/lib/dns/rdata.cc b/src/lib/dns/rdata.cc index 9e96378bc0..081f85506e 100644 --- a/src/lib/dns/rdata.cc +++ b/src/lib/dns/rdata.cc @@ -12,6 +12,20 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +#include + +#include + +#include +#include +#include +#include +#include +#include + +#include +#include + #include #include #include @@ -24,17 +38,6 @@ #include #include -#include -#include - -#include -#include -#include -#include -#include -#include -#include - using namespace std; using boost::lexical_cast; using namespace isc::util; @@ -113,7 +116,11 @@ fromtextError(bool& error_issued, const MasterLexer& lexer, token->getErrorText()); break; default: - assert(false); + // This case shouldn't happen based on how we use MasterLexer in + // createRdata(), so we could assert() that here. But since it + // depends on detailed behavior of other classes, we treat the case + // in a bit less harsh way. + isc_throw(Unexpected, "bug: createRdata() saw unexpected token type"); } } } From e66472386a716a31089a2306b9e5d51f7618feeb Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 10:18:48 -0800 Subject: [PATCH 50/69] [master] resolve additional conflicts due to change of token class name --- src/lib/dns/rdata.cc | 6 +++--- src/lib/dns/rrparamregistry-placeholder.cc | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib/dns/rdata.cc b/src/lib/dns/rdata.cc index f8deec6d22..46224c65b8 100644 --- a/src/lib/dns/rdata.cc +++ b/src/lib/dns/rdata.cc @@ -211,9 +211,9 @@ Generic::Generic(MasterLexer& lexer, const Name*, std::string s; while (true) { - const MasterLexer::Token& token = lexer.getNextToken(); - if ((token.getType() == MasterLexer::Token::END_OF_FILE) || - (token.getType() == MasterLexer::Token::END_OF_LINE)) { + const MasterToken& token = lexer.getNextToken(); + if ((token.getType() == MasterToken::END_OF_FILE) || + (token.getType() == MasterToken::END_OF_LINE)) { break; } diff --git a/src/lib/dns/rrparamregistry-placeholder.cc b/src/lib/dns/rrparamregistry-placeholder.cc index ed59f5de03..8160851a2e 100644 --- a/src/lib/dns/rrparamregistry-placeholder.cc +++ b/src/lib/dns/rrparamregistry-placeholder.cc @@ -51,9 +51,9 @@ AbstractRdataFactory::create(MasterLexer& lexer, const Name*, std::string s; while (true) { - const MasterLexer::Token& token = lexer.getNextToken(); - if ((token.getType() == MasterLexer::Token::END_OF_FILE) || - (token.getType() == MasterLexer::Token::END_OF_LINE)) { + const MasterToken& token = lexer.getNextToken(); + if ((token.getType() == MasterToken::END_OF_FILE) || + (token.getType() == MasterToken::END_OF_LINE)) { break; } From dbf7313f6bb3fd3ace97b50509a4208864c4c4ea Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 4 Dec 2012 20:29:20 +0100 Subject: [PATCH 51/69] [2479] Moved perfdhcp/template folder to perfdhcp/tests/testdata. --- configure.ac | 2 +- tests/tools/perfdhcp/Makefile.am | 2 +- tests/tools/perfdhcp/tests/Makefile.am | 2 +- tests/tools/perfdhcp/{templates => tests/testdata}/.gitignore | 0 tests/tools/perfdhcp/{templates => tests/testdata}/Makefile.am | 0 .../perfdhcp/{templates => tests/testdata}/discover-example.hex | 0 .../perfdhcp/{templates => tests/testdata}/request4-example.hex | 0 .../perfdhcp/{templates => tests/testdata}/request6-example.hex | 0 .../perfdhcp/{templates => tests/testdata}/solicit-example.hex | 0 9 files changed, 3 insertions(+), 3 deletions(-) rename tests/tools/perfdhcp/{templates => tests/testdata}/.gitignore (100%) rename tests/tools/perfdhcp/{templates => tests/testdata}/Makefile.am (100%) rename tests/tools/perfdhcp/{templates => tests/testdata}/discover-example.hex (100%) rename tests/tools/perfdhcp/{templates => tests/testdata}/request4-example.hex (100%) rename tests/tools/perfdhcp/{templates => tests/testdata}/request6-example.hex (100%) rename tests/tools/perfdhcp/{templates => tests/testdata}/solicit-example.hex (100%) diff --git a/configure.ac b/configure.ac index 636f8aa959..feeb9a9248 100644 --- a/configure.ac +++ b/configure.ac @@ -1308,7 +1308,7 @@ AC_CONFIG_FILES([Makefile tests/tools/badpacket/tests/Makefile tests/tools/perfdhcp/Makefile tests/tools/perfdhcp/tests/Makefile - tests/tools/perfdhcp/templates/Makefile + tests/tools/perfdhcp/tests/testdata/Makefile dns++.pc ]) AC_OUTPUT([doc/version.ent diff --git a/tests/tools/perfdhcp/Makefile.am b/tests/tools/perfdhcp/Makefile.am index 08a21a4c2d..c4b82b5d48 100644 --- a/tests/tools/perfdhcp/Makefile.am +++ b/tests/tools/perfdhcp/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = . tests templates +SUBDIRS = . tests AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib AM_CPPFLAGS += -I$(top_srcdir)/src/lib/log -I$(top_builddir)/src/lib/log diff --git a/tests/tools/perfdhcp/tests/Makefile.am b/tests/tools/perfdhcp/tests/Makefile.am index 54602aff4b..4879d57ab7 100644 --- a/tests/tools/perfdhcp/tests/Makefile.am +++ b/tests/tools/perfdhcp/tests/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = . +SUBDIRS = . testdata AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib AM_CPPFLAGS += $(BOOST_INCLUDES) diff --git a/tests/tools/perfdhcp/templates/.gitignore b/tests/tools/perfdhcp/tests/testdata/.gitignore similarity index 100% rename from tests/tools/perfdhcp/templates/.gitignore rename to tests/tools/perfdhcp/tests/testdata/.gitignore diff --git a/tests/tools/perfdhcp/templates/Makefile.am b/tests/tools/perfdhcp/tests/testdata/Makefile.am similarity index 100% rename from tests/tools/perfdhcp/templates/Makefile.am rename to tests/tools/perfdhcp/tests/testdata/Makefile.am diff --git a/tests/tools/perfdhcp/templates/discover-example.hex b/tests/tools/perfdhcp/tests/testdata/discover-example.hex similarity index 100% rename from tests/tools/perfdhcp/templates/discover-example.hex rename to tests/tools/perfdhcp/tests/testdata/discover-example.hex diff --git a/tests/tools/perfdhcp/templates/request4-example.hex b/tests/tools/perfdhcp/tests/testdata/request4-example.hex similarity index 100% rename from tests/tools/perfdhcp/templates/request4-example.hex rename to tests/tools/perfdhcp/tests/testdata/request4-example.hex diff --git a/tests/tools/perfdhcp/templates/request6-example.hex b/tests/tools/perfdhcp/tests/testdata/request6-example.hex similarity index 100% rename from tests/tools/perfdhcp/templates/request6-example.hex rename to tests/tools/perfdhcp/tests/testdata/request6-example.hex diff --git a/tests/tools/perfdhcp/templates/solicit-example.hex b/tests/tools/perfdhcp/tests/testdata/solicit-example.hex similarity index 100% rename from tests/tools/perfdhcp/templates/solicit-example.hex rename to tests/tools/perfdhcp/tests/testdata/solicit-example.hex From 501a5e337373d10c40c05d0838f5711fbb961ee2 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 4 Dec 2012 20:56:17 +0100 Subject: [PATCH 52/69] [2479] Use an absolute path to testdata directory. --- tests/tools/perfdhcp/tests/Makefile.am | 1 + .../perfdhcp/tests/test_control_unittest.cc | 30 +++++++++++++------ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/tests/tools/perfdhcp/tests/Makefile.am b/tests/tools/perfdhcp/tests/Makefile.am index 4879d57ab7..be67481fe1 100644 --- a/tests/tools/perfdhcp/tests/Makefile.am +++ b/tests/tools/perfdhcp/tests/Makefile.am @@ -1,6 +1,7 @@ SUBDIRS = . testdata AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib +AM_CPPFLAGS += -DTEST_DATA_DIR=\"$(abs_srcdir)/testdata\" AM_CPPFLAGS += $(BOOST_INCLUDES) AM_CXXFLAGS = $(B10_CXXFLAGS) diff --git a/tests/tools/perfdhcp/tests/test_control_unittest.cc b/tests/tools/perfdhcp/tests/test_control_unittest.cc index abe8282140..f52562f436 100644 --- a/tests/tools/perfdhcp/tests/test_control_unittest.cc +++ b/tests/tools/perfdhcp/tests/test_control_unittest.cc @@ -185,6 +185,18 @@ public: return (""); } + /// \brief Get full path to a file in testdata directory. + /// + /// \param filename filename being appended to absolute + /// path to testdata directory + /// + /// \return full path to a file in testdata directory. + std::string getFullPath(const std::string& filename) const { + std::ostringstream stream; + stream << TEST_DATA_DIR << "/" << filename; + return (stream.str()); + } + /// \brief Match requested options in the buffer with given list. /// /// This method iterates through options provided in the buffer @@ -925,8 +937,8 @@ TEST_F(TestControlTest, DISABLED_Packet4Exchange) { // Use templates for this test. processCmdLine("perfdhcp -l " + loopback_iface + " -r 100 -R 20 -n 20 -D 10% -L 10547" - + " -T ../templates/discover-example.hex" - + " -T ../templates/request4-example.hex" + + " -T " + getFullPath("discover-example.hex") + + " -T " + getFullPath("request4-example.hex") + " 127.0.0.1"); // The number iterations is restricted by the percentage of // dropped packets (-D 10%). We also have to bump up the number @@ -967,8 +979,8 @@ TEST_F(TestControlTest, DISABLED_Packet6Exchange) { use_templates = true; processCmdLine("perfdhcp -l " + loopback_iface + " -6 -r 100 -n 10 -R 20 -D 3 -L 10547" - + " -T ../templates/solicit-example.hex" - + " -T ../templates/request6-example.hex ::1"); + + " -T " + getFullPath("solicit-example.hex") + + " -T " + getFullPath("request6-example.hex ::1")); // For the first 3 packets we are simulating responses from server. // For other packets we don't so packet as 4,5,6 will be dropped and // then test should be interrupted and actual number of iterations will @@ -981,9 +993,9 @@ TEST_F(TestControlTest, DISABLED_Packet6Exchange) { TEST_F(TestControlTest, PacketTemplates) { std::vector template1(256); - std::string file1("../templates/test1.hex"); + std::string file1(getFullPath("test1.hex")); std::vector template2(233); - std::string file2("../templates/test2.hex"); + std::string file2(getFullPath("test2.hex")); for (int i = 0; i < template1.size(); ++i) { template1[i] = static_cast(random() % 256); } @@ -1011,7 +1023,7 @@ TEST_F(TestControlTest, PacketTemplates) { EXPECT_TRUE(std::equal(template2.begin(), template2.end(), buf2.begin())); // Try to read template file with odd number of digits. - std::string file3("../templates/test3.hex"); + std::string file3(getFullPath("test3.hex")); // Size of the file is 2 times larger than binary data size and it is always // even number. Substracting 1 makes file size odd. ASSERT_TRUE(createTemplateFile(file3, template1, template1.size() * 2 - 1)); @@ -1021,7 +1033,7 @@ TEST_F(TestControlTest, PacketTemplates) { EXPECT_THROW(tc.initPacketTemplates(), isc::OutOfRange); // Try to read empty file. - std::string file4("../templates/test4.hex"); + std::string file4(getFullPath("test4.hex")); ASSERT_TRUE(createTemplateFile(file4, template2, 0)); ASSERT_NO_THROW( processCmdLine("perfdhcp -l 127.0.0.1 -T " + file4 + " all") @@ -1029,7 +1041,7 @@ TEST_F(TestControlTest, PacketTemplates) { EXPECT_THROW(tc.initPacketTemplates(), isc::OutOfRange); // Try reading file with non hexadecimal characters. - std::string file5("../templates/test5.hex"); + std::string file5(getFullPath("test5.hex")); ASSERT_TRUE(createTemplateFile(file5, template1, template1.size() * 2, true)); ASSERT_NO_THROW( processCmdLine("perfdhcp -l 127.0.0.1 -T " + file5 + " all") From e0c2fad3471896467ad0ae3576f93b39bcb76a7d Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 4 Dec 2012 21:03:56 +0100 Subject: [PATCH 53/69] [2479] Renabled tests that were affected by invalid paths to testdata. --- tests/tools/perfdhcp/tests/test_control_unittest.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tools/perfdhcp/tests/test_control_unittest.cc b/tests/tools/perfdhcp/tests/test_control_unittest.cc index f52562f436..176644ca1c 100644 --- a/tests/tools/perfdhcp/tests/test_control_unittest.cc +++ b/tests/tools/perfdhcp/tests/test_control_unittest.cc @@ -908,7 +908,7 @@ TEST_F(TestControlTest, Packet6) { } } -TEST_F(TestControlTest, DISABLED_Packet4Exchange) { +TEST_F(TestControlTest, Packet4Exchange) { // Get the local loopback interface to open socket on // it and test packets exchanges. We don't want to fail // the test if interface is not available. @@ -951,7 +951,7 @@ TEST_F(TestControlTest, DISABLED_Packet4Exchange) { EXPECT_EQ(12, iterations_performed); } -TEST_F(TestControlTest, DISABLED_Packet6Exchange) { +TEST_F(TestControlTest, Packet6Exchange) { // Get the local loopback interface to open socket on // it and test packets exchanges. We don't want to fail // the test if interface is not available. From 238b343266ec6c5e7c610781021d235b7fcfb22d Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 4 Dec 2012 21:33:57 +0100 Subject: [PATCH 54/69] [2479] Remove unnecessary assignment. --- tests/tools/perfdhcp/tests/testdata/Makefile.am | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/tools/perfdhcp/tests/testdata/Makefile.am b/tests/tools/perfdhcp/tests/testdata/Makefile.am index c22787f3e5..bbd9a735fe 100644 --- a/tests/tools/perfdhcp/tests/testdata/Makefile.am +++ b/tests/tools/perfdhcp/tests/testdata/Makefile.am @@ -4,7 +4,5 @@ SUBDIRS = . # unit tests and have to be removed. CLEANFILES = test1.hex test2.hex test3.hex test4.hex test5.hex -perfdhcpdir = $(pkgdatadir) - EXTRA_DIST = discover-example.hex request4-example.hex EXTRA_DIST += solicit-example.hex request6-example.hex From 1780e7b66ee62456880ac5ed0d93c9830f233163 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 13:54:16 -0800 Subject: [PATCH 55/69] [2442] make sure from-string version still throws InvalidRdataText on error. --- src/lib/dns/rdata/generic/detail/txt_like.h | 17 ++++++++++------- src/lib/dns/tests/rdata_txt_like_unittest.cc | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/lib/dns/rdata/generic/detail/txt_like.h b/src/lib/dns/rdata/generic/detail/txt_like.h index fecf1b0860..b788dd80d3 100644 --- a/src/lib/dns/rdata/generic/detail/txt_like.h +++ b/src/lib/dns/rdata/generic/detail/txt_like.h @@ -79,17 +79,20 @@ public: /// \brief Constructor from string. /// - /// Exceptions - /// - /// \c CharStringTooLong is thrown if the parameter string length exceeds - /// maximum. - /// \c InvalidRdataText is thrown if the method cannot process the - /// parameter data. + /// \throw CharStringTooLong the parameter string length exceeds maximum. + /// \throw InvalidRdataText the method cannot process the parameter data explicit TXTLikeImpl(const std::string& txtstr) { std::istringstream ss(txtstr); MasterLexer lexer; lexer.pushSource(ss); - buildFromTextHelper(lexer); + + try { + buildFromTextHelper(lexer); + } catch (const MasterLexer::LexerError& ex) { + isc_throw(InvalidRdataText, "Failed to construct " << + RRType(typeCode) << " RDATA from " << txtstr << ": " + << ex.what()); + } } TXTLikeImpl(MasterLexer& lexer, const Name*, MasterLoader::Options, diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc index 6c980f84a3..b3c263013a 100644 --- a/src/lib/dns/tests/rdata_txt_like_unittest.cc +++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc @@ -154,7 +154,7 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { // The escape character makes the double quote a part of character-string, // so this is invalid input and should be rejected. - EXPECT_THROW(TypeParam("\"Test-String\\\""), MasterLexer::LexerError); + EXPECT_THROW(TypeParam("\"Test-String\\\""), InvalidRdataText); EXPECT_THROW(TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, this->loader_cb), MasterLexer::LexerError); EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); From 9bdc1899a220d369cb7ca1b86fd9a2ddde44eeb9 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 14:51:45 -0800 Subject: [PATCH 56/69] [2442] added some test cases for txt-like rdata with multi-char-string --- src/lib/dns/tests/rdata_txt_like_unittest.cc | 57 ++++++++++++++++---- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc index b3c263013a..e4bb173861 100644 --- a/src/lib/dns/tests/rdata_txt_like_unittest.cc +++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc @@ -25,6 +25,10 @@ #include +#include +#include +#include + using isc::UnitTestUtil; using namespace std; using namespace isc::dns; @@ -77,7 +81,6 @@ protected: const TXT_LIKE rdata_txt_like; const TXT_LIKE rdata_txt_like_empty; const TXT_LIKE rdata_txt_like_quoted; - ConstRdataPtr rdata_txt_like_fromwire; }; // The list of types we want to test. @@ -96,24 +99,21 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { ss << "\"Test-String\\\"\"\n"; this->lexer.pushSource(ss); - // Rdata to compare, created from wire - this->rdata_txt_like_fromwire = + // commonly used Rdata to compare below, created from wire + ConstRdataPtr const rdata = this->rdataFactoryFromFile(RRTYPE(), RRClass("IN"), "rdata_txt_fromWire1"); // normal case is covered in toWireBuffer. - EXPECT_EQ(0, this->rdata_txt_like.compare(*this->rdata_txt_like_fromwire)); + EXPECT_EQ(0, this->rdata_txt_like.compare(*rdata)); EXPECT_EQ(0, TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, - this->loader_cb).compare( - *this->rdata_txt_like_fromwire)); + this->loader_cb).compare(*rdata)); EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); // surrounding double-quotes shouldn't change the result. - EXPECT_EQ(0, this->rdata_txt_like_quoted.compare( - *this->rdata_txt_like_fromwire)); + EXPECT_EQ(0, this->rdata_txt_like_quoted.compare(*rdata)); EXPECT_EQ(0, TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, - this->loader_cb).compare( - *this->rdata_txt_like_fromwire)); + this->loader_cb).compare(*rdata)); EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); // Null character-string. @@ -160,6 +160,43 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); } +TYPED_TEST(Rdata_TXT_LIKE_Test, createMultiStringsFromText) { + // Tests for "from text" variants construction with various forms of + // multi character-strings. + + std::vector texts; + texts.push_back("\"Test-String\" \"Test-String\""); // most common form + texts.push_back("\"Test-String\"\"Test-String\""); // no space between'em + 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 + + std::stringstream ss; + for (std::vector::const_iterator it = texts.begin(); + it != texts.end(); ++it) { + ss << *it << "\n"; + } + this->lexer.pushSource(ss); + + // The corresponding Rdata built from wire to compare in the checks below. + ConstRdataPtr const rdata = + this->rdataFactoryFromFile(RRTYPE(), + RRClass("IN"), "rdata_txt_fromWire3.wire"); + + // Confirm we can construct the Rdata from the test text, both from + // std::string and with lexer, and that matches the from-wire data. + for (std::vector::const_iterator it = texts.begin(); + it != texts.end(); ++it) { + SCOPED_TRACE(*it); + EXPECT_EQ(0, TypeParam(*it).compare(*rdata)); + + EXPECT_EQ(0, TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, + this->loader_cb).compare(*rdata)); + EXPECT_EQ(MasterToken::END_OF_LINE, + this->lexer.getNextToken().getType()); + } +} + void makeLargest(vector& data) { uint8_t ch = 0; From 59861491526ae8b4f61f94e8ad4a6926178d9aea Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 18:56:04 -0800 Subject: [PATCH 57/69] [2442] added another case of multi-string with mixture of not-quoted + no spc. due to limitation of the current implementation it doesn't work right now, so commented out; planning to create a ticket to fix the underlying issue. --- src/lib/dns/tests/rdata_txt_like_unittest.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc index e4bb173861..05c4a04eb2 100644 --- a/src/lib/dns/tests/rdata_txt_like_unittest.cc +++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc @@ -170,6 +170,8 @@ 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 std::stringstream ss; for (std::vector::const_iterator it = texts.begin(); From 435309b12de7683b06fa29d0b2b9a0bb57fbd9ef Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 19:10:57 -0800 Subject: [PATCH 58/69] [2442] reject empty text input; adjuted one test case accordingly --- src/lib/dns/rdata/generic/detail/txt_like.h | 7 +++++++ src/lib/dns/tests/rdata_txt_like_unittest.cc | 12 ++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/lib/dns/rdata/generic/detail/txt_like.h b/src/lib/dns/rdata/generic/detail/txt_like.h index b788dd80d3..004229ca4c 100644 --- a/src/lib/dns/rdata/generic/detail/txt_like.h +++ b/src/lib/dns/rdata/generic/detail/txt_like.h @@ -113,7 +113,14 @@ private: string_list_.push_back(std::vector()); strToCharString(token.getStringRegion(), string_list_.back()); } + + // Let upper layer handle eol/eof. lexer.ungetToken(); + + if (string_list_.empty()) { + isc_throw(InvalidRdataText, "Failed to construct" << + RRType(typeCode) << " RDATA: empty input"); + } } public: diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc index 05c4a04eb2..b6e2676f61 100644 --- a/src/lib/dns/tests/rdata_txt_like_unittest.cc +++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc @@ -199,6 +199,14 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createMultiStringsFromText) { } } +TYPED_TEST(Rdata_TXT_LIKE_Test, fromTextEmpty) { + // If the input text doesn't contain any character-string, it should be + // rejected + EXPECT_THROW(TypeParam(""), InvalidRdataText); + EXPECT_THROW(TypeParam(" "), InvalidRdataText); // even with a space + EXPECT_THROW(TypeParam("(\n)"), InvalidRdataText); // or multi-line with () +} + void makeLargest(vector& data) { uint8_t ch = 0; @@ -331,8 +339,8 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, compare) { EXPECT_EQ(TypeParam(txt1).compare(TypeParam(txt1)), 0); - EXPECT_LT(TypeParam("").compare(TypeParam(txt1)), 0); - EXPECT_GT(TypeParam(txt1).compare(TypeParam("")), 0); + EXPECT_LT(TypeParam("\"\"").compare(TypeParam(txt1)), 0); + EXPECT_GT(TypeParam(txt1).compare(TypeParam("\"\"")), 0); EXPECT_LT(TypeParam(txt1).compare(TypeParam(txt2)), 0); EXPECT_GT(TypeParam(txt2).compare(TypeParam(txt1)), 0); From 8201bad569bfda39027198bd920e8da6b8af9539 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 19:16:29 -0800 Subject: [PATCH 59/69] [2442] added a test case for multi-line text input --- src/lib/dns/tests/rdata_txt_like_unittest.cc | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc index b6e2676f61..491aa2d909 100644 --- a/src/lib/dns/tests/rdata_txt_like_unittest.cc +++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc @@ -89,10 +89,17 @@ typedef testing::Types Implementations; TYPED_TEST_CASE(Rdata_TXT_LIKE_Test, Implementations); TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { + // Below we check the behavior for the "from text" constructors, both + // from std::string and with MasterLexer. The underlying implementation + // is the same, so both should work exactly same, but we confirm both + // cases. + + // test input for the lexer version std::stringstream ss; ss << "Test-String\n"; ss << "\"Test-String\"\n"; // explicitly surrounded by '"'s - ss << "\"\"\n"; // empty string + ss << "(\n\"Test-String\")\n"; // multi-line text with () + ss << "\"\"\n"; // empty string (note: still valid char-str) ss << string(255, 'a') << "\n"; // Longest possible character-string. ss << string(256, 'a') << "\n"; // char-string too long ss << "\"Test-String\\\"\n"; // unbalanced quote @@ -104,7 +111,9 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { this->rdataFactoryFromFile(RRTYPE(), RRClass("IN"), "rdata_txt_fromWire1"); - // normal case is covered in toWireBuffer. + // normal case is covered in toWireBuffer. First check the std::string + // case, then with MasterLexer. For the latter, we need to read and skip + // '\n'. These apply to most of the other cases below. EXPECT_EQ(0, this->rdata_txt_like.compare(*rdata)); EXPECT_EQ(0, TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, this->loader_cb).compare(*rdata)); @@ -116,6 +125,12 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { this->loader_cb).compare(*rdata)); EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); + // multi-line input with () + EXPECT_EQ(0, TypeParam("(\n\"Test-String\")").compare(*rdata)); + EXPECT_EQ(0, TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, + this->loader_cb).compare(*rdata)); + EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); + // Null character-string. this->obuffer.clear(); TypeParam(string("\"\"")).toWire(this->obuffer); From ec4fcc79563b93bfc1e300c46287d8f102261d68 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 19:26:40 -0800 Subject: [PATCH 60/69] [2442] rejected extra \n for std::string version of txt-like ctor. --- src/lib/dns/rdata/generic/detail/txt_like.h | 7 ++++++- src/lib/dns/tests/rdata_txt_like_unittest.cc | 12 ++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/lib/dns/rdata/generic/detail/txt_like.h b/src/lib/dns/rdata/generic/detail/txt_like.h index 004229ca4c..1ea73b3748 100644 --- a/src/lib/dns/rdata/generic/detail/txt_like.h +++ b/src/lib/dns/rdata/generic/detail/txt_like.h @@ -88,9 +88,14 @@ public: try { buildFromTextHelper(lexer); + if (lexer.getNextToken().getType() != MasterToken::END_OF_FILE) { + isc_throw(InvalidRdataText, "Failed to construct " << + RRType(typeCode) << " RDATA from '" << txtstr << + "': extra new line"); + } } catch (const MasterLexer::LexerError& ex) { isc_throw(InvalidRdataText, "Failed to construct " << - RRType(typeCode) << " RDATA from " << txtstr << ": " + RRType(typeCode) << " RDATA from '" << txtstr << "': " << ex.what()); } } diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc index 491aa2d909..23d13718f4 100644 --- a/src/lib/dns/tests/rdata_txt_like_unittest.cc +++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc @@ -98,7 +98,7 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { std::stringstream ss; ss << "Test-String\n"; ss << "\"Test-String\"\n"; // explicitly surrounded by '"'s - ss << "(\n\"Test-String\")\n"; // multi-line text with () + ss << "(\n \"Test-String\" )\n"; // multi-line text with () ss << "\"\"\n"; // empty string (note: still valid char-str) ss << string(255, 'a') << "\n"; // Longest possible character-string. ss << string(256, 'a') << "\n"; // char-string too long @@ -126,7 +126,7 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); // multi-line input with () - EXPECT_EQ(0, TypeParam("(\n\"Test-String\")").compare(*rdata)); + EXPECT_EQ(0, TypeParam("(\n \"Test-String\" )").compare(*rdata)); EXPECT_EQ(0, TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, this->loader_cb).compare(*rdata)); EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); @@ -214,6 +214,14 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createMultiStringsFromText) { } } +TYPED_TEST(Rdata_TXT_LIKE_Test, createFromTextExtra) { + // This is for the std::string version only: the input must end with EOF; + // an extra new-line will result in an exception. + EXPECT_THROW(TypeParam("\"Test-String\"\n"), InvalidRdataText); + // Same if there's a space before '\n' + EXPECT_THROW(TypeParam("\"Test-String\" \n"), InvalidRdataText); +} + TYPED_TEST(Rdata_TXT_LIKE_Test, fromTextEmpty) { // If the input text doesn't contain any character-string, it should be // rejected From 20ca327e9bcccb2e83ed437e7d1dfc9fa6d8fb3c Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 19:30:15 -0800 Subject: [PATCH 61/69] [2442] test for escaped characters --- src/lib/dns/tests/rdata_txt_like_unittest.cc | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc index 23d13718f4..b0a572d7ce 100644 --- a/src/lib/dns/tests/rdata_txt_like_unittest.cc +++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc @@ -94,11 +94,15 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { // is the same, so both should work exactly same, but we confirm both // cases. + const std::string multi_line = "(\n \"Test-String\" )"; + const std::string escaped_txt = "Test\\045Strin\\g"; + // test input for the lexer version std::stringstream ss; ss << "Test-String\n"; ss << "\"Test-String\"\n"; // explicitly surrounded by '"'s - ss << "(\n \"Test-String\" )\n"; // multi-line text with () + ss << multi_line << "\n"; // multi-line text with () + ss << escaped_txt << "\n"; // using the two types of escape with '\' ss << "\"\"\n"; // empty string (note: still valid char-str) ss << string(255, 'a') << "\n"; // Longest possible character-string. ss << string(256, 'a') << "\n"; // char-string too long @@ -126,7 +130,13 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, createFromText) { EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); // multi-line input with () - EXPECT_EQ(0, TypeParam("(\n \"Test-String\" )").compare(*rdata)); + EXPECT_EQ(0, TypeParam(multi_line).compare(*rdata)); + EXPECT_EQ(0, TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, + this->loader_cb).compare(*rdata)); + EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); + + // for the same data using escape + EXPECT_EQ(0, TypeParam(escaped_txt).compare(*rdata)); EXPECT_EQ(0, TypeParam(this->lexer, NULL, MasterLoader::MANY_ERRORS, this->loader_cb).compare(*rdata)); EXPECT_EQ(MasterToken::END_OF_LINE, this->lexer.getNextToken().getType()); From de92f31ea5fc5991e10bc61c175373b67f78deab Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 19:52:22 -0800 Subject: [PATCH 62/69] [2442] documentation updates --- src/lib/dns/rdata/generic/detail/char_string.h | 16 ++++++++++++++++ src/lib/dns/rdata/generic/detail/txt_like.h | 10 ++++++++++ src/lib/dns/rdata/generic/spf_99.cc | 12 ++++++++++-- src/lib/dns/rdata/generic/txt_16.cc | 10 ++++++++++ 4 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/lib/dns/rdata/generic/detail/char_string.h b/src/lib/dns/rdata/generic/detail/char_string.h index b5dc338e96..702af04ed5 100644 --- a/src/lib/dns/rdata/generic/detail/char_string.h +++ b/src/lib/dns/rdata/generic/detail/char_string.h @@ -32,6 +32,22 @@ namespace detail { /// be the bare char basis. typedef std::vector CharString; +/// \brief Convert a DNS character-string into corresponding binary data. +/// +/// This helper function takes a string object that is expected to be a +/// textual representation of a valid DNS character-string, and dumps +/// the corresponding binary sequence in the given placeholder (passed +/// via the \c result parameter). It handles escape notations of +/// character-strings with a backslash ('\'), and checks the length +/// restriction. +/// +/// \throw CharStringTooLong The resulting binary data are too large for a +/// valid character-string. +/// \throw InvalidRdataText Other syntax errors. +/// +/// \brief str_region A string that represents a character-string. +/// \brief result A placeholder vector where the resulting data are to be +/// stored. Expected to be empty, but it's not checked. void strToCharString(const MasterToken::StringRegion& str_region, CharString& result); diff --git a/src/lib/dns/rdata/generic/detail/txt_like.h b/src/lib/dns/rdata/generic/detail/txt_like.h index 1ea73b3748..9d90f403e6 100644 --- a/src/lib/dns/rdata/generic/detail/txt_like.h +++ b/src/lib/dns/rdata/generic/detail/txt_like.h @@ -100,6 +100,16 @@ public: } } + /// \brief Constructor using the master lexer. + /// + /// This implementation only uses the \c lexer parameters; others are + /// ignored. + /// + /// \throw CharStringTooLong the parameter string length exceeds maximum. + /// \throw InvalidRdataText the method cannot process the parameter data + /// + /// \param lexer A \c MasterLexer object parsing a master file for this + /// RDATA. TXTLikeImpl(MasterLexer& lexer, const Name*, MasterLoader::Options, MasterLoaderCallbacks&) { diff --git a/src/lib/dns/rdata/generic/spf_99.cc b/src/lib/dns/rdata/generic/spf_99.cc index 6be9ea5fde..94be9368a2 100644 --- a/src/lib/dns/rdata/generic/spf_99.cc +++ b/src/lib/dns/rdata/generic/spf_99.cc @@ -66,8 +66,16 @@ SPF::SPF(InputBuffer& buffer, size_t rdata_len) : impl_(new SPFImpl(buffer, rdata_len)) {} - -/// \brief Constructor from TBD +/// \brief Constructor using the master lexer. +/// +/// This implementation only uses the \c lexer parameters; others are +/// ignored. +/// +/// \throw CharStringTooLong the parameter string length exceeds maximum. +/// \throw InvalidRdataText the method cannot process the parameter data +/// +/// \param lexer A \c MasterLexer object parsing a master file for this +/// RDATA. SPF::SPF(MasterLexer& lexer, const Name* origin, MasterLoader::Options options, MasterLoaderCallbacks& callbacks) : impl_(new SPFImpl(lexer, origin, options, callbacks)) diff --git a/src/lib/dns/rdata/generic/txt_16.cc b/src/lib/dns/rdata/generic/txt_16.cc index 6e054d2e70..d35b4270d8 100644 --- a/src/lib/dns/rdata/generic/txt_16.cc +++ b/src/lib/dns/rdata/generic/txt_16.cc @@ -52,6 +52,16 @@ TXT::TXT(InputBuffer& buffer, size_t rdata_len) : impl_(new TXTImpl(buffer, rdata_len)) {} +/// \brief Constructor using the master lexer. +/// +/// This implementation only uses the \c lexer parameters; others are +/// ignored. +/// +/// \throw CharStringTooLong the parameter string length exceeds maximum. +/// \throw InvalidRdataText the method cannot process the parameter data +/// +/// \param lexer A \c MasterLexer object parsing a master file for this +/// RDATA. TXT::TXT(MasterLexer& lexer, const Name* origin, MasterLoader::Options options, MasterLoaderCallbacks& callbacks) : impl_(new TXTImpl(lexer, origin, options, callbacks)) From 9002ffc08e7c7fba799e8b417a1d80b77a20973e Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Dec 2012 20:00:31 -0800 Subject: [PATCH 63/69] [2442] simplified interface; txt-like ctor only needs to take lexer. --- src/lib/dns/rdata/generic/detail/txt_like.h | 9 +-------- src/lib/dns/rdata/generic/spf_99.cc | 6 +++--- src/lib/dns/rdata/generic/txt_16.cc | 6 +++--- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/lib/dns/rdata/generic/detail/txt_like.h b/src/lib/dns/rdata/generic/detail/txt_like.h index 9d90f403e6..d1916e35f3 100644 --- a/src/lib/dns/rdata/generic/detail/txt_like.h +++ b/src/lib/dns/rdata/generic/detail/txt_like.h @@ -16,8 +16,6 @@ #define TXT_LIKE_H 1 #include -#include -#include #include #include @@ -102,17 +100,12 @@ public: /// \brief Constructor using the master lexer. /// - /// This implementation only uses the \c lexer parameters; others are - /// ignored. - /// /// \throw CharStringTooLong the parameter string length exceeds maximum. /// \throw InvalidRdataText the method cannot process the parameter data /// /// \param lexer A \c MasterLexer object parsing a master file for this /// RDATA. - TXTLikeImpl(MasterLexer& lexer, const Name*, MasterLoader::Options, - MasterLoaderCallbacks&) - { + TXTLikeImpl(MasterLexer& lexer) { buildFromTextHelper(lexer); } diff --git a/src/lib/dns/rdata/generic/spf_99.cc b/src/lib/dns/rdata/generic/spf_99.cc index 94be9368a2..4bf24e9539 100644 --- a/src/lib/dns/rdata/generic/spf_99.cc +++ b/src/lib/dns/rdata/generic/spf_99.cc @@ -76,9 +76,9 @@ SPF::SPF(InputBuffer& buffer, size_t rdata_len) : /// /// \param lexer A \c MasterLexer object parsing a master file for this /// RDATA. -SPF::SPF(MasterLexer& lexer, const Name* origin, - MasterLoader::Options options, MasterLoaderCallbacks& callbacks) : - impl_(new SPFImpl(lexer, origin, options, callbacks)) +SPF::SPF(MasterLexer& lexer, const Name*, MasterLoader::Options, + MasterLoaderCallbacks&) : + impl_(new SPFImpl(lexer)) {} /// \brief Constructor from string. diff --git a/src/lib/dns/rdata/generic/txt_16.cc b/src/lib/dns/rdata/generic/txt_16.cc index d35b4270d8..1bd2eb1aea 100644 --- a/src/lib/dns/rdata/generic/txt_16.cc +++ b/src/lib/dns/rdata/generic/txt_16.cc @@ -62,9 +62,9 @@ TXT::TXT(InputBuffer& buffer, size_t rdata_len) : /// /// \param lexer A \c MasterLexer object parsing a master file for this /// RDATA. -TXT::TXT(MasterLexer& lexer, const Name* origin, - MasterLoader::Options options, MasterLoaderCallbacks& callbacks) : - impl_(new TXTImpl(lexer, origin, options, callbacks)) +TXT::TXT(MasterLexer& lexer, const Name*, MasterLoader::Options, + MasterLoaderCallbacks&) : + impl_(new TXTImpl(lexer)) {} TXT::TXT(const std::string& txtstr) : From 05fc975a3d825a10c8b9c8df509854f9f69d9c87 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 5 Dec 2012 10:36:13 -0800 Subject: [PATCH 64/69] [2442] use std::string instead of POD array for constructing obj of DDD. --- src/lib/dns/rdata/generic/detail/char_string.cc | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/lib/dns/rdata/generic/detail/char_string.cc b/src/lib/dns/rdata/generic/detail/char_string.cc index e9f900a49b..b08111ac10 100644 --- a/src/lib/dns/rdata/generic/detail/char_string.cc +++ b/src/lib/dns/rdata/generic/detail/char_string.cc @@ -41,20 +41,17 @@ decimalToNumber(const char* s, const char* s_end) { isc_throw(InvalidRdataText, "Escaped digits too short"); } - // Make a nul-terminated copy of the 'DDD' for lexical_cast - char buf[4]; - std::memcpy(buf, s, 3); - buf[3] = 0; - + const std::string num_str(s, s + 3); try { - const int i = boost::lexical_cast(buf); + const int i = boost::lexical_cast(num_str); if (i > 255) { - isc_throw(InvalidRdataText, "Escaped digits too large: " << buf); + isc_throw(InvalidRdataText, "Escaped digits too large: " + << num_str); } return (i); } catch (const boost::bad_lexical_cast&) { isc_throw(InvalidRdataText, - "Invalid form for escaped digits: " << buf); + "Invalid form for escaped digits: " << num_str); } } } From ea88e8907ab138e2fa1c5271c8a03ec53b4b08d0 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 5 Dec 2012 10:43:06 -0800 Subject: [PATCH 65/69] [2442] updated comment for string::operator[] when it's empty this is actually ensured by the C++ standard lib spec. --- src/lib/dns/tests/rdata_char_string_unittest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dns/tests/rdata_char_string_unittest.cc b/src/lib/dns/tests/rdata_char_string_unittest.cc index 83f0591280..9d236229c1 100644 --- a/src/lib/dns/tests/rdata_char_string_unittest.cc +++ b/src/lib/dns/tests/rdata_char_string_unittest.cc @@ -52,7 +52,7 @@ protected: MasterToken::StringRegion createStringRegion(const std::string& str) { MasterToken::StringRegion region; - region.beg = &str[0]; // note this works even if str is empty + region.beg = &str[0]; // note std ensures this works even if str is empty region.len = str.size(); return (region); } From 181226ac0ced78a1092c14d54764828637a4d814 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 6 Dec 2012 17:16:17 +0100 Subject: [PATCH 66/69] [master] Added ChangeLog entry for #2479. --- ChangeLog | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ChangeLog b/ChangeLog index 4b3a4c4553..18c6723f7c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +516. [bug] marcin + Fixed 'make distcheck' failure when running perfdhcp unit tests. + The unit tests used to read files from the folder specified + with the path relative to current folder, thus when the test was + run from a different folder the files could not be found. + (Trac #2479, git 4e8325e1b309f1d388a3055ec1e1df98c377f383) + 515. [bug] jinmei The in-memory data source now accepts an RRSIG provided without a covered RRset in loading. A subsequent query for its owner name From c366a4f8deb99014add34a7c79715beaeb1d63f3 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 6 Dec 2012 15:26:00 -0800 Subject: [PATCH 67/69] [2442] in CharStringTooLong what() msg, clarify it includes an extra octet. --- src/lib/dns/rdata/generic/detail/char_string.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dns/rdata/generic/detail/char_string.cc b/src/lib/dns/rdata/generic/detail/char_string.cc index b08111ac10..fb4c9b41db 100644 --- a/src/lib/dns/rdata/generic/detail/char_string.cc +++ b/src/lib/dns/rdata/generic/detail/char_string.cc @@ -86,7 +86,7 @@ strToCharString(const MasterToken::StringRegion& str_region, } if (result.size() > MAX_CHARSTRING_LEN + 1) { // '+ 1' due to the len field isc_throw(CharStringTooLong, "character-string is too long: " << - result.size() << " bytes"); + (result.size() - 1) << "(+1) characters"); } result[0] = result.size() - 1; } From 678ce7c7ba26b9728ae3a870896e8c880a618918 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 6 Dec 2012 17:50:53 -0800 Subject: [PATCH 68/69] [master] include notify's source IP in ZONEMGR_UNKNOWN_ZONE_NOTIFIED log msg. okayed on jabber. --- src/bin/zonemgr/zonemgr.py.in | 3 ++- src/bin/zonemgr/zonemgr_messages.mes | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/bin/zonemgr/zonemgr.py.in b/src/bin/zonemgr/zonemgr.py.in index 8cb616d917..86f89fa4ef 100755 --- a/src/bin/zonemgr/zonemgr.py.in +++ b/src/bin/zonemgr/zonemgr.py.in @@ -193,7 +193,8 @@ class ZonemgrRefresh: def zone_handle_notify(self, zone_name_class, master): """Handle zone notify""" if (self._zone_not_exist(zone_name_class)): - logger.error(ZONEMGR_UNKNOWN_ZONE_NOTIFIED, zone_name_class[0], zone_name_class[1]) + logger.error(ZONEMGR_UNKNOWN_ZONE_NOTIFIED, zone_name_class[0], + zone_name_class[1], master) raise ZonemgrException("[b10-zonemgr] Notified zone (%s, %s) " "doesn't belong to zonemgr" % zone_name_class) self._set_zone_notifier_master(zone_name_class, master) diff --git a/src/bin/zonemgr/zonemgr_messages.mes b/src/bin/zonemgr/zonemgr_messages.mes index 88f8dcf237..4f58271cc9 100644 --- a/src/bin/zonemgr/zonemgr_messages.mes +++ b/src/bin/zonemgr/zonemgr_messages.mes @@ -138,7 +138,7 @@ zone, or, if this error appears without the administrator giving transfer commands, it can indicate an error in the program, as it should not have initiated transfers of unknown zones on its own. -% ZONEMGR_UNKNOWN_ZONE_NOTIFIED notified zone %1 (class %2) is not known to the zone manager +% ZONEMGR_UNKNOWN_ZONE_NOTIFIED notified zone %1/%2 from %3 is not known to the zone manager A NOTIFY was received but the zone that was the subject of the operation is not being managed by the zone manager. This may indicate an error in the program (as the operation should not have been initiated if this From 8e39d7eb39a7174ac39b93e5425b9fc8f35feecf Mon Sep 17 00:00:00 2001 From: "Jeremy C. Reed" Date: Thu, 6 Dec 2012 21:01:51 -0600 Subject: [PATCH 69/69] [master] fix typo and standardize one spelling in docs. trivial. no review. --- doc/guide/bind10-guide.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml index 9d8f932c16..da1ac13b8c 100644 --- a/doc/guide/bind10-guide.xml +++ b/doc/guide/bind10-guide.xml @@ -472,7 +472,7 @@ var/ Packages - Some operating systems or softare package vendors may + Some operating systems or software package vendors may provide ready-to-use, pre-built software packages for the BIND 10 suite. Installing a pre-built package means you do not need to @@ -2157,7 +2157,7 @@ AND_MATCH := "ALL": [ RULE_RAW, RULE_RAW, ... ] you indicate that the system is not usable without the component and if such component fails, the system shuts down no matter when the failure happened. This is the - behaviour of the core components (the ones you can't turn + behavior of the core components (the ones you can't turn off), but you can declare any other components as core as well if you wish (but you can turn these off, they just can't fail).