From 6a1b17a579d2f75cca5fdef9570d0fad3ae817bc Mon Sep 17 00:00:00 2001
From: Stephen Morris <stephen@isc.org>
Date: Wed, 28 Oct 2015 11:27:12 +0000
Subject: [PATCH] [3259] Add test to check that mixed syntax element is
 rejected

---
 src/lib/dhcpsrv/parsers/dhcp_parsers.cc       |  6 ++-
 .../dhcpsrv/tests/dhcp_parsers_unittest.cc    | 43 ++++++++++++++++---
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
index 85bde6c8bd..1a0268ca43 100644
--- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
+++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
@@ -336,13 +336,15 @@ HooksLibrariesParser::build(ConstElementPtr value) {
             if (! lib_found) {
                 isc_throw(DhcpConfigError, "hooks library configuration error:"
                     " one or more hooks-libraries elements are missing the "
-                    " name of the library");
+                    " name of the library"  <<
+                    " (" << library_entry->getPosition() << ")");
             }
         }
     } else {
         isc_throw(DhcpConfigError, "hooks library configuration error:"
             " list of hooks libraries is not a list of maps, each map"
-            " containing a 'library' element");
+            " containing a 'library' element" <<
+            " (" << value->getPosition() << ")");
     }
 
     // Check if the list of libraries has changed.  If not, nothing is done
diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc
index 4e4639336e..d2b54f3f4f 100644
--- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc
+++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc
@@ -827,12 +827,13 @@ TEST_F(ParseConfigTest, optionDataMinimalWithOptionDef) {
 // Utility function for setting up the "hooks-libraries" configuration.
 //
 // Returns a hooks-libraries configuration element that contains zero to
-// four libraries, depending on what arguments are supplied.
+// four libraries, depending on what arguments are supplied.  The first
+/// three libraries have valid syntax, the last does not.
 //
 // This function uses the old syntax (Kea 0.9.1 and 0.9.2)
 std::string
 setHooksLibrariesConfig(const char* lib1 = NULL, const char* lib2 = NULL,
-                        const char* lib3 = NULL) {
+                        const char* lib3 = NULL, const char* lib4 = NULL) {
     const string quote("\"");
     const string comma_space(", ");
 
@@ -843,6 +844,10 @@ setHooksLibrariesConfig(const char* lib1 = NULL, const char* lib2 = NULL,
             config += (comma_space + quote + string(lib2) + quote);
             if (lib3 != NULL) {
                 config += (comma_space + quote + string(lib3) + quote);
+                if (lib4 != NULL) {
+                    config += comma_space + string("{ \"library\": ") + 
+                              quote + string(lib4) + quote + string(" }");
+                }
             }
         }
     }
@@ -858,9 +863,10 @@ setHooksLibrariesConfig(const char* lib1 = NULL, const char* lib2 = NULL,
 // lib1 - No parameters
 // lib2 - Empty parameters statement
 // lib3 - Valid parameters
+// lib4 - Invalid (old syntax)
 std::string
 setHooksLibrariesNewConfig(const char* lib1 = NULL, const char* lib2 = NULL,
-                           const char* lib3 = NULL) {
+                           const char* lib3 = NULL, const char* lib4 = NULL) {
     const string lbrace("{");
     const string rbrace("}");
     const string quote("\"");
@@ -892,6 +898,11 @@ setHooksLibrariesNewConfig(const char* lib1 = NULL, const char* lib2 = NULL,
                 config += string("    \"bvalue\": true");     // Boolean value
                 config += string("}");
                 config += rbrace;
+
+                if (lib4 != NULL) {
+                    // Library 4 is invalidly specified (as a plain string)
+                    config += comma_space + quote + string(lib4) + quote;
+                }
             }
         }
     }
@@ -915,7 +926,7 @@ setHooksLibrariesNewConfig(const char* lib1 = NULL, const char* lib2 = NULL,
 
 class HooksParseConfigTest : public ParseConfigTest,
     public ::testing::WithParamInterface<
-        std::string (*)(const char*, const char*, const char*)
+        std::string (*)(const char*, const char*, const char*, const char*)
     > {
 
 public:
@@ -936,13 +947,13 @@ public:
     // based on the number of libraries requested.
     std::string
     createConfig(const char* lib1 = NULL, const char* lib2 = NULL,
-		         const char* lib3 = NULL) {
-       return ((*generator_)(lib1, lib2, lib3));
+		         const char* lib3 = NULL, const char* lib4 = NULL) {
+       return ((*generator_)(lib1, lib2, lib3, lib4));
     }
 
     // Pointer to the configuration string generator function.
     std::string (*generator_)(const char* lib1, const char* lib2,
-                              const char* lib3);
+                              const char* lib3, const char* lib4);
 };
 
 };  // Anonymous namespace
@@ -1228,6 +1239,24 @@ TEST_P(HooksParseConfigTest, reconfigureInvalidHooksLibraries) {
     EXPECT_EQ(CALLOUT_LIBRARY_1, hooks_libraries[0]);
 }
 
+// Check that if the syntax is not valid (mixture of old and new syntaxes),
+// the parse will fail.
+TEST_P(HooksParseConfigTest, invalidHooksLibrarySyntax) {
+    // Create string with multiple valid libraries.  This should fail to
+    // parse.
+    string config = createConfig(CALLOUT_LIBRARY_1, CALLOUT_LIBRARY_2,
+                                 CALLOUT_LIBRARY_1, CALLOUT_LIBRARY_2);
+    int rcode = parseConfiguration(config);
+    ASSERT_NE(0, rcode);
+
+    // Check that it found that the error was down to the string not
+    // beiong a list of maps.
+    EXPECT_FALSE(error_text_.find("not a list of maps") == string::npos) <<
+        "Error text returned from parse failure is " << error_text_;
+    
+}
+
+
 INSTANTIATE_TEST_CASE_P(OldSyntax, HooksParseConfigTest,
                          ::testing::Values(setHooksLibrariesConfig));
 INSTANTIATE_TEST_CASE_P(NewSyntax, HooksParseConfigTest,
-- 
2.47.3