Browse Source

[5014_phase2] Began to write tests (and of course found and fixed some problems)

Francis Dupont 8 years ago
parent
commit
f04ce6bbac

+ 3 - 0
src/bin/dhcp6/Makefile.am

@@ -121,6 +121,9 @@ parser: dhcp6_lexer.cc location.hh position.hh stack.hh dhcp6_parser.cc dhcp6_pa
 # It can be used to manually follow what's going on in the parser.
 # This is especially useful if yydebug_ is set to 1 as that variable
 # will cause parser to print out its internal state.
+# Call flex with -s to check that the default rule can be suppressed
+# Call bison with -W to get warnings like unmarked empty rules
+# Note C++11 deprecated register still used by flex < 2.6.0
 location.hh position.hh stack.hh dhcp6_parser.cc dhcp6_parser.h: dhcp6_parser.yy
 	$(YACC) --defines=dhcp6_parser.h --report=all --report-file=dhcp6_parser.report -o dhcp6_parser.cc dhcp6_parser.yy
 

+ 22 - 16
src/bin/dhcp6/dhcp6_lexer.ll

@@ -42,7 +42,7 @@ std::vector<struct yy_buffer_state*> states;
 bool start_token_flag = false;
 
 isc::dhcp::Parser6Context::ParserType start_token_value;
-int comment_start_line = 0;
+unsigned int comment_start_line = 0;
 
 };
 
@@ -63,6 +63,9 @@ int comment_start_line = 0;
 /* batch means that we'll never use the generated lexer interactively. */
 %option batch
 
+/* avoid to get static global variables to remain with C++. */
+/* in last resort %option reentrant */
+
 /* Enables debug mode. To see the debug messages, one needs to also set
    yy_flex_debug to 1, then the debug messages will be printed on stderr. */
 %option debug
@@ -72,11 +75,6 @@ int comment_start_line = 0;
    be on the safe side and keep it. */
 %option noinput
 
-/* This line tells flex to track the line numbers. It's not really that
-   useful for client classes, which typically are one-liners, but it may be
-   useful in more complex cases. */
-%option yylineno
-
 %x COMMENT
 %x DIR_ENTER DIR_INCLUDE DIR_EXIT
 
@@ -112,6 +110,7 @@ JSONString                              \"{JSONStringCharacter}*\"
 
     if (start_token_flag) {
         start_token_flag = false;
+        BEGIN(0);
         switch (start_token_value) {
         case Parser6Context::PARSER_DHCP6:
             return isc::dhcp::Dhcp6Parser::make_TOPLEVEL_DHCP6(loc);
@@ -128,13 +127,13 @@ JSONString                              \"{JSONStringCharacter}*\"
 
 "/*" {
   BEGIN(COMMENT);
-  comment_start_line = yylineno;
+  comment_start_line = loc.end.line;;
 }
 
 <COMMENT>"*/" BEGIN(INITIAL);
 <COMMENT>. ;
 <COMMENT><<EOF>> {
-    isc_throw(isc::BadValue, "Comment not closed. (/* in line " << comment_start_line);
+    isc_throw(Dhcp6ParseError, "Comment not closed. (/* in line " << comment_start_line);
 }
 
 "<?" BEGIN(DIR_ENTER);
@@ -149,7 +148,7 @@ JSONString                              \"{JSONStringCharacter}*\"
     driver.includeFile(tmp);
 }
 <DIR_ENTER,DIR_INCLUDE,DIR_EXIT><<EOF>> {
-    isc_throw(isc::BadValue, "Directive not closed.");
+    isc_throw(Dhcp6ParseError, "Directive not closed.");
 }
 <DIR_EXIT>"?>" BEGIN(INITIAL);
     
@@ -213,7 +212,8 @@ JSONString                              \"{JSONStringCharacter}*\"
 
 \"type\" {
     switch(driver.ctx_) {
-    case isc::dhcp::Parser6Context::DATABASE:
+    case isc::dhcp::Parser6Context::LEASE_DATABASE:
+    case isc::dhcp::Parser6Context::HOSTS_DATABASE:
     case isc::dhcp::Parser6Context::SERVER_ID:
         return isc::dhcp::Dhcp6Parser::make_TYPE(loc);
     default:
@@ -223,7 +223,8 @@ JSONString                              \"{JSONStringCharacter}*\"
 
 \"user\" {
     switch(driver.ctx_) {
-    case isc::dhcp::Parser6Context::DATABASE:
+    case isc::dhcp::Parser6Context::LEASE_DATABASE:
+    case isc::dhcp::Parser6Context::HOSTS_DATABASE:
         return isc::dhcp::Dhcp6Parser::make_USER(loc);
     default:
         return isc::dhcp::Dhcp6Parser::make_STRING("user", loc);
@@ -232,7 +233,8 @@ JSONString                              \"{JSONStringCharacter}*\"
 
 \"password\" {
     switch(driver.ctx_) {
-    case isc::dhcp::Parser6Context::DATABASE:
+    case isc::dhcp::Parser6Context::LEASE_DATABASE:
+    case isc::dhcp::Parser6Context::HOSTS_DATABASE:
         return isc::dhcp::Dhcp6Parser::make_PASSWORD(loc);
     default:
         return isc::dhcp::Dhcp6Parser::make_STRING("password", loc);
@@ -241,7 +243,8 @@ JSONString                              \"{JSONStringCharacter}*\"
 
 \"host\" {
     switch(driver.ctx_) {
-    case isc::dhcp::Parser6Context::DATABASE:
+    case isc::dhcp::Parser6Context::LEASE_DATABASE:
+    case isc::dhcp::Parser6Context::HOSTS_DATABASE:
         return isc::dhcp::Dhcp6Parser::make_HOST(loc);
     default:
         return isc::dhcp::Dhcp6Parser::make_STRING("host", loc);
@@ -250,7 +253,8 @@ JSONString                              \"{JSONStringCharacter}*\"
 
 \"persist\" {
     switch(driver.ctx_) {
-    case isc::dhcp::Parser6Context::DATABASE:
+    case isc::dhcp::Parser6Context::LEASE_DATABASE:
+    case isc::dhcp::Parser6Context::HOSTS_DATABASE:
     case isc::dhcp::Parser6Context::SERVER_ID:
         return isc::dhcp::Dhcp6Parser::make_PERSIST(loc);
     default:
@@ -260,7 +264,8 @@ JSONString                              \"{JSONStringCharacter}*\"
 
 \"lfc-interval\" {
     switch(driver.ctx_) {
-    case isc::dhcp::Parser6Context::DATABASE:
+    case isc::dhcp::Parser6Context::LEASE_DATABASE:
+    case isc::dhcp::Parser6Context::HOSTS_DATABASE:
         return isc::dhcp::Dhcp6Parser::make_LFC_INTERVAL(loc);
     default:
         return isc::dhcp::Dhcp6Parser::make_STRING("lfc-interval", loc);
@@ -329,7 +334,8 @@ JSONString                              \"{JSONStringCharacter}*\"
 
 \"name\" {
     switch(driver.ctx_) {
-    case isc::dhcp::Parser6Context::DATABASE:
+    case isc::dhcp::Parser6Context::LEASE_DATABASE:
+    case isc::dhcp::Parser6Context::HOSTS_DATABASE:
     case isc::dhcp::Parser6Context::OPTION_DATA:
     case isc::dhcp::Parser6Context::CLIENT_CLASSES:
     case isc::dhcp::Parser6Context::CLIENT_CLASS:

+ 24 - 4
src/bin/dhcp6/dhcp6_parser.yy

@@ -225,6 +225,15 @@ not_empty_list: value {
 
 // ---- syntax checking parser starts here -----------------------------
 
+// Unknown keyword in a map
+unknown_map_entry: STRING COLON {
+    const std::string& where = ctx.context_name();
+    const std::string& keyword = $1;
+    error(@1,
+          "got unexpected keyword \"" + keyword + "\" in " + where + " map.");
+}
+
+
 // This defines the top-level { } that holds Dhcp6, Dhcp4, DhcpDdns or Logging
 // objects.
 // ctx_ = CONFIG
@@ -288,6 +297,7 @@ global_param: preferred_lifetime
             | server_id
             | dhcp4o6_port
             | dhcp_ddns
+            | unknown_map_entry
             ;
 
 preferred_lifetime: PREFERRED_LIFETIME COLON INTEGER {
@@ -334,7 +344,7 @@ lease_database: LEASE_DATABASE {
     ElementPtr i(new MapElement());
     ctx.stack_.back()->set("lease-database", i);
     ctx.stack_.push_back(i);
-    ctx.enter(ctx.DATABASE);
+    ctx.enter(ctx.LEASE_DATABASE);
 } COLON LCURLY_BRACKET database_map_params RCURLY_BRACKET {
     ctx.stack_.pop_back();
     ctx.leave();
@@ -344,7 +354,7 @@ hosts_database: HOSTS_DATABASE {
     ElementPtr i(new MapElement());
     ctx.stack_.back()->set("hosts-database", i);
     ctx.stack_.push_back(i);
-    ctx.enter(ctx.DATABASE);
+    ctx.enter(ctx.HOSTS_DATABASE);
 } COLON LCURLY_BRACKET database_map_params RCURLY_BRACKET {
     ctx.stack_.pop_back();
     ctx.leave();
@@ -360,7 +370,8 @@ database_map_param: type
                   | host
                   | name
                   | persist
-                  | lfc_interval;
+                  | lfc_interval
+                  | unknown_map_entry
 ;
 
 type: TYPE {
@@ -501,7 +512,7 @@ hooks_library: LCURLY_BRACKET {
 };
 
 hooks_params: hooks_param
-            | hooks_params hooks_param
+            | hooks_params COMMA hooks_param
             ;
 
 hooks_param: LIBRARY {
@@ -600,6 +611,7 @@ subnet6_param: option_data_list
              | id
              | client_class
              | reservations
+             | unknown_map_entry
              ;
 
 subnet: SUBNET {
@@ -680,6 +692,7 @@ option_data_param: option_data_name
                  | option_data_code
                  | option_data_space
                  | option_data_csv_format
+                 | unknown_map_entry
                  ;
 
 
@@ -748,6 +761,7 @@ pool_params: pool_param
 
 pool_param: pool_entry
           | option_data_list
+          | unknown_map_entry
           ;
 
 pool_entry: POOL {
@@ -797,6 +811,7 @@ pd_pool_param: pd_prefix
              | pd_prefix_len
              | pd_delegated_len
              | option_data_list
+             | unknown_map_entry
              ;
 
 pd_prefix: PREFIX {
@@ -862,6 +877,7 @@ reservation_param: duid
                  | hw_address
                  | hostname
                  | option_data_list
+                 | unknown_map_entry
                  ;
 
 ip_addresses: IP_ADDRESSES {
@@ -954,6 +970,7 @@ not_empty_client_class_params: client_class_param
 client_class_param: client_class_name
                   | client_class_test
                   | option_data_list
+                  | unknown_map_entry
                   ;
 
 client_class_name: name;
@@ -990,6 +1007,7 @@ server_id_param: type
                | htype
                | enterprise_id
                | persist
+               | unknown_map_entry
                ;
 
 htype: HTYPE COLON INTEGER {
@@ -1082,6 +1100,7 @@ logger_param: name
             | output_options_list
             | debuglevel
             | severity
+            | unknown_map_entry
             ;
 
 debuglevel: DEBUGLEVEL COLON INTEGER {
@@ -1146,6 +1165,7 @@ dhcp_ddns_params: dhcp_ddns_param
 
 dhcp_ddns_param: enable_updates
                | qualifying_suffix
+               | unknown_map_entry
                ;
 
 enable_updates: ENABLE_UPDATES COLON BOOLEAN {

+ 57 - 6
src/bin/dhcp6/parser_context.cc

@@ -40,7 +40,7 @@ Parser6Context::parseString(const std::string& str, ParserType parser_type)
     if (stack_.size() == 1) {
         return (stack_[0]);
     } else {
-        isc_throw(BadValue, "Expected exactly one terminal Element expected, found "
+        isc_throw(Dhcp6ParseError, "Expected exactly one terminal Element expected, found "
                   << stack_.size());
     }
 }
@@ -49,7 +49,7 @@ isc::data::ConstElementPtr
 Parser6Context::parseFile(const std::string& filename, ParserType parser_type) {
     FILE* f = fopen(filename.c_str(), "r");
     if (!f) {
-        isc_throw(BadValue, "Unable to open file " << filename);
+        isc_throw(Dhcp6ParseError, "Unable to open file " << filename);
     }
     scanFileBegin(f, filename, parser_type);
 
@@ -65,7 +65,7 @@ Parser6Context::parseFile(const std::string& filename, ParserType parser_type) {
     if (stack_.size() == 1) {
         return (stack_[0]);
     } else {
-        isc_throw(BadValue, "Expected exactly one terminal Element expected, found "
+        isc_throw(Dhcp6ParseError, "Expected exactly one terminal Element expected, found "
                   << stack_.size());
     }
 }
@@ -74,19 +74,19 @@ Parser6Context::parseFile(const std::string& filename, ParserType parser_type) {
 void
 Parser6Context::error(const isc::dhcp::location& loc, const std::string& what)
 {
-    isc_throw(EvalParseError, loc << ": " << what);
+    isc_throw(Dhcp6ParseError, loc << ": " << what);
 }
 
 void
 Parser6Context::error (const std::string& what)
 {
-    isc_throw(EvalParseError, what);
+    isc_throw(Dhcp6ParseError, what);
 }
 
 void
 Parser6Context::fatal (const std::string& what)
 {
-    isc_throw(Unexpected, what);
+    isc_throw(Dhcp6ParseError, what);
 }
 
 void
@@ -108,5 +108,56 @@ Parser6Context::leave()
     cstack_.pop_back();
 }
 
+const std::string
+Parser6Context::context_name()
+{
+    switch (ctx_) {
+    case NO_KEYWORD:
+        return ("__no keyword__");
+    case CONFIG:
+        return ("__config__");
+    case DHCP6:
+        return ("Dhcp6");
+    case LOGGING:
+        return ("Logging");
+    case INTERFACES_CONFIG:
+        return ("interfaces-config");
+    case LEASE_DATABASE:
+        return ("lease-database");
+    case HOSTS_DATABASE:
+        return ("hosts-database");
+    case MAC_SOURCES:
+        return ("mac-sources");
+    case HOST_RESERVATION_IDENTIFIERS:
+        return ("host-reservation-identifiers");
+    case HOOKS_LIBRARIES:
+        return ("hooks-librairies");
+    case SUBNET6:
+        return ("subnet6");
+    case OPTION_DATA:
+        return ("option-data");
+    case CLIENT_CLASSES:
+        return ("client-classes");
+    case SERVER_ID:
+        return ("server-id");
+    case DHCP_DDNS:
+        return ("dhcp-ddns");
+    case POOLS:
+        return ("pools");
+    case PD_POOLS:
+        return ("pd-pools");
+    case RESERVATIONS:
+        return ("reservations");
+    case CLIENT_CLASS:
+        return ("client-class");
+    case LOGGERS:
+        return ("loggers");
+    case OUTPUT_OPTIONS:
+        return ("output-options");
+    default:
+        return ("__unknown__");
+    }
+}
+
 };
 };

+ 9 - 5
src/bin/dhcp6/parser_context.h

@@ -22,10 +22,10 @@ YY_DECL;
 namespace isc {
 namespace dhcp {
 
-/// @brief Evaluation error exception raised when trying to parse an axceptions.
-class EvalParseError : public isc::Exception {
+/// @brief Evaluation error exception raised when trying to parse.
+class Dhcp6ParseError : public isc::Exception {
 public:
-    EvalParseError(const char* file, size_t line, const char* what) :
+    Dhcp6ParseError(const char* file, size_t line, const char* what) :
         isc::Exception(file, line, what) { };
 };
 
@@ -86,7 +86,7 @@ public:
     /// @brief Error handler
     ///
     /// This is a simplified error reporting tool for possible future
-    /// cases when the EvalParser is not able to handle the packet.
+    /// cases when the Dhcp6Parser is not able to handle the packet.
     void error(const std::string& what);
 
     /// @brief Fatal error handler
@@ -107,7 +107,8 @@ public:
         LOGGING,
         /// Dhcp6
         INTERFACES_CONFIG,
-        DATABASE,
+        LEASE_DATABASE,
+        HOSTS_DATABASE,
         MAC_SOURCES,
         HOST_RESERVATION_IDENTIFIERS,
         HOOKS_LIBRARIES,
@@ -138,6 +139,9 @@ public:
     /// @throw isc::Unexpected if unbalanced
     void leave();
 
+    /// @brief Get the syntactix context name
+    const std::string context_name();
+
  private:
     /// @brief Flag determining scanner debugging.
     bool trace_scanning_;

+ 75 - 0
src/bin/dhcp6/tests/parser_unittest.cc

@@ -247,4 +247,79 @@ TEST(ParserTest, file) {
     }
 }
 
+void testError(const std::string& txt,
+               Parser6Context::ParserType parser_type,
+               const std::string& msg)
+{
+    try {
+        Parser6Context ctx;
+        ConstElementPtr parsed = ctx.parseString(txt, parser_type);
+        FAIL() << "Expected Dhcp6ParseError but nothing was raised";
+    }
+    catch (const Dhcp6ParseError& ex) {
+        EXPECT_EQ(msg, ex.what());
+    }
+    catch (...) {
+        FAIL() << "Expected Dhcp6ParseError but something else was raised";
+    }
+}
+
+// Check lexer errors
+TEST(ParserTest, lexerErrors) {
+    testError("",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "<string>:1.1: syntax error, unexpected end of file, "
+              "expecting {");
+    testError(" ",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "<string>:1.2: syntax error, unexpected end of file, "
+              "expecting {");
+    testError("\n",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "<string>:2.1: syntax error, unexpected end of file, "
+              "expecting {");
+    testError("# nothing\n",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "<string>:2.1: syntax error, unexpected end of file, "
+              "expecting {");
+    testError(" #\n",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "<string>:2.1: syntax error, unexpected end of file, "
+              "expecting {");
+    testError("// nothing\n",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "<string>:2.1: syntax error, unexpected end of file, "
+              "expecting {");
+    testError("/* nothing */\n",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "<string>:2.1: syntax error, unexpected end of file, "
+              "expecting {");
+    testError("/* no\nthing */\n",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "<string>:3.1: syntax error, unexpected end of file, "
+              "expecting {");
+    testError("/* no\nthing */\n\n",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "<string>:4.1: syntax error, unexpected end of file, "
+              "expecting {");
+    testError("/* nothing\n",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "Comment not closed. (/* in line 1");
+    fprintf(stderr, "got 124?\n");
+    testError("/**/\n\n\n/* nothing\n",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "Comment not closed. (/* in line 4");
+    testError("<?\n",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "Directive not closed.");
+    testError("<?include\n",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "Directive not closed.");
+    string file = string(CFG_EXAMPLES) + "/" + "stateless.json";
+    fprintf(stderr, "doing include\n");
+    testError("<?include \"" + file + "\"\n",
+              Parser6Context::PARSER_GENERIC_JSON,
+              "Directive not closed.");
+}
+
 };