Merge branch 'master' of ssh://bind10.isc.org//var/bind10/git/bind10

src/lib/acl/Makefile.am

@@ -1,6 +1,15 @@
-SUBDIRS = tests
+SUBDIRS = . tests
 
-EXTRA_DIST = check.h acl.h
+AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
+AM_CPPFLAGS += $(BOOST_INCLUDES)
 
-# TODO: Once we have some cc file we are able to compile, create the library.
-# For now, we have only header files, not creating empty library.
+AM_CXXFLAGS = $(B10_CXXFLAGS)
+
+lib_LTLIBRARIES = libacl.la
+libacl_la_SOURCES = check.h acl.h
+libacl_la_SOURCES += loader.h loader.cc
+
+libacl_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
+libacl_la_LIBADD += $(top_builddir)/src/lib/cc/libcc.la
+
+CLEANFILES = *.gcno *.gcda

src/lib/acl/loader.cc

@@ -0,0 +1,46 @@
+// Copyright (C) 2011  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 "loader.h"
+
+using namespace std;
+
+namespace isc {
+namespace acl {
+
+BasicAction defaultActionLoader(data::ConstElementPtr actionEl) {
+    try {
+        const string action(actionEl->stringValue());
+        if (action == "ACCEPT") {
+            return (ACCEPT);
+        } else if (action == "REJECT") {
+            return (REJECT);
+        } else if (action == "DROP") {
+            return (DROP);
+        } else {
+            throw LoaderError(__FILE__, __LINE__,
+                              string("Unknown action '" + action + "'").
+                                  c_str(),
+                              actionEl);
+        }
+    }
+    catch (const data::TypeError&) {
+        throw LoaderError(__FILE__, __LINE__,
+                          "Invalid element type for action, must be string",
+                          actionEl);
+    }
+}
+
+}
+}

src/lib/acl/loader.h

@@ -0,0 +1,404 @@
+// Copyright (C) 2011  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 ACL_LOADER_H
+#define ACL_LOADER_H
+
+#include "acl.h"
+#include <cc/data.h>
+#include <boost/function.hpp>
+#include <boost/shared_ptr.hpp>
+#include <map>
+
+namespace isc {
+namespace acl {
+
+/**
+ * \brief Exception for bad ACL specifications.
+ *
+ * This will be thrown by the Loader if the ACL description is malformed
+ * in some way.
+ *
+ * It also can hold optional JSON element where was the error detected, so
+ * it can be examined.
+ *
+ * Checks may subclass this exception for similar errors if they see it fit.
+ */
+class LoaderError : public BadValue {
+private:
+    const data::ConstElementPtr element_;
+public:
+    /**
+     * \brief Constructor.
+     *
+     * Should be used with isc_throw if the fourth argument isn't used.
+     *
+     * \param file The file where the throw happened.
+     * \param line Similar as file, just for the line number.
+     * \param what Human readable description of what happened.
+     * \param element This might be passed to hold the JSON element where
+     *     the error was detected.
+     */
+    LoaderError(const char* file, size_t line, const char* what,
+                data::ConstElementPtr element = data::ConstElementPtr()) :
+        BadValue(file, line, what),
+        element_(element)
+    {}
+    ~ LoaderError() throw() {}
+    /**
+     * \brief Get the element.
+     *
+     * This returns the element where the error was detected. Note that it
+     * might be NULL in some situations.
+     */
+    const data::ConstElementPtr& element() const {
+        return (element_);
+    }
+};
+
+/**
+ * \brief Loader of the default actions of ACLs.
+ *
+ * Declared outside the Loader class, as this one does not need to be
+ * templated. This will throw LoaderError if the parameter isn't string
+ * or if it doesn't contain one of the accepted values.
+ *
+ * \param action The JSON representation of the action. It must be a string
+ *     and contain one of "ACCEPT", "REJECT" or "DENY".
+ * \note We could define different names or add aliases if needed.
+ */
+BasicAction defaultActionLoader(data::ConstElementPtr action);
+
+/**
+ * \brief Loader of ACLs.
+ *
+ * The goal of this class is to convert JSON description of an ACL to object
+ * of the ACL class (including the checks inside it).
+ *
+ * The class can be used to load the checks only. This is supposed to be used
+ * by compound checks to create the subexpressions.
+ *
+ * To allow any kind of checks to exist in the application, creators are
+ * registered for the names of the checks.
+ *
+ * An ACL definition looks like this:
+ * \verbatim
+ * [
+ *   {
+ *      "action": "ACCEPT",
+ *      "match-type": <parameter>
+ *   },
+ *   {
+ *      "action": "REJECT",
+ *      "match-type": <parameter>
+ *      "another-match-type": [<parameter1>, <parameter2>]
+*    },
+*    {
+*       "action": "DROP"
+*    }
+ * ]
+ * \endverbatim
+ *
+ * This is a list of elements. Each element must have an "action"
+ * entry/keyword. That one specifies which action is returned if this
+ * element matches (the value of the key is passed to the action loader
+ * (see the constructor). It may be any piece of JSON which the action
+ * loader expects.
+ *
+ * The rest of the element are matches. The left side is the name of the
+ * match type (for example match for source IP address or match for message
+ * size). The <parameter> is whatever is needed to describe the match and
+ * depends on the match type, the loader passes it verbatim to creator
+ * of that match type.
+ *
+ * There may be multiple match types in single element. In such case, all
+ * of the matches must match for the element to take action (so, in the second
+ * element, both "match-type" and "another-match-type" must be satisfied).
+ * If there's no match in the element, the action is taken/returned without
+ * conditions, every time (makes sense as the last entry, as the ACL will
+ * never get past it).
+ *
+ * The second entry shows another thing - if there's a list as the value
+ * for some match and the match itself is not expecting a list, it is taken
+ * as an "or" - a match for at last one of the choices in the list must match.
+ * So, for the second entry, both "match-type" and "another-match-type" must
+ * be satisfied, but the another one is satisfied by either parameter1 or
+ * parameter2.
+ */
+template<typename Context, typename Action = BasicAction> class Loader {
+public:
+    /**
+     * \brief Constructor.
+     *
+     * \param default_action The default action for created ACLs.
+     * \param actionLoader is the loader which will be used to convert actions
+     *     from their JSON representation. The default value is suitable for
+     *     the BasicAction enum. If you did not specify the second
+     *     template argument, you don't need to specify this loader.
+     */
+    Loader(const Action& defaultAction,
+           const boost::function1<Action, data::ConstElementPtr>
+               &actionLoader = &defaultActionLoader) :
+        default_action_(defaultAction),
+        action_loader_(actionLoader)
+    {}
+    /**
+     * \brief Creator of the checks.
+     *
+     * This can be registered within the Loader and will be used to create the
+     * checks. It is expected multiple creators (for multiple types, one can
+     * handle even multiple names) will be created and registered to support
+     * range of things we could check. This allows for customizing/extending
+     * the loader.
+     */
+    class CheckCreator {
+    public:
+        /**
+         * \brief List of names supported by this loader.
+         *
+         * List of all names for which this loader is able to create the
+         * checks. There can be multiple names, to support both aliases
+         * to the same checks and creators capable of creating multiple
+         * types of checks.
+         */
+        virtual std::vector<std::string> names() const = 0;
+        /**
+         * \brief Creates the check.
+         *
+         * This function does the actual creation. It is passed all the
+         * relevant data and is supposed to return shared pointer to the
+         * check.
+         *
+         * It is expected to throw the LoaderError exception when the
+         * definition is invalid.
+         *
+         * \param name The type name of the check. If the creator creates
+         *     only one type of check, it can safely ignore this parameter.
+         * \param definition The part of JSON describing the parameters of
+         *     check. As there's no way for the loader to know how the
+         *     parameters might look like, they are not checked in any way.
+         *     Therefore it's up to the creator (or the check being created)
+         *     to validate the data and throw if it is bad.
+         * \param Current loader calling this creator. This can be used
+         *     to load subexpressions in case of compound check.
+         */
+        virtual boost::shared_ptr<Check<Context> > create(
+            const std::string& name, data::ConstElementPtr definition,
+            const Loader<Context, Action>& loader) = 0;
+        /**
+         * \brief Is list or-abbreviation allowed?
+         *
+         * If this returns true and the parameter (eg. the value we check
+         * against, the one that is passed as the second parameter of create)
+         * is list, the loader will call the create method with each element of
+         * the list and aggregate all the results in OR compound check. If it
+         * is false, the parameter is passed verbatim no matter if it is or
+         * isn't a list. For example, IP check will have this as true (so
+         * multiple IP addresses can be passed as options), but AND operator
+         * will return false and handle the list of subexpressions itself.
+         *
+         * The rationale behind this is that it is common to specify list of
+         * something that matches (eg. list of IP addresses).
+         */
+        virtual bool allowListAbbreviation() const {
+            return (true);
+        }
+    };
+    /**
+     * \brief Register another check creator.
+     *
+     * Adds a creator to the list of known ones. The creator's list of names
+     * must be disjoint with the names already known to the creator or the
+     * LoaderError exception is thrown. In such case, the creator is not
+     * registered under any of the names. In case of other exceptions, like
+     * bad_alloc, only weak exception safety is guaranteed.
+     *
+     * \param creator Shared pointer to the creator.
+     * \note We don't support deregistration yet, but it is expected it will
+     *     be needed in future, when we have some kind of plugins. These
+     *     plugins might want to unload, in which case they would need to
+     *     deregister their creators. It is expected they would pass the same
+     *     pointer to such method as they pass here.
+     */
+    void registerCreator(boost::shared_ptr<CheckCreator> creator) {
+        // First check we can insert all the names
+        typedef std::vector<std::string> Strings;
+        const Strings names(creator->names());
+        for (Strings::const_iterator i(names.begin()); i != names.end();
+             ++i) {
+            if (creators_.find(*i) != creators_.end()) {
+                isc_throw(LoaderError, "The loader already contains creator "
+                          "named " << *i);
+            }
+        }
+        // Now insert them
+        for (Strings::const_iterator i(names.begin()); i != names.end();
+             ++i) {
+            creators_[*i] = creator;
+        }
+    }
+    /**
+     * \brief Load a check.
+     *
+     * This parses a check dict (block, the one element of ACL) and calls a
+     * creator (or creators, if more than one check is found inside) for it. It
+     * ignores the "action" key, as it is a reserved keyword used to specify
+     * actions inside the ACL.
+     *
+     * This may throw LoaderError if it is not a dict or if some of the type
+     * names is not known (there's no creator registered for it). The
+     * exceptions from creators aren't caught.
+     *
+     * \param description The JSON description of the check.
+     */
+    boost::shared_ptr<Check<Context> > loadCheck(const data::ConstElementPtr&
+                                                 description)
+    {
+        // Get the description as a map
+        typedef std::map<std::string, data::ConstElementPtr> Map;
+        Map map;
+        try {
+            map = description->mapValue();
+        }
+        catch (const data::TypeError&) {
+            isc_throw_1(LoaderError, "Check description is not a map",
+                        description);
+        }
+        // Call the internal part with extracted map
+        return (loadCheck(description, map));
+    }
+    /**
+     * \brief Load an ACL.
+     *
+     * This parses an ACL list, creates the checks and actions of each element
+     * and returns it. It may throw LoaderError if it isn't a list or the
+     * "action" key is missing in some element. Also, no exceptions from
+     * loadCheck (therefore from whatever creator is used) and from the
+     * actionLoader passed to constructor are not caught.
+     *
+     * \param description The JSON list of ACL.
+     */
+    boost::shared_ptr<ACL<Context, Action> > load(const data::ConstElementPtr&
+                                                  description)
+    {
+        // We first check it's a list, so we can use the list reference
+        // (the list may be huge)
+        if (description->getType() != data::Element::list) {
+            isc_throw_1(LoaderError, "ACL not a list", description);
+        }
+        // First create an empty ACL
+        const List &list(description->listValue());
+        boost::shared_ptr<ACL<Context, Action> > result(
+            new ACL<Context, Action>(default_action_));
+        // Run trough the list of elements
+        for (List::const_iterator i(list.begin()); i != list.end(); ++i) {
+            Map map;
+            try {
+                map = (*i)->mapValue();
+            }
+            catch (const data::TypeError&) {
+                isc_throw_1(LoaderError, "ACL element not a map", *i);
+            }
+            // Create an action for the element
+            const Map::const_iterator action(map.find("action"));
+            if (action == map.end()) {
+                isc_throw_1(LoaderError, "No action in ACL element", *i);
+            }
+            const Action acValue(action_loader_(action->second));
+            // Now create the check if there's one
+            if (map.size() >= 2) { // One is the action, another one the check
+                result->append(loadCheck(*i, map), acValue);
+            } else {
+                // In case there's no check, this matches every time. We
+                // simulate it by our own private "True" check.
+                result->append(boost::shared_ptr<Check<Context> >(new True()),
+                               acValue);
+            }
+        }
+        return (result);
+    }
+private:
+    // Some type aliases to save typing
+    typedef std::map<std::string, boost::shared_ptr<CheckCreator> > Creators;
+    typedef std::map<std::string, data::ConstElementPtr> Map;
+    typedef std::vector<data::ConstElementPtr> List;
+    // Private members
+    Creators creators_;
+    const Action default_action_;
+    const boost::function1<Action, data::ConstElementPtr> action_loader_;
+    /**
+     * \brief Internal version of loadCheck.
+     *
+     * This is the internal part, shared between load and loadCheck.
+     * \param description The bit of JSON (used in exceptions).
+     * \param map The extracted map describing the check. It does change
+     *     the map.
+     */
+    boost::shared_ptr<Check<Context> > loadCheck(const data::ConstElementPtr&
+                                                 description, Map& map)
+    {
+        // Remove the action keyword
+        map.erase("action");
+        // Now, do we have any definition? Or is it and abbreviation?
+        switch (map.size()) {
+            case 0:
+                isc_throw_1(LoaderError, "Check description is empty",
+                            description);
+            case 1: {
+                // Get the first and only item
+                const Map::const_iterator checkDesc(map.begin());
+                const std::string& name(checkDesc->first);
+                const typename Creators::const_iterator
+                    creatorIt(creators_.find(name));
+                if (creatorIt == creators_.end()) {
+                    isc_throw_1(LoaderError, "No creator for ACL check " <<
+                                name, description);
+                }
+                if (creatorIt->second->allowListAbbreviation() &&
+                    checkDesc->second->getType() == data::Element::list) {
+                    isc_throw_1(LoaderError,
+                                "Not implemented (OR-abbreviated form)",
+                                checkDesc->second);
+                }
+                // Create the check and return it
+                return (creatorIt->second->create(name, checkDesc->second,
+                                                  *this));
+            }
+            default:
+                isc_throw_1(LoaderError,
+                            "Not implemented (AND-abbreviated form)",
+                            description);
+        }
+    }
+    /**
+     * \brief Check that always matches.
+     *
+     * This one is used internally for ACL elements without condition. We may
+     * want to make this publicly accesible sometime maybe, but for now,
+     * there's no need.
+     */
+    class True : public Check<Context> {
+    public:
+        virtual bool matches(const Context&) const { return (true); };
+        virtual unsigned cost() const { return (1); }
+        // We don't write "true" here, as this one was created using empty
+        // input
+        virtual std::string toText() const { return ""; }
+    };
+};
+
+}
+}
+
+#endif

src/lib/acl/tests/Makefile.am

@@ -5,12 +5,13 @@ TESTS =
 if HAVE_GTEST
 TESTS += run_unittests
 run_unittests_SOURCES = run_unittests.cc
-run_unittests_SOURCES += check_test.cc acl_test.cc
+run_unittests_SOURCES += check_test.cc acl_test.cc loader_test.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 
 run_unittests_LDADD = $(GTEST_LDADD)
 run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
+run_unittests_LDADD += $(top_builddir)/src/lib/acl/libacl.la
 endif
 
 noinst_PROGRAMS = $(TESTS)

src/lib/acl/tests/acl_test.cc

@@ -12,75 +12,11 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <gtest/gtest.h>
-#include <acl/acl.h>
-#include <cassert>
-
-using namespace isc::acl;
-using boost::shared_ptr;
+#include "logcheck.h"
 
 namespace {
 
-// This is arbitrary guess of size for the log. If it's too small for your
-// test, just make it bigger.
-const size_t LOG_SIZE = 10;
-
-// This will remember which checks did run already.
-struct Log {
-    // The actual log cells, if i-th check did run
-    mutable bool run[LOG_SIZE];
-    Log() {
-        // Nothing run yet
-        for (size_t i(0); i < LOG_SIZE; ++i) {
-            run[i] = false;
-        }
-    }
-    // Checks that the first amount of checks did run and the rest didn't.
-    void checkFirst(size_t amount) const {
-        ASSERT_LE(amount, LOG_SIZE) << "Wrong test: amount bigger than size "
-            "of log";
-        {
-            SCOPED_TRACE("Checking that the first amount of checks did run");
-            for (size_t i(0); i < amount; ++i) {
-                EXPECT_TRUE(run[i]) << "Check #" << i << " did not run.";
-            }
-        }
-
-        {
-            SCOPED_TRACE("Checking that the rest did not run");
-            for (size_t i(amount); i < LOG_SIZE; ++i) {
-                EXPECT_FALSE(run[i]) << "Check #" << i << "did run.";
-            }
-        }
-    }
-};
-
-// This returns true or false every time, no matter what is passed to it.
-// But it logs that it did run.
-class ConstCheck : public Check<Log> {
-public:
-    ConstCheck(bool accepts, size_t log_num) :
-        log_num_(log_num),
-        accepts_(accepts)
-    {
-        assert(log_num < LOG_SIZE); // If this fails, the LOG_SIZE is too small
-    }
-    /*
-     * This use of mutable log context is abuse for testing purposes.
-     * It is expected that the context will not be modified in the real
-     * applications of ACLs, but we want to know which checks were called
-     * and this is an easy way.
-     */
-    virtual bool matches(const Log& log) const {
-        log.run[log_num_] = true;
-        return (accepts_);
-    }
-private:
-    size_t log_num_;
-    bool accepts_;
-};
-
-// Test version of the ACL class. It adds few methods to examine the protected
+// Test version of the Acl class. It adds few methods to examine the protected
 // data, but does not change the implementation.
 class TestACL : public ACL<Log> {
 public:

src/lib/acl/tests/loader_test.cc

@@ -0,0 +1,426 @@
+// Copyright (C) 2011  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 "logcheck.h"
+#include <acl/loader.h>
+#include <string>
+#include <gtest/gtest.h>
+
+using namespace std;
+using namespace boost;
+using isc::data::ConstElementPtr;
+
+namespace {
+
+// Just for convenience, create JSON objects from JSON string
+ConstElementPtr el(const string& JSON) {
+    return (isc::data::Element::fromJSON(JSON));
+}
+
+// We don't use the EXPECT_THROW macro, as it doesn't allow us
+// to examine the exception. We want to check the element is stored
+// there as well.
+void testActionLoaderException(const string& JSON) {
+    SCOPED_TRACE("Should throw with input: " + JSON);
+    ConstElementPtr elem(el(JSON));
+    try {
+        defaultActionLoader(elem);
+        FAIL() << "It did not throw";
+    }
+    catch (const LoaderError& error) {
+        // Yes, comparing for pointer equality, that is enough, it
+        // should return the exact instance of the JSON object
+        EXPECT_EQ(elem, error.element());
+    }
+}
+
+// Test the defaultActionLoader function
+TEST(LoaderHelpers, DefaultActionLoader) {
+    // First the three valid inputs
+    EXPECT_EQ(ACCEPT, defaultActionLoader(el("\"ACCEPT\"")));
+    EXPECT_EQ(REJECT, defaultActionLoader(el("\"REJECT\"")));
+    EXPECT_EQ(DROP, defaultActionLoader(el("\"DROP\"")));
+    // Now few invalid ones
+    // String, but unknown one
+    testActionLoaderException("\"UNKNOWN\"");
+    testActionLoaderException("42");
+    testActionLoaderException("true");
+    testActionLoaderException("null");
+    testActionLoaderException("[]");
+    testActionLoaderException("{}");
+}
+
+// A check that doesn't check anything but remembers it's own name
+// and data
+class NamedCheck : public Check<Log> {
+public:
+    NamedCheck(const string& name, ConstElementPtr data) :
+        name_(name),
+        data_(data)
+    {}
+    virtual bool matches(const Log&) const { return (true); }
+    const string name_;
+    const ConstElementPtr data_;
+};
+
+// The creator of NamedCheck
+class NamedCreator : public Loader<Log>::CheckCreator {
+public:
+    NamedCreator(const string& name, bool abbreviatedList = true) :
+        abbreviated_list_(abbreviatedList)
+    {
+        names_.push_back(name);
+    }
+    NamedCreator(const vector<string>& names) :
+        names_(names),
+        abbreviated_list_(true)
+    {}
+    vector<string> names() const {
+        return (names_);
+    }
+    shared_ptr<Check<Log> > create(const string& name, ConstElementPtr data,
+                                   const Loader<Log>&)
+    {
+        bool found(false);
+        for (vector<string>::const_iterator i(names_.begin());
+             i != names_.end(); ++i) {
+            if (*i == name) {
+                found = true;
+                break;
+            }
+        }
+        EXPECT_TRUE(found) << "Name " << name << " passed to creator which "
+            "doesn't handle it.";
+        return (shared_ptr<Check<Log> >(new NamedCheck(name, data)));
+    }
+    bool allowListAbbreviation() const {
+        return (abbreviated_list_);
+    }
+private:
+    vector<string> names_;
+    const bool abbreviated_list_;
+};
+
+// To be thrown in tests internally
+class TestCreatorError {};
+
+// This will throw every time it should create something
+class ThrowCreator : public Loader<Log>::CheckCreator {
+public:
+    vector<string> names() const {
+        vector<string> result;
+        result.push_back("throw");
+        return (result);
+    }
+    shared_ptr<Check<Log> > create(const string&, ConstElementPtr,
+                                   const Loader<Log>&)
+    {
+        throw TestCreatorError();
+    }
+};
+
+// This throws whenever the match is called on it
+class ThrowCheck : public Check<Log> {
+public:
+    virtual bool matches(const Log&) const {
+        throw TestCreatorError();
+    }
+};
+
+// And creator for it
+class ThrowCheckCreator : public Loader<Log>::CheckCreator {
+public:
+    vector<string> names() const {
+        vector<string> result;
+        result.push_back("throwcheck");
+        return (result);
+    }
+    shared_ptr<Check<Log> > create(const string&, ConstElementPtr,
+                                   const Loader<Log>&)
+    {
+        return (shared_ptr<Check<Log> >(new ThrowCheck()));
+    }
+};
+
+class LogCreator : public Loader<Log>::CheckCreator {
+public:
+    vector<string> names() const {
+        vector<string> result;
+        result.push_back("logcheck");
+        return (result);
+    }
+    /*
+     * For simplicity, we just take two values as a list, first is the
+     * logging cell used, the second is result of the check. No error checking
+     * is done, if there's bug in the test, it will throw TypeError for us.
+     */
+    shared_ptr<Check<Log> > create(const string&, ConstElementPtr definition,
+                                   const Loader<Log>&)
+    {
+        vector<ConstElementPtr> list(definition->listValue());
+        int logpos(list[0]->intValue());
+        bool accept(list[1]->boolValue());
+        return (shared_ptr<ConstCheck>(new ConstCheck(accept, logpos)));
+    }
+    // We take a list, so don't interpret it for us
+    virtual bool allowListAbbreviation() const { return (false); }
+};
+
+class LoaderTest : public ::testing::Test {
+public:
+    LoaderTest() :
+        loader_(REJECT)
+    {}
+    Loader<Log> loader_;
+    Log log_;
+    // Some convenience functions to set up
+
+    // Create a NamedCreator, convert to shared pointer
+    shared_ptr<NamedCreator> namedCreator(const string& name,
+                                          bool abbreviatedList = true)
+    {
+        return (shared_ptr<NamedCreator>(new NamedCreator(name,
+                                                          abbreviatedList)));
+    }
+    // Create and add a NamedCreator
+    void addNamed(const string& name, bool abbreviatedList = true) {
+        EXPECT_NO_THROW(loader_.registerCreator(
+            namedCreator(name, abbreviatedList)));
+    }
+    // Load a check and convert it to named check to examine it
+    shared_ptr<NamedCheck> loadCheck(const string& definition) {
+        SCOPED_TRACE("Loading check " + definition);
+        shared_ptr<Check<Log> > loaded;
+        EXPECT_NO_THROW(loaded = loader_.loadCheck(el(definition)));
+        shared_ptr<NamedCheck> result(dynamic_pointer_cast<NamedCheck>(
+            loaded));
+        EXPECT_TRUE(result);
+        return (result);
+    }
+    // The loadCheck throws an exception
+    void checkException(const string& JSON) {
+        SCOPED_TRACE("Loading check exception: " + JSON);
+        ConstElementPtr input(el(JSON));
+        // Not using EXPECT_THROW, we want to examine the exception
+        try {
+            loader_.loadCheck(input);
+            FAIL() << "Should have thrown";
+        }
+        catch (const LoaderError& e) {
+            // It should be identical copy, so checking pointers
+            EXPECT_EQ(input, e.element());
+        }
+    }
+    // Insert the throw, throwcheck and logcheck checks into the loader
+    void aclSetup() {
+        try {
+            loader_.registerCreator(shared_ptr<ThrowCreator>(new
+                                                             ThrowCreator()));
+            loader_.registerCreator(shared_ptr<ThrowCheckCreator>(
+                new ThrowCheckCreator()));
+            loader_.registerCreator(shared_ptr<LogCreator>(new LogCreator()));
+        }
+        // We ignore this exception here, because it happens when we try to
+        // insert the creators multiple times. This is harmless.
+        catch (const LoaderError&) {}
+    }
+    // Create an ACL, run it, check it's result and how many first
+    // log items it marked
+    //
+    // Works with preset names throw and logcheck
+    void aclRun(const string& JSON, BasicAction expectedResult,
+                size_t logged)
+    {
+        SCOPED_TRACE("Running ACL for " + JSON);
+        aclSetup();
+        shared_ptr<ACL<Log> > acl;
+        EXPECT_NO_THROW(acl = loader_.load(el(JSON)));
+        EXPECT_EQ(expectedResult, acl->execute(log_));
+        log_.checkFirst(logged);
+    }
+    // Check it throws an error when creating the ACL
+    void aclException(const string& JSON) {
+        SCOPED_TRACE("Trying to load bad " + JSON);
+        aclSetup();
+        EXPECT_THROW(loader_.load(el(JSON)), LoaderError);
+    }
+};
+
+// Test that it does not accept duplicate creator
+TEST_F(LoaderTest, CreatorDuplicity) {
+    addNamed("name");
+    EXPECT_THROW(loader_.registerCreator(namedCreator("name")), LoaderError);
+}
+
+// Test that when it does not accept a duplicate, nothing is inserted
+TEST_F(LoaderTest, CreatorDuplicateUnchanged) {
+    addNamed("name1");
+    vector<string> names;
+    names.push_back("name2");
+    names.push_back("name1");
+    names.push_back("name3");
+    EXPECT_THROW(loader_.registerCreator(
+        shared_ptr<NamedCreator>(new NamedCreator(names))), LoaderError);
+    // It should now reject both name2 and name3 as not known
+    checkException("{\"name2\": null}");
+    checkException("{\"name3\": null}");
+}
+
+// Test that we can register a creator and load a check with the name
+TEST_F(LoaderTest, SimpleCheckLoad) {
+    addNamed("name");
+    shared_ptr<NamedCheck> check(loadCheck("{\"name\": 42}"));
+    EXPECT_EQ("name", check->name_);
+    EXPECT_TRUE(check->data_->equals(*el("42")));
+}
+
+// As above, but there are multiple creators registered within the loader
+TEST_F(LoaderTest, MultiCreatorCheckLoad) {
+    addNamed("name1");
+    addNamed("name2");
+    shared_ptr<NamedCheck> check(loadCheck("{\"name2\": 42}"));
+    EXPECT_EQ("name2", check->name_);
+    EXPECT_TRUE(check->data_->equals(*el("42")));
+}
+
+// Similar to above, but there's a creator with multiple names
+TEST_F(LoaderTest, MultiNameCheckLoad) {
+    addNamed("name1");
+    vector<string> names;
+    names.push_back("name2");
+    names.push_back("name3");
+    EXPECT_NO_THROW(loader_.registerCreator(shared_ptr<NamedCreator>(
+        new NamedCreator(names))));
+    shared_ptr<NamedCheck> check(loadCheck("{\"name3\": 42}"));
+    EXPECT_EQ("name3", check->name_);
+    EXPECT_TRUE(check->data_->equals(*el("42")));
+}
+
+// Invalid format is rejected
+TEST_F(LoaderTest, InvalidFormatCheck) {
+    checkException("[]");
+    checkException("42");
+    checkException("\"hello\"");
+    checkException("null");
+}
+
+// Empty check is rejected
+TEST_F(LoaderTest, EmptyCheck) {
+    checkException("{}");
+}
+
+// The name isn't known
+TEST_F(LoaderTest, UnkownName) {
+    checkException("{\"unknown\": null}");
+}
+
+// Exception from the creator is propagated
+TEST_F(LoaderTest, CheckPropagate) {
+    loader_.registerCreator(shared_ptr<ThrowCreator>(new ThrowCreator()));
+    EXPECT_THROW(loader_.loadCheck(el("{\"throw\": null}")), TestCreatorError);
+}
+
+// The abbreviated form is not yet implemented
+// (we need the operators to be implemented)
+TEST_F(LoaderTest, AndAbbrev) {
+    addNamed("name1");
+    addNamed("name2");
+    EXPECT_THROW(loader_.loadCheck(el("{\"name1\": 1, \"name2\": 2}")),
+                 LoaderError);
+}
+
+TEST_F(LoaderTest, OrAbbrev) {
+    addNamed("name1");
+    EXPECT_THROW(loader_.loadCheck(el("{\"name1\": [1, 2]}")),
+                 LoaderError);
+}
+
+// But this is not abbreviated form, this should be passed directly to the
+// creator
+TEST_F(LoaderTest, ListCheck) {
+    addNamed("name1", false);
+    shared_ptr<NamedCheck> check(loadCheck("{\"name1\": [1, 2]}"));
+    EXPECT_EQ("name1", check->name_);
+    EXPECT_TRUE(check->data_->equals(*el("[1, 2]")));
+}
+
+// Check the action key is ignored as it should be
+TEST_F(LoaderTest, CheckNoAction) {
+    addNamed("name1");
+    shared_ptr<NamedCheck> check(loadCheck("{\"name1\": 1, \"action\": 2}"));
+    EXPECT_EQ("name1", check->name_);
+    EXPECT_TRUE(check->data_->equals(*el("1")));
+}
+
+// The empty ACL can be created and run, providing the default action
+TEST_F(LoaderTest, EmptyACL) {
+    aclRun("[]", REJECT, 0);
+}
+
+// We can create a simple ACL, which will return the correct default
+// action
+TEST_F(LoaderTest, NoMatchACL) {
+    aclRun("[{\"logcheck\": [0, false], \"action\": \"ACCEPT\"}]",
+           REJECT, 1);
+}
+
+// We can created more complicated ACL, it will match at the second
+// check
+TEST_F(LoaderTest, MatchACL) {
+    aclRun("["
+           "  {\"logcheck\": [0, false], \"action\": \"DROP\"},"
+           "  {\"logcheck\": [1, true], \"action\": \"ACCEPT\"}"
+           "]", ACCEPT, 2);
+}
+
+// ACL without a check (matches unconditionally)
+// We add another one check after it, to make sure it is really not run
+TEST_F(LoaderTest, NoCheckACL) {
+    aclRun("["
+           "  {\"action\": \"DROP\"},"
+           "  {\"throwcheck\": 1, \"action\": \"ACCEPT\"}"
+           "]", DROP, 0);
+}
+
+// Malformed things are rejected
+TEST_F(LoaderTest, InvalidACLFormat) {
+    // Not a list
+    aclException("{}");
+    aclException("42");
+    aclException("true");
+    aclException("null");
+    aclException("\"hello\"");
+    // Malformed element
+    aclException("[42]");
+    aclException("[\"hello\"]");
+    aclException("[[]]");
+    aclException("[true]");
+    aclException("[null]");
+}
+
+// If there's no action keyword, it is rejected
+TEST_F(LoaderTest, NoAction) {
+    aclException("[{}]");
+    aclException("[{\"logcheck\": [0, true]}]");
+}
+
+// Exceptions from check creation is propagated
+TEST_F(LoaderTest, ACLPropagate) {
+    aclSetup();
+    EXPECT_THROW(loader_.load(el("[{\"action\": \"ACCEPT\", \"throw\": 1}]")),
+                 TestCreatorError);
+
+}
+
+}

src/lib/acl/tests/logcheck.h

@@ -0,0 +1,86 @@
+// Copyright (C) 2011  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 <gtest/gtest.h>
+#include <acl/acl.h>
+#include <cassert>
+
+// This is not a public header, it is used only inside the tests. Therefore
+// we lower the standards a bit and use anonymous namespace in the header
+// and "using", just for convenience. This is just to share little bit of code
+// between multiple tests.
+using namespace isc::acl;
+using boost::shared_ptr;
+
+namespace {
+
+// This is arbitrary guess of size for the log. If it's too small for your
+// test, just make it bigger.
+const size_t LOG_SIZE = 10;
+
+// This will remember which checks did run already.
+struct Log {
+    // The actual log cells, if i-th check did run
+    mutable bool run[LOG_SIZE];
+    Log() {
+        // Nothing run yet
+        for (size_t i(0); i < LOG_SIZE; ++ i) {
+            run[i] = false;
+        }
+    }
+    // Checks that the first amount of checks did run and the rest didn't.
+    void checkFirst(size_t amount) const {
+        ASSERT_LE(amount, LOG_SIZE) << "Wrong test: amount bigger than size "
+            "of log";
+        {
+            SCOPED_TRACE("Checking that the first amount of checks did run");
+            for (size_t i(0); i < amount; ++ i) {
+                EXPECT_TRUE(run[i]) << "Check #" << i << " did not run.";
+            }
+        }
+
+        {
+            SCOPED_TRACE("Checking that the rest did not run");
+            for (size_t i(amount); i < LOG_SIZE; ++ i) {
+                EXPECT_FALSE(run[i]) << "Check #" << i << "did run.";
+            }
+        }
+    }
+};
+
+// This returns true or false every time, no matter what is passed to it.
+// But it logs that it did run.
+class ConstCheck : public Check<Log> {
+public:
+    ConstCheck(bool accepts, size_t logNum) :
+        logNum_(logNum),
+        accepts_(accepts)
+    {
+        assert(logNum < LOG_SIZE); // If this fails, the LOG_SIZE is too small
+    }
+    virtual bool matches(const Log& log) const {
+        /*
+         * This is abuse of the context. It is designed to carry the
+         * information to check, not to modify it. However, this is the
+         * easiest way to do the test, so we go against the design.
+         */
+        log.run[logNum_] = true;
+        return (accepts_);
+    }
+private:
+    size_t logNum_;
+    bool accepts_;
+};
+
+}

src/lib/exceptions/exceptions.h

@@ -163,6 +163,17 @@ public:
         oss__ << stream; \
         throw type(__FILE__, __LINE__, oss__.str().c_str()); \
     } while (1)
+
+///
+/// Similar as isc_throw, but allows the exception to have one additional
+/// parameter (the stream/text goes first)
+#define isc_throw_1(type, stream, param1) \
+    do { \
+        std::ostringstream oss__; \
+        oss__ << stream; \
+        throw type(__FILE__, __LINE__, oss__.str().c_str(), param1); \
+    } while (1)
+
 }
 #endif // __EXCEPTIONS_H