From eabcd010be0c7f35723efd97d7cae7a754d2856f Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Tue, 20 Dec 2016 15:54:23 +0100 Subject: [PATCH] [5017] dhcp-socket-type is now supported - added doc/examples/kea4/advanced.json - unit-tests now remove comments before passing to legacy JSON parser --- doc/Makefile.am | 1 + src/bin/dhcp4/dhcp4_lexer.ll | 9 ++ src/bin/dhcp4/dhcp4_parser.yy | 23 ++++- src/bin/dhcp4/tests/parser_unittest.cc | 132 ++++++++++++++++++++----- 4 files changed, 136 insertions(+), 29 deletions(-) diff --git a/doc/Makefile.am b/doc/Makefile.am index 9bb83c60e..dff8c492e 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -8,6 +8,7 @@ EXTRA_DIST += devel/unit-tests.dox nobase_dist_doc_DATA = examples/ddns/sample1.json nobase_dist_doc_DATA += examples/ddns/template.json +nobase_dist_doc_DATA += examples/kea4/advanced.json nobase_dist_doc_DATA += examples/kea4/backends.json nobase_dist_doc_DATA += examples/kea4/classify.json nobase_dist_doc_DATA += examples/kea4/dhcpv4-over-dhcpv6.json diff --git a/src/bin/dhcp4/dhcp4_lexer.ll b/src/bin/dhcp4/dhcp4_lexer.ll index 7f0097346..b993b645f 100644 --- a/src/bin/dhcp4/dhcp4_lexer.ll +++ b/src/bin/dhcp4/dhcp4_lexer.ll @@ -187,6 +187,15 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence} } } +\"dhcp-socket-type\" { + switch(driver.ctx_) { + case isc::dhcp::Parser4Context::INTERFACES_CONFIG: + return isc::dhcp::Dhcp4Parser::make_DHCP_SOCKET_TYPE(driver.loc_); + default: + return isc::dhcp::Dhcp4Parser::make_STRING("dhcp-socket-type", driver.loc_); + } +} + \"interfaces\" { switch(driver.ctx_) { case isc::dhcp::Parser4Context::INTERFACES_CONFIG: diff --git a/src/bin/dhcp4/dhcp4_parser.yy b/src/bin/dhcp4/dhcp4_parser.yy index b9b51e34f..fc2512459 100644 --- a/src/bin/dhcp4/dhcp4_parser.yy +++ b/src/bin/dhcp4/dhcp4_parser.yy @@ -52,6 +52,7 @@ using namespace std; DHCP4 "Dhcp4" INTERFACES_CONFIG "interfaces-config" INTERFACES "interfaces" + DHCP_SOCKET_TYPE "dhcp-socket-type" ECHO_CLIENT_ID "echo-client-id" MATCH_CLIENT_ID "match-client-id" @@ -414,20 +415,28 @@ interfaces_config: INTERFACES_CONFIG { ctx.stack_.back()->set("interfaces-config", i); ctx.stack_.push_back(i); ctx.enter(ctx.INTERFACES_CONFIG); -} COLON LCURLY_BRACKET interface_config_map RCURLY_BRACKET { +} COLON LCURLY_BRACKET interfaces_config_params RCURLY_BRACKET { ctx.stack_.pop_back(); ctx.leave(); }; +interfaces_config_params: interfaces_config_param + | interfaces_config_params COMMA interfaces_config_param + ; + +interfaces_config_param: interfaces_list + | dhcp_socket_type + ; + sub_interfaces4: LCURLY_BRACKET { // Parse the interfaces-config map ElementPtr m(new MapElement(ctx.loc2pos(@1))); ctx.stack_.push_back(m); -} interface_config_map RCURLY_BRACKET { +} interfaces_config_params RCURLY_BRACKET { // parsing completed }; -interface_config_map: INTERFACES { +interfaces_list: INTERFACES { ElementPtr l(new ListElement(ctx.loc2pos(@1))); ctx.stack_.back()->set("interfaces", l); ctx.stack_.push_back(l); @@ -437,6 +446,14 @@ interface_config_map: INTERFACES { ctx.leave(); }; +dhcp_socket_type: DHCP_SOCKET_TYPE { + ctx.enter(ctx.NO_KEYWORD); +} COLON STRING { + ElementPtr type(new StringElement($4, ctx.loc2pos(@4))); + ctx.stack_.back()->set("dhcp-socket-type", type); + ctx.leave(); +}; + lease_database: LEASE_DATABASE { ElementPtr i(new MapElement(ctx.loc2pos(@1))); ctx.stack_.back()->set("lease-database", i); diff --git a/src/bin/dhcp4/tests/parser_unittest.cc b/src/bin/dhcp4/tests/parser_unittest.cc index 60204afa8..f62f5094a 100644 --- a/src/bin/dhcp4/tests/parser_unittest.cc +++ b/src/bin/dhcp4/tests/parser_unittest.cc @@ -7,20 +7,18 @@ #include #include #include +#include +#include +#include using namespace isc::data; using namespace std; namespace { -void compareJSON(ConstElementPtr a, ConstElementPtr b, bool print = true) { +void compareJSON(ConstElementPtr a, ConstElementPtr b) { ASSERT_TRUE(a); ASSERT_TRUE(b); - if (print) { - // std::cout << "JSON A: -----" << endl << a->str() << std::endl; - // std::cout << "JSON B: -----" << endl << b->str() << std::endl; - // cout << "---------" << endl << endl; - } EXPECT_EQ(a->str(), b->str()); } @@ -124,7 +122,7 @@ TEST(ParserTest, keywordDhcp4) { " \"subnet\": \"192.0.2.0/24\", " " \"interface\": \"test\" } ],\n" "\"valid-lifetime\": 4000 } }"; - testParser2(txt, Parser4Context::PARSER_DHCP4); + testParser(txt, Parser4Context::PARSER_DHCP4); } TEST(ParserTest, bashComments) { @@ -142,7 +140,7 @@ TEST(ParserTest, bashComments) { " \"interface\": \"eth0\"" " } ]," "\"valid-lifetime\": 4000 } }"; - testParser2(txt, Parser4Context::PARSER_DHCP4); + testParser(txt, Parser4Context::PARSER_DHCP4); } TEST(ParserTest, cComments) { @@ -192,14 +190,96 @@ TEST(ParserTest, multilineComments) { testParser2(txt, Parser4Context::PARSER_DHCP4); } +/// @brief removes comments from a JSON file +/// +/// This is rather naive implementation, but it's probably sufficient for +/// testing. It won't be able to pick any trickier cases, like # or // +/// appearing in strings, nested C++ comments etc. +/// +/// @param input_file file to be stripped of comments +/// @return a new file that has comments stripped from it +std::string decommentJSONfile(const std::string& input_file) { + ifstream f(input_file); + if (!f.is_open()) { + isc_throw(isc::BadValue, "can't open input file for reading: " + input_file); + } + + string outfile; + size_t last_slash = input_file.find_last_of("/"); + if (last_slash != string::npos) { + outfile = input_file.substr(last_slash + 1); + } else { + outfile = input_file; + } + outfile += "-decommented"; + + ofstream out(outfile); + if (!out.is_open()) { + isc_throw(isc::BadValue, "can't open output file for writing: " + input_file); + } + + bool in_comment = false; + string line; + while (std::getline(f, line)) { + // First, let's get rid of the # comments + size_t hash_pos = line.find("#"); + if (hash_pos != string::npos) { + line = line.substr(0, hash_pos); + } + + // Second, let's get rid of the // comments + size_t dblslash_pos = line.find("//"); + if (dblslash_pos != string::npos) { + line = line.substr(0, dblslash_pos); + } + + // Now the tricky part: c comments. + size_t begin_pos = line.find("/*"); + size_t end_pos = line.find("*/"); + if (in_comment && end_pos == string::npos) { + // we continue through multiline comment + line = ""; + } else { + + if (begin_pos != string::npos) { + in_comment = true; + if (end_pos != string::npos) { + // sigle line comment. Let's get rid of the content in between + line = line.replace(begin_pos, end_pos + 2, end_pos + 2 - begin_pos, ' '); + in_comment = false; + } else { + line = line.substr(0, begin_pos); + } + } else { + if (in_comment && end_pos != string::npos) { + line = line.replace(0, end_pos +2 , end_pos + 2, ' '); + in_comment = false; + } + } + } -void testFile(const std::string& fname, bool print) { + // Finally, write the line to the output file. + out << line << endl; + } + f.close(); + out.close(); + + return (outfile); +} + +void testFile(const std::string& fname) { ElementPtr reference_json; ConstElementPtr test_json; - cout << "Attempting to load file " << fname << endl; + string decommented = decommentJSONfile(fname); - EXPECT_NO_THROW(reference_json = Element::fromJSONFile(fname, true)); + cout << "Attempting to load file " << fname << " (" << decommented + << ")" << endl; + + EXPECT_NO_THROW(reference_json = Element::fromJSONFile(decommented, true)); + + // remove the temporary file + EXPECT_NO_THROW(::remove(decommented.c_str())); EXPECT_NO_THROW( try { @@ -213,7 +293,7 @@ void testFile(const std::string& fname, bool print) { ASSERT_TRUE(reference_json); ASSERT_TRUE(test_json); - compareJSON(reference_json, test_json, print); + compareJSON(reference_json, test_json); } @@ -222,21 +302,21 @@ void testFile(const std::string& fname, bool print) { // twice: first with the existing Element::fromJSONFile() and then // the second time with Parser4. Both JSON trees are then compared. TEST(ParserTest, file) { - vector configs; - configs.push_back("backends.json"); - configs.push_back("classify.json"); - configs.push_back("dhcpv4-over-dhcpv6.json"); - configs.push_back("hooks.json"); - configs.push_back("leases-expiration.json"); - configs.push_back("multiple-options.json"); - configs.push_back("mysql-reservations.json"); - configs.push_back("pgsql-reservations.json"); - configs.push_back("reservations.json"); - configs.push_back("several-subnets.json"); - configs.push_back("single-subnet.json"); + vector configs = { "advanced.json" , + "backends.json", + "classify.json", + "dhcpv4-over-dhcpv6.json", + "hooks.json", + "leases-expiration.json", + "multiple-options.json", + "mysql-reservations.json", + "pgsql-reservations.json", + "reservations.json", + "several-subnets.json", + "single-subnet.json" }; for (int i = 0; igetType()); EXPECT_EQ("////", result->stringValue()); -} +} }; -- GitLab