BIND 10 trac772, updated. df79b8d3306394ae123fb4c558f7239146e9f0d6 [trac772] Logging

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Jul 13 12:49:31 UTC 2011


The branch, trac772 has been updated
  discards  37e8a3fb0129dea790d371aaa416686d66bb6eb5 (commit)
  discards  bb0f6cff9d043993fe537a9ba2a3c874781f4a1b (commit)
  discards  3b780b61f6f586c0a6bbe5586f3d0c3e72a57e13 (commit)
  discards  1188ca7fd05a8c241e4905d168144ffa5dd29db1 (commit)
  discards  80bf2d9bd104c9664f322bf1b92538fefacc7633 (commit)
       via  df79b8d3306394ae123fb4c558f7239146e9f0d6 (commit)
       via  6d784213ea929dfa06099d7d85ed87709a7f408e (commit)
       via  e77575c3c85c7e219137b2c616ad104e5b28eb20 (commit)
       via  49f1d2d2e7f75432465ddd4acae2579c018aab33 (commit)
       via  ed9c17ed1627872d701c76336aff407d3ad5c44e (commit)
       via  b0e38303e79e2a487e37a9dcadd5f1730cdeae9e (commit)
       via  93145c09728dbfb7fe5bd77b5a3671e911c41deb (commit)
       via  1c1bd99f0add79535b62f6723d7e942661007653 (commit)
       via  1d03e4212cffa7fcf57d0f3a4fcdc1920c959e40 (commit)
       via  cca39b307de50546d7e3c4cd9fe4c2435223bf21 (commit)
       via  dffeeebd09195ad602090501c8c9b05b55885596 (commit)
       via  673a619cd628130b0506a5d3669fd6a4d139c790 (commit)
       via  f8092952b50ef238e2ffc63ccb6d17a469f22966 (commit)
       via  7cb53c7b33c41bc8c5d76c6994caae800692108d (commit)
       via  d0df4daafee6703a7b52609b5681846f83310182 (commit)
       via  d23f84732df2786fad5bf31f3446e0e088d941ec (commit)
       via  fd2daaa2c1a27140568cf5a4f04baf57682214d2 (commit)
       via  78942e3fc11f22f1bdbbd8fdd629691d5c510a55 (commit)
       via  8945eccce758dd466ac42c6521a3fc4ada5a9226 (commit)
       via  f29890eed7bad4aead5e95cfa6aae147287a0b10 (commit)
       via  7469b1f920d47306f87aab0e2fa0533903bc61af (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (37e8a3fb0129dea790d371aaa416686d66bb6eb5)
            \
             N -- N -- N (df79b8d3306394ae123fb4c558f7239146e9f0d6)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit df79b8d3306394ae123fb4c558f7239146e9f0d6
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Wed Jul 13 13:07:05 2011 +0200

    [trac772] Logging

commit 6d784213ea929dfa06099d7d85ed87709a7f408e
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Wed Jul 13 13:06:30 2011 +0200

    [trac772] Send the response based on ACL

commit e77575c3c85c7e219137b2c616ad104e5b28eb20
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Wed Jul 13 12:49:48 2011 +0200

    [trac772] Perform the ACL check

commit 49f1d2d2e7f75432465ddd4acae2579c018aab33
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Wed Jul 13 12:08:54 2011 +0200

    [trac772] Propagate the ACL as well

commit ed9c17ed1627872d701c76336aff407d3ad5c44e
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Wed Jul 13 11:55:04 2011 +0200

    [trac772] Propagate the remote endpoint to XfrOut session

commit b0e38303e79e2a487e37a9dcadd5f1730cdeae9e
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Mon Jul 11 14:55:26 2011 +0200

    [trac772] Loading of ACL from configuration

commit 93145c09728dbfb7fe5bd77b5a3671e911c41deb
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Mon Jul 11 14:39:50 2011 +0200

    [trac772] Default ACL

commit 1c1bd99f0add79535b62f6723d7e942661007653
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Mon Jul 11 13:57:24 2011 +0200

    [trac772] Add ACL to spec file

commit 1d03e4212cffa7fcf57d0f3a4fcdc1920c959e40
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Mon Jul 11 13:46:54 2011 +0200

    [trac772] Comment cleanup

-----------------------------------------------------------------------

Summary of changes:
 src/bin/xfrout/tests/xfrout_test.py.in             |   28 ++-
 src/bin/xfrout/xfrout.py.in                        |   38 +++-
 src/bin/xfrout/xfrout_messages.mes                 |    8 +
 src/lib/acl/loader.h                               |   30 ++--
 src/lib/python/isc/acl/Makefile.am                 |    8 +-
 src/lib/python/isc/acl/acl.cc                      |   29 ++-
 src/lib/python/isc/acl/acl_inc.cc                  |   16 ++
 src/lib/python/isc/acl/dns.cc                      |   93 +++++---
 src/lib/python/isc/acl/dns.h                       |   52 ++++
 src/lib/python/isc/acl/dns_requestacl_inc.cc       |    4 +-
 src/lib/python/isc/acl/dns_requestacl_python.cc    |   10 +-
 .../python/isc/acl/dns_requestcontext_python.cc    |    5 +-
 src/lib/python/isc/acl/dns_requestloader_inc.cc    |   86 +++++++
 src/lib/python/isc/acl/dns_requestloader_python.cc |  255 ++++++++++++++++++++
 .../isc/acl/{acl.h => dns_requestloader_python.h}  |   23 ++-
 src/lib/python/isc/acl/dnsacl_inc.cc               |   28 +--
 src/lib/python/isc/acl/tests/dns_test.py           |  182 ++++++++++----
 src/lib/util/python/pycppwrapper_util.h            |   29 +++-
 src/lib/util/python/wrapper_template.cc            |    2 +-
 19 files changed, 767 insertions(+), 159 deletions(-)
 create mode 100644 src/lib/python/isc/acl/acl_inc.cc
 create mode 100644 src/lib/python/isc/acl/dns.h
 create mode 100644 src/lib/python/isc/acl/dns_requestloader_inc.cc
 create mode 100644 src/lib/python/isc/acl/dns_requestloader_python.cc
 rename src/lib/python/isc/acl/{acl.h => dns_requestloader_python.h} (69%)

-----------------------------------------------------------------------
diff --git a/src/bin/xfrout/tests/xfrout_test.py.in b/src/bin/xfrout/tests/xfrout_test.py.in
index c361777..b20e090 100644
--- a/src/bin/xfrout/tests/xfrout_test.py.in
+++ b/src/bin/xfrout/tests/xfrout_test.py.in
@@ -118,7 +118,10 @@ class TestXfroutSession(unittest.TestCase):
     def setUp(self):
         self.sock = MySocket(socket.AF_INET,socket.SOCK_STREAM)
         self.xfrsess = MyXfroutSession(self.sock, None, Dbserver(),
-                                       TSIGKeyRing(), ('127.0.0.1', 12345))
+                                       TSIGKeyRing(), ('127.0.0.1', 12345),
+                                       # When not testing ACLs, simply accept
+                                       isc.acl.dns.REQUEST_LOADER.load(
+                                           [{"action": "ACCEPT"}]))
         self.mdata = bytes(b'\xd6=\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x07example\x03com\x00\x00\xfc\x00\x01')
         self.soa_record = (4, 3, 'example.com.', 'com.example.', 3600, 'SOA', None, 'master.example.com. admin.example.com. 1234 3600 1800 2419200 7200')
 
@@ -138,6 +141,29 @@ class TestXfroutSession(unittest.TestCase):
         self.assertEqual(rcode.to_text(), "NOERROR")
         self.assertTrue(self.xfrsess._tsig_ctx is not None)
 
+        # ACL checks, put some ACL inside
+        self.xfrsess._acl = isc.acl.dns.REQUEST_LOADER.load([
+            {
+                "from": "127.0.0.1",
+                "action": "ACCEPT"
+            },
+            {
+                "from": "192.0.2.1",
+                "action": "DROP"
+            }
+        ])
+        # Localhost (the default in this test) is accepted
+        rcode, msg = self.xfrsess._parse_query_message(self.mdata)
+        self.assertEqual(rcode.to_text(), "NOERROR")
+        # This should be dropped completely, therefore returning None
+        self.xfrsess._remote = ('192.0.2.1', 12345)
+        rcode, msg = self.xfrsess._parse_query_message(self.mdata)
+        self.assertTrue(rcode is None)
+        # This should be rejected, therefore NOTAUTH
+        self.xfrsess._remote = ('192.0.2.2', 12345)
+        rcode, msg = self.xfrsess._parse_query_message(self.mdata)
+        self.assertEqual(rcode.to_text(), "REFUSED")
+
     def test_get_query_zone_name(self):
         msg = self.getmsg()
         self.assertEqual(self.xfrsess._get_query_zone_name(msg), "example.com.")
diff --git a/src/bin/xfrout/xfrout.py.in b/src/bin/xfrout/xfrout.py.in
index 98a48c8..1b71d47 100755
--- a/src/bin/xfrout/xfrout.py.in
+++ b/src/bin/xfrout/xfrout.py.in
@@ -49,7 +49,7 @@ except ImportError as e:
     log.error(XFROUT_IMPORT, str(e))
 
 from isc.acl.acl import ACCEPT, REJECT, DROP
-from isc.acl.dns import load_request_acl
+from isc.acl.dns import REQUEST_LOADER
 
 isc.util.process.rename()
 
@@ -95,7 +95,8 @@ def get_rrset_len(rrset):
 
 
 class XfroutSession():
-    def __init__(self, sock_fd, request_data, server, tsig_key_ring, remote):
+    def __init__(self, sock_fd, request_data, server, tsig_key_ring, remote,
+                 acl):
         self._sock_fd = sock_fd
         self._request_data = request_data
         self._server = server
@@ -103,6 +104,7 @@ class XfroutSession():
         self._tsig_ctx = None
         self._tsig_len = 0
         self._remote = remote
+        self._acl = acl
         self.handle()
 
     def create_tsig_ctx(self, tsig_record, tsig_key_ring):
@@ -115,7 +117,7 @@ class XfroutSession():
             self.dns_xfrout_start(self._sock_fd, self._request_data)
             #TODO, avoid catching all exceptions
         except Exception as e:
-            logger.error(XFROUT_HANDLE_QUERY_ERROR, str(e))
+            logger.error(XFROUT_HANDLE_QUERY_ERROR, e)
             pass
 
         os.close(self._sock_fd)
@@ -142,10 +144,24 @@ class XfroutSession():
             # TSIG related checks
             rcode = self._check_request_tsig(msg, mdata)
 
-            # TODO The ACL check comes here
+            # ACL checks
+            acl_result = self._acl.execute(
+                isc.acl.dns.RequestContext(self._remote))
+            if acl_result == isc.acl.acl.DROP:
+                logger.info(XFROUT_QUERY_DROPPED,
+                            self._get_query_zone_name(msg),
+                            self._get_query_zone_class(msg),
+                            self._remote[0], self._remote[1])
+                return None, None
+            elif acl_result == isc.acl.acl.REJECT:
+                logger.info(XFROUT_QUERY_REJECTED,
+                            self._get_query_zone_name(msg),
+                            self._get_query_zone_class(msg),
+                            self._remote[0], self._remote[1])
+                return Rcode.REFUSED(), msg
 
         except Exception as err:
-            logger.error(XFROUT_PARSE_QUERY_ERROR, str(err))
+            logger.error(XFROUT_PARSE_QUERY_ERROR, err)
             return Rcode.FORMERR(), None
 
         return rcode, msg
@@ -247,7 +263,9 @@ class XfroutSession():
     def dns_xfrout_start(self, sock_fd, msg_query):
         rcode_, msg = self._parse_query_message(msg_query)
         #TODO. create query message and parse header
-        if rcode_ == Rcode.NOTAUTH():
+        if rcode_ is None: # Dropped by ACL
+            return
+        elif rcode_ == Rcode.NOTAUTH() or rcode_ == Rcode.REFUSED():
             return self._reply_query_with_error_rcode(msg, sock_fd, rcode_)
         elif rcode_ != Rcode.NOERROR():
             return self._reply_query_with_format_error(msg, sock_fd)
@@ -260,7 +278,7 @@ class XfroutSession():
         if rcode_ != Rcode.NOERROR():
             logger.info(XFROUT_AXFR_TRANSFER_FAILED, zone_name,
                         zone_class_str, rcode_.to_text())
-            return self. _reply_query_with_error_rcode(msg, sock_fd, rcode_)
+            return self._reply_query_with_error_rcode(msg, sock_fd, rcode_)
 
         try:
             logger.info(XFROUT_AXFR_TRANSFER_STARTED, zone_name, zone_class_str)
@@ -388,7 +406,7 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
     def _common_init(self):
         self._lock = threading.Lock()
         self._transfers_counter = 0
-        self._acl = load_request_acl("[]")
+        self._acl = REQUEST_LOADER.load("[]")
 
     def _receive_query_message(self, sock):
         ''' receive request message from sock'''
@@ -493,7 +511,7 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
         '''Finish one request by instantiating RequestHandlerClass.'''
         self.RequestHandlerClass(sock_fd, request_data, self,
                                  self.tsig_key_ring,
-                                 self._guess_remote(sock_fd))
+                                 self._guess_remote(sock_fd), self._acl)
 
     def _remove_unused_sock_file(self, sock_file):
         '''Try to remove the socket file. If the file is being used
@@ -537,7 +555,7 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
     def update_config_data(self, new_config):
         '''Apply the new config setting of xfrout module. '''
         if 'ACL' in new_config:
-            self._acl = load_request_acl(json.dumps(new_config['ACL']))
+            self._acl = REQUEST_LOADER.load(new_config['ACL'])
         logger.info(XFROUT_NEW_CONFIG)
         self._lock.acquire()
         self._max_transfers_out = new_config.get('transfers_out')
diff --git a/src/bin/xfrout/xfrout_messages.mes b/src/bin/xfrout/xfrout_messages.mes
index 2dada54..7a234d0 100644
--- a/src/bin/xfrout/xfrout_messages.mes
+++ b/src/bin/xfrout/xfrout_messages.mes
@@ -95,6 +95,14 @@ in the log message, but at this point no specific information other
 than that could be given. This points to incomplete exception handling
 in the code.
 
+% XFROUT_QUERY_DROPPED request to transfer %1/%2 to %3:%4 dropped
+The xfrout process silently dropped a request to transfer zone to given host.
+This is required by the ACLs.
+
+% XFROUT_QUERY_REJECTED request to transfer %1/%2 to %3:%4 rejected
+The xfrout process rejected (by REFUSED rcode) a request to transfer zone to
+given host. This is because of ACLs.
+
 % XFROUT_RECEIVE_FILE_DESCRIPTOR_ERROR error receiving the file descriptor for an XFR connection
 There was an error receiving the file descriptor for the transfer
 request. Normally, the request is received by b10-auth, and passed on
diff --git a/src/lib/acl/loader.h b/src/lib/acl/loader.h
index c86373e..f60b144 100644
--- a/src/lib/acl/loader.h
+++ b/src/lib/acl/loader.h
@@ -101,21 +101,21 @@ BasicAction defaultActionLoader(data::ConstElementPtr action);
  *
  * 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
+ [
+   {
+      "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
diff --git a/src/lib/python/isc/acl/Makefile.am b/src/lib/python/isc/acl/Makefile.am
index 4a9c5f0..cabc0a3 100644
--- a/src/lib/python/isc/acl/Makefile.am
+++ b/src/lib/python/isc/acl/Makefile.am
@@ -10,13 +10,14 @@ pythondir = $(PYTHON_SITEPKG_DIR)/isc/acl
 pyexec_LTLIBRARIES = acl.la dns.la
 pyexecdir = $(PYTHON_SITEPKG_DIR)/isc/acl
 
-acl_la_SOURCES = acl.h acl.cc
+acl_la_SOURCES = acl.cc
 acl_la_CPPFLAGS = $(AM_CPPFLAGS) $(PYTHON_INCLUDES)
 acl_la_LDFLAGS = $(PYTHON_LDFLAGS)
 acl_la_CXXFLAGS = $(AM_CXXFLAGS) $(PYTHON_CXXFLAGS)
 
-dns_la_SOURCES = dns.cc dns_requestacl_python.h dns_requestacl_python.cc
+dns_la_SOURCES = dns.h dns.cc dns_requestacl_python.h dns_requestacl_python.cc
 dns_la_SOURCES += dns_requestcontext_python.h dns_requestcontext_python.cc
+dns_la_SOURCES += dns_requestloader_python.h dns_requestloader_python.cc
 dns_la_CPPFLAGS = $(AM_CPPFLAGS) $(PYTHON_INCLUDES)
 dns_la_LDFLAGS = $(PYTHON_LDFLAGS)
 # Note: PYTHON_CXXFLAGS may have some -Wno... workaround, which must be
@@ -31,11 +32,12 @@ acl_la_LIBADD += $(PYTHON_LIB)
 
 dns_la_LDFLAGS += -module
 dns_la_LIBADD = $(top_builddir)/src/lib/acl/libdnsacl.la
-dns_la_LIBADD += acl.la
 dns_la_LIBADD += $(PYTHON_LIB)
 
 EXTRA_DIST = acl.py dns.py
+EXTRA_DIST += acl_inc.cc
 EXTRA_DIST += dnsacl_inc.cc dns_requestacl_inc.cc dns_requestcontext_inc.cc
+EXTRA_DIST += dns_requestloader_inc.cc
 
 CLEANDIRS = __pycache__
 
diff --git a/src/lib/python/isc/acl/acl.cc b/src/lib/python/isc/acl/acl.cc
index 6427906..6517a12 100644
--- a/src/lib/python/isc/acl/acl.cc
+++ b/src/lib/python/isc/acl/acl.cc
@@ -18,26 +18,25 @@
 
 #include <acl/acl.h>
 
-#include "acl.h"
-
 using namespace isc::util::python;
-using namespace isc::acl::python;
 
-namespace isc {
-namespace acl {
-namespace python {
+#include "acl_inc.cc"
+
+namespace {
+// Commonly used Python exception objects.  Right now the acl module consists
+// of only one .cc file, so we hide them in an unnamed namespace.  If and when
+// we extend this module with multiple .cc files, we should move them to
+// a named namespace, say isc::acl::python, and declare them in a separate
+// header file.
 PyObject* po_ACLError;
 PyObject* po_LoaderError;
 }
-}
-}
 
 namespace {
 PyModuleDef acl = {
     { PyObject_HEAD_INIT(NULL) NULL, 0, NULL},
-    "isc.acl",
-    "This module provides Python bindings for the C++ classes in the "
-    "isc::acl namespace",
+    "isc.acl.acl",
+    acl_doc,
     -1,
     NULL,
     NULL,
@@ -61,9 +60,11 @@ PyInit_acl(void) {
         po_LoaderError = PyErr_NewException("isc.acl.LoaderError", NULL, NULL);
         PyObjectContainer(po_LoaderError).installToModule(mod, "LoaderError");
 
-        // Install module constants.  Note that we can release our own
-        // references to these objects because we don't have corresponding
-        // C++ variables.
+        // Install module constants.  Note that we can let Py_BuildValue
+        // "steal" the references to these object (by specifying false to
+        // installToModule), because, unlike the exception cases above,
+        // we don't have corresponding C++ variables (see the note in
+        // pycppwrapper_util for more details).
         PyObjectContainer(Py_BuildValue("I", isc::acl::ACCEPT)).
             installToModule(mod, "ACCEPT", false);
         PyObjectContainer(Py_BuildValue("I", isc::acl::REJECT)).
diff --git a/src/lib/python/isc/acl/acl.h b/src/lib/python/isc/acl/acl.h
deleted file mode 100644
index 59e083f..0000000
--- a/src/lib/python/isc/acl/acl.h
+++ /dev/null
@@ -1,35 +0,0 @@
-// 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 __PYTHON_ACL_H
-#define __PYTHON_ACL_H 1
-
-#include <Python.h>
-
-namespace isc {
-namespace acl {
-namespace python {
-
-extern PyObject* po_ACLError;
-extern PyObject* po_LoaderError;
-
-} // namespace python
-} // namespace acl
-} // namespace isc
-
-#endif // __PYTHON_ACL_H
-
-// Local Variables:
-// mode: c++
-// End:
diff --git a/src/lib/python/isc/acl/acl_inc.cc b/src/lib/python/isc/acl/acl_inc.cc
new file mode 100644
index 0000000..a9f7c9d
--- /dev/null
+++ b/src/lib/python/isc/acl/acl_inc.cc
@@ -0,0 +1,16 @@
+namespace {
+const char* const acl_doc = "\
+Implementation module for ACL operations\n\n\
+This module provides Python bindings for the C++ classes in the\n\
+isc::acl namespace.\n\
+\n\
+Integer constants:\n\
+\n\
+ACCEPT, REJECT, DROP -- Default actions an ACL could perform.\n\
+  These are the commonly used actions in specific ACLs.\n\
+  It is possible to specify any other values, as the ACL class does\n\
+  nothing about them, but these look reasonable, so they are provided\n\
+  for convenience. It is not specified what exactly these mean and it's\n\
+  up to whoever uses them.\n\
+";
+} // unnamed namespace
diff --git a/src/lib/python/isc/acl/dns.cc b/src/lib/python/isc/acl/dns.cc
index 83d7642..351a8b3 100644
--- a/src/lib/python/isc/acl/dns.cc
+++ b/src/lib/python/isc/acl/dns.cc
@@ -24,9 +24,10 @@
 #include <acl/acl.h>
 #include <acl/dns.h>
 
-#include "acl.h"
+#include "dns.h"
 #include "dns_requestcontext_python.h"
 #include "dns_requestacl_python.h"
+#include "dns_requestloader_python.h"
 
 using namespace std;
 using boost::shared_ptr;
@@ -38,45 +39,21 @@ using namespace isc::acl::dns::python;
 #include "dnsacl_inc.cc"
 
 namespace {
-PyObject*
-loadRequestACL(PyObject*, PyObject* args) {
-    const char* acl_config;
-
-    if (PyArg_ParseTuple(args, "s", &acl_config)) {
-        try {
-            shared_ptr<RequestACL> acl(
-                getRequestLoader().load(Element::fromJSON(acl_config)));
-            s_RequestACL* py_acl = static_cast<s_RequestACL*>(
-                requestacl_type.tp_alloc(&requestacl_type, 0));
-            if (py_acl != NULL) {
-                py_acl->cppobj = acl;
-            }
-            return (py_acl);
-        } catch (const exception& ex) {
-            PyErr_SetString(isc::acl::python::po_LoaderError, ex.what());
-            return (NULL);
-        } catch (...) {
-            PyErr_SetString(PyExc_SystemError, "Unexpected C++ exception");
-            return (NULL);
-        }
-    }
-
-    return (NULL);
-}
+// This is a Python binding object corresponding to the singleton loader used
+// in the C++ version of the library.
+// We can define it as a pure object rather than through an accessor function,
+// because in Python we can ensure it has been created and initialized
+// in the module initializer by the time it's actually used.
+s_RequestLoader* po_REQUEST_LOADER;
 
 PyMethodDef methods[] = {
-    { "load_request_acl", loadRequestACL, METH_VARARGS, load_request_acl_doc },
     { NULL, NULL, 0, NULL }
 };
 
 PyModuleDef dnsacl = {
     { PyObject_HEAD_INIT(NULL) NULL, 0, NULL},
     "isc.acl.dns",
-    "This module provides Python bindings for the C++ classes in the "
-    "isc::acl::dns namespace.  Specifically, it defines Python interfaces of "
-    "handling access control lists (ACLs) with DNS related contexts.\n\n"
-    "These bindings are close match to the C++ API, but they are not complete "
-    "(some parts are not needed) and some are done in more python-like ways.",
+    dnsacl_doc,
     -1,
     methods,
     NULL,
@@ -86,6 +63,32 @@ PyModuleDef dnsacl = {
 };
 } // end of unnamed namespace
 
+namespace isc {
+namespace acl {
+namespace dns {
+namespace python {
+PyObject*
+getACLException(const char* ex_name) {
+    PyObject* ex_obj = NULL;
+
+    PyObject* acl_module = PyImport_AddModule("isc.acl.acl");
+    if (acl_module != NULL) {
+        PyObject* acl_dict = PyModule_GetDict(acl_module);
+        if (acl_dict != NULL) {
+            ex_obj = PyDict_GetItemString(acl_dict, ex_name);
+        }
+    }
+
+    if (ex_obj == NULL) {
+        ex_obj = PyExc_RuntimeError;
+    }
+    return (ex_obj);
+}
+}
+}
+}
+}
+
 PyMODINIT_FUNC
 PyInit_dns(void) {
     PyObject* mod = PyModule_Create(&dnsacl);
@@ -101,6 +104,32 @@ PyInit_dns(void) {
         Py_DECREF(mod);
         return (NULL);
     }
+    if (!initModulePart_RequestLoader(mod)) {
+        Py_DECREF(mod);
+        return (NULL);
+    }
+
+    // Module constants
+    try {
+        if (po_REQUEST_LOADER == NULL) {
+            po_REQUEST_LOADER = static_cast<s_RequestLoader*>(
+                requestloader_type.tp_alloc(&requestloader_type, 0));
+        }
+        if (po_REQUEST_LOADER != NULL) {
+            // We gain and keep our own reference to the singleton object
+            // for the same reason as that for exception objects (see comments
+            // in pycppwrapper_util for more details).  Note also that we don't
+            // bother to release the reference even if exception is thrown
+            // below (in fact, we cannot delete the singleton loader).
+            po_REQUEST_LOADER->cppobj = &getRequestLoader();
+            Py_INCREF(po_REQUEST_LOADER);
+        }
+        PyObjectContainer(po_REQUEST_LOADER).installToModule(mod,
+                                                             "REQUEST_LOADER");
+    } catch (...) {
+        Py_DECREF(mod);
+        return (NULL);
+    }
 
     return (mod);
 }
diff --git a/src/lib/python/isc/acl/dns.h b/src/lib/python/isc/acl/dns.h
new file mode 100644
index 0000000..76849c5
--- /dev/null
+++ b/src/lib/python/isc/acl/dns.h
@@ -0,0 +1,52 @@
+// 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 __PYTHON_ACL_DNS_H
+#define __PYTHON_ACL_DNS_H 1
+
+#include <Python.h>
+
+namespace isc {
+namespace acl {
+namespace dns {
+namespace python {
+
+// Return a Python exception object of the given name (ex_name) defined in
+// the isc.acl.acl loadable module.
+//
+// Since the acl module is a different binary image and is loaded separately
+// from the dns module, it would be very tricky to directly access to
+// C/C++ symbols defined in that module.  So we get access to these object
+// using the Python interpretor through this wrapper function.
+//
+// The __init__.py file should ensure isc.acl.acl has been loaded by the time
+// whenever this function is called, and there shouldn't be any operation
+// within this function that can fail (such as dynamic memory allocation),
+// so this function should always succeed.  Yet there may be an overlooked
+// failure mode, perhaps due to a bug in the binding implementation, or
+// due to invalid usage.  As a last resort for such cases, this function
+// returns PyExc_RuntimeError (a C binding of Python's RuntimeError) should
+// it encounters an unexpected failure.
+extern PyObject* getACLException(const char* ex_name);
+
+} // namespace python
+} // namespace dns
+} // namespace acl
+} // namespace isc
+
+#endif // __PYTHON_ACL_DNS_H
+
+// Local Variables:
+// mode: c++
+// End:
diff --git a/src/lib/python/isc/acl/dns_requestacl_inc.cc b/src/lib/python/isc/acl/dns_requestacl_inc.cc
index 9637170..673fa23 100644
--- a/src/lib/python/isc/acl/dns_requestacl_inc.cc
+++ b/src/lib/python/isc/acl/dns_requestacl_inc.cc
@@ -4,7 +4,7 @@ The DNS Request ACL.\n\
 \n\
 It holds bunch of ordered entries, each one consisting of a check for\n\
 a given DNS Request context and an action, which is one of ACCEPT,\n\
-REJECT, or DROP, as defined in the isc.acl module.\n\
+REJECT, or DROP, as defined in the isc.acl.acl module.\n\
 The checks are tested in the order and first match counts.\n\
 \n\
 A RequestACL object cannot be constructed directly; an application\n\
@@ -16,7 +16,7 @@ const char* const RequestACL_execute_doc = "\
 execute(context) -> action \n\
 \n\
 The returned action is one of ACCEPT, REJECT or DROP as defined in\n\
-the isc.acl module.\n\
+the isc.acl.acl module.\n\
 \n\
 This is the function that takes the ACL entries one by one, checks the\n\
 context against conditions and if it matches, returns the action that\n\
diff --git a/src/lib/python/isc/acl/dns_requestacl_python.cc b/src/lib/python/isc/acl/dns_requestacl_python.cc
index 326c717..5e5acea 100644
--- a/src/lib/python/isc/acl/dns_requestacl_python.cc
+++ b/src/lib/python/isc/acl/dns_requestacl_python.cc
@@ -28,14 +28,13 @@
 #include <acl/acl.h>
 #include <acl/dns.h>
 
-#include "acl.h"
+#include "dns.h"
 #include "dns_requestacl_python.h"
 #include "dns_requestcontext_python.h"
 
 using namespace std;
 using namespace isc::util::python;
 using namespace isc::acl;
-using namespace isc::acl::python;
 using namespace isc::acl::dns;
 using namespace isc::acl::dns::python;
 
@@ -60,12 +59,13 @@ s_RequestACL::s_RequestACL() {}
 namespace {
 int
 RequestACL_init(PyObject*, PyObject*, PyObject*) {
-    PyErr_SetString(po_ACLError, "RequestACL cannot be directly constructed");
+    PyErr_SetString(getACLException("Error"),
+                    "RequestACL cannot be directly constructed");
     return (-1);
 }
 
 void
-RequestACL_destroy(PyObject* const po_self) {
+RequestACL_destroy(PyObject* po_self) {
     s_RequestACL* const self = static_cast<s_RequestACL*>(po_self);
     self->cppobj.reset();
     Py_TYPE(self)->tp_free(self);
@@ -84,7 +84,7 @@ RequestACL_execute(PyObject* po_self, PyObject* args) {
         }
     } catch (const exception& ex) {
         const string ex_what = "Failed to execute ACL: " + string(ex.what());
-        PyErr_SetString(po_ACLError, ex_what.c_str());
+        PyErr_SetString(getACLException("Error"), ex_what.c_str());
     } catch (...) {
         PyErr_SetString(PyExc_RuntimeError,
                         "Unexpected exception in executing ACL");
diff --git a/src/lib/python/isc/acl/dns_requestcontext_python.cc b/src/lib/python/isc/acl/dns_requestcontext_python.cc
index 2da0c38..6c63b59 100644
--- a/src/lib/python/isc/acl/dns_requestcontext_python.cc
+++ b/src/lib/python/isc/acl/dns_requestcontext_python.cc
@@ -42,7 +42,7 @@
 #include <acl/dns.h>
 #include <acl/ip_check.h>
 
-#include "acl.h"
+#include "dns.h"
 #include "dns_requestcontext_python.h"
 
 using namespace std;
@@ -51,7 +51,6 @@ using boost::lexical_cast;
 using namespace isc;
 using namespace isc::util::python;
 using namespace isc::acl::dns;
-using namespace isc::acl::python;
 using namespace isc::acl::dns::python;
 
 namespace isc {
@@ -177,7 +176,7 @@ RequestContext_init(PyObject* po_self, PyObject* args, PyObject*) {
     } catch (const exception& ex) {
         const string ex_what = "Failed to construct RequestContext object: " +
             string(ex.what());
-        PyErr_SetString(po_ACLError, ex_what.c_str());
+        PyErr_SetString(getACLException("Error"), ex_what.c_str());
         return (-1);
     } catch (...) {
         PyErr_SetString(PyExc_RuntimeError,
diff --git a/src/lib/python/isc/acl/dns_requestloader_inc.cc b/src/lib/python/isc/acl/dns_requestloader_inc.cc
new file mode 100644
index 0000000..e05a580
--- /dev/null
+++ b/src/lib/python/isc/acl/dns_requestloader_inc.cc
@@ -0,0 +1,86 @@
+namespace {
+const char* const RequestLoader_doc = "\
+Loader of DNS Request ACLs.\n\
+\n\
+The goal of this class is to convert JSON description of an ACL to\n\
+object of the ACL class (including the checks inside it).\n\
+\n\
+The class can be used to load the checks only. This is supposed to be\n\
+used by compound checks to create the subexpressions.\n\
+\n\
+To allow any kind of checks to exist in the application, creators are\n\
+registered for the names of the checks (this feature is not yet\n\
+available for the python API).\n\
+\n\
+An ACL definition looks like this:  [\n\
+   {\n\
+      \"action\": \"ACCEPT\",\n\
+      \"match-type\": <parameter>\n\
+   },\n\
+   {\n\
+      \"action\": \"REJECT\",\n\
+      \"match-type\": <parameter>,\n\
+      \"another-match-type\": [<parameter1>, <parameter2>]\n\
+   },\n\
+   {\n\
+      \"action\": \"DROP\"\n\
+   }\n\
+ ]\n\
+ \n\
+\n\
+This is a list of elements. Each element must have an \"action\"\n\
+entry/keyword. That one specifies which action is returned if this\n\
+element matches (the value of the key is passed to the action loader\n\
+(see the constructor), which is one of ACCEPT,\n\
+REJECT, or DROP, as defined in the isc.acl.acl module.\n\
+\n\
+The rest of the element are matches. The left side is the name of the\n\
+match type (for example \"from\" to match for source IP address).\n\
+The <parameter> is whatever is needed to describe the\n\
+match and depends on the match type, the loader passes it verbatim to\n\
+creator of that match type.\n\
+\n\
+There may be multiple match types in single element. In such case, all\n\
+of the matches must match for the element to take action (so, in the\n\
+second element, both \"match-type\" and \"another-match-type\" must be\n\
+satisfied). If there's no match in the element, the action is\n\
+taken/returned without conditions, every time (makes sense as the last\n\
+entry, as the ACL will never get past it).\n\
+\n\
+The second entry shows another thing - if there's a list as the value\n\
+for some match and the match itself is not expecting a list, it is\n\
+taken as an \"or\" - a match for at last one of the choices in the\n\
+list must match. So, for the second entry, both \"match-type\" and\n\
+\"another-match-type\" must be satisfied, but the another one is\n\
+satisfied by either parameter1 or parameter2.\n\
+\n\
+Currently, a RequestLoader object cannot be constructed directly;\n\
+an application must use the singleton loader defined in the\n\
+isc.acl.dns module, i.e., isc.acl.dns.REQUEST_LOADER.\n\
+A future version of this implementation may be extended to give\n\
+applications full flexibility of creating arbitrary loader, when\n\
+this restriction may be removed.\n\
+";
+
+const char* const RequestLoader_load_doc = "\
+load(description) -> RequestACL\n\
+\n\
+Load a DNS (Request) ACL.\n\
+\n\
+This parses an ACL list, creates internal data for each rule\n\
+and returns a RequestACl object that contains all given rules.\n\
+\n\
+Exceptions:\n\
+  LoaderError Load failed.  The most likely cause of this is a syntax\n\
+              error in the description.  Other internal errors such as\n\
+              memory allocation failure is also converted to this\n\
+              exception.\n\
+\n\
+Parameters:\n\
+  description String or Python representation of the JSON list of\n\
+              ACL. The Python representation is ones accepted by the\n\
+              standard json module.\n\
+\n\
+Return Value(s): The newly created RequestACL object\n\
+";
+} // unnamed namespace
diff --git a/src/lib/python/isc/acl/dns_requestloader_python.cc b/src/lib/python/isc/acl/dns_requestloader_python.cc
new file mode 100644
index 0000000..aa5df67
--- /dev/null
+++ b/src/lib/python/isc/acl/dns_requestloader_python.cc
@@ -0,0 +1,255 @@
+// 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.
+
+// Enable this if you use s# variants with PyArg_ParseTuple(), see
+// http://docs.python.org/py3k/c-api/arg.html#strings-and-buffers
+//#define PY_SSIZE_T_CLEAN
+
+// Python.h needs to be placed at the head of the program file, see:
+// http://docs.python.org/py3k/extending/extending.html#a-simple-example
+#include <Python.h>
+
+#include <string>
+#include <stdexcept>
+
+#include <boost/shared_ptr.hpp>
+
+#include <util/python/pycppwrapper_util.h>
+
+#include <cc/data.h>
+
+#include <acl/dns.h>
+
+#include "dns.h"
+#include "dns_requestacl_python.h"
+#include "dns_requestloader_python.h"
+
+using namespace std;
+using boost::shared_ptr;
+using namespace isc::util::python;
+using namespace isc::data;
+using namespace isc::acl::dns;
+using namespace isc::acl::dns::python;
+
+//
+// Definition of the classes
+//
+
+// For each class, we need a struct, a helper functions (init, destroy,
+// and static wrappers around the methods we export), a list of methods,
+// and a type description
+
+//
+// RequestLoader
+//
+
+// Trivial constructor.
+s_RequestLoader::s_RequestLoader() : cppobj(NULL) {
+}
+
+// Import pydoc text
+#include "dns_requestloader_inc.cc"
+
+namespace {
+//
+// We declare the functions here, the definitions are below
+// the type definition of the object, since both can use the other
+//
+
+int
+RequestLoader_init(PyObject*, PyObject*, PyObject*) {
+    PyErr_SetString(getACLException("Error"),
+                    "RequestLoader cannot be directly constructed");
+    return (-1);
+}
+
+void
+RequestLoader_destroy(PyObject* po_self) {
+    s_RequestLoader* const self = static_cast<s_RequestLoader*>(po_self);
+    delete self->cppobj;
+    self->cppobj = NULL;
+    Py_TYPE(self)->tp_free(self);
+}
+
+// This helper function essentially does:
+//   import json
+//   return json.dumps
+// Getting access to the json module this way and call one of its functions
+// via PyObject_CallObject() may exceed the reasonably acceptable level for
+// straightforward bindings.  But the alternative would be to write a Python
+// frontend for the entire module only for this conversion, which would also
+// be too much.  So, right now, we implement everything within the binding
+// implementation.  If future extensions require more such non trivial
+// wrappers, we should consider the frontend approach more seriously.
+PyObject*
+getJSONDumpsObj() {
+    PyObject* json_dump_obj = NULL;
+    PyObject* json_module = PyImport_AddModule("json");
+    if (json_module != NULL) {
+        PyObject* json_dict = PyModule_GetDict(json_module);
+        if (json_dict != NULL) {
+            json_dump_obj = PyDict_GetItemString(json_dict, "dumps");
+        }
+    }
+    return (json_dump_obj);
+}
+
+PyObject*
+RequestLoader_load(PyObject* po_self, PyObject* args) {
+    s_RequestLoader* const self = static_cast<s_RequestLoader*>(po_self);
+
+    try {
+        PyObjectContainer c1, c2; // placeholder for temporary py objects
+        const char* acl_config;
+
+        // First, try string
+        int py_result = PyArg_ParseTuple(args, "s", &acl_config);
+        if (!py_result) {
+            PyErr_Clear();  // need to clear the error from ParseTuple
+
+            // If that fails, confirm the argument is a single Python object,
+            // and pass the argument to json.dumps() without conversion.
+            // Note that we should pass 'args', not 'json_obj' to
+            // PyObject_CallObject(), since this function expects a form of
+            // tuple as its argument parameter, just like ParseTuple.
+            PyObject* json_obj;
+            if (PyArg_ParseTuple(args, "O", &json_obj)) {
+                PyObject* json_dumps_obj = getJSONDumpsObj();
+                if (json_dumps_obj != NULL) {
+                    c1.reset(PyObject_CallObject(json_dumps_obj, args));
+                    c2.reset(Py_BuildValue("(O)", c1.get()));
+                    py_result = PyArg_ParseTuple(c2.get(), "s", &acl_config);
+                }
+            }
+        }
+        if (py_result) {
+            shared_ptr<RequestACL> acl(
+                self->cppobj->load(Element::fromJSON(acl_config)));
+            s_RequestACL* py_acl = static_cast<s_RequestACL*>(
+                requestacl_type.tp_alloc(&requestacl_type, 0));
+            if (py_acl != NULL) {
+                py_acl->cppobj = acl;
+            }
+            return (py_acl);
+        }
+    } catch (const PyCPPWrapperException&) {
+        // If the wrapper utility throws, it's most likely because an invalid
+        // type of argument is passed (and the call to json.dumps() failed
+        // above), rather than a rare case of system errors such as memory
+        // allocation failure.  So we fall through to the end of this function
+        // and raise a TypeError.
+        ;
+    } catch (const exception& ex) {
+        PyErr_SetString(getACLException("LoaderError"), ex.what());
+        return (NULL);
+    } catch (...) {
+        PyErr_SetString(PyExc_SystemError, "Unexpected C++ exception");
+        return (NULL);
+    }
+
+    PyErr_SetString(PyExc_TypeError, "RequestLoader.load() "
+                    "expects str or python representation of JSON");
+    return (NULL);
+}
+
+// This list contains the actual set of functions we have in
+// python. Each entry has
+// 1. Python method name
+// 2. Our static function here
+// 3. Argument type
+// 4. Documentation
+PyMethodDef RequestLoader_methods[] = {
+    { "load", RequestLoader_load, METH_VARARGS, RequestLoader_load_doc },
+    { NULL, NULL, 0, NULL }
+};
+} // end of unnamed namespace
+
+namespace isc {
+namespace acl {
+namespace dns {
+namespace python {
+// This defines the complete type for reflection in python and
+// parsing of PyObject* to s_RequestLoader
+// Most of the functions are not actually implemented and NULL here.
+PyTypeObject requestloader_type = {
+    PyVarObject_HEAD_INIT(NULL, 0)
+    "isc.acl.dns.RequestLoader",
+    sizeof(s_RequestLoader),                 // tp_basicsize
+    0,                                  // tp_itemsize
+    RequestLoader_destroy,       // tp_dealloc
+    NULL,                               // tp_print
+    NULL,                               // tp_getattr
+    NULL,                               // tp_setattr
+    NULL,                               // tp_reserved
+    NULL,                               // tp_repr
+    NULL,                               // tp_as_number
+    NULL,                               // tp_as_sequence
+    NULL,                               // tp_as_mapping
+    NULL,                               // tp_hash
+    NULL,                               // tp_call
+    NULL,                       // tp_str
+    NULL,                               // tp_getattro
+    NULL,                               // tp_setattro
+    NULL,                               // tp_as_buffer
+    Py_TPFLAGS_DEFAULT,                 // tp_flags
+    RequestLoader_doc,
+    NULL,                               // tp_traverse
+    NULL,                               // tp_clear
+    NULL, // tp_richcompare
+    0,                                  // tp_weaklistoffset
+    NULL,                               // tp_iter
+    NULL,                               // tp_iternext
+    RequestLoader_methods,                   // tp_methods
+    NULL,                               // tp_members
+    NULL,                               // tp_getset
+    NULL,                               // tp_base
+    NULL,                               // tp_dict
+    NULL,                               // tp_descr_get
+    NULL,                               // tp_descr_set
+    0,                                  // tp_dictoffset
+    RequestLoader_init,            // tp_init
+    NULL,                               // tp_alloc
+    PyType_GenericNew,                  // tp_new
+    NULL,                               // tp_free
+    NULL,                               // tp_is_gc
+    NULL,                               // tp_bases
+    NULL,                               // tp_mro
+    NULL,                               // tp_cache
+    NULL,                               // tp_subclasses
+    NULL,                               // tp_weaklist
+    NULL,                               // tp_del
+    0                                   // tp_version_tag
+};
+
+bool
+initModulePart_RequestLoader(PyObject* mod) {
+    // We initialize the static description object with PyType_Ready(),
+    // then add it to the module. This is not just a check! (leaving
+    // this out results in segmentation faults)
+    if (PyType_Ready(&requestloader_type) < 0) {
+        return (false);
+    }
+    void* p = &requestloader_type;
+    if (PyModule_AddObject(mod, "RequestLoader",
+                           static_cast<PyObject*>(p)) < 0) {
+        return (false);
+    }
+    Py_INCREF(&requestloader_type);
+
+    return (true);
+}
+} // namespace python
+} // namespace dns
+} // namespace acl
+} // namespace isc
diff --git a/src/lib/python/isc/acl/dns_requestloader_python.h b/src/lib/python/isc/acl/dns_requestloader_python.h
new file mode 100644
index 0000000..9d0b63e
--- /dev/null
+++ b/src/lib/python/isc/acl/dns_requestloader_python.h
@@ -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.
+
+#ifndef __PYTHON_REQUESTLOADER_H
+#define __PYTHON_REQUESTLOADER_H 1
+
+#include <Python.h>
+
+#include <acl/dns.h>
+
+namespace isc {
+namespace acl {
+namespace dns {
+namespace python {
+
+// The s_* Class simply covers one instantiation of the object
+class s_RequestLoader : public PyObject {
+public:
+    s_RequestLoader();
+    RequestLoader* cppobj;
+};
+
+extern PyTypeObject requestloader_type;
+
+bool initModulePart_RequestLoader(PyObject* mod);
+
+} // namespace python
+} // namespace dns
+} // namespace acl
+} // namespace isc
+#endif // __PYTHON_REQUESTLOADER_H
+
+// Local Variables:
+// mode: c++
+// End:
diff --git a/src/lib/python/isc/acl/dnsacl_inc.cc b/src/lib/python/isc/acl/dnsacl_inc.cc
index f68e193..b2e7338 100644
--- a/src/lib/python/isc/acl/dnsacl_inc.cc
+++ b/src/lib/python/isc/acl/dnsacl_inc.cc
@@ -1,21 +1,17 @@
 namespace {
-const char* const load_request_acl_doc = "\
-load_request_acl(description) -> RequestACL\n\
+const char* const dnsacl_doc = "\
+Implementation module for DNS ACL operations\n\n\
+This module provides Python bindings for the C++ classes in the\n\
+isc::acl::dns namespace.  Specifically, it defines Python interfaces of\n\
+handling access control lists (ACLs) with DNS related contexts.\n\
+These bindings are close match to the C++ API, but they are not complete\n\
+(some parts are not needed) and some are done in more python-like ways.\n\
 \n\
-Load a DNS ACL.\n\
+Special objects:\n\
 \n\
-This parses an ACL list, creates internal data for each rule\n\
-and returns a RequestACl object that contains all given rules.\n\
-\n\
-Exceptions:\n\
-  LoaderError Load failed.  The most likely cause of this is a syntax\n\
-              error in the description.  Other internal errors such as\n\
-              memory allocation failure is also converted to this\n\
-              exception.\n\
-\n\
-Parameters:\n\
-  description String representation of the JSON list of ACL.\n\
-\n\
-Return Value(s): The newly created RequestACL object\n\
+REQUEST_LOADER -- A singleton loader of ACLs. It is expected applications\n\
+  will use this function instead of creating their own loaders, because\n\
+  one is enough, this one will have registered default checks and it is\n\
+  known one, so any plugins can registrer additional checks as well.\n\
 ";
 } // unnamed namespace
diff --git a/src/lib/python/isc/acl/tests/dns_test.py b/src/lib/python/isc/acl/tests/dns_test.py
index 3924222..acaf32b 100644
--- a/src/lib/python/isc/acl/tests/dns_test.py
+++ b/src/lib/python/isc/acl/tests/dns_test.py
@@ -20,7 +20,8 @@ from isc.acl.dns import *
 
 def get_sockaddr(address, port):
     '''This is a simple shortcut wrapper for getaddrinfo'''
-    ai = socket.getaddrinfo(address, port, 0, 0, 0, socket.AI_NUMERICHOST)[0]
+    ai = socket.getaddrinfo(address, port, 0, socket.SOCK_DGRAM,
+                            socket.IPPROTO_UDP, socket.AI_NUMERICHOST)[0]
     return ai[4]
 
 def get_acl(prefix):
@@ -28,7 +29,15 @@ def get_acl(prefix):
     that accepts addresses for the given IP prefix (and reject any others
     by default)
     '''
-    return load_request_acl('[{"action": "ACCEPT", "from": "' + prefix + '"}]')
+    return REQUEST_LOADER.load('[{"action": "ACCEPT", "from": "' + \
+                                   prefix + '"}]')
+
+def get_acl_json(prefix):
+    '''Same as get_acl, but this function passes a Python representation of
+    JSON to the loader, not a string.'''
+    json = [{"action": "ACCEPT"}]
+    json[0]["from"] = prefix
+    return REQUEST_LOADER.load(json)
 
 def get_context(address):
     '''This is a simple shortcut wrapper for creating a RequestContext
@@ -97,89 +106,155 @@ class RequestACLTest(unittest.TestCase):
 
     def test_request_loader(self):
         # these shouldn't raise an exception
-        load_request_acl('[{"action": "DROP"}]')
-        load_request_acl('[{"action": "DROP", "from": "192.0.2.1"}]')
+        REQUEST_LOADER.load('[{"action": "DROP"}]')
+        REQUEST_LOADER.load([{"action": "DROP"}])
+        REQUEST_LOADER.load('[{"action": "DROP", "from": "192.0.2.1"}]')
+        REQUEST_LOADER.load([{"action": "DROP", "from": "192.0.2.1"}])
 
-        # Invalid types
-        self.assertRaises(TypeError, load_request_acl, 1)
-        self.assertRaises(TypeError, load_request_acl, [])
+        # Invalid types (note that arguments like '1' or '[]' is of valid
+        # 'type' (but syntax error at a higher level)).  So we need to use
+        # something that is not really JSON nor string.
+        self.assertRaises(TypeError, REQUEST_LOADER.load, b'')
 
         # Incorrect number of arguments
-        self.assertRaises(TypeError, load_request_acl,
+        self.assertRaises(TypeError, REQUEST_LOADER.load,
                           '[{"action": "DROP"}]', 0)
 
     def test_bad_acl_syntax(self):
         # the following are derived from loader_test.cc
-        self.assertRaises(LoaderError, load_request_acl, '{}');
-        self.assertRaises(LoaderError, load_request_acl, '42');
-        self.assertRaises(LoaderError, load_request_acl, 'true');
-        self.assertRaises(LoaderError, load_request_acl, 'null');
-        self.assertRaises(LoaderError, load_request_acl, '"hello"');
-        self.assertRaises(LoaderError, load_request_acl, '[42]');
-        self.assertRaises(LoaderError, load_request_acl, '["hello"]');
-        self.assertRaises(LoaderError, load_request_acl, '[[]]');
-        self.assertRaises(LoaderError, load_request_acl, '[true]');
-        self.assertRaises(LoaderError, load_request_acl, '[null]');
-        self.assertRaises(LoaderError, load_request_acl, '[{}]');
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, '{}');
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, {});
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, '42');
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, 42);
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, 'true');
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, True);
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, 'null');
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, None);
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, '"hello"');
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, "hello");
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, '[42]');
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, [42]);
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, '["hello"]');
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, ["hello"]);
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, '[[]]');
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, [[]]);
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, '[true]');
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, [True]);
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, '[null]');
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, [None]);
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, '[{}]');
+        self.assertRaises(LoaderError, REQUEST_LOADER.load, [{}]);
 
         # the following are derived from dns_test.cc
-        self.assertRaises(LoaderError, load_request_acl,
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
                           '[{"action": "ACCEPT", "bad": "192.0.2.1"}]')
-        self.assertRaises(LoaderError, load_request_acl,
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "ACCEPT", "bad": "192.0.2.1"}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
                           '[{"action": "ACCEPT", "from": 4}]')
-        self.assertRaises(LoaderError, load_request_acl,
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "ACCEPT", "from": 4}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
                           '[{"action": "ACCEPT", "from": []}]')
-        self.assertRaises(LoaderError, load_request_acl,
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "ACCEPT", "from": []}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
                           '[{"action": "ACCEPT", "from": "bad"}]')
-        self.assertRaises(LoaderError, load_request_acl,
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "ACCEPT", "from": "bad"}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
                           '[{"action": "ACCEPT", "from": null}]')
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "ACCEPT", "from": None}])
 
     def test_bad_acl_ipsyntax(self):
         # this test is derived from ip_check_unittest.cc
-        self.assertRaises(LoaderError, load_request_acl,
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
                           '[{"action": "DROP", "from": "192.0.2.43/-1"}]')
-        self.assertRaises(LoaderError, load_request_acl,
-                          '[{"action": "DROP", "from": "192.0.2.43//1"')
-        self.assertRaises(LoaderError, load_request_acl,
-                          '[{"action": "DROP", "from": "192.0.2.43/1/"')
-        self.assertRaises(LoaderError, load_request_acl,
-                          '[{"action": "DROP", "from": "/192.0.2.43/1"')
-        self.assertRaises(LoaderError, load_request_acl,
-                          '[{"action": "DROP", "from": "2001:db8::/xxxx"')
-        self.assertRaises(LoaderError, load_request_acl,
-                          '[{"action": "DROP", "from": "2001:db8::/32/s"')
-        self.assertRaises(LoaderError, load_request_acl,
-                          '[{"action": "DROP", "from": "1/"')
-        self.assertRaises(LoaderError, load_request_acl,
-                          '[{"action": "DROP", "from": "/1"')
-        self.assertRaises(LoaderError, load_request_acl,
-                          '[{"action": "DROP", "from": "192.0.2.0/33"')
-        self.assertRaises(LoaderError, load_request_acl,
-                          '[{"action": "DROP", "from": "::1/129"')
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "DROP", "from": "192.0.2.43/-1"}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          '[{"action": "DROP", "from": "192.0.2.43//1"}]')
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "DROP", "from": "192.0.2.43//1"}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          '[{"action": "DROP", "from": "192.0.2.43/1/"}]')
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "DROP", "from": "192.0.2.43/1/"}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          '[{"action": "DROP", "from": "/192.0.2.43/1"}]')
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "DROP", "from": "/192.0.2.43/1"}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          '[{"action": "DROP", "from": "2001:db8::/xxxx"}]')
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "DROP", "from": "2001:db8::/xxxx"}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          '[{"action": "DROP", "from": "2001:db8::/32/s"}]')
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "DROP", "from": "2001:db8::/32/s"}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          '[{"action": "DROP", "from": "1/"}]')
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "DROP", "from": "1/"}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          '[{"action": "DROP", "from": "/1"}]')
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "DROP", "from": "/1"}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          '[{"action": "DROP", "from": "192.0.2.0/33"}]')
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "DROP", "from": "192.0.2.0/33"}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          '[{"action": "DROP", "from": "::1/129"}]')
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "DROP", "from": "::1/129"}])
 
     def test_execute(self):
         # tests derived from dns_test.cc.  We don't directly expose checks
         # in the python wrapper, so we test it via execute().
         self.assertEqual(ACCEPT, get_acl('192.0.2.1').execute(CONTEXT4))
+        self.assertEqual(ACCEPT, get_acl_json('192.0.2.1').execute(CONTEXT4))
         self.assertEqual(REJECT, get_acl('192.0.2.53').execute(CONTEXT4))
+        self.assertEqual(REJECT, get_acl_json('192.0.2.53').execute(CONTEXT4))
         self.assertEqual(ACCEPT, get_acl('192.0.2.0/24').execute(CONTEXT4))
+        self.assertEqual(ACCEPT, get_acl_json('192.0.2.0/24').execute(CONTEXT4))
         self.assertEqual(REJECT, get_acl('192.0.1.0/24').execute(CONTEXT4))
+        self.assertEqual(REJECT, get_acl_json('192.0.1.0/24').execute(CONTEXT4))
         self.assertEqual(REJECT, get_acl('192.0.1.0/24').execute(CONTEXT4))
+        self.assertEqual(REJECT, get_acl_json('192.0.1.0/24').execute(CONTEXT4))
 
         self.assertEqual(ACCEPT, get_acl('2001:db8::1').execute(CONTEXT6))
+        self.assertEqual(ACCEPT, get_acl_json('2001:db8::1').execute(CONTEXT6))
         self.assertEqual(REJECT, get_acl('2001:db8::53').execute(CONTEXT6))
+        self.assertEqual(REJECT, get_acl_json('2001:db8::53').execute(CONTEXT6))
         self.assertEqual(ACCEPT, get_acl('2001:db8::/64').execute(CONTEXT6))
+        self.assertEqual(ACCEPT,
+                         get_acl_json('2001:db8::/64').execute(CONTEXT6))
         self.assertEqual(REJECT, get_acl('2001:db8:1::/64').execute(CONTEXT6))
+        self.assertEqual(REJECT,
+                         get_acl_json('2001:db8:1::/64').execute(CONTEXT6))
         self.assertEqual(REJECT, get_acl('32.1.13.184').execute(CONTEXT6))
+        self.assertEqual(REJECT, get_acl_json('32.1.13.184').execute(CONTEXT6))
 
         # A bit more complicated example, derived from resolver_config_unittest
-        acl = load_request_acl('[ {"action": "ACCEPT", ' +
-                               '     "from": "192.0.2.1"},' +
-                               '    {"action": "REJECT",' +
-                               '     "from": "192.0.2.0/24"},' +
-                               '    {"action": "DROP",' +
-                               '     "from": "2001:db8::1"},' +
-                               '] }')
+        acl = REQUEST_LOADER.load('[ {"action": "ACCEPT", ' +
+                                  '     "from": "192.0.2.1"},' +
+                                  '    {"action": "REJECT",' +
+                                  '     "from": "192.0.2.0/24"},' +
+                                  '    {"action": "DROP",' +
+                                  '     "from": "2001:db8::1"},' +
+                                  '] }')
+        self.assertEqual(ACCEPT, acl.execute(CONTEXT4))
+        self.assertEqual(REJECT, acl.execute(get_context('192.0.2.2')))
+        self.assertEqual(DROP, acl.execute(get_context('2001:db8::1')))
+        self.assertEqual(REJECT, acl.execute(get_context('2001:db8::2')))
+
+        # same test using the JSON representation
+        acl = REQUEST_LOADER.load([{"action": "ACCEPT", "from": "192.0.2.1"},
+                                   {"action": "REJECT",
+                                    "from": "192.0.2.0/24"},
+                                   {"action": "DROP", "from": "2001:db8::1"}])
         self.assertEqual(ACCEPT, acl.execute(CONTEXT4))
         self.assertEqual(REJECT, acl.execute(get_context('192.0.2.2')))
         self.assertEqual(DROP, acl.execute(get_context('2001:db8::1')))
@@ -194,5 +269,12 @@ class RequestACLTest(unittest.TestCase):
         # type mismatch
         self.assertRaises(TypeError, acl.execute, 'bad parameter')
 
+class RequestLoaderTest(unittest.TestCase):
+    # Note: loading ACLs is tested in other test cases.
+
+    def test_construct(self):
+        # at least for now, we don't allow direct construction.
+        self.assertRaises(Error, RequestLoader)
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/src/lib/util/python/pycppwrapper_util.h b/src/lib/util/python/pycppwrapper_util.h
index fd55c19..3f396e2 100644
--- a/src/lib/util/python/pycppwrapper_util.h
+++ b/src/lib/util/python/pycppwrapper_util.h
@@ -94,6 +94,22 @@ public:
 /// the reference to be decreased, the original bare pointer should be
 /// extracted using the \c release() method.
 ///
+/// In some other cases, it would be convenient if it's possible to create
+/// an "empty" container and reset it with a Python object later.
+/// For example, we may want to create a temporary Python object in the
+/// middle of a function and make sure that it's valid within the rest of
+/// the function scope, while we want to make sure its reference is released
+/// when the function returns (either normally or as a result of exception).
+/// To allow this scenario, this class defines the default constructor
+/// and the \c reset() method.  The default constructor allows the class
+/// object with an "empty" (NULL) Python object, while \c reset() allows
+/// the stored object to be replaced with a new one.  If there's a valid
+/// object was already set, \c reset() releases its reference.
+/// In general, it's safer to construct the container object with a valid
+/// Python object pointer.  The use of the default constructor and
+/// \c reset() should therefore be restricted to cases where it's
+/// absolutely necessary.
+///
 /// There are two convenience methods for commonly used operations:
 /// \c installAsClassVariable() to add the PyObject as a class variable
 /// and \c installToModule to add the PyObject to a specified python module.
@@ -166,16 +182,27 @@ public:
 /// exception in a python biding written in C/C++.  See the code comment
 /// of the method for more details.
 struct PyObjectContainer {
+    PyObjectContainer() : obj_(NULL) {}
     PyObjectContainer(PyObject* obj) : obj_(obj) {
         if (obj_ == NULL) {
             isc_throw(PyCPPWrapperException, "Unexpected NULL PyObject, "
                       "probably due to short memory");
         }
     }
-    virtual ~PyObjectContainer() {
+    ~PyObjectContainer() {
+        if (obj_ != NULL) {
+            Py_DECREF(obj_);
+        }
+    }
+    void reset(PyObject* obj) {
+        if (obj == NULL) {
+            isc_throw(PyCPPWrapperException, "Unexpected NULL PyObject, "
+                      "probably due to short memory");
+        }
         if (obj_ != NULL) {
             Py_DECREF(obj_);
         }
+        obj_ = obj;
     }
     PyObject* get() {
         return (obj_);
diff --git a/src/lib/util/python/wrapper_template.cc b/src/lib/util/python/wrapper_template.cc
index 3a9282f..a703731 100644
--- a/src/lib/util/python/wrapper_template.cc
+++ b/src/lib/util/python/wrapper_template.cc
@@ -210,7 +210,7 @@ namespace python {
 // Most of the functions are not actually implemented and NULL here.
 PyTypeObject @cppclass at _type = {
     PyVarObject_HEAD_INIT(NULL, 0)
-    "pydnspp. at CPPCLASS@",
+    "@MODULE at .@CPPCLASS@",
     sizeof(s_ at CPPCLASS@),                 // tp_basicsize
     0,                                  // tp_itemsize
     reinterpret_cast<destructor>(@CPPCLASS at _destroy),       // tp_dealloc




More information about the bind10-changes mailing list