[2252] Merge branch 'master' of git://git.bind10.isc.org/bind10 into trac2252

Conflicts:
	tests/lettuce/features/xfrin_notify_handling.feature

AUTHORS

@@ -14,6 +14,7 @@ Michael Graff
 Michal Vaner
 Mukund Sivaraman
 Naoki Kambe
+Paul Selkirk
 Shane Kerr
 Shen Tingting
 Stephen Morris

ChangeLog

@@ -1,3 +1,98 @@
+594.	[func]		muks, pselkirk
+	libdns++: the NSEC, DS, DLV, and AFSDB Rdata classes now use the
+	generic lexer in constructors from text.  This means that the name
+	fields in such RRs in a zone file can now be non-absolute (the
+	origin name in that context will be used), e.g., when loaded by
+	b10-loadzone.
+	(Trac #2386, git dc0f34afb1eccc574421a802557198e6cd2363fa)
+	(Trac #2391, git 1450d8d486cba3bee8be46e8001d66898edd370c)
+
+593.	[func]		jelte
+	Address + port output and logs is now consistent according to our
+	coding guidelines, e.g. <address>:<port> in the case of IPv4, and
+	[<address>]:<port> in the case of IPv6, instead of <address>#<port>
+	(Trac #1086, git bcefe1e95cdd61ee4a09b20522c3c56b315a1acc)
+
+592.	[bug]		jinmei
+	b10-auth and zonemgr now handle some uncommon NOTIFY messages more
+	gracefully: auth immediately returns a NOTAUTH response if the
+	server does not have authority for the zone (the behavior
+	compatible with BIND 9) without bothering zonemgr; zonemgr now
+	simply skips retransfer if the specified zone is not in its
+	secondary zone list, instead of producing noisy error logs.
+	(Trac #1938, git 89d7de8e2f809aef2184b450e7dee1bfec98ad14)
+
+591.	[func]		vorner
+	Ported the remaining tests from the old shell/perl based system to
+	lettuce. Make target `systest' is now gone. Currently, the lettuce
+	tests are in git only, not part of the release tarball.
+	(Trac #2624, git df1c5d5232a2ab551cd98b77ae388ad568a683ad)
+
+590.	[bug]		tmark
+	Modified "include" statements in DHCP MySQL lease manager code to
+	fix build problems if MySQL is installed in a non-standard location.
+	(Trac #2825, git 4813e06cf4e0a9d9f453890557b639715e081eca)
+
+589.	[bug]		jelte
+	b10-cmdctl now automatically re-reads the user accounts file when
+	it is updated.
+	(Trac #2710, git 16e8be506f32de668699e6954f5de60ca9d14ddf)
+
+588.	[bug]*		jreed
+	b10-xfrout: Log message id XFROUT_QUERY_QUOTA_EXCCEEDED
+	changed to XFROUT_QUERY_QUOTA_EXCEEDED.
+	(git be41be890f1349ae4c870a887f7acd99ba1eaac5)
+
+587.	[bug]		jelte
+	When used from python, the dynamic datasource factory now
+	explicitely loads the logging messages dictionary, so that correct
+	logging messages does not depend on incidental earlier import
+	statements. Also, the sqlite3-specific log messages have been moved
+	from the general datasource library to the sqlite3 datasource
+	(which also explicitely loads its messages).
+	(Trac 2746, git 1c004d95a8b715500af448683e4a07e9b66ea926)
+
+586.	[func]		marcin
+	libdhcp++: Removed unnecesary calls to the function which
+	validates option definitions used to create instances of options
+	being decoded in the received packets. Eliminating these calls
+	lowered the CPU utilization by the server by approximately 10%.
+	Also, added the composite search indexes on the container used to
+	store DHCP leases by Memfile backend. This resulted in the
+	significant performance rise when using this backend to store
+	leases.
+	(Trac #2701, git b96a30b26a045cfaa8ad579b0a8bf84f5ed4e73f)
+
+585.	[func]		jinmei, muks
+	The zone data loader now accepts RRs in any order during load.
+	Before it used to reject adding non-consecutive RRsets. It
+	expected records for a single owner name and its type to be
+	grouped together. These restrictions are now removed.  It now also
+	suppresses any duplicate RRs in the zone file when loading them
+	into memory.
+	(Trac #2440, git 232307060189c47285121f696d4efb206f632432)
+	(Trac #2441, git 0860ae366d73314446d4886a093f4e86e94863d4)
+
+584.	[bug]		jinmei
+	Fixed build failure with Boost 1.53 (and probably higher) in the
+	internal utility library.  Note that with -Werror it may still
+	fail, but it's due to a Boost bug that is reportedly fixed in their
+	development trunk.  See https://svn.boost.org/trac/boost/ticket/8080
+	Until the fix is available in a released Boost version you may need
+	to specify the --without-werror configure option to build BIND 10.
+	(Trac #2764, git ca1da8aa5de24358d7d4e7e9a4625347457118cf)
+
+583.	[func]*		jelte
+	b10-cmdctl-usermgr has been updated and its options and arguments
+	have changed; it now defaults to the same accounts file as
+	b10-cmdctl defaults to. It can now be used to remove users from the
+	accounts file as well, and it now accepts command-line arguments to
+	specify the username and password to add or remove, in which case
+	it will not prompt for them.
+	Note that using a password on the command line is not recommended,
+	as this can be viewed by other users.
+	(Trac #2713, git 9925af3b3f4daa47ba8c2eb66f556b01ed6f0502)
+
 582.	[func]		naokikambe
 	New statistics items related unixdomain sockets added into Xfrout :
 	open, openfail, close, bindfail, acceptfail, accept, senderr, and

Makefile.am

@@ -115,11 +115,6 @@ cppcheck:
 		--template '{file}:{line}: check_fail: {message} ({severity},{id})' \
 		src
 
-# system tests
-systest:
-	cd tests/system; \
-	sh $(abs_srcdir)/tests/system/runall.sh
-
 ### include tool to generate documentation from log message specifications
 ### in the distributed tarball:
 EXTRA_DIST = tools/system_messages.py

README

@@ -25,6 +25,12 @@ the DHCPv4 and DHCPv6 servers must be considered experimental.
 Limitations and known issues with this DHCP release can be found
 at http://bind10.isc.org/wiki/KeaKnownIssues
 
+NOTE: The API/ABI provided by libraries in BIND 10 may change in future
+point releases. So please do not assume currently that any code that you
+compile for a particular version of a BIND 10 library will work in
+future versions of the library. We aim to stabilize the public API/ABI
+interface of BIND 10 libraries in future releases.
+
 Documentation is included with the source. See doc/guide/bind10-guide.txt
 (or bind10-guide.html) for installation instructions.  The
 documentation is also available via the BIND 10 website at

configure.ac

@@ -323,9 +323,9 @@ if test -x ${PYTHON}-config; then
 	# so we only go through the flag if it's contained; also, protecting
 	# the output with [] seems necessary for environment to avoid getting
 	# an empty output accidentally.
-	python_config_ldflags=[`${PYTHON}-config --ldflags | sed -ne 's/\([ \t]*-L\)[ ]*\([^ \t]*[ \t]*\)/\1\2/pg'`]
+	python_config_ldflags=[`${PYTHON}-config --ldflags | ${SED} -ne 's/\([ \t]*-L\)[ ]*\([^ \t]*[ \t]*\)/\1\2/gp'`]
 	for flag in $python_config_ldflags; do
-		flag=`echo $flag | sed -ne 's/^\(\-L.*\)$/\1/p'`
+		flag=`echo $flag | ${SED} -ne 's/^\(\-L.*\)$/\1/p'`
 		if test "X${flag}" != X; then
 			PYTHON_LDFLAGS="$PYTHON_LDFLAGS ${flag}"
 		fi
@@ -351,7 +351,7 @@ fi
 if test "x$ISC_RPATH_FLAG" != "x"; then
 	python_rpath=
 	for flag in ${PYTHON_LDFLAGS}; do
-		python_rpath="${python_rpath} `echo $flag | sed -ne "s/^\(\-L\)/${ISC_RPATH_FLAG}/p"`"
+		python_rpath="${python_rpath} `echo $flag | ${SED} -ne "s/^\(\-L\)/${ISC_RPATH_FLAG}/p"`"
 	done
 	PYTHON_LDFLAGS="${PYTHON_LDFLAGS} ${python_rpath}"
 fi
@@ -536,7 +536,7 @@ if test "$lcov" != "no"; then
 		AC_MSG_ERROR([Cannot find lcov.])
 	fi
 	# is genhtml always in the same directory?
-	GENHTML=`echo "$LCOV" | sed s/lcov$/genhtml/`
+	GENHTML=`echo "$LCOV" | ${SED} s/lcov$/genhtml/`
 	if test ! -x $GENHTML; then
 		AC_MSG_ERROR([genhtml not found, needed for lcov])
 	fi
@@ -712,15 +712,15 @@ fi
 BOTAN_LDFLAGS=
 BOTAN_NEWLIBS=
 for flag in ${BOTAN_LIBS}; do
-    BOTAN_LDFLAGS="${BOTAN_LDFLAGS} `echo $flag | sed -ne '/^\(\-L\)/p'`"
-    BOTAN_LIBS="${BOTAN_LIBS} `echo $flag | sed -ne '/^\(\-l\)/p'`"
+    BOTAN_LDFLAGS="${BOTAN_LDFLAGS} `echo $flag | ${SED} -ne '/^\(\-L\)/p'`"
+    BOTAN_LIBS="${BOTAN_LIBS} `echo $flag | ${SED} -ne '/^\(\-l\)/p'`"
 done
 
 # See python_rpath for some info on why we do this
 if test "x$ISC_RPATH_FLAG" != "x"; then
     BOTAN_RPATH=
     for flag in ${BOTAN_LIBS}; do
-            BOTAN_RPATH="${BOTAN_RPATH} `echo $flag | sed -ne "s/^\(\-L\)/${ISC_RPATH_FLAG}/p"`"
+            BOTAN_RPATH="${BOTAN_RPATH} `echo $flag | ${SED} -ne "s/^\(\-L\)/${ISC_RPATH_FLAG}/p"`"
     done
 AC_SUBST(BOTAN_RPATH)
 
@@ -1204,6 +1204,7 @@ AC_CONFIG_FILES([Makefile
                  src/bin/stats/tests/Makefile
                  src/bin/stats/tests/testdata/Makefile
                  src/bin/usermgr/Makefile
+                 src/bin/usermgr/tests/Makefile
                  src/bin/tests/Makefile
                  src/lib/Makefile
                  src/lib/asiolink/Makefile
@@ -1304,7 +1305,6 @@ AC_CONFIG_FILES([Makefile
                  src/lib/statistics/Makefile
                  src/lib/statistics/tests/Makefile
                  tests/Makefile
-                 tests/system/Makefile
                  tests/tools/Makefile
                  tests/tools/badpacket/Makefile
                  tests/tools/badpacket/tests/Makefile
@@ -1388,23 +1388,6 @@ AC_OUTPUT([doc/version.ent
            src/lib/util/python/gen_wiredata.py
            src/lib/server_common/tests/data_path.h
            tests/lettuce/setup_intree_bind10.sh
-           tests/system/conf.sh
-           tests/system/run.sh
-           tests/system/glue/setup.sh
-           tests/system/glue/nsx1/b10-config.db
-           tests/system/bindctl/nsx1/b10-config.db.template
-           tests/system/ixfr/db.example.n0
-           tests/system/ixfr/db.example.n2
-           tests/system/ixfr/db.example.n2.refresh
-           tests/system/ixfr/db.example.n4
-           tests/system/ixfr/db.example.n6
-           tests/system/ixfr/ixfr_init.sh
-           tests/system/ixfr/b10-config.db
-           tests/system/ixfr/common_tests.sh
-           tests/system/ixfr/in-1/setup.sh
-           tests/system/ixfr/in-2/setup.sh
-           tests/system/ixfr/in-3/setup.sh
-           tests/system/ixfr/in-4/setup.sh
           ], [
            chmod +x src/bin/cmdctl/run_b10-cmdctl.sh
            chmod +x src/bin/xfrin/run_b10-xfrin.sh
@@ -1435,14 +1418,6 @@ AC_OUTPUT([doc/version.ent
            chmod +x src/lib/util/python/mkpywrapper.py
            chmod +x src/lib/util/python/gen_wiredata.py
            chmod +x src/lib/python/isc/log/tests/log_console.py
-           chmod +x tests/system/conf.sh
-           chmod +x tests/system/run.sh
-           chmod +x tests/system/ixfr/ixfr_init.sh
-           chmod +x tests/system/ixfr/common_tests.sh
-           chmod +x tests/system/ixfr/in-1/setup.sh
-           chmod +x tests/system/ixfr/in-2/setup.sh
-           chmod +x tests/system/ixfr/in-3/setup.sh
-           chmod +x tests/system/ixfr/in-4/setup.sh
           ])
 AC_OUTPUT
 

doc/design/cc-protocol.txt

@@ -1,296 +1,185 @@
-protocol version 0x536b616e
+The CC protocol
+===============
 
-DATA        0x01
-HASH        0x02
-LIST        0x03
-NULL        0x04
-TYPE_MASK   0x0f
+We use our home-grown protocol for IPC between modules. There's a
+central daemon routing the messages.
 
-LENGTH_32   0x00
-LENGTH_16   0x10
-LENGTH_8    0x20
-LENGTH_MASK 0xf0
-
-
-MESSAGE ENCODING
-----------------
-
-When decoding, the entire message length must be known.  If this is
-transmitted over a raw stream such as TCP, this is usually encoded
-with a 4-byte length followed by the message itself.  If some other
-wrapping is used (say as part of a different message structure) the
-length of the message must be preserved and included for decoding.
-
-The first 4 bytes of the message is the protocol version encoded
-directly as a 4-byte value.  Immediately following this is a HASH
-element.  The length of the hash element is the remainder of the
-message after subtracting 4 bytes for the protocol version.
-
-This initial HASH is intended to be used by the message routing system
-if one is in use.
-
-
-ITEM TYPES
+Addressing
 ----------
 
-There are four basic types encoded in this protocol.  A simple data
-blob (DATA), a tag-value series (HASH), an ordered list (LIST), and
-a NULL type (which is used internally to encode DATA types which are
-empty and can be used to indicate existance without data in a hash.)
-
-Each item can be of any type, so a hash of hashes and hashes of lists
-are typical.
-
-All multi-byte integers which are encoded in binary are in network
-byte order.
-
-
-ITEM ENCODING
--------------
-
-Each item is preceeded by a single byte which describes that item.
-This byte contains the item type and item length encoding:
-
-    Thing             Length    Description
-    ----------------  --------  ------------------------------------
-    TyLen             1 byte    Item type and length encoding
-    Length            variable  Item data blob length
-    Item Data         variable  Item data blob
-
-The TyLen field includes both the item data type and the item's
-length.  The length bytes are encoded depending on the length of data
-portion, and the smallest data encoding type supported should be
-used.  Note that this length compression is used just for data
-compactness.  It is wasteful to encode the most common length (8-bit
-length) as 4 bytes, so this method allows one byte to be used rather
-than 4, three of which are nearly always zero.
-
-
-HASH
-----
-
-This is a tag/value pair where each tag is an opaque unique blob and
-the data elements are of any type.  Hashes are not encoded in any
-specific tag or item order.
-
-The length of the HASH's data area is processed for tag/value pairs
-until the entire area is consumed.  Running out of data prematurely
-indicates an incorrectly encoded message.
-
-The data area consists of repeated items:
-
-    Thing             Length    Description
-    ----------------  --------  ------------------------------------
-    Tag Length       1 byte    The length of the tag.
-    Tag              Variable  The tag name
-    Item             Variable  Encoded item
-
-The Tag Length field is always one byte, which limits the tag name to
-255 bytes maximum.  A tag length of zero is invalid.
-
-
-LIST
-----
-
-A LIST is a list of items encoded and decoded in a specific order.
-The order is chosen entirely by the source curing encoding.
-
-The length of the LIST's data is consumed by the ITEMs it contains.
-Running out of room prematurely indicates an incorrectly encoded
-message.
-
-The data area consists of repeated items:
+Each connected client gets an unique address, called ``l-name''. A
+message can be sent directly to such l-name, if it is known to the
+sender.
 
-     Thing           Length    Description
-     --------------  ------    ----------------------------------------
-     Item	     Variable  Encoded item
+A client may subscribe to a group of communication. A message can be
+broadcasted to a whole group instead of a single client. There's also
+an instance parameter to addressing, but we didn't find any actual use
+for it and it is not used for anything. It is left in the default `*`
+for most of our code and should be done so in any new code. It wasn't
+priority to remove it yet.
 
+Wire format
+-----------
 
-DATA
-----
+Each message on the wire looks like this:
 
-A DATA item is a simple blob of data.  No further processing of this
-data is performed by this protocol on these elements.
+  <message length><header length><header><body>
 
-The data blob is the entire data area.  The data area can be 0 or more
-bytes long.
+The message length is 4-byte unsigned integer in network byte order,
+specifying the number of bytes of the rest of the message (eg. header
+length, header and body put together).
 
-It is typical to encode integers as strings rather than binary
-integers.  However, so long as both sender and recipient agree on the
-format of the data blob itself, any blob encoding may be used.
+The header length is 2-byte unsigned integer in network byte order,
+specifying the length of the header.
 
+The header is a string representation of single JSON object. It
+specifies the type of message and routing information.
 
-NULL
-----
+The body is the payload of the message. It takes the whole rest of
+size of the message (so its length is message length - 2 - header
+length). The content is not examined by the routing daemon, but the
+clients expect it to be valid JSON object.
 
-This data element indicates no data is actually present.  This can be
-used to indicate that a tag is present in a HASH but no data is
-actually at that location, or in a LIST to indicate empty item
-positions.
+The body may be empty in case the message is not to be routed to
+client, but it is instruction for the routing daemon. See message
+types below.
 
-There is no data portion of this type, and the encoded length is
-ignored and is always zero.
+The message is sent in this format to the routing daemon, the daemon
+optionally modifies the headers and delivers it in the same format to
+the recipient(s).
 
-Note that this is different than a DATA element with a zero length.
+The headers
+-----------
 
+The header object can contain following information:
 
-EXAMPLE
--------
-
-This is Ruby syntax, but should be clear enough for anyone to read.
-
-Example data encoding:
-
-{
-  "from" => "sender@host",
-  "to" => "recipient@host",
-  "seq" => 1234,
-  "data" => {
-    "list" => [ 1, 2, nil, "this" ],
-    "description" => "Fun for all",
-  },
-}
-
-
-Wire-format:
-
-In this format, strings are not shown in hex, but are included "like
-this."  Descriptions are written (like this.)
-
-Message Length: 0x64 (100 bytes)
-Protocol Version:  0x53 0x6b 0x61 0x6e
-(remaining length: 96 bytes)
-
-0x04 "from" 0x21 0x0b "sender@host"
-0x02 "to" 0x21 0x0e "recipient@host"
-0x03 "seq" 0x21 0x04 "1234"
-0x04 "data" 0x22
-  0x04 "list" 0x23 
-    0x21 0x01 "1"
-    0x21 0x01 "2"
-    0x04
-    0x21 0x04 "this"
-  0x0b "description" 0x0b "Fun for all"
-
-
-MESSAGE ROUTING
----------------
-
-The message routing daemon uses the top-level hash to contain routing
-instructions and additional control data.  Not all of these are
-required for various control message types; see the individual
-descriptions for more information.
-
-    Tag      Description
-    -------  ----------------------------------------
-    msg      Sender-supplied data
-    from     sender's identity
-    group    Group name this message is being sent to
-    instance Instance in this group
-    repl     if present, this message is a reply.
-    seq	     sequence number, used in replies
-    to	     recipient or "*" for no specific receiver
-    type     "send" for a channel message
-
-
-"type" is a DATA element, which indicates to the message routing
-system what the purpose of this message is.
+|====================================================================================================
+|Name       |type  |Description
+|====================================================================================================
+|from       |string|Sender's l-name
+|type       |string|Type of the message. The routed message is "send".
+|group      |string|The group to deliver to.
+|instance   |string|Instance in the group. Purpose lost in history. Defaults to "*".
+|to         |string|Override recipient (group/instance ignored).
+|seq        |int   |Tracking number of the message.
+|reply      |int   |If present, contains a seq number of message this is a reply to.
+|want_answer|bool  |If present and true, the daemon generates error if there's no matching recipient.
+|====================================================================================================
 
+Types of messages
+-----------------
 
 Get Local Name (type "getlname")
---------------------------------
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Upon connection, this is the first message to be sent to the control
-daemon.  It will return the local name of this client.  Each
-connection gets its own unique local name, and local names are never
-repeated.  They should be considered opaque strings, in a format
-useful only to the message routing system.  They are used in replies
-or to send to a specific destination.
+Upon connection, this is the first message to be sent to the daemon.
+It will return the local name of this client.  Each connection gets
+its own unique local name, and local names are never repeated.  They
+should be considered opaque strings, in a format useful only to the
+message routing system.  They are used in replies or to send to a
+specific destination.
 
 To request the local name, the only element included is the
-  "type" => "getlname"
+  {"type": "getlname"}
 tuple.  The response is also a simple, single tuple:
-  "lname" => "UTF-8 encoded local name blob"
+  {"lname" => "Opaque utf-8 string"}
 
 Until this message is sent, no other types of messages may be sent on
 this connection.
 
-
 Regular Group Messages (type "send")
-------------------------------------
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-When sending a message:
+Message routed to other client. This one expects the body to be
+non-empty.
 
-"msg" is the sender supplied data.  It is encoded as per its type.
-It is a required field, but may be the NULL type if not needed.
-In OpenReg, this was another wire format message, stored as an
-ITEM_DATA.  This was done to make it easy to decode the routing
-information without having to decode arbitrary application-supplied
-data, but rather treat this application data as an opaque blob.
+Expected headers are:
 
-"from" is a DATA element, and its value is a UTF-8 encoded sender
-identity.  It MUST be the "local name" supplied by the message
-routing system upon connection.  The message routing system will
-enforce this, but will not add it.  It is a required field.
+* from
+* group
+* instance (set to "*" if no specific instance desired)
+* seq (should be unique for the sender)
+* to (set to "*" if not directed to specific client)
+* reply (optional, only if it is reply)
+* want_answer (optional, only when not a reply)
 
-"group" is a DATA element, and its value is the UTF-8 encoded group
-name this message is being transmitted to.  It is a required field for
-all messages of type "send".
+A client does not see its own transmissions.
 
-"instance" is a DATA element, and its value is the UTF-8 encoded
-instance name, with "*" meaning all instances.
+Group Subscriptions (type "subscribe")
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-"repl" is the sequence number being replied to, if this is a reply.
+Indicates the sender wants to be included in the given group.
 
-"seq" is a unique identity per client.  That is, the <lname, seq>
-tuple must be unique over the lifetime of the connection, or at least
-over the lifetime of the expected reply duration.
+Expected headers are:
 
-"to" is a DATA element, and its value is a UTF-8 encoded recipient
-identity.  This must be a specific recipient name or "*" to indicate
-"all listeners on this channel."  It is a required field.
+* group
+* instance (leave at "*" for default)
 
-When a message of type "send" is received by the client, all the data
-is used as above.  This indicates a message of the given type was
-received.
+There is no response to this message and the client is subscribed to
+the given group and instance.
 
-A client does not see its own transmissions. (XXXMLG Need to check this)
+The group can be any utf-8 string and the group doesn't have to exist
+before (it is created when at least one client is in it). A client may
+be subscribed in multiple groups.
 
+Group Unsubscribe (type "unsubscribe")
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Group Subscriptions (type "subscribe")
---------------------------------------
+The headers to be included are "group" and "instance" and have the same
+meaning as a "subscribe" message. Only, the client is removed from the
+group.
 
-A subscription requires the "group", "instance", and a flag to
-indicate the subscription type ("sybtype").  If instance is "*" the
-instance name will be ignored when decising to forward a message to
-this client or not.
+Transmitted messages
+--------------------
 
-"subtype" is a DATA element, and contains "normal" for normal channel
-subscriptions, "meonly" for only those messages on a channel with the
-recipient specified exactly as the local name, or "promisc" to receive
-all channel messages regardless of other filters.  As its name
-implies, "normal" is for typical subscriptions, and "promisc" is
-intended for channel message debugging.
+These are the messages generally transmitted in the body of the
+message.
 
-There is no response to this message.
+Command
+~~~~~~~
 
+It is a command from one process to another, to do something or send
+some information. It is identified by a name and can optionally have
+parameters. It'd look like this:
 
-Group Unsubscribe (type "unsubscribe")
--------------------------------
+  {"command": ["name", <parameters>]}
+
+The parameters may be omitted (then the array is 1 element long). If
+present, it may be any JSON element. However, the most usual is an
+object with named parameter values.
+
+It is usually transmitted with the `want_answer` header turned on to
+cope with the situation the remote end doesn't exist, and sent to a
+group (eg. `to` with value of `*`).
+
+Success reply
+~~~~~~~~~~~~~
+
+When the command is successful, the other side answers by a reply of
+the following format:
+
+  {"result": [0, <result>]}
+
+The result is the return value of the command. It may be any JSON
+element and it may be omitted (for the case of ``void'' function).
 
-The fields to be included are "group" and "instance" and have the same
-meaning as a "subscribe" message.
+This is transmitted with the `reply` header set to the `seq` number of
+the original command. It is sent with the `to` header set.
 
-There is no response to this message.
+Error reply
+~~~~~~~~~~~
 
+In case something goes wrong, an error reply is sent. This is similar
+as throwing an exception from local function. The format is similar:
 
-Statistics (type "stats")
--------------------------
+  {"result": [ecode, "Error description"]}
 
-Request statistics from the message router.  No other fields are
-inclued in the request.
+The `ecode` is non-zero error code. Most of the current code uses `1`
+for all errors. The string after that is mandatory and must contain a
+human-readable description of the error.
 
-The response contains a single element "stats" which is an opaque
-element.  This is used mostly for debugging, and its format is
-specific to the message router.  In general, some method to simply
-dump raw messages would produce something useful during debugging.
+The negative error codes are reserved for errors from the daemon.
+Currently, only `-1` is used and it is generated when a message with
+`reply` not included is sent, it has the `want_answer` header set to
+`true` and there's no recipient to deliver the message to. This
+usually means a command was sent to a non-existent recipient.

doc/devel/01-dns.dox

@@ -8,7 +8,7 @@
  *
  * @section b10-cfgmgr b10-cfgmgr Overview
  *
- * @todo: Descibe b10-cfgmgr here.
+ * @todo: Describe b10-cfgmgr here.
  *
  *
  */

doc/devel/02-dhcp.dox

@@ -21,7 +21,7 @@
  *
  * DHCPv4 server component is now integrated with the BIND10 message queue.
  * The integration is performed by establishSession() and disconnectSession()
- * functions in isc::dhcp::ControlledDhcpv4Srv class. main() method deifined
+ * functions in isc::dhcp::ControlledDhcpv4Srv class. main() method defined
  * in the src/bin/dhcp4/main.cc file instantiates isc::dhcp::ControlledDhcpv4Srv
  * class that establishes connection with msgq and install necessary handlers
  * for receiving commands and configuration updates. It is derived from

examples/README

@@ -31,7 +31,7 @@ sinclude(m4/ax_boost_include.m4)
 sinclude(m4/ax_isc_bind10.m4)
 (and same for other m4 files as they are added under m4/)
 
-On some systems, espeically if you have installed the BIND 10
+On some systems, especially if you have installed the BIND 10
 libraries in an uncommon path, programs linked with the BIND 10
 library may not work at run time due to the "missing" shared library.
 Normally, you should be able to avoid this problem by making sure

examples/host/host.cc

@@ -95,7 +95,7 @@ host_lookup(const char* const name, const char* const dns_class,
         cout << "Name: " << server << "\n";
         // TODO: I guess I have to do a lookup to get that address and aliases
         // too
-        //cout << "Address: " << address << "\n" ; // "#" << port << "\n";
+        //cout << "Address: " << address << "\n" ;
         //cout << "Aliases: " << server << "\n";
     }
 

src/bin/auth/auth_messages.mes

@@ -266,9 +266,16 @@ bug ticket for this issue.
 This is a debug message issued when the authoritative server has received
 a command on the command channel.
 
-% AUTH_RECEIVED_NOTIFY received incoming NOTIFY for zone name %1, zone class %2
+% AUTH_RECEIVED_NOTIFY received incoming NOTIFY for zone %1/%2 from %3
 This is a debug message reporting that an incoming NOTIFY was received.
 
+% AUTH_RECEIVED_NOTIFY_NOTAUTH received bad NOTIFY for zone %1/%2 from %3
+The authoritative server received a NOTIFY message, but the specified zone
+doesn't match any of the zones served by the server.  The server doesn't
+process the message further, and returns a response with the Rcode being
+NOTAUTH.  Note: RFC 1996 does not specify the server behavior in this case;
+responding with Rcode of NOTAUTH follows BIND 9's behavior.
+
 % AUTH_RESPONSE_FAILURE exception while building response to query: %1
 This is a debug message, generated by the authoritative server when an
 attempt to create a response to a received DNS packet has failed. The

src/bin/auth/auth_srv.cc

@@ -747,6 +747,8 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, Message& message,
                            std::auto_ptr<TSIGContext> tsig_context,
                            MessageAttributes& stats_attrs)
 {
+    const IOEndpoint& remote_ep = io_message.getRemoteEndpoint(); // for logs
+
     // The incoming notify must contain exactly one question for SOA of the
     // zone name.
     if (message.getRRCount(Message::SECTION_QUESTION) != 1) {
@@ -769,8 +771,23 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, Message& message,
     // on, but we don't check these conditions.  This behavior is compatible
     // with BIND 9.
 
-    // TODO check with the conf-mgr whether current server is the auth of the
-    // zone
+    // See if we have the specified zone in our data sources; if not return
+    // NOTAUTH, following BIND 9 (this is not specified in RFC 1996).
+    bool is_auth = false;
+    {
+        auth::DataSrcClientsMgr::Holder datasrc_holder(datasrc_clients_mgr_);
+        const shared_ptr<datasrc::ClientList> dsrc_clients =
+            datasrc_holder.findClientList(question->getClass());
+        is_auth = dsrc_clients &&
+            dsrc_clients->find(question->getName(), true, false).exact_match_;
+    }
+    if (!is_auth) {
+        LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_RECEIVED_NOTIFY_NOTAUTH)
+            .arg(question->getName()).arg(question->getClass()).arg(remote_ep);
+        makeErrorMessage(renderer_, message, buffer, Rcode::NOTAUTH(),
+                         stats_attrs, tsig_context);
+        return (true);
+    }
 
     // In the code that follows, we simply ignore the notify if any internal
     // error happens rather than returning (e.g.) SERVFAIL.  RFC 1996 is
@@ -782,10 +799,9 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, Message& message,
     }
 
     LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_RECEIVED_NOTIFY)
-      .arg(question->getName()).arg(question->getClass());
+        .arg(question->getName()).arg(question->getClass()).arg(remote_ep);
 
-    const string remote_ip_address =
-        io_message.getRemoteEndpoint().getAddress().toText();
+    const string remote_ip_address = remote_ep.getAddress().toText();
     static const string command_template_start =
         "{\"command\": [\"notify\", {\"zone_name\" : \"";
     static const string command_template_master = "\", \"master\" : \"";

src/bin/auth/auth_srv.h

@@ -228,7 +228,7 @@ public:
      * \brief Set and get the addresses we listen on.
      */
     void setListenAddresses(const isc::server_common::portconfig::AddressList&
-                            addreses);
+                            addresses);
     const isc::server_common::portconfig::AddressList& getListenAddresses()
         const;
 

src/bin/auth/gen-statisticsitems.py.pre.in

@@ -289,7 +289,7 @@ def generate_cxx(itemsfile, ccfile, utfile, def_mtime):
         This method recursively builds two lists:
         - msg_counter_types consists of strings for all leaf items, each
           defines one enum element with a comment, e.g.
-          COUNTER_ITEM, ///< item's descriptin
+          COUNTER_ITEM, ///< item's description
         - item_names consists of tuples of three elements, depending on
           whether it's a leaf element (no child from it) or not:
           (leaf)   ( "item_name", NULL, COUNTER_ITEM )

src/bin/auth/query.cc

@@ -373,7 +373,7 @@ Query::process(datasrc::ClientList& client_list,
     // If we have no matching authoritative zone for the query name, return
     // REFUSED.  In short, this is to be compatible with BIND 9, but the
     // background discussion is not that simple.  See the relevant topic
-    // at the BIND 10 developers's ML:
+    // at the BIND 10 developers' ML:
     // https://lists.isc.org/mailman/htdig/bind10-dev/2010-December/001633.html
     if (result.dsrc_client_ == NULL) {
         // If we tried to find a "parent zone" for a DS query and failed,

src/bin/auth/query.h

@@ -329,8 +329,8 @@ public:
 
     /// \short Bad zone data encountered.
     ///
-    /// This is thrown when process encounteres misconfigured zone in a way
-    /// it can't continue. This throws, not sets the Rcode, because such
+    /// This is thrown when a process encounters a misconfigured zone in a
+    /// way it can't continue. This throws, not sets the Rcode, because such
     /// misconfigured zone should not be present in the data source and
     /// should have been rejected sooner.
     struct BadZone : public isc::Exception {

src/bin/auth/tests/auth_srv_unittest.cc

@@ -244,6 +244,62 @@ createBuiltinVersionResponse(const qid_t qid, vector<uint8_t>& data) {
                 renderer.getLength());
 }
 
+void
+installDataSrcClientLists(AuthSrv& server, ClientListMapPtr lists) {
+    // For now, we use explicit swap than reconfigure() because the latter
+    // involves a separate thread and cannot guarantee the new config is
+    // available for the subsequent test.
+    server.getDataSrcClientsMgr().setDataSrcClientLists(lists);
+}
+
+void
+updateDatabase(AuthSrv& server, const char* params) {
+    const ConstElementPtr config(Element::fromJSON("{"
+        "\"IN\": [{"
+        "    \"type\": \"sqlite3\","
+        "    \"params\": " + string(params) +
+        "}]}"));
+    installDataSrcClientLists(server, configureDataSource(config));
+}
+
+// Note: if with_static is set to true, the corresponding test should be
+// disabled in case of USE_STATIC_LINK.
+void
+updateInMemory(AuthSrv& server, const char* origin, const char* filename,
+               bool with_static = true)
+{
+    string spec_txt = "{"
+        "\"IN\": [{"
+        "   \"type\": \"MasterFiles\","
+        "   \"params\": {"
+        "       \"" + string(origin) + "\": \"" + string(filename) + "\""
+        "   },"
+        "   \"cache-enable\": true"
+        "}]";
+    if (with_static) {
+        spec_txt += ", \"CH\": [{"
+        "   \"type\": \"static\","
+        "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
+            "}]";
+    }
+    spec_txt += "}";
+
+    const ConstElementPtr config(Element::fromJSON(spec_txt));
+    installDataSrcClientLists(server, configureDataSource(config));
+}
+
+// Note: tests using this function should be disabled in case of
+// USE_STATIC_LINK.
+void
+updateBuiltin(AuthSrv& server) {
+    const ConstElementPtr config(Element::fromJSON("{"
+        "\"CH\": [{"
+        "   \"type\": \"static\","
+        "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
+        "}]}"));
+    installDataSrcClientLists(server, configureDataSource(config));
+}
+
 // We did not configure any client lists. Therefore it should be REFUSED
 TEST_F(AuthSrvTest, noClientList) {
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
@@ -647,8 +703,10 @@ TEST_F(AuthSrvTest, IXFRDisconnectFail) {
 }
 
 TEST_F(AuthSrvTest, notify) {
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE, false);
+
     UnitTestUtil::createRequestMessage(request_message, Opcode::NOTIFY(),
-                                       default_qid, Name("example.com"),
+                                       default_qid, Name("example"),
                                        RRClass::IN(), RRType::SOA());
     request_message.setHeaderFlag(Message::HEADERFLAG_AA);
     createRequestPacket(request_message, IPPROTO_UDP);
@@ -664,7 +722,7 @@ TEST_F(AuthSrvTest, notify) {
                   stringValue());
     ConstElementPtr notify_args =
         notify_session.getSentMessage()->get("command")->get(1);
-    EXPECT_EQ("example.com.", notify_args->get("zone_name")->stringValue());
+    EXPECT_EQ("example.", notify_args->get("zone_name")->stringValue());
     EXPECT_EQ(DEFAULT_REMOTE_ADDRESS,
               notify_args->get("master")->stringValue());
     EXPECT_EQ("IN", notify_args->get("zone_class")->stringValue());
@@ -675,7 +733,7 @@ TEST_F(AuthSrvTest, notify) {
 
     // The question must be identical to that of the received notify
     ConstQuestionPtr question = *parse_message->beginQuestion();
-    EXPECT_EQ(Name("example.com"), question->getName());
+    EXPECT_EQ(Name("example"), question->getName());
     EXPECT_EQ(RRClass::IN(), question->getClass());
     EXPECT_EQ(RRType::SOA(), question->getType());
 
@@ -690,10 +748,17 @@ TEST_F(AuthSrvTest, notify) {
     checkStatisticsCounters(stats_after, expect);
 }
 
+#ifdef USE_STATIC_LINK
+TEST_F(AuthSrvTest, DISABLED_notifyForCHClass) {
+#else
 TEST_F(AuthSrvTest, notifyForCHClass) {
-    // Same as the previous test, but for the CH RRClass.
+#endif
+    // Same as the previous test, but for the CH RRClass (so we install the
+    // builtin (static) data source.
+    updateBuiltin(server);
+
     UnitTestUtil::createRequestMessage(request_message, Opcode::NOTIFY(),
-                                       default_qid, Name("example.com"),
+                                       default_qid, Name("bind"),
                                        RRClass::CH(), RRType::SOA());
     request_message.setHeaderFlag(Message::HEADERFLAG_AA);
     createRequestPacket(request_message, IPPROTO_UDP);
@@ -773,9 +838,11 @@ TEST_F(AuthSrvTest, notifyNonSOAQuestion) {
 }
 
 TEST_F(AuthSrvTest, notifyWithoutAA) {
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE, false);
+
     // implicitly leave the AA bit off.  our implementation will accept it.
     UnitTestUtil::createRequestMessage(request_message, Opcode::NOTIFY(),
-                                       default_qid, Name("example.com"),
+                                       default_qid, Name("example"),
                                        RRClass::IN(), RRType::SOA());
     createRequestPacket(request_message, IPPROTO_UDP);
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -786,8 +853,10 @@ TEST_F(AuthSrvTest, notifyWithoutAA) {
 }
 
 TEST_F(AuthSrvTest, notifyWithErrorRcode) {
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE, false);
+
     UnitTestUtil::createRequestMessage(request_message, Opcode::NOTIFY(),
-                                       default_qid, Name("example.com"),
+                                       default_qid, Name("example"),
                                        RRClass::IN(), RRType::SOA());
     request_message.setHeaderFlag(Message::HEADERFLAG_AA);
     request_message.setRcode(Rcode::SERVFAIL());
@@ -800,10 +869,12 @@ TEST_F(AuthSrvTest, notifyWithErrorRcode) {
 }
 
 TEST_F(AuthSrvTest, notifyWithoutSession) {
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE, false);
+
     server.setXfrinSession(NULL);
 
     UnitTestUtil::createRequestMessage(request_message, Opcode::NOTIFY(),
-                                       default_qid, Name("example.com"),
+                                       default_qid, Name("example"),
                                        RRClass::IN(), RRType::SOA());
     request_message.setHeaderFlag(Message::HEADERFLAG_AA);
     createRequestPacket(request_message, IPPROTO_UDP);
@@ -816,10 +887,12 @@ TEST_F(AuthSrvTest, notifyWithoutSession) {
 }
 
 TEST_F(AuthSrvTest, notifySendFail) {
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE, false);
+
     notify_session.disableSend();
 
     UnitTestUtil::createRequestMessage(request_message, Opcode::NOTIFY(),
-                                       default_qid, Name("example.com"),
+                                       default_qid, Name("example"),
                                        RRClass::IN(), RRType::SOA());
     request_message.setHeaderFlag(Message::HEADERFLAG_AA);
     createRequestPacket(request_message, IPPROTO_UDP);
@@ -830,10 +903,12 @@ TEST_F(AuthSrvTest, notifySendFail) {
 }
 
 TEST_F(AuthSrvTest, notifyReceiveFail) {
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE, false);
+
     notify_session.disableReceive();
 
     UnitTestUtil::createRequestMessage(request_message, Opcode::NOTIFY(),
-                                       default_qid, Name("example.com"),
+                                       default_qid, Name("example"),
                                        RRClass::IN(), RRType::SOA());
     request_message.setHeaderFlag(Message::HEADERFLAG_AA);
     createRequestPacket(request_message, IPPROTO_UDP);
@@ -843,10 +918,12 @@ TEST_F(AuthSrvTest, notifyReceiveFail) {
 }
 
 TEST_F(AuthSrvTest, notifyWithBogusSessionMessage) {
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE, false);
+
     notify_session.setMessage(Element::fromJSON("{\"foo\": 1}"));
 
     UnitTestUtil::createRequestMessage(request_message, Opcode::NOTIFY(),
-                                       default_qid, Name("example.com"),
+                                       default_qid, Name("example"),
                                        RRClass::IN(), RRType::SOA());
     request_message.setHeaderFlag(Message::HEADERFLAG_AA);
     createRequestPacket(request_message, IPPROTO_UDP);
@@ -856,11 +933,13 @@ TEST_F(AuthSrvTest, notifyWithBogusSessionMessage) {
 }
 
 TEST_F(AuthSrvTest, notifyWithSessionMessageError) {
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE, false);
+
     notify_session.setMessage(
         Element::fromJSON("{\"result\": [1, \"FAIL\"]}"));
 
     UnitTestUtil::createRequestMessage(request_message, Opcode::NOTIFY(),
-                                       default_qid, Name("example.com"),
+                                       default_qid, Name("example"),
                                        RRClass::IN(), RRType::SOA());
     request_message.setHeaderFlag(Message::HEADERFLAG_AA);
     createRequestPacket(request_message, IPPROTO_UDP);
@@ -869,49 +948,50 @@ TEST_F(AuthSrvTest, notifyWithSessionMessageError) {
     EXPECT_FALSE(dnsserv.hasAnswer());
 }
 
-void
-installDataSrcClientLists(AuthSrv& server, ClientListMapPtr lists) {
-    // For now, we use explicit swap than reconfigure() because the latter
-    // involves a separate thread and cannot guarantee the new config is
-    // available for the subsequent test.
-    server.getDataSrcClientsMgr().setDataSrcClientLists(lists);
-}
+TEST_F(AuthSrvTest, notifyNotAuth) {
+    // If the server doesn't have authority of the specified zone in NOTIFY,
+    // it will return NOTAUTH
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE, false);
 
-void
-updateDatabase(AuthSrv& server, const char* params) {
-    const ConstElementPtr config(Element::fromJSON("{"
-        "\"IN\": [{"
-        "    \"type\": \"sqlite3\","
-        "    \"params\": " + string(params) +
-        "}]}"));
-    installDataSrcClientLists(server, configureDataSource(config));
+    UnitTestUtil::createRequestMessage(request_message, Opcode::NOTIFY(),
+                                       default_qid, Name("example.com"),
+                                       RRClass::IN(), RRType::SOA());
+    request_message.setHeaderFlag(Message::HEADERFLAG_AA);
+    createRequestPacket(request_message, IPPROTO_UDP);
+    server.processMessage(*io_message, *parse_message, *response_obuffer,
+                          &dnsserv);
+    EXPECT_TRUE(dnsserv.hasAnswer());
+    headerCheck(*parse_message, default_qid, Rcode::NOTAUTH(),
+                Opcode::NOTIFY().getCode(), QR_FLAG /* no AA */, 1, 0, 0, 0);
 }
 
-void
-updateInMemory(AuthSrv& server, const char* origin, const char* filename) {
-    const ConstElementPtr config(Element::fromJSON("{"
-        "\"IN\": [{"
-        "   \"type\": \"MasterFiles\","
-        "   \"params\": {"
-        "       \"" + string(origin) + "\": \"" + string(filename) + "\""
-        "   },"
-        "   \"cache-enable\": true"
-        "}],"
-        "\"CH\": [{"
-        "   \"type\": \"static\","
-        "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
-        "}]}"));
-    installDataSrcClientLists(server, configureDataSource(config));
+TEST_F(AuthSrvTest, notifyNotAuthSubDomain) {
+    // Similar to the previous case, but checking partial match doesn't confuse
+    // the processing.
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE, false);
+
+    UnitTestUtil::createRequestMessage(request_message, Opcode::NOTIFY(),
+                                       default_qid, Name("child.example"),
+                                       RRClass::IN(), RRType::SOA());
+    createRequestPacket(request_message, IPPROTO_UDP);
+    server.processMessage(*io_message, *parse_message, *response_obuffer,
+                          &dnsserv);
+    headerCheck(*parse_message, default_qid, Rcode::NOTAUTH(),
+                Opcode::NOTIFY().getCode(), QR_FLAG, 1, 0, 0, 0);
 }
 
-void
-updateBuiltin(AuthSrv& server) {
-    const ConstElementPtr config(Element::fromJSON("{"
-        "\"CH\": [{"
-        "   \"type\": \"static\","
-        "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
-        "}]}"));
-    installDataSrcClientLists(server, configureDataSource(config));
+TEST_F(AuthSrvTest, notifyNotAuthNoClass) {
+    // Likewise, and there's not even a data source in the specified class.
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE, false);
+
+    UnitTestUtil::createRequestMessage(request_message, Opcode::NOTIFY(),
+                                       default_qid, Name("example"),
+                                       RRClass::CH(), RRType::SOA());
+    createRequestPacket(request_message, IPPROTO_UDP);
+    server.processMessage(*io_message, *parse_message, *response_obuffer,
+                          &dnsserv);
+    headerCheck(*parse_message, default_qid, Rcode::NOTAUTH(),
+                Opcode::NOTIFY().getCode(), QR_FLAG, 1, 0, 0, 0);
 }
 
 // Try giving the server a TSIG signed request and see it can anwer signed as
@@ -1679,7 +1759,7 @@ public:
              data_sources_.push_back(
                  DataSourceInfo(client.get(),
                                 isc::datasrc::DataSourceClientContainerPtr(),
-                                false, RRClass::IN(), ztable_segment_));
+                                false, RRClass::IN(), ztable_segment_, ""));
         }
     }
 private:

src/bin/bind10/TODO

@@ -6,7 +6,7 @@
   - Stop a component
   - Force-stop a component
 - Mechanism to wait for child to start before continuing
-- Use .spec file to define comands
+- Use .spec file to define commands
 - Rename "c-channel" stuff to msgq for clarity
 - Reply to shutdown message?
 - Some sort of group creation so termination signals can be sent to

src/bin/bind10/init.py.in

@@ -38,6 +38,7 @@ __main__.
 
 import sys; sys.path.append ('@@PYTHONPATH@@')
 import os
+from isc.util.address_formatter import AddressFormatter
 
 # If B10_FROM_SOURCE is set in the environment, we use data files
 # from a directory relative to that, otherwise we use the ones
@@ -151,7 +152,7 @@ class ProcessInfo:
         """Function used before running a program that needs to run as a
         different user."""
         # First, put us into a separate process group so we don't get
-        # SIGINT signals on Ctrl-C (b10-init will shut everthing down by
+        # SIGINT signals on Ctrl-C (b10-init will shut everything down by
         # other means).
         os.setpgrp()
 
@@ -428,7 +429,7 @@ class Init:
                         port)
         else:
             logger.info(BIND10_STARTING_PROCESS_PORT_ADDRESS,
-                        self.curproc, address, port)
+                        self.curproc, AddressFormatter((address, port)))
 
     def log_started(self, pid = None):
         """

src/bin/bind10/init_messages.mes

@@ -34,7 +34,7 @@ The named component is about to be started by the b10-init process.
 
 % BIND10_COMPONENT_START_EXCEPTION component %1 failed to start: %2
 An exception (mentioned in the message) happened during the startup of the
-named component. The componet is not considered started and further actions
+named component. The component is not considered started and further actions
 will be taken about it.
 
 % BIND10_COMPONENT_STOP component %1 is being stopped
@@ -285,9 +285,9 @@ The b10-init module is starting the given process.
 The b10-init module is starting the given process, which will listen on the
 given port number.
 
-% BIND10_STARTING_PROCESS_PORT_ADDRESS starting process %1 (to listen on %2#%3)
+% BIND10_STARTING_PROCESS_PORT_ADDRESS starting process %1 (to listen on %2)
 The b10-init module is starting the given process, which will listen on the
-given address and port number (written as <address>#<port>).
+given address and port number (written as <address>:<port>).
 
 % BIND10_STARTUP_COMPLETE BIND 10 started
 All modules have been successfully started, and BIND 10 is now running.

src/bin/bind10/tests/init_test.py.in

@@ -2339,6 +2339,16 @@ class SocketSrvTest(unittest.TestCase):
         self.assertEqual({}, self.__b10_init._unix_sockets)
         self.assertTrue(sock.closed)
 
+    def test_log_starting(self):
+        """
+        Checks the log_starting call doesn't raise any errors
+        (does not check actual log output)
+        """
+        self.__b10_init.log_starting("foo")
+        self.__b10_init.log_starting("foo", 1)
+        self.__b10_init.log_starting("foo", 1, "192.0.2.1")
+
+
 class TestFunctions(unittest.TestCase):
     def setUp(self):
         self.lockfile_testpath = \

src/bin/bindctl/README

@@ -1,2 +1,2 @@
 1. Start bindctl by run command "sh bindctl".
-2. Login to b10-cmdctld with the username and password. The username and password will be saved in file default_user.csv automatcally after logining successfully, so next time bindctl will login with the username and password saved in default_user.csv. For more usage information, please turn to "man bindctl".
+2. Login to b10-cmdctl with the username and password. The username and password will be saved in file default_user.csv automatically after logining successfully, so next time bindctl will login with the username and password saved in default_user.csv. For more usage information, please turn to "man bindctl".

src/bin/bindctl/bindcmd.py

@@ -33,6 +33,7 @@ import inspect
 import pprint
 import ssl, socket
 import os, time, random, re
+import os.path
 import getpass
 from hashlib import sha1
 import csv
@@ -214,22 +215,16 @@ WARNING: Python readline module isn't available, so the command line editor
 
         return True
 
-    def __print_check_ssl_msg(self):
-        self._print("Please check the logs of b10-cmdctl, there may "
-                    "be a problem accepting SSL connections, such "
-                    "as a permission problem on the server "
-                    "certificate file.")
-
     def _try_login(self, username, password):
         '''
-        Attempts to log in to cmdctl by sending a POST with
-        the given username and password.
-        On success of the POST (mind, not the login, only the network
-        operation), returns a tuple (response, data).
-        On failure, raises a FailToLogin exception, and prints some
-        information on the failure.
-        This call is essentially 'private', but made 'protected' for
-        easier testing.
+        Attempts to log into cmdctl by sending a POST with the given
+        username and password. On success of the POST (not the login,
+        but the network operation), it returns a tuple (response, data).
+        We check for some failures such as SSL errors and socket errors
+        which could happen due to the environment in which BIND 10 runs.
+        On failure, it raises a FailToLogin exception and prints some
+        information on the failure.  This call is essentially 'private',
+        but made 'protected' for easier testing.
         '''
         param = {'username': username, 'password' : password}
         try:
@@ -237,16 +232,8 @@ WARNING: Python readline module isn't available, so the command line editor
             data = response.read().decode()
             # return here (will raise error after try block)
             return (response, data)
-        except ssl.SSLError as err:
-            self._print("SSL error while sending login information: ", err)
-            if err.errno == ssl.SSL_ERROR_EOF:
-                self.__print_check_ssl_msg()
-        except socket.error as err:
-            self._print("Socket error while sending login information: ", err)
-            # An SSL setup error can also bubble up as a plain CONNRESET...
-            # (on some systems it usually does)
-            if err.errno == errno.ECONNRESET:
-                self.__print_check_ssl_msg()
+        except (ssl.SSLError, socket.error) as err:
+            self._print('Error while sending login information:', err)
             pass
         raise FailToLogin()
 
@@ -270,9 +257,21 @@ WARNING: Python readline module isn't available, so the command line editor
 
         # No valid logins were found, prompt the user for a username/password
         count = 0
-        self._print('No stored password file found, please see sections '
-              '"Configuration specification for b10-cmdctl" and "bindctl '
-              'command-line options" of the BIND 10 guide.')
+        if not os.path.exists(self.csv_file_dir + CSV_FILE_NAME):
+            self._print('\nNo stored password file found.\n\n'
+                        'When the system is first set up you need to create '
+                        'at least one user account.\n'
+                        'For information on how to set up a BIND 10 system, '
+                        'please check see the\n'
+                        'BIND 10 Guide: \n\n'
+                        'http://bind10.isc.org/docs/bind10-guide.html#quick-start-auth-dns\n\n'
+
+                        'If a user account has been set up, please check the '
+                        'b10-cmdctl log for other\n'
+                        'information.\n')
+        else:
+            self._print('Login failed: either the user name or password is '
+                        'invalid.\n')
         while True:
             count = count + 1
             if count > 3:
@@ -317,14 +316,14 @@ WARNING: Python readline module isn't available, so the command line editor
             return {}
 
 
-    def send_POST(self, url, post_param = None):
+    def send_POST(self, url, post_param=None):
         '''Send POST request to cmdctl, session id is send with the name
         'cookie' in header.
         Format: /module_name/command_name
         parameters of command is encoded as a map
         '''
         param = None
-        if (len(post_param) != 0):
+        if post_param is not None and len(post_param) != 0:
             param = json.dumps(post_param)
 
         headers = {"cookie" : self.session_id}
@@ -938,5 +937,3 @@ WARNING: Python readline module isn't available, so the command line editor
         if data != "" and data != "{}":
             self._print(json.dumps(json.loads(data), sort_keys=True,
                                    indent=4))
-
-

src/bin/bindctl/tests/bindctl_test.py

@@ -26,6 +26,7 @@ import http.client
 import pwd
 import getpass
 import re
+import json
 from optparse import OptionParser
 from isc.config.config_data import ConfigData, MultiConfigData
 from isc.config.module_spec import ModuleSpec
@@ -386,7 +387,7 @@ class TestConfigCommands(unittest.TestCase):
             self.tool.send_POST = send_POST_raiseImmediately
             self.assertRaises(FailToLogin, self.tool._try_login, "foo", "bar")
             expected_printed_messages.append(
-                'Socket error while sending login information:  test error')
+                'Error while sending login information: test error')
             self.__check_printed_messages(expected_printed_messages)
 
             def create_send_POST_raiseOnRead(exception):
@@ -405,7 +406,7 @@ class TestConfigCommands(unittest.TestCase):
                 create_send_POST_raiseOnRead(socket.error("read error"))
             self.assertRaises(FailToLogin, self.tool._try_login, "foo", "bar")
             expected_printed_messages.append(
-                'Socket error while sending login information:  read error')
+                'Error while sending login information: read error')
             self.__check_printed_messages(expected_printed_messages)
 
             # connection reset
@@ -415,13 +416,7 @@ class TestConfigCommands(unittest.TestCase):
                 create_send_POST_raiseOnRead(exc)
             self.assertRaises(FailToLogin, self.tool._try_login, "foo", "bar")
             expected_printed_messages.append(
-                'Socket error while sending login information:  '
-                'connection reset')
-            expected_printed_messages.append(
-                'Please check the logs of b10-cmdctl, there may be a '
-                'problem accepting SSL connections, such as a permission '
-                'problem on the server certificate file.'
-            )
+                'Error while sending login information: connection reset')
             self.__check_printed_messages(expected_printed_messages)
 
             # 'normal' SSL error
@@ -430,7 +425,7 @@ class TestConfigCommands(unittest.TestCase):
                 create_send_POST_raiseOnRead(exc)
             self.assertRaises(FailToLogin, self.tool._try_login, "foo", "bar")
             expected_printed_messages.append(
-                'SSL error while sending login information:  .*')
+                'Error while sending login information: .*')
             self.__check_printed_messages(expected_printed_messages)
 
             # 'EOF' SSL error
@@ -440,12 +435,7 @@ class TestConfigCommands(unittest.TestCase):
                 create_send_POST_raiseOnRead(exc)
             self.assertRaises(FailToLogin, self.tool._try_login, "foo", "bar")
             expected_printed_messages.append(
-                'SSL error while sending login information: .*')
-            expected_printed_messages.append(
-                'Please check the logs of b10-cmdctl, there may be a '
-                'problem accepting SSL connections, such as a permission '
-                'problem on the server certificate file.'
-            )
+                'Error while sending login information: .*')
             self.__check_printed_messages(expected_printed_messages)
 
             # any other exception should be passed through
@@ -457,6 +447,44 @@ class TestConfigCommands(unittest.TestCase):
         finally:
             self.tool.send_POST = orig_send_POST
 
+    def test_try_login_calls_cmdctl(self):
+        # Make sure _try_login() makes the right API call to cmdctl.
+        orig_conn = self.tool.conn
+        try:
+            class MyConn:
+                def __init__(self):
+                    self.method = None
+                    self.url = None
+                    self.param = None
+                    self.headers = None
+
+                def request(self, method, url, param, headers):
+                    self.method = method
+                    self.url = url
+                    self.param = param
+                    self.headers = headers
+
+                def getresponse(self):
+                    class MyResponse:
+                        def __init__(self):
+                            self.status = http.client.OK
+                        def read(self):
+                            class MyData:
+                                def decode(self):
+                                    return json.dumps(True)
+                            return MyData()
+                    return MyResponse()
+
+            self.tool.conn = MyConn()
+            self.assertTrue(self.tool._try_login('user32', 'pass64'))
+            self.assertEqual(self.tool.conn.method, 'POST')
+            self.assertEqual(self.tool.conn.url, '/login')
+            self.assertEqual(json.loads(self.tool.conn.param),
+                             {"password": "pass64", "username": "user32"})
+            self.assertIn('cookie', self.tool.conn.headers)
+        finally:
+            self.tool.conn = orig_conn
+
     def test_run(self):
         def login_to_cmdctl():
             return True

src/bin/cfgmgr/plugins/datasrc.spec.pre.in

@@ -63,6 +63,11 @@
                                     "item_optional": false,
                                     "item_default": ""
                                 }
+                            },
+                            {
+                                "item_name": "name",
+                                "item_type": "string",
+                                "item_optional": true
                             }
                         ]
                     }

src/bin/cfgmgr/plugins/tests/datasrc_test.py

@@ -142,7 +142,7 @@ class DatasrcTest(unittest.TestCase):
 
     def test_no_such_file_mem(self):
         """
-        We also check the existance of master files. Not the actual content,
+        We also check the existence of master files. Not the actual content,
         though.
         """
         self.reject({"IN": [{
@@ -153,6 +153,101 @@ class DatasrcTest(unittest.TestCase):
             }
         }]})
 
+    def test_names_present(self):
+        """
+        Test we don't choke on configuration with the "name" being present on
+        some items.
+        """
+        self.accept({"IN": [{
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {},
+            "name": "Whatever"
+        }]})
+
+    def test_names_default_classes(self):
+        """
+        Test we can have a client of the same type in different classes
+        without specified name. The defaults should be derived both from
+        the type and the class.
+        """
+        self.accept({
+        "IN": [{
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {}
+        }],
+        "CH": [{
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {}
+        }]})
+
+    def test_names_collision(self):
+        """
+        Reject when two names are the same.
+
+        Cases are:
+        - Explicit names.
+        - Two default names turn out to be the same (same type and class).
+        - One explicit is set to the same as the default one.
+        """
+        self.reject({"IN": [
+        {
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {},
+            "name": "Whatever"
+        },
+        {
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {},
+            "name": "Whatever"
+        }]})
+        # The same, but across different classes is allowed (we would
+        # identify the data source by class+name tuple)
+        self.accept({
+        "IN": [
+            {
+                "type": "MasterFiles",
+                "cache-enable": True,
+                "params": {},
+                "name": "Whatever"
+            }
+        ],
+        "CH": [
+            {
+                "type": "MasterFiles",
+                "cache-enable": True,
+                "params": {},
+                "name": "Whatever"
+            }
+        ]})
+        self.reject({"IN": [
+        {
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {}
+        },
+        {
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {}
+        }]})
+        self.reject({"IN": [
+        {
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {},
+            "name": "MasterFiles"
+        },
+        {
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {}
+        }]})
+
 if __name__ == '__main__':
     isc.log.init("bind10")
     isc.log.resetUnitTestRootLogger()

src/bin/cfgmgr/plugins/tests/tsig_keys_test.py

@@ -86,7 +86,7 @@ class TSigKeysTest(unittest.TestCase):
         self.assertEqual("TSIG: Invalid TSIG key string: invalid.key",
                          tsig_keys.check({'keys': ['invalid.key']}))
         self.assertEqual(
-            "TSIG: Unexpected end of input in BASE decoder",
+            "TSIG: Incomplete input for base64: 123",
             tsig_keys.check({'keys': ['invalid.key:123']}))
 
     def test_bad_format(self):

src/bin/cmdctl/Makefile.am

@@ -11,20 +11,18 @@ pylogmessagedir = $(pyexecdir)/isc/log_messages/
 
 b10_cmdctldir = $(pkgdatadir)
 
-USERSFILES = cmdctl-accounts.csv
 CERTFILES = cmdctl-keyfile.pem cmdctl-certfile.pem
 
 b10_cmdctl_DATA = cmdctl.spec
 
-EXTRA_DIST = $(USERSFILES)
-
 CLEANFILES= b10-cmdctl cmdctl.pyc cmdctl.spec
 CLEANFILES += $(PYTHON_LOGMSGPKG_DIR)/work/cmdctl_messages.py
 CLEANFILES += $(PYTHON_LOGMSGPKG_DIR)/work/cmdctl_messages.pyc
 
 man_MANS = b10-cmdctl.8 b10-certgen.1
 DISTCLEANFILES = $(man_MANS) cmdctl-certfile.pem cmdctl-keyfile.pem
-EXTRA_DIST += $(man_MANS) b10-certgen.xml b10-cmdctl.xml cmdctl_messages.mes
+EXTRA_DIST  = $(man_MANS) b10-certgen.xml b10-cmdctl.xml cmdctl_messages.mes
+EXTRA_DIST += cmdctl-accounts.csv
 
 if GENERATE_DOCS
 

src/bin/cmdctl/b10-certgen.cc

@@ -62,7 +62,7 @@ usage() {
     std::cout << "Options:" << std::endl;
     std::cout << "-c, --certfile=FILE\t\tfile to read or store the certificate"
               << std::endl;
-    std::cout << "-f, --force\t\t\toverwrite existing certficate even if it"
+    std::cout << "-f, --force\t\t\toverwrite existing certificate even if it"
               << std::endl <<"\t\t\t\tis valid" << std::endl;
     std::cout << "-h, --help\t\t\tshow this help" << std::endl;
     std::cout << "-k, --keyfile=FILE\t\tfile to store the generated private key"
@@ -194,7 +194,7 @@ public:
 
             print("Creating certificate file " + cert_file_name);
 
-            // The exact call changed aftert 1.8, adding the
+            // The exact call changed after 1.8, adding the
             // hash function option
 #if BOTAN_VERSION_CODE >= BOTAN_VERSION_CODE_FOR(1,9,0)
             X509_Certificate cert =
@@ -229,7 +229,7 @@ public:
     validateCertificate(const std::string& certfile) {
         // Since we are dealing with a self-signed certificate here, we
         // also use the certificate to check itself; i.e. we add it
-        // as a trusted certificate, then validate the certficate itself.
+        // as a trusted certificate, then validate the certificate itself.
         //const X509_Certificate cert(certfile);
         try {
             X509_Store store;

src/bin/cmdctl/cmdctl.py.in

@@ -42,6 +42,7 @@ import random
 import time
 import signal
 from isc.config import ccsession
+import isc.cc.proto_defs
 import isc.util.process
 import isc.net.parse
 from optparse import OptionParser, OptionValueError
@@ -97,6 +98,11 @@ def check_file(file_name):
 class SecureHTTPRequestHandler(http.server.BaseHTTPRequestHandler):
     '''https connection request handler.
     Currently only GET and POST are supported.  '''
+    def __init__(self, request, client_address, server):
+        http.server.BaseHTTPRequestHandler.__init__(self, request,
+                                                    client_address, server)
+        self.session_id = None
+
     def do_GET(self):
         '''The client should send its session id in header with
         the name 'cookie'
@@ -121,7 +127,7 @@ class SecureHTTPRequestHandler(http.server.BaseHTTPRequestHandler):
         return self.server.get_reply_data_for_GET(id, module)
 
     def _is_session_valid(self):
-        return self.session_id
+        return self.session_id is not None
 
     def _is_user_logged_in(self):
         login_time = self.server.user_sessions.get(self.session_id)
@@ -171,7 +177,7 @@ class SecureHTTPRequestHandler(http.server.BaseHTTPRequestHandler):
         is_user_valid, error_info = self._check_user_name_and_pwd()
         if is_user_valid:
             self.server.save_user_session_id(self.session_id)
-            return http.client.OK, ["login success "]
+            return http.client.OK, ["login success"]
         else:
             return http.client.UNAUTHORIZED, error_info
 
@@ -454,7 +460,8 @@ class CommandControl():
                     else:
                         return rcode, {}
                 else:
-                    errstr = str(answer['result'][1])
+                    errstr = \
+                        str(answer[isc.cc.proto_defs.CC_PAYLOAD_RESULT][1])
             except ccsession.ModuleCCSessionError as mcse:
                 errstr = str("Error in ccsession answer:") + str(mcse)
 
@@ -502,12 +509,25 @@ class SecureHTTPServer(socketserver_mixin.NoPollMixIn,
         self._verbose = verbose
         self._lock = threading.Lock()
         self._user_infos = {}
-        self._accounts_file = None
+        self.__accounts_file = None
+        self.__accounts_file_mtime = 0
 
     def _create_user_info(self, accounts_file):
         '''Read all user's name and its' salt, hashed password
         from accounts file.'''
-        if (self._accounts_file == accounts_file) and (len(self._user_infos) > 0):
+
+        # If the file does not exist, set accounts to empty, and return
+        if not os.path.exists(accounts_file):
+            self._user_infos = {}
+            self.__accounts_file = None
+            self.__accounts_file_mtime = 0
+            return
+
+        # If the filename hasn't changed, and the file itself
+        # has neither, do nothing
+        accounts_file_mtime = os.stat(accounts_file).st_mtime
+        if self.__accounts_file == accounts_file and\
+           accounts_file_mtime <= self.__accounts_file_mtime:
             return
 
         with self._lock:
@@ -525,7 +545,8 @@ class SecureHTTPServer(socketserver_mixin.NoPollMixIn,
                 if csvfile:
                     csvfile.close()
 
-        self._accounts_file = accounts_file
+        self.__accounts_file = accounts_file
+        self.__accounts_file_mtime = accounts_file_mtime
         if len(self._user_infos) == 0:
             logger.error(CMDCTL_NO_USER_ENTRIES_READ)
 

src/bin/cmdctl/tests/Makefile.am

@@ -25,8 +25,8 @@ endif
 	echo Running test: $$pytest ; \
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/bin/cmdctl \
-	CMDCTL_BUILD_PATH=$(abs_top_builddir)/src/bin/cmdctl \
 	CMDCTL_SRC_PATH=$(abs_top_srcdir)/src/bin/cmdctl \
+	CMDCTL_BUILD_PATH=$(abs_top_builddir)/src/bin/cmdctl \
 	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/bin/cmdctl/tests/b10-certgen_test.py

@@ -172,10 +172,7 @@ class TestCertGenTool(unittest.TestCase):
         """
         Tests a few pre-created certificates with the -c option
         """
-        if ('CMDCTL_SRC_PATH' in os.environ):
-            path = os.environ['CMDCTL_SRC_PATH'] + "/tests/testdata/"
-        else:
-            path = "testdata/"
+        path = os.environ['CMDCTL_SRC_PATH'] + '/tests/testdata/'
         self.validate_certificate(10, path + 'expired-certfile.pem')
         self.validate_certificate(100, path + 'mangled-certfile.pem')
         self.validate_certificate(17, path + 'noca-certfile.pem')

src/bin/cmdctl/tests/cmdctl_test.py

@@ -17,6 +17,7 @@
 import unittest
 import socket
 import tempfile
+import time
 import stat
 import sys
 from cmdctl import *
@@ -33,7 +34,7 @@ BUILD_FILE_PATH = os.environ['CMDCTL_BUILD_PATH'] + os.sep
 # Rewrite the class for unittest.
 class MySecureHTTPRequestHandler(SecureHTTPRequestHandler):
     def __init__(self):
-        pass
+        self.session_id = None
 
     def send_response(self, rcode):
         self.rcode = rcode
@@ -41,19 +42,6 @@ class MySecureHTTPRequestHandler(SecureHTTPRequestHandler):
     def end_headers(self):
         pass
 
-    def do_GET(self):
-        self.wfile = open('tmp.file', 'wb')
-        super().do_GET()
-        self.wfile.close()
-        os.remove('tmp.file')
-
-    def do_POST(self):
-        self.wfile = open("tmp.file", 'wb')
-        super().do_POST()
-        self.wfile.close()
-        os.remove('tmp.file')
-
-
 class FakeSecureHTTPServer(SecureHTTPServer):
     def __init__(self):
         self.user_sessions = {}
@@ -84,6 +72,26 @@ class UnreadableFile:
     def __exit__(self, type, value, traceback):
         os.chmod(self.file_name, self.orig_mode)
 
+class TmpTextFile:
+    """
+    Context class for temporarily creating a text file with some
+    lines of content.
+
+    The file is automatically deleted if the context is left, so
+    make sure to not use the path of an existing file!
+    """
+    def __init__(self, path, contents):
+        self.__path = path
+        self.__contents = contents
+
+    def __enter__(self):
+        with open(self.__path, 'w') as f:
+            f.write("\n".join(self.__contents) + "\n")
+
+    def __exit__(self, type, value, traceback):
+        os.unlink(self.__path)
+
+
 class TestSecureHTTPRequestHandler(unittest.TestCase):
     def setUp(self):
         self.old_stdout = sys.stdout
@@ -93,13 +101,22 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
         self.handler.server.user_sessions = {}
         self.handler.server._user_infos = {}
         self.handler.headers = {}
-        self.handler.rfile = open("check.tmp", 'w+b')
+        self.handler.rfile = open('input.tmp', 'w+b')
+        self.handler.wfile = open('output.tmp', 'w+b')
 
     def tearDown(self):
         sys.stdout.close()
         sys.stdout = self.old_stdout
+        self.handler.wfile.close()
+        os.remove('output.tmp')
         self.handler.rfile.close()
-        os.remove('check.tmp')
+        os.remove('input.tmp')
+
+    def test_is_session_valid(self):
+        self.assertIsNone(self.handler.session_id)
+        self.assertFalse(self.handler._is_session_valid())
+        self.handler.session_id = 4234
+        self.assertTrue(self.handler._is_session_valid())
 
     def test_parse_request_path(self):
         self.handler.path = ''
@@ -160,7 +177,7 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
             self.handler.do_GET()
             self.assertEqual(self.handler.rcode, http.client.OK)
 
-    def test_user_logged_in(self):
+    def test_is_user_logged_in(self):
         self.handler.server.user_sessions = {}
         self.handler.session_id = 12345
         self.assertTrue(self.handler._is_user_logged_in() == False)
@@ -294,6 +311,68 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
         rcode, reply = self.handler._handle_post_request()
         self.assertEqual(http.client.BAD_REQUEST, rcode)
 
+    def test_handle_login(self):
+        orig_is_user_logged_in = self.handler._is_user_logged_in
+        orig_check_user_name_and_pwd = self.handler._check_user_name_and_pwd
+        try:
+            def create_is_user_logged_in(status):
+                '''Create a replacement _is_user_logged_in() method.'''
+                def my_is_user_logged_in():
+                    return status
+                return my_is_user_logged_in
+
+            # Check case where _is_user_logged_in() returns True
+            self.handler._is_user_logged_in = create_is_user_logged_in(True)
+            self.handler.headers['cookie'] = 12345
+            self.handler.path = '/login'
+            self.handler.do_POST()
+            self.assertEqual(self.handler.rcode, http.client.OK)
+            self.handler.wfile.seek(0, 0)
+            d = self.handler.wfile.read()
+            self.assertEqual(json.loads(d.decode()),
+                             ['user has already login'])
+
+            # Clear the output
+            self.handler.wfile.seek(0, 0)
+            self.handler.wfile.truncate()
+
+            # Check case where _is_user_logged_in() returns False
+            self.handler._is_user_logged_in = create_is_user_logged_in(False)
+
+            def create_check_user_name_and_pwd(status, error_info=None):
+                '''Create a replacement _check_user_name_and_pwd() method.'''
+                def my_check_user_name_and_pwd():
+                    return status, error_info
+                return my_check_user_name_and_pwd
+
+            # (a) Check case where _check_user_name_and_pwd() returns
+            # valid user status
+            self.handler._check_user_name_and_pwd = \
+                create_check_user_name_and_pwd(True)
+            self.handler.do_POST()
+            self.assertEqual(self.handler.rcode, http.client.OK)
+            self.handler.wfile.seek(0, 0)
+            d = self.handler.wfile.read()
+            self.assertEqual(json.loads(d.decode()), ['login success'])
+
+            # Clear the output
+            self.handler.wfile.seek(0, 0)
+            self.handler.wfile.truncate()
+
+            # (b) Check case where _check_user_name_and_pwd() returns
+            # invalid user status
+            self.handler._check_user_name_and_pwd = \
+                create_check_user_name_and_pwd(False, ['login failed'])
+            self.handler.do_POST()
+            self.assertEqual(self.handler.rcode, http.client.UNAUTHORIZED)
+            self.handler.wfile.seek(0, 0)
+            d = self.handler.wfile.read()
+            self.assertEqual(json.loads(d.decode()), ['login failed'])
+
+        finally:
+            self.handler._is_user_logged_in = orig_is_user_logged_in
+            self.handler._check_user_name_and_pwd = orig_check_user_name_and_pwd
+
 class MyCommandControl(CommandControl):
     def _get_modules_specification(self):
         return {}
@@ -470,6 +549,88 @@ class TestSecureHTTPServer(unittest.TestCase):
         self.assertEqual(1, len(self.server._user_infos))
         self.assertTrue('root' in self.server._user_infos)
 
+    def test_get_user_info(self):
+        self.assertIsNone(self.server.get_user_info('root'))
+        self.server._create_user_info(SRC_FILE_PATH + 'cmdctl-accounts.csv')
+        self.assertIn('6f0c73bd33101a5ec0294b3ca39fec90ef4717fe',
+                      self.server.get_user_info('root'))
+
+        # When the file is not changed calling _create_user_info() again
+        # should have no effect. In order to test this, we overwrite the
+        # user-infos that were just set and make sure it isn't touched by
+        # the call (so make sure it isn't set to some empty value)
+        fake_users_val = { 'notinfile': [] }
+        self.server._user_infos = fake_users_val
+        self.server._create_user_info(SRC_FILE_PATH + 'cmdctl-accounts.csv')
+        self.assertEqual(fake_users_val, self.server._user_infos)
+
+    def test_create_user_info_changing_file_time(self):
+        self.assertEqual(0, len(self.server._user_infos))
+
+        # Create a file
+        accounts_file = BUILD_FILE_PATH + 'new_file.csv'
+        with TmpTextFile(accounts_file, ['root,foo,bar']):
+            self.server._create_user_info(accounts_file)
+            self.assertEqual(1, len(self.server._user_infos))
+            self.assertTrue('root' in self.server._user_infos)
+
+            # Make sure re-reading is a noop if file was not modified
+            fake_users_val = { 'notinfile': [] }
+            self.server._user_infos = fake_users_val
+            self.server._create_user_info(accounts_file)
+            self.assertEqual(fake_users_val, self.server._user_infos)
+
+        # create the file again, this time read should not be a noop
+        with TmpTextFile(accounts_file, ['otherroot,foo,bar']):
+            # Set mtime in future
+            stat = os.stat(accounts_file)
+            os.utime(accounts_file, (stat.st_atime, stat.st_mtime + 10))
+            self.server._create_user_info(accounts_file)
+            self.assertEqual(1, len(self.server._user_infos))
+            self.assertTrue('otherroot' in self.server._user_infos)
+
+    def test_create_user_info_changing_file_name(self):
+        """
+        Check that the accounts file is re-read if the file name is different
+        """
+        self.assertEqual(0, len(self.server._user_infos))
+
+        # Create two files
+        accounts_file1 = BUILD_FILE_PATH + 'new_file.csv'
+        accounts_file2 = BUILD_FILE_PATH + 'new_file2.csv'
+        with TmpTextFile(accounts_file2, ['otherroot,foo,bar']):
+            with TmpTextFile(accounts_file1, ['root,foo,bar']):
+                self.server._create_user_info(accounts_file1)
+                self.assertEqual(1, len(self.server._user_infos))
+                self.assertTrue('root' in self.server._user_infos)
+
+                # Make sure re-reading is a noop if file was not modified
+                fake_users_val = { 'notinfile': [] }
+                self.server._user_infos = fake_users_val
+                self.server._create_user_info(accounts_file1)
+                self.assertEqual(fake_users_val, self.server._user_infos)
+
+                # But a different file should be read
+                self.server._create_user_info(accounts_file2)
+                self.assertEqual(1, len(self.server._user_infos))
+                self.assertTrue('otherroot' in self.server._user_infos)
+
+    def test_create_user_info_nonexistent_file(self):
+        # Even if there was data initially, if set to a nonexistent
+        # file it should result in no users
+        accounts_file = BUILD_FILE_PATH + 'new_file.csv'
+        self.assertFalse(os.path.exists(accounts_file))
+        fake_users_val = { 'notinfile': [] }
+        self.server._user_infos = fake_users_val
+        self.server._create_user_info(accounts_file)
+        self.assertEqual({}, self.server._user_infos)
+
+        # Should it now be created it should be read
+        with TmpTextFile(accounts_file, ['root,foo,bar']):
+            self.server._create_user_info(accounts_file)
+            self.assertEqual(1, len(self.server._user_infos))
+            self.assertTrue('root' in self.server._user_infos)
+
     def test_check_file(self):
         # Just some file that we know exists
         file_name = BUILD_FILE_PATH + 'cmdctl-keyfile.pem'

src/bin/dbutil/dbutil.py.in

@@ -16,7 +16,7 @@
 # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 """
-@file Dabase Utilities
+@file Database Utilities
 
 This file holds the "dbutil" program, a general utility program for doing
 management of the BIND 10 database.  There are two modes of operation:

src/bin/dbutil/tests/dbutil_test.sh.in

@@ -132,7 +132,7 @@ check_no_backup() {
 # .schema command, with spaces removed and upper converted to lowercase.
 #
 # The database is copied before the schema is taken (and removed after)
-# as SQLite3 assummes a writeable database, which may not be the case if
+# as SQLite3 assumes a writeable database, which may not be the case if
 # getting the schema from a reference copy.
 #
 # @param $1 Database for which the schema is required
@@ -161,7 +161,7 @@ get_schema() {
 # @param $2 Expected backup file
 upgrade_ok_test() {
     copy_file $1 $tempfile
-    ${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile
+    @SHELL@ ../run_dbutil.sh --upgrade --noconfirm $tempfile
     if [ $? -eq 0 ]
     then
         # Compare schema with the reference
@@ -199,7 +199,7 @@ upgrade_ok_test() {
 # @param $2 Expected backup file
 upgrade_fail_test() {
     copy_file $1 $tempfile
-    ${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile
+    @SHELL@ ../run_dbutil.sh --upgrade --noconfirm $tempfile
     failzero $?
     check_backup $1 $backupfile
 }
@@ -222,7 +222,7 @@ record_count_test() {
     records_count=`sqlite3 $tempfile 'select count(*) from records'`
     zones_count=`sqlite3 $tempfile 'select count(*) from zones'`
 
-    ${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile
+    @SHELL@ ../run_dbutil.sh --upgrade --noconfirm $tempfile
     if [ $? -ne 0 ]
     then
         # Reason for failure should already have been output
@@ -268,12 +268,12 @@ record_count_test() {
 # @param $2 Expected version string
 check_version() {
     copy_file $1 $verfile
-    ${SHELL} ../run_dbutil.sh --check $verfile
+    @SHELL@ ../run_dbutil.sh --check $verfile
     if [ $? -gt 2 ]
     then
         fail "version check failed on database $1; return code $?"
     else
-        ${SHELL} ../run_dbutil.sh --check $verfile 2>&1 | grep "$2" > /dev/null
+        @SHELL@ ../run_dbutil.sh --check $verfile 2>&1 | grep "$2" > /dev/null
         if [ $? -ne 0 ]
         then
             fail "database $1 not at expected version $2 (output: $?)"
@@ -293,7 +293,7 @@ check_version() {
 # @param $2 Backup file
 check_version_fail() {
     copy_file $1 $verfile
-    ${SHELL} ../run_dbutil.sh --check $verfile
+    @SHELL@ ../run_dbutil.sh --check $verfile
     failzero $?
     check_no_backup $tempfile $backupfile
 }
@@ -310,12 +310,12 @@ sec=0
 # Test: check that the utility fails if the database does not exist
 sec=`expr $sec + 1`
 echo $sec".1. Non-existent database - check"
-${SHELL} ../run_dbutil.sh --check $tempfile
+@SHELL@ ../run_dbutil.sh --check $tempfile
 failzero $?
 check_no_backup $tempfile $backupfile
 
 echo $sec".2. Non-existent database - upgrade"
-${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile
+@SHELL@ ../run_dbutil.sh --upgrade --noconfirm $tempfile
 failzero $?
 check_no_backup $tempfile $backupfile
 rm -f $tempfile $backupfile
@@ -330,7 +330,7 @@ rm -f $tempfile $backupfile
 
 echo $sec".2. Database is an empty file - upgrade"
 touch $tempfile
-${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile
+@SHELL@ ../run_dbutil.sh --upgrade --noconfirm $tempfile
 failzero $?
 # A backup is performed before anything else, so the backup should exist.
 check_backup $tempfile $backupfile
@@ -344,7 +344,7 @@ rm -f $tempfile $backupfile
 
 echo $sec".2. Database is not an SQLite file - upgrade"
 echo "This is not an sqlite3 database" > $tempfile
-${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile
+@SHELL@ ../run_dbutil.sh --upgrade --noconfirm $tempfile
 failzero $?
 # ...and as before, a backup should have been created
 check_backup $tempfile $backupfile
@@ -459,31 +459,31 @@ rm -f $tempfile $backupfile ${backupfile}-1 ${backupfile}-2
 sec=`expr $sec + 1`
 echo $sec".1 Command-line errors"
 copy_file $testdata/old_v1.sqlite3 $tempfile
-${SHELL} ../run_dbutil.sh $tempfile
+@SHELL@ ../run_dbutil.sh $tempfile
 failzero $?
-${SHELL} ../run_dbutil.sh --upgrade --check $tempfile
+@SHELL@ ../run_dbutil.sh --upgrade --check $tempfile
 failzero $?
-${SHELL} ../run_dbutil.sh --noconfirm --check $tempfile
+@SHELL@ ../run_dbutil.sh --noconfirm --check $tempfile
 failzero $?
-${SHELL} ../run_dbutil.sh --check
+@SHELL@ ../run_dbutil.sh --check
 failzero $?
-${SHELL} ../run_dbutil.sh --upgrade --noconfirm
+@SHELL@ ../run_dbutil.sh --upgrade --noconfirm
 failzero $?
-${SHELL} ../run_dbutil.sh --check $tempfile $backupfile
+@SHELL@ ../run_dbutil.sh --check $tempfile $backupfile
 failzero $?
-${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile $backupfile
+@SHELL@ ../run_dbutil.sh --upgrade --noconfirm $tempfile $backupfile
 failzero $?
 rm -f $tempfile $backupfile
 
 echo $sec".2 verbose flag"
 copy_file $testdata/old_v1.sqlite3 $tempfile
-${SHELL} ../run_dbutil.sh --upgrade --noconfirm --verbose $tempfile
+@SHELL@ ../run_dbutil.sh --upgrade --noconfirm --verbose $tempfile
 passzero $?
 rm -f $tempfile $backupfile
 
 echo $sec".3 Interactive prompt - yes"
 copy_file $testdata/old_v1.sqlite3 $tempfile
-${SHELL} ../run_dbutil.sh --upgrade $tempfile << .
+@SHELL@ ../run_dbutil.sh --upgrade $tempfile << .
 Yes
 .
 passzero $?
@@ -492,7 +492,7 @@ rm -f $tempfile $backupfile
 
 echo $sec".4 Interactive prompt - no"
 copy_file $testdata/old_v1.sqlite3 $tempfile
-${SHELL} ../run_dbutil.sh --upgrade $tempfile << .
+@SHELL@ ../run_dbutil.sh --upgrade $tempfile << .
 no
 .
 passzero $?
@@ -502,7 +502,7 @@ rm -f $tempfile $backupfile
 
 echo $sec".5 quiet flag"
 copy_file $testdata/old_v1.sqlite3 $tempfile
-${SHELL} ../run_dbutil.sh --check --quiet $tempfile 2>&1 | grep .
+@SHELL@ ../run_dbutil.sh --check --quiet $tempfile 2>&1 | grep .
 failzero $?
 rm -f $tempfile $backupfile
 

src/bin/ddns/ddns.py.in

@@ -236,7 +236,7 @@ class DDNSServer:
         '''Exception for internal errors in an update session.
 
         This exception is expected to be caught within the server class,
-        only used for controling the code flow.
+        only used for controlling the code flow.
 
         '''
         pass
@@ -354,7 +354,7 @@ class DDNSServer:
                 zname = Name(zone_spec['name'])
                 # class has the default value in case it's unspecified.
                 # ideally this should be merged within the config module, but
-                # the current implementation doesn't esnure that, so we need to
+                # the current implementation doesn't ensure that, so we need to
                 # subsitute it ourselves.
                 if 'class' in zone_spec:
                     zclass = RRClass(zone_spec['class'])
@@ -510,8 +510,8 @@ class DDNSServer:
         '''Send DDNS response to the client.
 
         Right now, this is a straightforward subroutine of handle_request(),
-        but is intended to be extended evetually so that it can handle more
-        comlicated operations for TCP (which requires asynchronous write).
+        but is intended to be extended eventually so that it can handle more
+        complicated operations for TCP (which requires asynchronous write).
         Further, when we support multiple requests over a single TCP
         connection, this method may even be shared by multiple methods.
 

src/bin/ddns/ddns_messages.mes

@@ -141,7 +141,7 @@ logged.
 
 % DDNS_RESPONSE_TCP_SOCKET_SEND_FAILED failed to complete sending update response to %1 over TCP
 b10-ddns had tried to send an update response over TCP, and it hadn't
-been completed at that time, and a followup attempt to complete the
+been completed at that time, and a follow-up attempt to complete the
 send operation failed due to some network I/O error.  While a network
 error can happen any time, this event is quite unexpected for two
 reasons.  First, since the size of a response to an update request

src/bin/ddns/tests/ddns_test.py

@@ -40,7 +40,7 @@ READ_ZONE_DB_FILE = TESTDATA_PATH + "rwtest.sqlite3" # original, to be copied
 TEST_ZONE_NAME = Name('example.org')
 TEST_ZONE_NAME_STR = TEST_ZONE_NAME.to_text()
 UPDATE_RRTYPE = RRType.SOA
-TEST_QID = 5353                 # arbitrary chosen
+TEST_QID = 5353                 # arbitrarily chosen
 TEST_RRCLASS = RRClass.IN
 TEST_RRCLASS_STR = TEST_RRCLASS.to_text()
 TEST_SERVER6 = ('2001:db8::53', 53, 0, 0)
@@ -53,7 +53,7 @@ TEST_ACL_CONTEXT = isc.acl.dns.RequestContext(
                        socket.IPPROTO_UDP, socket.AI_NUMERICHOST)[0][4])
 # TSIG key for tests when needed.  The key name is TEST_ZONE_NAME.
 TEST_TSIG_KEY = TSIGKey("example.org:SFuWd/q99SzF8Yzd1QbB9g==")
-# TSIG keyring that contanins the test key
+# TSIG keyring that contains the test key
 TEST_TSIG_KEYRING = TSIGKeyRing()
 TEST_TSIG_KEYRING.add(TEST_TSIG_KEY)
 # Another TSIG key not in the keyring, making verification fail
@@ -450,7 +450,7 @@ class TestDDNSServer(unittest.TestCase):
         self.assertEqual(1, isc.config.parse_answer(answer)[0])
         self.assertEqual({}, self.ddns_server._zone_config)
 
-        # the first zone cofig is valid, but not the second.  the first one
+        # the first zone config is valid, but not the second.  the first one
         # shouldn't be installed.
         bad_config = { 'zones': [ { 'origin': TEST_ZONE_NAME_STR,
                                     'class': TEST_RRCLASS_STR,
@@ -856,7 +856,7 @@ class TestDDNSServer(unittest.TestCase):
     def test_select_multi_tcp(self):
         '''Test continuation of sending a TCP response, multiple sockets.'''
         # Check if the implementation still works with multiple outstanding
-        # TCP contexts.  We use three (arbitray choice), of which two will be
+        # TCP contexts.  We use three (arbitrary choice), of which two will be
         # writable after select and complete the send.
         tcp_socks = []
         for i in range(0, 3):

src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -156,7 +156,7 @@ void ControlledDhcpv4Srv::establishSession() {
     // Dumy configuration handler is internally invoked by the
     // constructor and on success the constructor updates
     // the current session with the configuration that had been
-    // commited in the previous session. If we did not install
+    // committed in the previous session. If we did not install
     // the dummy handler, the previous configuration would have
     // been lost.
     config_session_ = new ModuleCCSession(specfile, *cc_session_,

src/bin/dhcp4/ctrl_dhcp4_srv.h

@@ -136,7 +136,7 @@ protected:
     /// @brief Helper session object that represents raw connection to msgq.
     isc::cc::Session* cc_session_;
 
-    /// @brief Session that receives configuation and commands
+    /// @brief Session that receives configuration and commands
     isc::config::ModuleCCSession* config_session_;
 };
 

src/bin/dhcp4/dhcp4.dox

@@ -33,7 +33,7 @@ assigned is not implemented in IfaceMgr yet.
 
 DHCPv4 server component is now integrated with BIND10 message queue.
 The integration is performed by establishSession() and disconnectSession()
-functions in isc::dhcp::ControlledDhcpv4Srv class. main() method deifined
+functions in isc::dhcp::ControlledDhcpv4Srv class. main() method defined
 in the src/bin/dhcp4/main.cc file instantiates isc::dhcp::ControlledDhcpv4Srv
 class that establishes connection with msgq and install necessary handlers
 for receiving commands and configuration updates. It is derived from

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -487,7 +487,7 @@ TEST_F(Dhcp4ParserTest, optionDefIpv4Address) {
     EXPECT_TRUE(def->getEncapsulatedSpace().empty());
 }
 
-// The goal of this test is to check whether an option definiiton
+// The goal of this test is to check whether an option definition
 // that defines an option carrying a record of data fields can
 // be created.
 TEST_F(Dhcp4ParserTest, optionDefRecord) {
@@ -1060,7 +1060,7 @@ TEST_F(Dhcp4ParserTest, optionDataTwoSpaces) {
     ASSERT_TRUE(status);
     checkResult(status, 0);
 
-    // Options should be now availabe for the subnet.
+    // Options should be now available for the subnet.
     Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
     // Try to get the option from the space dhcp4.
@@ -1430,7 +1430,7 @@ TEST_F(Dhcp4ParserTest, optionDataInvalidChar) {
     testInvalidOptionParam("01020R", "data");
 }
 
-// Verify that option data containins '0x' prefix is rejected
+// Verify that option data containing '0x' prefix is rejected
 // by the configuration.
 TEST_F(Dhcp4ParserTest, optionDataUnexpectedPrefix) {
     // Option code 0 is reserved and should not be accepted
@@ -1530,7 +1530,7 @@ TEST_F(Dhcp4ParserTest, stdOptionData) {
     boost::shared_ptr<Option4AddrLst> option_addrs =
         boost::dynamic_pointer_cast<Option4AddrLst>(option);
     // If cast is unsuccessful than option returned was of a
-    // differnt type than Option6IA. This is wrong.
+    // different type than Option6IA. This is wrong.
     ASSERT_TRUE(option_addrs);
 
     // Get addresses from the option.

src/bin/dhcp6/ctrl_dhcp6_srv.cc

@@ -155,7 +155,7 @@ void ControlledDhcpv6Srv::establishSession() {
     // Dumy configuration handler is internally invoked by the
     // constructor and on success the constructor updates
     // the current session with the configuration that had been
-    // commited in the previous session. If we did not install
+    // committed in the previous session. If we did not install
     // the dummy handler, the previous configuration would have
     // been lost.
     config_session_ = new ModuleCCSession(specfile, *cc_session_,

src/bin/dhcp6/ctrl_dhcp6_srv.h

@@ -134,7 +134,7 @@ protected:
     /// @brief Helper session object that represents raw connection to msgq.
     isc::cc::Session* cc_session_;
 
-    /// @brief Session that receives configuation and commands
+    /// @brief Session that receives configuration and commands
     isc::config::ModuleCCSession* config_session_;
 };
 

src/bin/dhcp6/dhcp6.dox

@@ -51,7 +51,7 @@
  list of parsers for each received entry. Parser is an object that is derived
  from a DhcpConfigParser class. Once a parser is created
  (constructor), its value is set (using build() method). Once all parsers are
- build, the configuration is then applied ("commited") and commit() method is
+ build, the configuration is then applied ("committed") and commit() method is
  called.
 
  All parsers are defined in src/bin/dhcp6/config_parser.cc file. Some of them

src/bin/dhcp6/dhcp6_srv.cc

@@ -353,7 +353,7 @@ Dhcpv6Srv::generateServerID() {
         }
 
         // Some interfaces (like lo on Linux) report 6-bytes long
-        // MAC adress 00:00:00:00:00:00. Let's not use such weird interfaces
+        // MAC address 00:00:00:00:00:00. Let's not use such weird interfaces
         // to generate DUID.
         if (isRangeZero(iface->getMac(), iface->getMac() + iface->getMacLen())) {
             continue;

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -598,7 +598,7 @@ TEST_F(Dhcp6ParserTest, optionDefIpv6Address) {
     EXPECT_EQ(OPT_IPV6_ADDRESS_TYPE, def->getType());
 }
 
-// The goal of this test is to check whether an option definiiton
+// The goal of this test is to check whether an option definition
 // that defines an option carrying a record of data fields can
 // be created.
 TEST_F(Dhcp6ParserTest, optionDefRecord) {
@@ -1176,7 +1176,7 @@ TEST_F(Dhcp6ParserTest, optionDataTwoSpaces) {
     ASSERT_TRUE(status);
     checkResult(status, 0);
 
-    // Options should be now availabe for the subnet.
+    // Options should be now available for the subnet.
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     ASSERT_TRUE(subnet);
     // Try to get the option from the space dhcp6.
@@ -1484,7 +1484,7 @@ TEST_F(Dhcp6ParserTest, optionDataInvalidChar) {
     testInvalidOptionParam("01020R", "data");
 }
 
-// Verify that option data containins '0x' prefix is rejected
+// Verify that option data containing '0x' prefix is rejected
 // by the configuration.
 TEST_F(Dhcp6ParserTest, optionDataUnexpectedPrefix) {
     // Option code 0 is reserved and should not be accepted
@@ -1582,7 +1582,7 @@ TEST_F(Dhcp6ParserTest, stdOptionData) {
     boost::shared_ptr<Option6IA> optionIA =
         boost::dynamic_pointer_cast<Option6IA>(option);
     // If cast is unsuccessful than option returned was of a
-    // differnt type than Option6IA. This is wrong.
+    // different type than Option6IA. This is wrong.
     ASSERT_TRUE(optionIA);
     // If cast was successful we may use accessors exposed by
     // Option6IA to validate that the content of this option

src/bin/loadzone/tests/correct/correct_test.sh.in

@@ -53,7 +53,7 @@ echo "I:test master file BIND 8 compatibility TTL and \$TTL semantics"
 echo "I:test master file RFC1035 TTL and \$TTL semantics"
 echo "I:test master file BIND8 compatibility and mixed \$INCLUDE with \$TTL semantics"
 echo "I:test master file RFC1035 TTL and mixed \$INCLUDE with \$TTL semantics"
-echo "I:test master file BIND9 extenstion of TTL"
+echo "I:test master file BIND9 extension of TTL"
 echo "I:test master file RFC1035 missing CLASS, TTL, NAME semantics"
 echo "I:test master file comments"
 

src/bin/loadzone/tests/correct/ttlext.db

@@ -11,7 +11,7 @@ ns			A	10.53.0.1
 a			TXT	"soa minttl 3"
 b		2S	TXT	"explicit ttl 2"
 c			TXT	"soa minttl 3"
-$TTL 10M  ; bind9 extention ttl
+$TTL 10M  ; bind9 extension ttl
 d			TXT	"default ttl 600"
 e		4	TXT	"explicit ttl 4"
 f			TXT	"default ttl 600"

src/bin/loadzone/tests/loadzone_test.py

@@ -41,7 +41,7 @@ ORIG_SOA_TXT = 'example.org. 3600 IN SOA ns1.example.org. ' +\
     'admin.example.org. 1234 3600 1800 2419200 7200\n'
 NEW_SOA_TXT = 'example.org. 3600 IN SOA ns.example.org. ' +\
     'admin.example.org. 1235 3600 1800 2419200 7200\n'
-# This is the brandnew SOA for a newly created zone
+# This is the brand new SOA for a newly created zone
 ALT_NEW_SOA_TXT = 'example.com. 3600 IN SOA ns.example.com. ' +\
     'admin.example.com. 1234 3600 1800 2419200 7200\n'
 

src/bin/msgq/Makefile.am

@@ -10,6 +10,7 @@ b10_msgq_DATA = msgq.spec
 CLEANFILES = b10-msgq msgq.pyc
 CLEANFILES += $(PYTHON_LOGMSGPKG_DIR)/work/msgq_messages.py
 CLEANFILES += $(PYTHON_LOGMSGPKG_DIR)/work/msgq_messages.pyc
+CLEANFILES += $(PYTHON_LOGMSGPKG_DIR)/work/msgq_messages.pyo
 
 man_MANS = b10-msgq.8
 DISTCLEANFILES = $(man_MANS)

src/bin/msgq/msgq.py.in

@@ -143,7 +143,7 @@ class SubscriptionManager:
         this group, instance pair.  This includes wildcard subscriptions."""
         target = (group, instance)
         partone = self.find_sub(group, instance)
-        parttwo = self.find_sub(group, "*")
+        parttwo = self.find_sub(group, CC_INSTANCE_WILDCARD)
         return list(set(partone + parttwo))
 
 class MsgQ:
@@ -406,7 +406,7 @@ class MsgQ:
             routing, data = self.read_packet(fd, sock)
         except (MsgQReceiveError, MsgQCloseOnReceive) as err:
             # If it's MsgQCloseOnReceive and that happens without reading
-            # any data, it basically means the remote clinet has closed the
+            # any data, it basically means the remote client has closed the
             # socket, so we log it as debug information.  Otherwise, it's
             # a somewhat unexpected event, so we consider it an "error".
             if isinstance(err, MsgQCloseOnReceive) and not err.partial_read:
@@ -429,19 +429,19 @@ class MsgQ:
         """Process a single command.  This will split out into one of the
            other functions."""
         logger.debug(TRACE_DETAIL, MSGQ_RECV_HDR, routing)
-        cmd = routing["type"]
-        if cmd == 'send':
+        cmd = routing[CC_HEADER_TYPE]
+        if cmd == CC_COMMAND_SEND:
             self.process_command_send(sock, routing, data)
-        elif cmd == 'subscribe':
+        elif cmd == CC_COMMAND_SUBSCRIBE:
             self.process_command_subscribe(sock, routing, data)
-        elif cmd == 'unsubscribe':
+        elif cmd == CC_COMMAND_UNSUBSCRIBE:
             self.process_command_unsubscribe(sock, routing, data)
-        elif cmd == 'getlname':
+        elif cmd == CC_COMMAND_GET_LNAME:
             self.process_command_getlname(sock, routing, data)
-        elif cmd == 'ping':
+        elif cmd == CC_COMMAND_PING:
             # Command for testing purposes
             self.process_command_ping(sock, routing, data)
-        elif cmd == 'stop':
+        elif cmd == CC_COMMAND_STOP:
             self.stop()
         else:
             logger.error(MSGQ_INVALID_CMD, cmd)
@@ -570,11 +570,12 @@ class MsgQ:
         return "%x_%x@%s" % (time.time(), self.connection_counter, self.hostname)
 
     def process_command_ping(self, sock, routing, data):
-        self.sendmsg(sock, { "type" : "pong" }, data)
+        self.sendmsg(sock, { CC_HEADER_TYPE : CC_COMMAND_PONG }, data)
 
     def process_command_getlname(self, sock, routing, data):
         lname = [ k for k, v in self.lnames.items() if v == sock ][0]
-        self.sendmsg(sock, { "type" : "getlname" }, { "lname" : lname })
+        self.sendmsg(sock, { CC_HEADER_TYPE : CC_COMMAND_GET_LNAME },
+                     { CC_PAYLOAD_LNAME : lname })
 
     def process_command_send(self, sock, routing, data):
         group = routing[CC_HEADER_GROUP]
@@ -638,15 +639,15 @@ class MsgQ:
             self.send_prepared_msg(sock, errmsg)
 
     def process_command_subscribe(self, sock, routing, data):
-        group = routing["group"]
-        instance = routing["instance"]
+        group = routing[CC_HEADER_GROUP]
+        instance = routing[CC_HEADER_INSTANCE]
         if group == None or instance == None:
             return  # ignore invalid packets entirely
         self.subs.subscribe(group, instance, sock)
 
     def process_command_unsubscribe(self, sock, routing, data):
-        group = routing["group"]
-        instance = routing["instance"]
+        group = routing[CC_HEADER_GROUP]
+        instance = routing[CC_HEADER_INSTANCE]
         if group == None or instance == None:
             return  # ignore invalid packets entirely
         self.subs.unsubscribe(group, instance, sock)

src/bin/msgq/run_msgq.sh.in

@@ -20,7 +20,7 @@ export PYTHON_EXEC
 
 MYPATH_PATH=@abs_top_builddir@/src/bin/msgq
 
-PYTHONPATH=@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python/isc/cc:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/log/.libs
+PYTHONPATH=@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python/isc/cc:@abs_top_srcdir@/src/lib/python:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs
 export PYTHONPATH
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries

src/bin/msgq/tests/Makefile.am

@@ -1,5 +1,5 @@
-PYCOVERAGE_RUN = @PYCOVERAGE_RUN@          
-PYTESTS = msgq_test.py
+PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
+PYTESTS = msgq_test.py msgq_run_test.py
 EXTRA_DIST = $(PYTESTS)
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries
@@ -12,7 +12,7 @@ endif
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
 if ENABLE_PYTHON_COVERAGE
-	touch $(abs_top_srcdir)/.coverage 
+	touch $(abs_top_srcdir)/.coverage
 	rm -f .coverage
 	${LN_S} $(abs_top_srcdir)/.coverage .coverage
 endif

src/bin/msgq/tests/msgq_run_test.py

@@ -0,0 +1,269 @@
+# Copyright (C) 2013  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and 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 INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM 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.
+
+"""
+In this test file, we actually start msgq as a process and test it
+as a whole. It may be considered a system test instead of unit test,
+but apart from the terminology, we don't care much. We need to test
+the message queue works as expected, together with the libraries.
+
+In each test, we first start a timeout (because we do some waits
+for messages and if they wouldn't come, the test could block indefinitely).
+The timeout is long, because it is for the case the test fails.
+
+We then start the msgq and wait for the socket file to appear
+(that should indicate it is ready to receive connections). Then the
+actual test starts. After the test, we kill it and remove the test file.
+
+We also register signal handlers for many signals. Even in the case
+the test is interrupted or crashes, we should ensure the message queue
+itself is terminated.
+"""
+
+import unittest
+import os
+import signal
+import sys
+import subprocess
+import time
+
+import isc.log
+import isc.cc.session
+from isc.cc.proto_defs import *
+
+# Due to problems with too long path on build bots, we place the socket
+# into the top-level build directory. That is ugly, but works.
+SOCKET_PATH = os.path.abspath(os.environ['B10_FROM_BUILD'] + '/msgq.sock')
+MSGQ_PATH = os.environ['B10_FROM_BUILD'] + '/src/bin/msgq/run_msgq.sh'
+TIMEOUT = 15 # Some long time (seconds), for single test.
+
+class MsgqRunTest(unittest.TestCase):
+    def setUp(self):
+        """
+        As described above - check the socket file does not exist.
+        Then register signals and timeouts. Finally, launch msgq
+        and wait for it to start.
+        """
+        self.__msgq = None
+        self.__opened_connections = []
+        # A precondition check
+        self.assertFalse(os.path.exists(SOCKET_PATH))
+        signal.alarm(TIMEOUT)
+        self.__orig_signals = {}
+        # Register handlers for many signals. Most of them probably
+        # can't happen in python, but we register them anyway just to be
+        # safe.
+        for sig in [signal.SIGHUP, signal.SIGINT, signal.SIGQUIT,
+            signal.SIGILL, signal.SIGTRAP, signal.SIGABRT, signal.SIGBUS,
+            signal.SIGFPE, signal.SIGALRM, signal.SIGTERM]:
+            self.__orig_signals[sig] = signal.signal(sig, self.__signal)
+        # Start msgq
+        self.__msgq = subprocess.Popen([MSGQ_PATH, '-s', SOCKET_PATH],
+                                       close_fds=True)
+        # Wait for it to become ready (up to the alarm-set timeout)
+        while not os.path.exists(SOCKET_PATH):
+            # Just a short wait, so we don't hog CPU, but don't wait too long
+            time.sleep(0.01)
+        # Some testing data
+        self.__no_recpt = {"result": [-1, "No such recipient"]}
+
+    def __message(self, data):
+        """
+        Provide some testing message. The data will be included in it, so
+        several different messages can be created.
+        """
+        return {"Message": "Text", "Data": data}
+
+    def tearDown(self):
+        """
+        Perform cleanup after the test.
+        """
+        self.__cleanup()
+
+    def __signal(self, signal, frame):
+        """
+        Called from a signal handler. We perform some cleanup, output
+        a complain and terminate with error.
+        """
+        self.__cleanup()
+        sys.stderr.write("Test terminating from signal " + str(signal) +
+                         " in " + str(frame) + "\n")
+        sys.exit(1)
+
+    def __cleanup(self):
+        """
+        Kill msgq (if running) and restore original signal handlers.
+        """
+        # Remove the socket (as we kill, msgq might not clean up)
+        for conn in self.__opened_connections:
+            conn.close()
+        self.__opened_connections = []
+        if self.__msgq:
+            self.__msgq.kill()
+            self.__msgq = None
+        if os.path.exists(SOCKET_PATH):
+            os.unlink(SOCKET_PATH)
+        for sig in self.__orig_signals:
+            signal.signal(sig, self.__orig_signals[sig])
+        # Cancel timeout (so someone else is not hit by it)
+        signal.alarm(0)
+
+    def __get_connection(self):
+        """
+        Create a connection to the daemon and make sure it is properly closed
+        at the end of the test.
+        """
+        connection = isc.cc.session.Session(SOCKET_PATH)
+        self.__opened_connections.append(connection)
+        return connection
+
+    def test_send_direct(self):
+        """
+        Connect twice to msgq, send a message from one to another using direct
+        l-name and see it comes.
+        """
+        # Create the connections
+        conn1 = self.__get_connection()
+        conn2 = self.__get_connection()
+        # Send the message
+        lname1 = conn1.lname
+        conn2.group_sendmsg(self.__message(1), "*", to=lname1)
+        # Receive the message and see it contains correct data
+        (msg, env) = conn1.group_recvmsg(nonblock=False)
+        self.assertEqual(self.__message(1), msg)
+        # We don't check there are no extra headers, just that none are missing
+        # or wrong.
+        self.assertEqual(lname1, env[CC_HEADER_TO])
+        self.assertEqual(conn2.lname, env[CC_HEADER_FROM])
+        self.assertEqual("*", env[CC_HEADER_GROUP])
+        self.assertEqual(CC_INSTANCE_WILDCARD, env[CC_HEADER_INSTANCE])
+        self.assertEqual(CC_COMMAND_SEND, env[CC_HEADER_TYPE])
+        self.assertFalse(env[CC_HEADER_WANT_ANSWER])
+
+    def __barrier(self, connections):
+        """
+        Make sure all previous commands on all supplied connections are
+        processed, by sending a ping and waiting for an answer.
+        """
+        for c in connections:
+            c.sendmsg({"type": "ping"})
+        for c in connections:
+            pong = c.recvmsg(nonblock=False)
+            self.assertEqual(({"type": "pong"}, None), pong)
+
+    def test_send_group(self):
+        """
+        Create several connections. First, try to send a message to a (empty)
+        group and see an error is bounced back. Then subscribe the others
+        to the group and send it again. Send to a different group and see it
+        bounced back. Unsubscribe and see it is bounced again.
+
+        Then the other connections answer (after unsubscribing, strange, but
+        legal). See both answers come.
+
+        Then, look there are no more waiting messages.
+        """
+        conn_a = self.__get_connection()
+        conn_b = []
+        for i in range(0, 10):
+            conn_b.append(self.__get_connection())
+        # Send a message to empty group and get an error answer
+        seq = conn_a.group_sendmsg(self.__message(1), "group",
+                                   want_answer=True)
+        (msg, env) = conn_a.group_recvmsg(nonblock=False, seq=seq)
+        self.assertEqual(self.__no_recpt, msg)
+        self.assertEqual(conn_a.lname, env[CC_HEADER_TO])
+        # Subscribe the two connections
+        for c in conn_b:
+            c.group_subscribe("group")
+        # The subscribe doesn't wait for answer, so make sure it is
+        # all processed before continuing.
+        self.__barrier(conn_b)
+        # Send a message to the group (this time not empty)
+        seq = conn_a.group_sendmsg(self.__message(2), "group",
+                                   want_answer=True)
+        envs = []
+        for c in conn_b:
+            (msg, env) = c.group_recvmsg(nonblock=False)
+            self.assertEqual(self.__message(2), msg)
+            self.assertEqual(conn_a.lname, env[CC_HEADER_FROM])
+            # The daemon does not mangle the headers. Is it OK?
+            self.assertEqual(CC_TO_WILDCARD, env[CC_HEADER_TO])
+            self.assertEqual("group", env[CC_HEADER_GROUP])
+            self.assertEqual(CC_INSTANCE_WILDCARD, env[CC_HEADER_INSTANCE])
+            self.assertEqual(CC_COMMAND_SEND, env[CC_HEADER_TYPE])
+            self.assertTrue(env[CC_HEADER_WANT_ANSWER])
+            envs.append(env)
+        # Send to non-existing group
+        seq_ne = conn_a.group_sendmsg(self.__message(3), "no-group",
+                                      want_answer=True)
+        (msg, env) = conn_a.group_recvmsg(nonblock=False, seq=seq_ne)
+        self.assertEqual(self.__no_recpt, msg)
+        self.assertEqual(conn_a.lname, env[CC_HEADER_TO])
+        # Unsubscribe the connections
+        for c in conn_b:
+            c.group_unsubscribe("group")
+        # Synchronize the unsubscriptions
+        self.__barrier(conn_b)
+        seq_ne = conn_a.group_sendmsg(self.__message(4), "group",
+                                      want_answer=True)
+        (msg, env) = conn_a.group_recvmsg(nonblock=False, seq=seq_ne)
+        self.assertEqual(self.__no_recpt, msg)
+        self.assertEqual(conn_a.lname, env[CC_HEADER_TO])
+        # Send answers for the original message that was delivered
+        lnames = set()
+        for (c, env) in zip(conn_b, envs):
+            c.group_reply(env, self.__message("Reply"))
+            lnames.add(c.lname)
+        # Check the both answers come
+        while lnames:
+            # While there are still connections we didn't get the answer from
+            # (the order is not guaranteed, therefore the juggling with set)
+            (msg, env) = conn_a.group_recvmsg(nonblock=False, seq=seq)
+            self.assertEqual(self.__message("Reply"), msg)
+            lname = env[CC_HEADER_FROM]
+            self.assertTrue(lname in lnames)
+            lnames.remove(lname)
+
+        # The barrier makes the msgq process everything we sent. As the
+        # processing is single-threaded in it, any stray message would have
+        # arrived before the barrier ends.
+        self.__barrier(conn_b)
+        self.__barrier([conn_a])
+        for c in conn_b:
+            self.assertEqual((None, None), c.group_recvmsg())
+        self.assertEqual((None, None), conn_a.group_recvmsg())
+
+    def test_conn_disconn(self):
+        """
+        Keep connecting and disconnecting, checking we can still send
+        and receive messages.
+        """
+        conn = self.__get_connection()
+        conn.group_subscribe("group")
+        for i in range(0, 50):
+            new = self.__get_connection()
+            new.group_subscribe("group")
+            self.__barrier([conn, new])
+            new.group_sendmsg(self.__message(i), "group")
+            (msg, env) = conn.group_recvmsg(nonblock=False)
+            self.assertEqual(self.__message(i), msg)
+            conn.close()
+            conn = new
+
+if __name__ == '__main__':
+    isc.log.init("msgq-tests")
+    isc.log.resetUnitTestRootLogger()
+    unittest.main()

src/bin/resolver/response_scrubber.h

@@ -25,7 +25,7 @@
 /// unsigned data in a response is more of a problem. (Note that even data from
 /// signed zones may be not be signed, e.g. delegations are not signed.) In
 /// particular, how do we know that the server from which the response was
-/// received was authoritive for the data it returned?
+/// received was authoritative for the data it returned?
 ///
 /// The part of the code that checks for this is the "Data Scrubbing" module.
 /// Although it includes the checking of IP addresses and ports, it is called
@@ -219,7 +219,7 @@
 /// referral been to the com nameservers, it would be a valid response; the com
 /// zone could well be serving all the data for example.com.  Having said that,
 /// the A record for ns1.example.net would still be regarded as being out of
-/// bailiwick becase the nameserver is not authoritative for the .net zone.
+/// bailiwick because the nameserver is not authoritative for the .net zone.
 ///
 /// \subsection DataScrubbingEx4 Example 4: Inconsistent Answer Section
 /// Qu: www.example.com\n

src/bin/sockcreator/README

@@ -35,7 +35,7 @@ must be a socket, not pipe.
   The answer to this is either 'S' directly followed by the socket (using
   sendmsg) if it is successful. If it fails, 'E' is returned instead, followed
   by either 'S' or 'B' (either socket() or bind() call failed). Then there is
-  one int (architecture-dependent length and endianess), which is the errno
+  one int (architecture-dependent length and endianness), which is the errno
   value after the failure.
 
 The creator may also send these messages at any time (but not in the middle

src/bin/sockcreator/sockcreator.h

@@ -85,7 +85,7 @@ typedef int (*close_t)(int);
 /// \param type The type of socket to create (SOCK_STREAM, SOCK_DGRAM, etc).
 /// \param bind_addr The address to bind.
 /// \param addr_len The actual length of bind_addr.
-/// \param close_fun The furction used to close a socket if there's an error
+/// \param close_fun The function used to close a socket if there's an error
 ///     after the creation.
 ///
 /// \return The file descriptor of the newly created socket, if everything

src/bin/sockcreator/tests/sockcreator_tests.cc

@@ -99,7 +99,7 @@ udpCheck(const int socknum) {
 }
 
 // The check function (tcpCheck/udpCheck) is passed as a parameter to the test
-// code, so provide a conveniet typedef.
+// code, so provide a convenient typedef.
 typedef void (*socket_check_t)(const int);
 
 // Address-family-specific scoket checks.

src/bin/stats/stats_httpd.py.in

@@ -35,6 +35,7 @@ import re
 import isc.cc
 import isc.config
 import isc.util.process
+from isc.util.address_formatter import AddressFormatter
 
 import isc.log
 from isc.log_messages.stats_httpd_messages import *
@@ -325,8 +326,8 @@ class StatsHttpd:
                 server_address, HttpHandler,
                 self.xml_handler, self.xsd_handler, self.xsl_handler,
                 self.write_log)
-            logger.info(STATSHTTPD_STARTED, server_address[0],
-                        server_address[1])
+            logger.info(STATSHTTPD_STARTED,
+                        AddressFormatter(server_address, address_family))
             return httpd
         except (socket.gaierror, socket.error,
                 OverflowError, TypeError) as err:
@@ -341,8 +342,8 @@ class StatsHttpd:
         """Closes sockets for HTTP"""
         while len(self.httpd)>0:
             ht = self.httpd.pop()
-            logger.info(STATSHTTPD_CLOSING, ht.server_address[0],
-                        ht.server_address[1])
+            logger.info(STATSHTTPD_CLOSING,
+                        AddressFormatter(ht.server_address))
             ht.server_close()
 
     def start(self):

src/bin/stats/stats_httpd_messages.mes

@@ -24,7 +24,7 @@ The stats-httpd module was unable to connect to the BIND 10 command
 and control bus. A likely problem is that the message bus daemon
 (b10-msgq) is not running. The stats-httpd module will now shut down.
 
-% STATSHTTPD_CLOSING closing %1#%2
+% STATSHTTPD_CLOSING closing %1
 The stats-httpd daemon will stop listening for requests on the given
 address and port number.
 
@@ -80,7 +80,7 @@ and an error is sent back.
 % STATSHTTPD_SHUTDOWN shutting down
 The stats-httpd daemon is shutting down.
 
-% STATSHTTPD_STARTED listening on %1#%2
+% STATSHTTPD_STARTED listening on %1
 The stats-httpd daemon will now start listening for requests on the
 given address and port number.
 

src/bin/stats/tests/b10-stats-httpd_test.py

@@ -173,7 +173,7 @@ class TestItemNameList(unittest.TestCase):
                          stats_httpd.item_name_list({'a':[1,2,3]}, 'a'))
         self.assertEqual(['a', 'a[0]', 'a[1]', 'a[2]'],
                          stats_httpd.item_name_list({'a':[1,2,3]}, ''))
-        # for a list under adict under a dict
+        # for a list under a dict under a dict
         self.assertEqual(['a', 'a/b', 'a/b[0]', 'a/b[1]', 'a/b[2]'],
                          stats_httpd.item_name_list({'a':{'b':[1,2,3]}}, 'a'))
         self.assertEqual(['a', 'a/b', 'a/b[0]', 'a/b[1]', 'a/b[2]'],

src/bin/stats/tests/b10-stats_test.py

@@ -398,8 +398,8 @@ class TestStats(unittest.TestCase):
             (0, "Stats is up. (PID " + str(os.getpid()) + ")"))
 
         # 'showschema' command.  update_modules() will be called, which
-        # (implicitly) cofirms the correct method is called; further details
-        # are tested seprately.
+        # (implicitly) confirms the correct method is called; further details
+        # are tested separately.
         call_log = []
         (rcode, value) = self.__send_command(__stats, 'showschema')
         self.assertEqual([('update_module', ())], call_log)

src/bin/stats/tests/test_utils.py

@@ -48,7 +48,7 @@ class SignalHandler():
         signal.signal(signal.SIGALRM, self.orig_handler)
 
     def sig_handler(self, signal, frame):
-        """envokes unittest.TestCase.fail as a signal handler"""
+        """invokes unittest.TestCase.fail as a signal handler"""
         self.fail_handler("A deadlock might be detected")
 
 def send_command(command_name, module_name, params=None):

src/bin/usermgr/Makefile.am

@@ -1,8 +1,11 @@
+SUBDIRS = tests
+
 sbin_SCRIPTS = b10-cmdctl-usermgr
+noinst_SCRIPTS = run_b10-cmdctl-usermgr.sh
 
 b10_cmdctl_usermgrdir = $(pkgdatadir)
 
-CLEANFILES=	b10-cmdctl-usermgr
+CLEANFILES=	b10-cmdctl-usermgr b10-cmdctl-usermgr.pyc
 
 man_MANS = b10-cmdctl-usermgr.8
 DISTCLEANFILES = $(man_MANS)
@@ -25,3 +28,7 @@ endif
 b10-cmdctl-usermgr: b10-cmdctl-usermgr.py
 	$(SED) "s|@@PYTHONPATH@@|@pyexecdir@|" b10-cmdctl-usermgr.py >$@
 	chmod a+x $@
+
+CLEANDIRS = __pycache__
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/usermgr/b10-cmdctl-usermgr.py.in

@@ -11,115 +11,234 @@
 # IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
 # INTERNET SYSTEMS CONSORTIUM 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
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN COMMAND OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS COMMAND, ARISING OUT OF OR IN CONNECTION
 # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 '''
-This file implements user management program. The user name and 
-its password is appended to csv file.
+This tool implements user management for b10-cmdctl. It is used to
+add and remove users from the accounts file.
 '''
+import sys; sys.path.append ('@@PYTHONPATH@@')
+from bind10_config import SYSCONFPATH
+from collections import OrderedDict
 import random
 from hashlib import sha1
 import csv
 import getpass
-import getopt
-import sys; sys.path.append ('@@PYTHONPATH@@')
+from optparse import OptionParser, OptionValueError
+import os
 import isc.util.process
 
 isc.util.process.rename()
 
-VERSION_NUMBER = 'bind10'
-DEFAULT_FILE = 'cmdctl-accounts.csv'
-
-def gen_password_hash(password):
-    salt = "".join(chr(random.randint(33, 127)) for x in range(64))
-    saltedpwd = sha1((password + salt).encode()).hexdigest()
-    return salt, saltedpwd
-
-def username_exist(name, filename):
-    # The file may doesn't exist.
-    exist = False
-    csvfile = None
-    try:
-        csvfile = open(filename)        
-        reader = csv.reader(csvfile)
-        for row in reader:
-            if name == row[0]:
-                exist = True               
-                break
-    except Exception:
-        pass
-            
-    if csvfile:    
-        csvfile.close()
-    return exist
-
-def save_userinfo(username, pw, salt, filename):
-    csvfile = open(filename, 'a')
-    writer = csv.writer(csvfile)
-    writer.writerow([username, pw, salt])
-    csvfile.close()
-    print("\n create new account successfully! \n")
-
-def usage():
-    print('''Usage: usermgr [options]
-           -h, --help \t Show this help message and exit
-           -f, --file \t Specify the file to append user name and password
-           -v, --version\t Get version number
-           ''')           
+VERSION_STRING = "b10-cmdctl-usermgr @PACKAGE_VERSION@"
+DEFAULT_FILE = SYSCONFPATH + "/cmdctl-accounts.csv"
 
-def main():
-    filename = DEFAULT_FILE
-    try: 
-        opts, args = getopt.getopt(sys.argv[1:], 'f:hv', 
-                                   ['file=', 'help', 'version']) 
-    except getopt.GetoptError as err: 
-        print(err) 
-        usage() 
-        sys.exit(2)
-    for op, param in opts:        
-        if op in ('-h', '--help'): 
-            usage()
-            sys.exit()
-        elif op in ('-v', '--version'): 
-            print(VERSION_NUMBER)
-            sys.exit()                     
-        elif op in ('-f', "--file"):
-            filename = param
-        else: 
-            assert False, 'unknown option' 
-            usage()
-    
-    try:
-        while True :
-            name = input("Desired Login Name:")
-            if name == '':
-                print("error, user name can't be empty")
+# Actions that can be performed (used for argument parsing,
+# code paths, and output)
+COMMAND_ADD = "add"
+COMMAND_DELETE = "delete"
+
+# Non-zero return codes, used in tests
+BAD_ARGUMENTS = 1
+FILE_ERROR = 2
+USER_EXISTS = 3
+USER_DOES_NOT_EXIST = 4
+
+class UserManager:
+    def __init__(self, options, args):
+        self.options = options
+        self.args = args
+
+    def __print(self, msg):
+        if not self.options.quiet:
+            print(msg)
+
+    def __gen_password_hash(self, password):
+        salt = "".join(chr(random.randint(ord('!'), ord('~')))\
+                    for x in range(64))
+        saltedpwd = sha1((password + salt).encode()).hexdigest()
+        return salt, saltedpwd
+
+    def __read_user_info(self):
+        """
+        Read the existing user info
+        Raises an IOError if the file can't be read
+        """
+        # Currently, this is done quite naively (there is no
+        # check that the file is modified between read and write)
+        # But taking multiple simultaneous users of this tool on the
+        # same file seems unnecessary at this point.
+        self.user_info = OrderedDict()
+        if os.path.exists(self.options.output_file):
+            # Just let any file read error bubble up; it will
+            # be caught in the run() method
+            with open(self.options.output_file, newline='') as csvfile:
+                reader = csv.reader(csvfile)
+                for row in reader:
+                    self.user_info[row[0]] = row
+
+    def __save_user_info(self):
+        """
+        Write out the (modified) user info
+        Raises an IOError if the file can't be written
+        """
+        # Just let any file write error bubble up; it will
+        # be caught in the run() method
+        with open(self.options.output_file, 'w',
+                  newline='') as csvfile:
+            writer = csv.writer(csvfile)
+            for row in self.user_info.values():
+                writer.writerow(row)
+
+    def __add_user(self, name, password):
+        """
+        Add the given username/password combination to the stored user_info.
+        First checks if the username exists, and returns False if so.
+        If not, it is added, and this method returns True.
+        """
+        if name in self.user_info:
+            return False
+        salt, pw = self.__gen_password_hash(password)
+        self.user_info[name] = [name, pw, salt]
+        return True
+
+    def __delete_user(self, name):
+        """
+        Removes the row with the given name from the stored user_info
+        First checks if the username exists, and returns False if not.
+        Otherwise, it is removed, and this mehtod returns True
+        """
+        if name not in self.user_info:
+            return False
+        del self.user_info[name]
+        return True
+
+    # overridable input() call, used in testing
+    def _input(self, prompt):
+        return input(prompt)
+
+    # in essence this is private, but made 'protected' for ease
+    # of testing
+    def _prompt_for_username(self, command):
+        # Note, direct prints here are intentional
+        while True:
+            name = self._input("Username to " + command + ": ")
+            if name == "":
+                print("Error username can't be empty")
                 continue
 
-            if username_exist(name, filename):
-                 print("user name already exists!")
+            if command == COMMAND_ADD and name in self.user_info:
+                 print("user already exists")
+                 continue
+            elif command == COMMAND_DELETE and name not in self.user_info:
+                 print("user does not exist")
                  continue
 
-            while True:
-                pwd1 = getpass.getpass("Choose a password:")
-                pwd2 = getpass.getpass("Re-enter password:")
-                if pwd1 != pwd2:
-                    print("password is not same, please input again")
+            return name
+
+    # in essence this is private, but made 'protected' for ease
+    # of testing
+    def _prompt_for_password(self):
+        # Note, direct prints here are intentional
+        while True:
+            pwd1 = getpass.getpass("Choose a password: ")
+            if pwd1 == "":
+                print("Error: password cannot be empty")
+                continue
+            pwd2 = getpass.getpass("Re-enter password: ")
+            if pwd1 != pwd2:
+                print("passwords do not match, try again")
+                continue
+            return pwd1
+
+    def __verify_options_and_args(self):
+        """
+        Basic sanity checks on command line arguments.
+        Returns False if there is a problem, True if everything seems OK.
+        """
+        if len(self.args) < 1:
+            self.__print("Error: no command specified")
+            return False
+        if len(self.args) > 3:
+            self.__print("Error: extraneous arguments")
+            return False
+        if self.args[0] not in [ COMMAND_ADD, COMMAND_DELETE ]:
+            self.__print("Error: command must be either add or delete")
+            return False
+        if self.args[0] == COMMAND_DELETE and len(self.args) > 2:
+            self.__print("Error: delete only needs username, not a password")
+            return False
+        return True
+
+    def run(self):
+        if not self.__verify_options_and_args():
+            return BAD_ARGUMENTS
+
+        try:
+            self.__print("Using accounts file: " + self.options.output_file)
+            self.__read_user_info()
+
+            command = self.args[0]
+
+            if len(self.args) > 1:
+                username = self.args[1]
+            else:
+                username = self._prompt_for_username(command)
+
+            if command == COMMAND_ADD:
+                if len(self.args) > 2:
+                    password = self.args[2]
                 else:
-                    break;
-            
-            salt, pw = gen_password_hash(pwd1)
-            save_userinfo(name, pw, salt, filename)
-            inputdata = input('continue to create new account by input \'y\' or \'Y\':')
-            if inputdata not in ['y', 'Y']:
-                break                
-                
-    except KeyboardInterrupt:
-        pass                
+                    password = self._prompt_for_password()
+                if not self.__add_user(username, password):
+                    print("Error: username exists")
+                    return USER_EXISTS
+            elif command == COMMAND_DELETE:
+                if not self.__delete_user(username):
+                    print("Error: username does not exist")
+                    return USER_DOES_NOT_EXIST
+
+            self.__save_user_info()
+            return 0
+        except IOError as ioe:
+            self.__print("Error accessing " + ioe.filename +\
+                         ": " + str(ioe.strerror))
+            return FILE_ERROR
+        except csv.Error as csve:
+            self.__print("Error parsing csv file: " + str(csve))
+            return FILE_ERROR
+
+def set_options(parser):
+    parser.add_option("-f", "--file",
+                      dest="output_file", default=DEFAULT_FILE,
+                      help="Accounts file to modify"
+                     )
+    parser.add_option("-q", "--quiet",
+                      dest="quiet", action="store_true", default=False,
+                      help="Quiet mode, don't print any output"
+                     )
+
+def main():
+    usage = "usage: %prog [options] <command> [username] [password]\n\n"\
+            "Arguments:\n"\
+            "  command\t\teither 'add' or 'delete'\n"\
+            "  username\t\tthe username to add or delete\n"\
+            "  password\t\tthe password to set for the added user\n"\
+            "\n"\
+            "If username or password are not specified, %prog will\n"\
+            "prompt for them. It is recommended practice to let the\n"\
+            "tool prompt for the password, as command-line\n"\
+            "arguments can be visible through history or process\n"\
+            "viewers."
+    parser = OptionParser(usage=usage, version=VERSION_STRING)
+    set_options(parser)
+    (options, args) = parser.parse_args()
 
+    usermgr = UserManager(options, args)
+    return usermgr.run()
 
 if __name__ == '__main__':
-    main()
+    sys.exit(main())
 

src/bin/usermgr/b10-cmdctl-usermgr.xml

@@ -12,8 +12,8 @@
  - 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
+ - LOSS OF USE, DATA OR PROFITS, WHETHER IN AN COMMAND OF CONTRACT, NEGLIGENCE
+ - OR OTHER TORTIOUS COMMAND, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
  - PERFORMANCE OF THIS SOFTWARE.
 -->
 
@@ -47,10 +47,12 @@
 
       <arg><option>-f <replaceable>filename</replaceable></option></arg>
       <arg><option>-h</option></arg>
-      <arg><option>-v</option></arg>
-      <arg><option>--file <replaceable>filename</replaceable></option></arg>
+      <arg><option>--file=<replaceable>filename</replaceable></option></arg>
       <arg><option>--help</option></arg>
       <arg><option>--version</option></arg>
+      <arg choice="plain"><replaceable>command</replaceable></arg>
+      <arg><option>username</option></arg>
+      <arg><option>password</option></arg>
 
     </cmdsynopsis>
   </refsynopsisdiv>
@@ -58,24 +60,22 @@
   <refsect1>
     <title>DESCRIPTION</title>
     <para>The <command>b10-cmdctl-usermgr</command> tool may be used
-      to add accounts with passwords for the
+      to add and remove accounts with passwords for the
       <citerefentry><refentrytitle>b10-cmdctl</refentrytitle><manvolnum>8</manvolnum></citerefentry>
       daemon.
     </para>
 
     <para>
       By default, the accounts are saved in the
-      <filename>cmdctl-accounts.csv</filename> file in the current directory,
-      unless the <option>--filename</option> switch is used.
-      The entry is appended to the file.
-<!-- TODO: default should have full path? -->
+      <filename>cmdctl-accounts.csv</filename> file in the system config
+      directory, unless the <option>--filename</option> switch is used.
+      The entry is appended to or removed from the file.
     </para>
 
     <para>
-      The tool can't remove or replace existing entries.
+      The tool can't replace existing entries, but this can easily be
+      accomplished by removing the entry and adding a new one.
     </para>
-<!-- TODO: the tool can't remove or replace existing entries -->
-
   </refsect1>
 
   <refsect1>
@@ -83,6 +83,18 @@
 
     <para>The arguments are as follows:</para>
 
+    <para>
+      command is either 'add' or 'delete', respectively to add or delete users.
+    </para>
+
+    <para>
+      If a username and password are given (or just a username in case of
+      deletion), these are used. Otherwise, the tool shall prompt for a
+      username and/or password. It is recommended practice to let the
+      tool prompt for the password, as command-line arguments can be
+      visible through history or process viewers.
+    </para>
+
     <variablelist>
 
       <varlistentry>
@@ -97,14 +109,13 @@
         <term><option>-f <replaceable>filename</replaceable></option></term>
         <term><option>--file <replaceable>filename</replaceable></option></term>
         <listitem><para>
-          Define the filename to append the account to. The default
-          is <filename>cmdctl-accounts.csv</filename> in the current directory.
-<!-- TODO: default should have full path? -->
+          Specify the accounts file to update. The default is
+          <filename>cmdctl-accounts.csv</filename> in the system config
+          directory.
         </para></listitem>
       </varlistentry>
 
       <varlistentry>
-        <term><option>-v</option></term>
         <term><option>--version</option></term>
         <listitem><para>
           Report the version and exit.

src/bin/usermgr/run_b10-cmdctl-usermgr.sh.in

@@ -18,6 +18,9 @@
 PYTHON_EXEC=${PYTHON_EXEC:-@PYTHON@}
 export PYTHON_EXEC
 
+PYTHONPATH=@abs_top_builddir@/src/lib/python
+export PYTHONPATH
+
 MYPATH_PATH=@abs_top_builddir@/src/bin/usermgr
 
 BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket

src/bin/usermgr/tests/Makefile.am

@@ -0,0 +1,22 @@
+PYCOVERAGE_RUN=@PYCOVERAGE_RUN@
+PYTESTS = b10-cmdctl-usermgr_test.py
+EXTRA_DIST = $(PYTESTS)
+
+CLEANFILES = *.csv
+
+# test using command-line arguments, so use check-local target instead of TESTS
+check-local:
+if ENABLE_PYTHON_COVERAGE
+	touch $(abs_top_srcdir)/.coverage
+	rm -f .coverage
+	${LN_S} $(abs_top_srcdir)/.coverage .coverage
+endif
+	for pytest in $(PYTESTS) ; do \
+	echo Running test: $$pytest ; \
+	$(LIBRARY_PATH_PLACEHOLDER) \
+	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/bin/cmdctl \
+	CMDCTL_BUILD_PATH=$(abs_top_builddir)/src/bin/cmdctl \
+	CMDCTL_SRC_PATH=$(abs_top_srcdir)/src/bin/cmdctl \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
+	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
+	done

src/bin/usermgr/tests/b10-cmdctl-usermgr_test.py

@@ -0,0 +1,483 @@
+# Copyright (C) 2013  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and 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 INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM 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 COMMAND OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS COMMAND, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+import csv
+from hashlib import sha1
+import getpass
+import imp
+import os
+import subprocess
+import stat
+import sys
+import unittest
+from bind10_config import SYSCONFPATH
+
+class PrintCatcher:
+    def __init__(self):
+        self.stdout_lines = []
+
+    def __enter__(self):
+        self.__orig_stdout_write = sys.stdout.write
+        def new_write(line):
+            self.stdout_lines.append(line)
+
+        sys.stdout.write = new_write
+        return self
+
+    def __exit__(self, type, value, traceback):
+        sys.stdout.write = self.__orig_stdout_write
+
+class OverrideGetpass:
+    def __init__(self, new_getpass):
+        self.__new_getpass = new_getpass
+        self.__orig_getpass = getpass.getpass
+
+    def __enter__(self):
+        getpass.getpass = self.__new_getpass
+        return self
+
+    def __exit__(self, type, value, traceback):
+        getpass.getpass = self.__orig_getpass
+
+# input() is a built-in function and not easily overridable
+# so this one uses usermgr for that
+class OverrideInput:
+    def __init__(self, usermgr, new_getpass):
+        self.__usermgr = usermgr
+        self.__new_input = new_getpass
+        self.__orig_input = usermgr._input
+
+    def __enter__(self):
+        self.__usermgr._input = self.__new_input
+        return self
+
+    def __exit__(self, type, value, traceback):
+        self.__usermgr._input = self.__orig_input
+
+def run(command):
+    """
+    Small helper function that returns a tuple of (rcode, stdout, stderr)
+    after running the given command (an array of command and arguments, as
+    passed on to subprocess).
+    Parameters:
+    command: an array of command and argument strings, which will be
+             passed to subprocess.Popen()
+    """
+    subp = subprocess.Popen(command, stdout=subprocess.PIPE,
+                            stderr=subprocess.PIPE)
+    (stdout, stderr) = subp.communicate()
+    return (subp.returncode, stdout, stderr)
+
+class TestUserMgr(unittest.TestCase):
+    TOOL = '../b10-cmdctl-usermgr'
+    OUTPUT_FILE = 'test_users.csv'
+
+    def setUp(self):
+        self.delete_output_file()
+        # For access to the actual module, we load it directly
+        self.usermgr_module = imp.load_source('usermgr',
+                                              '../b10-cmdctl-usermgr.py')
+        # And instantiate 1 instance (with fake options/args)
+        self.usermgr = self.usermgr_module.UserManager(object(), object())
+
+    def tearDown(self):
+        self.delete_output_file()
+
+    def delete_output_file(self):
+        if os.path.exists(self.OUTPUT_FILE):
+            os.remove(self.OUTPUT_FILE)
+
+    def check_output_file(self, expected_content):
+        self.assertTrue(os.path.exists(self.OUTPUT_FILE))
+
+        csv_entries = []
+        with open(self.OUTPUT_FILE, newline='') as csvfile:
+            reader = csv.reader(csvfile)
+            csv_entries = [row for row in reader]
+
+        self.assertEqual(len(expected_content), len(csv_entries))
+        csv_entries.reverse()
+        for expected_entry in expected_content:
+            expected_name = expected_entry[0]
+            expected_pass = expected_entry[1]
+
+            csv_entry = csv_entries.pop()
+            entry_name = csv_entry[0]
+            entry_salt = csv_entry[2]
+            entry_hash = csv_entry[1]
+
+            self.assertEqual(expected_name, entry_name)
+            expected_hash =\
+                sha1((expected_pass + entry_salt).encode()).hexdigest()
+            self.assertEqual(expected_hash, entry_hash)
+
+    def run_check(self, expected_returncode, expected_stdout, expected_stderr,
+                  command):
+        """
+        Runs the given command, and checks return code, and outputs (if provided).
+        Arguments:
+        expected_returncode, return code of the command
+        expected_stdout, (multiline) string that is checked against stdout.
+                         May be None, in which case the check is skipped.
+        expected_stderr, (multiline) string that is checked against stderr.
+                         May be None, in which case the check is skipped.
+        """
+        (returncode, stdout, stderr) = run(command)
+        if expected_stderr is not None:
+            self.assertEqual(expected_stderr, stderr.decode())
+        if expected_stdout is not None:
+            self.assertEqual(expected_stdout, stdout.decode())
+        self.assertEqual(expected_returncode, returncode, " ".join(command))
+
+    def test_help(self):
+        self.run_check(0,
+'''Usage: b10-cmdctl-usermgr [options] <command> [username] [password]
+
+Arguments:
+  command		either 'add' or 'delete'
+  username		the username to add or delete
+  password		the password to set for the added user
+
+If username or password are not specified, b10-cmdctl-usermgr will
+prompt for them. It is recommended practice to let the
+tool prompt for the password, as command-line
+arguments can be visible through history or process
+viewers.
+
+Options:
+  --version             show program's version number and exit
+  -h, --help            show this help message and exit
+  -f OUTPUT_FILE, --file=OUTPUT_FILE
+                        Accounts file to modify
+  -q, --quiet           Quiet mode, don't print any output
+''',
+                       '',
+                       [self.TOOL, '-h'])
+
+    def test_add_delete_users_ok(self):
+        """
+        Test that a file is created, and users are added.
+        Also tests quiet mode for adding a user to an existing file.
+        """
+        # content is a list of (user, pass) tuples
+        expected_content = []
+
+        # Creating a file
+        self.run_check(0,
+                       'Using accounts file: test_users.csv\n',
+                       '',
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'add', 'user1', 'pass1'
+                       ])
+        expected_content.append(('user1', 'pass1'))
+        self.check_output_file(expected_content)
+
+        # Add to existing file
+        self.run_check(0,
+                       'Using accounts file: test_users.csv\n',
+                       '',
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'add', 'user2', 'pass2'
+                       ])
+        expected_content.append(('user2', 'pass2'))
+        self.check_output_file(expected_content)
+
+        # Quiet mode
+        self.run_check(0,
+                       '',
+                       '',
+                       [ self.TOOL, '-q',
+                         '-f', self.OUTPUT_FILE,
+                         'add', 'user3', 'pass3'
+                       ])
+        expected_content.append(('user3', 'pass3'))
+        self.check_output_file(expected_content)
+
+        # Delete a user (let's pick the middle one)
+        self.run_check(0,
+                       '',
+                       '',
+                       [ self.TOOL, '-q',
+                         '-f', self.OUTPUT_FILE,
+                         'delete', 'user2'
+                       ])
+        del expected_content[1]
+        self.check_output_file(expected_content)
+
+    def test_add_delete_users_bad(self):
+        """
+        More add/delete tests, this time for some error scenarios
+        """
+        # content is a list of (user, pass) tuples
+        expected_content = []
+        # First add one
+        self.run_check(0, None, None,
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'add', 'user', 'pass'
+                       ])
+        expected_content.append(('user', 'pass'))
+        self.check_output_file(expected_content)
+
+        # Adding it again should error
+        self.run_check(3,
+                       'Using accounts file: test_users.csv\n'
+                       'Error: username exists\n',
+                       '',
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'add', 'user', 'pass'
+                       ])
+        self.check_output_file(expected_content)
+
+        # Deleting a non-existent one should fail too
+        self.run_check(4,
+                       'Using accounts file: test_users.csv\n'
+                       'Error: username does not exist\n',
+                       '',
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'delete', 'nosuchuser'
+                       ])
+        self.check_output_file(expected_content)
+
+    def test_bad_arguments(self):
+        """
+        Assorted tests with bad command-line arguments
+        """
+        self.run_check(1,
+                       'Error: no command specified\n',
+                       '',
+                       [ self.TOOL ])
+        self.run_check(1,
+                       'Error: command must be either add or delete\n',
+                       '',
+                       [ self.TOOL, 'foo' ])
+        self.run_check(1,
+                       'Error: extraneous arguments\n',
+                       '',
+                       [ self.TOOL, 'add', 'user', 'pass', 'toomuch' ])
+        self.run_check(1,
+                       'Error: delete only needs username, not a password\n',
+                       '',
+                       [ self.TOOL, 'delete', 'user', 'pass' ])
+
+    def test_default_file(self):
+        """
+        Check the default file is the correct one.
+        """
+        # Hardcoded path .. should be ok since this is run from make check
+        self.assertEqual(SYSCONFPATH + '/cmdctl-accounts.csv',
+                         self.usermgr_module.DEFAULT_FILE)
+
+    def test_prompt_for_password_different(self):
+        """
+        Check that the method that prompts for a password verifies that
+        the same value is entered twice
+        """
+        # returns a different string (the representation of the number
+        # of times it has been called), until it has been called
+        # over 10 times, in which case it will always return "11"
+        getpass_different_called = 0
+        def getpass_different(question):
+            nonlocal getpass_different_called
+            getpass_different_called += 1
+            if getpass_different_called > 10:
+                return "11"
+            else:
+                return str(getpass_different_called)
+
+        with PrintCatcher() as pc:
+            with OverrideGetpass(getpass_different):
+                pwd = self.usermgr._prompt_for_password()
+                self.assertEqual(12, getpass_different_called)
+                self.assertEqual("11", pwd)
+                # stdout should be 5 times the no match string;
+                expected_output = "passwords do not match, try again\n"*5
+                self.assertEqual(expected_output, ''.join(pc.stdout_lines))
+
+    def test_prompt_for_password_empty(self):
+        """
+        Check that the method that prompts for a password verifies that
+        the value entered is not empty
+        """
+        # returns an empty string until it has been called over 10
+        # times
+        getpass_empty_called = 0
+        def getpass_empty(prompt):
+            nonlocal getpass_empty_called
+            getpass_empty_called += 1
+            if getpass_empty_called > 10:
+                return "nonempty"
+            else:
+                return ""
+
+        with PrintCatcher() as pc:
+            with OverrideGetpass(getpass_empty):
+                pwd = self.usermgr._prompt_for_password()
+                self.assertEqual("nonempty", pwd)
+                self.assertEqual(12, getpass_empty_called)
+                # stdout should be 10 times the 'cannot be empty' string
+                expected_output = "Error: password cannot be empty\n"*10
+                self.assertEqual(expected_output, ''.join(pc.stdout_lines))
+
+    def test_prompt_for_user(self):
+        """
+        Test that the method that prompts for a username verifies that
+        is not empty, and that it exists (or does not, depending on the
+        action that is specified)
+        """
+        new_input_called = 0
+        input_results = [ '', '', 'existinguser', 'nonexistinguser',
+                          '', '', 'nonexistinguser', 'existinguser' ]
+        def new_input(prompt):
+            nonlocal new_input_called
+
+            if new_input_called < len(input_results):
+                result = input_results[new_input_called]
+            else:
+                result = 'empty'
+            new_input_called += 1
+            return result
+
+        # add fake user (value doesn't matter, method only checks for key)
+        self.usermgr.user_info = { 'existinguser': None }
+
+        expected_output = ''
+
+        with PrintCatcher() as pc:
+            with OverrideInput(self.usermgr, new_input):
+                # should skip the first three since empty or existing
+                # are not allowed, then return 'nonexistinguser'
+                username = self.usermgr._prompt_for_username(
+                                self.usermgr_module.COMMAND_ADD)
+                self.assertEqual('nonexistinguser', username)
+                expected_output += "Error username can't be empty\n"*2
+                expected_output += "user already exists\n"
+                self.assertEqual(expected_output, ''.join(pc.stdout_lines))
+
+                # For delete, should again not accept empty (in a while true
+                # loop), and this time should not accept nonexisting users
+                username = self.usermgr._prompt_for_username(
+                                self.usermgr_module.COMMAND_DELETE)
+                self.assertEqual('existinguser', username)
+                expected_output += "Error username can't be empty\n"*2
+                expected_output += "user does not exist\n"
+                self.assertEqual(expected_output, ''.join(pc.stdout_lines))
+
+    def test_bad_file(self):
+        """
+        Check for graceful handling of bad file argument
+        """
+        self.run_check(2,
+                       'Using accounts file: /\n'
+                       'Error accessing /: Is a directory\n',
+                       '',
+                       [ self.TOOL, '-f', '/', 'add', 'user', 'pass' ])
+
+        # Make sure we can initially write to the test file
+        self.run_check(0, None, None,
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'add', 'user1', 'pass1'
+                       ])
+
+        # Make it non-writable (don't worry about cleanup, the
+        # file should be deleted after each test anyway
+        os.chmod(self.OUTPUT_FILE, stat.S_IRUSR)
+        self.run_check(2,
+                       'Using accounts file: test_users.csv\n'
+                       'Error accessing test_users.csv: Permission denied\n',
+                       '',
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'add', 'user2', 'pass1'
+                       ])
+
+        self.run_check(2,
+                       'Using accounts file: test_users.csv\n'
+                       'Error accessing test_users.csv: Permission denied\n',
+                       '',
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'delete', 'user1'
+                       ])
+
+        # Making it write-only should have the same effect
+        os.chmod(self.OUTPUT_FILE, stat.S_IWUSR)
+        self.run_check(2,
+                       'Using accounts file: test_users.csv\n'
+                       'Error accessing test_users.csv: Permission denied\n',
+                       '',
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'add', 'user2', 'pass1'
+                       ])
+
+        self.run_check(2,
+                       'Using accounts file: test_users.csv\n'
+                       'Error accessing test_users.csv: Permission denied\n',
+                       '',
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'delete', 'user1'
+                       ])
+
+    def test_missing_fields(self):
+        """
+        Test that an invalid csv file is handled gracefully
+        """
+        # Valid but incomplete csv; should be handled
+        # correctly
+        with open(self.OUTPUT_FILE, 'w', newline='') as f:
+            f.write('onlyuserfield\n')
+            f.write('userfield,saltfield\n')
+            f.write(',emptyuserfield,passwordfield\n')
+
+        self.run_check(0, None, None,
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'add', 'user1', 'pass1'
+                       ])
+        self.run_check(0, None, None,
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'delete', 'onlyuserfield'
+                       ])
+        self.run_check(0, None, None,
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'delete', ''
+                       ])
+
+    def test_bad_data(self):
+        # I can only think of one invalid format, an unclosed string
+        with open(self.OUTPUT_FILE, 'w', newline='') as f:
+            f.write('a,"\n')
+        self.run_check(2,
+                       'Using accounts file: test_users.csv\n'
+                       'Error parsing csv file: newline inside string\n',
+                       '',
+                       [ self.TOOL,
+                         '-f', self.OUTPUT_FILE,
+                         'add', 'user1', 'pass1'
+                       ])
+
+
+if __name__== '__main__':
+    unittest.main()
+

src/bin/xfrin/tests/xfrin_test.py

@@ -608,7 +608,7 @@ class TestXfrinIXFRAdd(TestXfrinState):
         # signed, rejecting it.
         self.assertRaises(xfrin.XfrinProtocolError, self.state.handle_rr,
                           self.conn, end_soa_rrset)
-        # No diffs were commited
+        # No diffs were committed
         self.assertEqual([], self.conn._datasrc_client.committed_diffs)
 
     def test_handle_out_of_sync(self):
@@ -2578,7 +2578,7 @@ class TestXfrin(unittest.TestCase):
         # there can be one more outstanding transfer.
         self.assertEqual(self.xfr.command_handler("retransfer",
                                                   self.args)['result'][0], 0)
-        # make sure the # xfrs would excceed the quota
+        # make sure the # xfrs would exceed the quota
         self.xfr.recorder.increment(Name(str(self.xfr._max_transfers_in) + TEST_ZONE_NAME_STR))
         # this one should fail
         self.assertEqual(self.xfr.command_handler("retransfer",

src/bin/xfrin/xfrin.py.in

@@ -433,7 +433,7 @@ class XfrinIXFRAdd(XfrinState):
             if soa_serial == conn._end_serial:
                 # The final part is there. Finish the transfer by
                 # checking the last TSIG (if required), the zone data and
-                # commiting.
+                # committing.
                 conn.finish_transfer()
                 self.set_xfrstate(conn, XfrinIXFREnd())
                 return True
@@ -624,7 +624,7 @@ class XfrinConnection(asyncore.dispatcher):
         it if the constructor raises an exception after opening the socket.
         '''
         self.create_socket(self._master_addrinfo[0], self._master_addrinfo[1])
-        self.setblocking(1)
+        self.socket.setblocking(1)
 
     def _get_zone_soa(self):
         '''Retrieve the current SOA RR of the zone to be transferred.
@@ -822,7 +822,7 @@ class XfrinConnection(asyncore.dispatcher):
     def finish_transfer(self):
         """
         Perform any necessary checks after a transfer. Then complete the
-        transfer by commiting the transaction into the data source.
+        transfer by committing the transaction into the data source.
         """
         self._check_response_tsig_last()
         if not check_zone(self._zone_name, self._rrclass,
@@ -1129,7 +1129,7 @@ def __process_xfrin(server, zone_name, rrclass, db_file,
         if db_file is not None:
             # temporary hardcoded sqlite initialization. Once we decide on
             # the config specification, we need to update this (TODO)
-            # this may depend on #1207, or any followup ticket created for #1207
+            # this may depend on #1207, or any follow-up ticket created for #1207
             datasrc_type = "sqlite3"
             datasrc_config = "{ \"database_file\": \"" + db_file + "\"}"
             datasrc_client = DataSourceClient(datasrc_type, datasrc_config)

src/bin/xfrout/tests/xfrout_test.py.in

@@ -191,7 +191,7 @@ class Dbserver:
         self.transfer_counter -= 1
 
 class TestXfroutSessionBase(unittest.TestCase):
-    '''Base classs for tests related to xfrout sessions
+    '''Base class for tests related to xfrout sessions
 
     This class defines common setup/teadown and utility methods.  Actual
     tests are delegated to subclasses.

src/bin/xfrout/xfrout.py.in

@@ -186,7 +186,7 @@ class XfroutSession():
 
         '''
         # Check the xfrout quota.  We do both increase/decrease in this
-        # method so it's clear we always release it once acuired.
+        # method so it's clear we always release it once acquired.
         quota_ok = self._server.increase_transfers_counter()
         ex = None
         try:
@@ -502,7 +502,7 @@ class XfroutSession():
             return self._reply_query_with_error_rcode(msg, sock_fd,
                                                       Rcode.FORMERR)
         elif not quota_ok:
-            logger.warn(XFROUT_QUERY_QUOTA_EXCCEEDED, self._request_typestr,
+            logger.warn(XFROUT_QUERY_QUOTA_EXCEEDED, self._request_typestr,
                         format_addrinfo(self._remote),
                         self._server._max_transfers_out)
             return self._reply_query_with_error_rcode(msg, sock_fd,

src/bin/xfrout/xfrout_messages.mes

@@ -131,7 +131,7 @@ given host.  This is required by the ACLs.  The %2 represents the IP
 address and port of the peer requesting the transfer, and the %3
 represents the zone name and class.
 
-% XFROUT_QUERY_QUOTA_EXCCEEDED %1 client %2: request denied due to quota (%3)
+% XFROUT_QUERY_QUOTA_EXCEEDED %1 client %2: request denied due to quota (%3)
 The xfr request was rejected because the server was already handling
 the maximum number of allowable transfers as specified in the transfers_out
 configuration parameter, which is also shown in the log message.  The

src/bin/zonemgr/tests/zonemgr_test.py

@@ -314,16 +314,22 @@ class TestZonemgrRefresh(unittest.TestCase):
         sqlite3_ds.get_zone_soa = old_get_zone_soa
 
     def test_zone_handle_notify(self):
-        self.zone_refresh.zone_handle_notify(ZONE_NAME_CLASS1_IN,"127.0.0.1")
-        notify_master = self.zone_refresh._zonemgr_refresh_info[ZONE_NAME_CLASS1_IN]["notify_master"]
+        self.assertTrue(self.zone_refresh.zone_handle_notify(
+                ZONE_NAME_CLASS1_IN, "127.0.0.1"))
+        notify_master = self.zone_refresh.\
+            _zonemgr_refresh_info[ZONE_NAME_CLASS1_IN]["notify_master"]
         self.assertEqual("127.0.0.1", notify_master)
-        zone_timeout = self.zone_refresh._zonemgr_refresh_info[ZONE_NAME_CLASS1_IN]["next_refresh_time"]
+        zone_timeout = self.zone_refresh.\
+            _zonemgr_refresh_info[ZONE_NAME_CLASS1_IN]["next_refresh_time"]
         current_time = time.time()
         self.assertTrue(zone_timeout <= current_time)
-        self.assertRaises(ZonemgrException, self.zone_refresh.zone_handle_notify,\
-                          ZONE_NAME_CLASS3_CH, "127.0.0.1")
-        self.assertRaises(ZonemgrException, self.zone_refresh.zone_handle_notify,\
-                          ZONE_NAME_CLASS3_IN, "127.0.0.1")
+
+        # If the specified zone does not in the configured secondary list,
+        # it should return False.
+        self.assertFalse(self.zone_refresh.zone_handle_notify(
+                ZONE_NAME_CLASS3_CH, "127.0.0.1"))
+        self.assertFalse(self.zone_refresh.zone_handle_notify(
+                ZONE_NAME_CLASS3_IN, "127.0.0.1"))
 
     def test_zone_refresh_success(self):
         soa_rdata = 'a.example.net. root.example.net. 2009073106 1800 900 2419200 21600'
@@ -607,6 +613,19 @@ class TestZonemgrRefresh(unittest.TestCase):
                           config, self.cc_session)
 
 class MyZonemgr(Zonemgr):
+    class DummySocket:
+        """This dummy class simply steal send() to record any transmitted data.
+
+        """
+        def __init__(self):
+            self.sent_data = []
+
+        def send(self, data):
+            self.sent_data.append(data)
+
+    class DummyLock:
+        def __enter__(self): pass
+        def __exit__(self, type, value, traceback): pass
 
     def __init__(self):
         self._db_file = TEST_SQLITE3_DBFILE
@@ -621,6 +640,8 @@ class MyZonemgr(Zonemgr):
                     "reload_jitter" : 0.75,
                     "secondary_zones": []
                     }
+        self._lock = self.DummyLock()
+        self._master_socket = self.DummySocket()
 
     def _start_zone_refresh_timer(self):
         pass
@@ -672,15 +693,21 @@ class TestZonemgr(unittest.TestCase):
         self.assertEqual(TEST_SQLITE3_DBFILE, self.zonemgr.get_db_file())
 
     def test_parse_cmd_params(self):
-        params1 = {"zone_name" : "example.com.", "zone_class" : "CH", "master" : "127.0.0.1"}
+        params1 = {"zone_name" : "example.com.", "zone_class" : "CH",
+                   "master" : "127.0.0.1"}
         answer1 = (ZONE_NAME_CLASS3_CH, "127.0.0.1")
-        self.assertEqual(answer1, self.zonemgr._parse_cmd_params(params1, ZONE_NOTIFY_COMMAND))
+        self.assertEqual(answer1,
+                         self.zonemgr._parse_cmd_params(params1,
+                                                        ZONE_NOTIFY_COMMAND))
         params2 = {"zone_name" : "example.com.", "zone_class" : "IN"}
         answer2 = ZONE_NAME_CLASS3_IN
-        self.assertEqual(answer2, self.zonemgr._parse_cmd_params(params2, notify_out.ZONE_NEW_DATA_READY_CMD))
-        self.assertRaises(ZonemgrException, self.zonemgr._parse_cmd_params, params2, ZONE_NOTIFY_COMMAND)
+        self.assertEqual(answer2, self.zonemgr._parse_cmd_params(
+                params2, notify_out.ZONE_NEW_DATA_READY_CMD))
+        self.assertRaises(ZonemgrException, self.zonemgr._parse_cmd_params,
+                          params2, ZONE_NOTIFY_COMMAND)
         params1 = {"zone_class" : "CH"}
-        self.assertRaises(ZonemgrException, self.zonemgr._parse_cmd_params, params2, ZONE_NOTIFY_COMMAND)
+        self.assertRaises(ZonemgrException, self.zonemgr._parse_cmd_params,
+                          params2, ZONE_NOTIFY_COMMAND)
 
     def test_config_data_check(self):
         # jitter should not be bigger than half of the original value
@@ -697,6 +724,26 @@ class TestZonemgr(unittest.TestCase):
         self.zonemgr.run()
         self.assertTrue(self.zonemgr._module_cc.stopped)
 
+    def test_command_handler_notify(self):
+        """Check the result of NOTIFY command."""
+        self.zonemgr._zone_refresh = MyZonemgrRefresh()
+
+        # On successful case, the other thread will be notified via
+        # _master_socket.
+        self.zonemgr._zone_refresh.zone_handle_notify = lambda x, y: True
+        self.zonemgr.command_handler("notify", {"zone_name": "example.",
+                                                "zone_class": "IN",
+                                                "master": "192.0.2.1"})
+        self.assertEqual([b" "], self.zonemgr._master_socket.sent_data)
+
+        # If the specified is not found in the secondary list, it doesn't
+        # bother to wake the thread (sent_data shouldn't change)
+        self.zonemgr._zone_refresh.zone_handle_notify = lambda x, y: False
+        self.zonemgr.command_handler("notify", {"zone_name": "example.",
+                                                "zone_class": "IN",
+                                                "master": "192.0.2.1"})
+        self.assertEqual([b" "], self.zonemgr._master_socket.sent_data)
+
 if __name__== "__main__":
     isc.log.resetUnitTestRootLogger()
     unittest.main()

src/bin/zonemgr/zonemgr.py.in

@@ -191,14 +191,31 @@ class ZonemgrRefresh:
         self._set_zone_retry_timer(zone_name_class)
 
     def zone_handle_notify(self, zone_name_class, master):
-        """Handle zone notify"""
-        if (self._zone_not_exist(zone_name_class)):
-            logger.error(ZONEMGR_UNKNOWN_ZONE_NOTIFIED, zone_name_class[0],
-                         zone_name_class[1], master)
-            raise ZonemgrException("[b10-zonemgr] Notified zone (%s, %s) "
-                                   "doesn't belong to zonemgr" % zone_name_class)
+        """Handle an incomding NOTIFY message via the Auth module.
+
+        It returns True if the specified zone matches one of the locally
+        configured list of secondary zones; otherwise returns False.
+        In the latter case it assumes the server is a primary (master) of the
+        zone; the Auth module should have rejected the case where it's not
+        even authoritative for the zone.
+
+        Note: to be more robust and less independent from other module's
+        behavior, it's probably safer to check the authority condition here,
+        too.  But right now it uses SQLite3 specific API (to be deprecated),
+        so we rather rely on Auth.
+
+        Parameters:
+        zone_name_class (Name, RRClass): the notified zone name and class.
+        master (str): textual address of the NOTIFY sender.
+
+        """
+        if self._zone_not_exist(zone_name_class):
+            logger.debug(DBG_ZONEMGR_BASIC, ZONEMGR_ZONE_NOTIFY_NOT_SECONDARY,
+                         zone_name_class[0], zone_name_class[1], master)
+            return False
         self._set_zone_notifier_master(zone_name_class, master)
         self._set_zone_notify_timer(zone_name_class)
+        return True
 
     def zonemgr_reload_zone(self, zone_name_class):
         """ Reload a zone."""
@@ -423,7 +440,7 @@ class ZonemgrRefresh:
 
         # Ask the thread to stop
         self._running = False
-        self._write_sock.send(b'shutdown') # make self._read_sock readble
+        self._write_sock.send(b'shutdown') # make self._read_sock readable
         # Wait for it to actually finnish
         self._thread.join()
         # Wipe out what we do not need
@@ -630,27 +647,33 @@ class Zonemgr:
             """ Handle Auth notify command"""
             # master is the source sender of the notify message.
             zone_name_class, master = self._parse_cmd_params(args, command)
-            logger.debug(DBG_ZONEMGR_COMMAND, ZONEMGR_RECEIVE_NOTIFY, zone_name_class[0], zone_name_class[1])
+            logger.debug(DBG_ZONEMGR_COMMAND, ZONEMGR_RECEIVE_NOTIFY,
+                         zone_name_class[0], zone_name_class[1])
             with self._lock:
-                self._zone_refresh.zone_handle_notify(zone_name_class, master)
-            # Send notification to zonemgr timer thread
-            self._master_socket.send(b" ")# make self._slave_socket readble
+                need_refresh = self._zone_refresh.zone_handle_notify(
+                    zone_name_class, master)
+            if need_refresh:
+                # Send notification to zonemgr timer thread by making
+                # self._slave_socket readable.
+                self._master_socket.send(b" ")
 
         elif command == notify_out.ZONE_NEW_DATA_READY_CMD:
             """ Handle xfrin success command"""
             zone_name_class = self._parse_cmd_params(args, command)
-            logger.debug(DBG_ZONEMGR_COMMAND, ZONEMGR_RECEIVE_XFRIN_SUCCESS, zone_name_class[0], zone_name_class[1])
+            logger.debug(DBG_ZONEMGR_COMMAND, ZONEMGR_RECEIVE_XFRIN_SUCCESS,
+                         zone_name_class[0], zone_name_class[1])
             with self._lock:
                 self._zone_refresh.zone_refresh_success(zone_name_class)
-            self._master_socket.send(b" ")# make self._slave_socket readble
+            self._master_socket.send(b" ")# make self._slave_socket readable
 
         elif command == notify_out.ZONE_XFRIN_FAILED:
             """ Handle xfrin fail command"""
             zone_name_class = self._parse_cmd_params(args, command)
-            logger.debug(DBG_ZONEMGR_COMMAND, ZONEMGR_RECEIVE_XFRIN_FAILED, zone_name_class[0], zone_name_class[1])
+            logger.debug(DBG_ZONEMGR_COMMAND, ZONEMGR_RECEIVE_XFRIN_FAILED,
+                         zone_name_class[0], zone_name_class[1])
             with self._lock:
                 self._zone_refresh.zone_refresh_fail(zone_name_class)
-            self._master_socket.send(b" ")# make self._slave_socket readble
+            self._master_socket.send(b" ")# make self._slave_socket readable
 
         elif command == "shutdown":
             logger.debug(DBG_ZONEMGR_COMMAND, ZONEMGR_RECEIVE_SHUTDOWN)
@@ -667,7 +690,19 @@ class Zonemgr:
         self.running = True
         try:
             while not self._shutdown_event.is_set():
-                self._module_cc.check_command(False)
+                fileno = self._module_cc.get_socket().fileno()
+                # Wait with select() until there is something to read,
+                # and then read it using a non-blocking read
+                # This may or may not be relevant data for this loop,
+                # but due to the way the zonemgr does threading, we
+                # can't have a blocking read loop here.
+                try:
+                    (reads, _, _) = select.select([fileno], [], [])
+                except select.error as se:
+                    if se.args[0] != errno.EINTR:
+                        raise
+                if fileno in reads:
+                    self._module_cc.check_command(True)
         finally:
             self._module_cc.send_stopping()
 

src/bin/zonemgr/zonemgr_messages.mes

@@ -138,14 +138,19 @@ zone, or, if this error appears without the administrator giving transfer
 commands, it can indicate an error in the program, as it should not have
 initiated transfers of unknown zones on its own.
 
-% ZONEMGR_UNKNOWN_ZONE_NOTIFIED notified zone %1/%2 from %3 is not known to the zone manager
-A NOTIFY was received but the zone that was the subject of the operation
-is not being managed by the zone manager.  This may indicate an error
-in the program (as the operation should not have been initiated if this
-were the case).  Please submit a bug report.
-
 % ZONEMGR_UNKNOWN_ZONE_SUCCESS zone %1 (class %2) is not known to the zone manager
 An XFRIN operation has succeeded but the zone received is not being
 managed by the zone manager.  This may indicate an error in the program
 (as the operation should not have been initiated if this were the case).
 Please submit a bug report.
+
+% ZONEMGR_ZONE_NOTIFY_NOT_SECONDARY notify for zone %1/%2 from %3 received but not in secondaries
+A NOTIFY was received but the zone is not listed in the configured
+secondary zones of the zone manager.  The most common reason for this
+is that it's simply received by a primary server of the zone.  Another
+possibility is a configuration error that it's not configured as a
+secondary while it should be.  In either case, the zone manager does
+not take action in terms of zone management, and the authoritative
+server will respond to it like in the secondary case.  If this is a
+configuration error, it will be noticed by the fact that the zone
+isn't updated even after a change is made in the primary server.

src/lib/asiodns/dns_answer.h

@@ -32,7 +32,7 @@ namespace asiodns {
 /// via its constructor.
 ///
 /// A DNS Answer provider function takes answer data that has been obtained
-/// from a DNS Lookup provider functon and readies it to be sent to the
+/// from a DNS Lookup provider function and readies it to be sent to the
 /// client.  After it has run, the OutputBuffer object passed to it should
 /// contain the answer to the query rendered into wire format.
 class DNSAnswer {

src/lib/asiodns/dns_server.h

@@ -71,7 +71,7 @@ public:
     /// without losing access to derived class data.
     ///
     //@{
-    /// \brief The funtion operator
+    /// \brief The function operator
     virtual void operator()(asio::error_code ec = asio::error_code(),
                             size_t length = 0)
     {

src/lib/asiodns/io_fetch.h

@@ -143,7 +143,7 @@ public:
     /// \brief Constructor
     ///  This constructor has one parameter "query_message", which
     ///  is the shared_ptr to a full query message. It's different
-    ///  with above contructor which has only question section. All
+    ///  with above constructor which has only question section. All
     ///  other parameters are same.
     ///
     /// \param query_message the shared_ptr to a full query message

src/lib/asiodns/sync_udp_server.cc

@@ -174,13 +174,13 @@ SyncUDPServer::operator()(asio::error_code, size_t) {
 void
 SyncUDPServer::stop() {
     /// Using close instead of cancel, because cancel
-    /// will only cancel the asynchornized event already submitted
+    /// will only cancel the asynchronized event already submitted
     /// to io service, the events post to io service after
     /// cancel still can be scheduled by io service, if
-    /// the socket is cloesed, all the asynchronized event
+    /// the socket is closed, all the asynchronized event
     /// for it won't be scheduled by io service not matter it is
-    /// submit to io serice before or after close call. And we will
-    //. get bad_descriptor error
+    /// submit to io service before or after close call. And we will
+    /// get bad_descriptor error.
     socket_->close();
     stopped_ = true;
 }

src/lib/asiodns/tests/dns_server_unittest.cc

@@ -51,8 +51,8 @@
 ///   Before the server start to run
 ///   After we get the query and check whether it's valid
 ///   After we lookup the query
-///   After we compoisite the answer
-///   After user get the final result.
+///   After we compose the answer
+///   After user gets the final result.
 
 /// The standard about whether we stop the server successfully or not
 /// is based on the fact that if the server is still running, the io
@@ -83,7 +83,7 @@ const char* const server_port_str = "5553";
 const char* const query_message = "BIND10 is awesome";
 
 // \brief provide capacity to derived class the ability
-// to stop DNSServer at certern point
+// to stop DNSServer at certain point
 class ServerStopper {
     public:
         ServerStopper() : server_to_stop_(NULL) {}
@@ -148,7 +148,7 @@ class SimpleAnswer : public DNSAnswer, public ServerStopper {
 };
 
 // \brief simple client, send one string to server and wait for response
-//  in case, server stopped and client cann't get response, there is a timer wait
+//  in case, server stopped and client can't get response, there is a timer wait
 //  for specified seconds (the value is just a estimate since server process logic is quite
 //  simple, and all the intercommunication is local) then cancel the waiting.
 class SimpleClient : public ServerStopper {

src/lib/asiodns/udp_server.cc

@@ -323,13 +323,13 @@ UDPServer::asyncLookup() {
 void
 UDPServer::stop() {
     /// Using close instead of cancel, because cancel
-    /// will only cancel the asynchornized event already submitted
+    /// will only cancel the asynchronized event already submitted
     /// to io service, the events post to io service after
     /// cancel still can be scheduled by io service, if
-    /// the socket is cloesed, all the asynchronized event
+    /// the socket is closed, all the asynchronized event
     /// for it won't be scheduled by io service not matter it is
-    /// submit to io serice before or after close call. And we will
-    //. get bad_descriptor error
+    /// submit to io service before or after close call. And we will
+    //  get bad_descriptor error.
     data_->socket_->close();
 }
 

src/lib/asiolink/io_asio_socket.h

@@ -158,7 +158,7 @@ public:
     /// (The reason there is a need to know is because the call to open() passes
     /// in the state of the coroutine at the time the call is made.  On an
     /// asynchronous I/O, we need to set the state to point to the statement
-    /// after the call to open() _before_ we pass the corouine to the open()
+    /// after the call to open() _before_ we pass the coroutine to the open()
     /// call.  Unfortunately, the macros that set the state of the coroutine
     /// also yield control - which we don't want to do if the open is
     /// synchronous.  Hence we need to know before we make the call to open()

src/lib/asiolink/tcp_endpoint.h

@@ -100,7 +100,7 @@ public:
         return (asio_endpoint_.protocol().family());
     }
 
-    // This is not part of the exosed IOEndpoint API but allows
+    // This is not part of the exposed IOEndpoint API but allows
     // direct access to the ASIO implementation of the endpoint
     inline const asio::ip::tcp::endpoint& getASIOEndpoint() const {
         return (asio_endpoint_);

src/lib/asiolink/tests/tcp_socket_unittest.cc

@@ -14,7 +14,7 @@
 
 /// \brief Test of TCPSocket
 ///
-/// Tests the fuctionality of a TCPSocket by working through an open-send-
+/// Tests the functionality of a TCPSocket by working through an open-send-
 /// receive-close sequence and checking that the asynchronous notifications
 /// work.
 

src/lib/asiolink/tests/udp_socket_unittest.cc

@@ -14,7 +14,7 @@
 
 /// \brief Test of UDPSocket
 ///
-/// Tests the fuctionality of a UDPSocket by working through an open-send-
+/// Tests the functionality of a UDPSocket by working through an open-send-
 /// receive-close sequence and checking that the asynchronous notifications
 /// work.
 

src/lib/asiolink/udp_endpoint.h

@@ -100,7 +100,7 @@ public:
         return (asio_endpoint_.protocol().family());
     }
 
-    // This is not part of the exosed IOEndpoint API but allows
+    // This is not part of the exposed IOEndpoint API but allows
     // direct access to the ASIO implementation of the endpoint
     inline const asio::ip::udp::endpoint& getASIOEndpoint() const {
         return (asio_endpoint_);

src/lib/cc/cc_messages.mes

@@ -37,7 +37,7 @@ socket listed in the output.
 This debug message indicates that the connection was successfully made, this
 should follow CC_ESTABLISH.
 
-% CC_GROUP_RECEIVE trying to receive a message
+% CC_GROUP_RECEIVE trying to receive a message with seq %1
 Debug message, noting that a message is expected to come over the command
 channel.
 

src/lib/cc/data.h

@@ -216,7 +216,7 @@ public:
     //@{
     /// Returns the ElementPtr at the given key
     /// \param name The key of the Element to return
-    /// \return The ElementPtr at the given key
+    /// \return The ElementPtr at the given key, or null if not present
     virtual ConstElementPtr get(const std::string& name) const;
 
     /// Sets the ElementPtr at the given key
@@ -504,7 +504,7 @@ public:
 
     // find the Element at 'id', and store the element pointer in t
     // returns true if found, or false if not found (either because
-    // it doesnt exist or one of the elements in the path is not
+    // it doesn't exist or one of the elements in the path is not
     // a MapElement)
     bool find(const std::string& id, ConstElementPtr& t) const;
 

src/lib/cc/proto_defs.cc

@@ -34,12 +34,22 @@ const char* const CC_HEADER_WANT_ANSWER = "want_answer";
 const char* const CC_HEADER_REPLY = "reply";
 // The commands in the "type" header
 const char* const CC_COMMAND_SEND = "send";
+const char* const CC_COMMAND_SUBSCRIBE = "subscribe";
+const char* const CC_COMMAND_UNSUBSCRIBE = "unsubscribe";
+const char* const CC_COMMAND_GET_LNAME = "getlname";
+const char* const CC_COMMAND_PING = "ping";
+const char* const CC_COMMAND_PONG = "pong";
+const char* const CC_COMMAND_STOP = "stop";
 // The wildcards of some headers
 const char* const CC_TO_WILDCARD = "*";
 const char* const CC_INSTANCE_WILDCARD = "*";
 // Reply codes
-const int CC_REPLY_SUCCESS = 0;
 const int CC_REPLY_NO_RECPT = -1;
+const int CC_REPLY_SUCCESS = 0;
+// Payload in the message
+const char *const CC_PAYLOAD_LNAME = "lname";
+const char *const CC_PAYLOAD_RESULT = "result";
+const char *const CC_PAYLOAD_COMMAND = "command";
 
 }
 }

src/lib/cc/session.cc

@@ -260,7 +260,7 @@ SessionImpl::getSocketDesc() {
     /// @todo boost 1.42 uses native() method, but it is deprecated
     /// in 1.49 and native_handle() is recommended instead
     if (!socket_.is_open()) {
-        isc_throw(InvalidOperation, "Can't return socket desciptor: no socket opened.");
+        isc_throw(InvalidOperation, "Can't return socket descriptor: no socket opened.");
     }
     return socket_.native();
 }
@@ -325,14 +325,14 @@ Session::establish(const char* socket_file) {
     //
     // send a request for our local name, and wait for a response
     //
-    ConstElementPtr get_lname_msg =
-        Element::fromJSON("{ \"type\": \"getlname\" }");
+    ElementPtr get_lname_msg(Element::createMap());
+    get_lname_msg->set(CC_HEADER_TYPE, Element::create(CC_COMMAND_GET_LNAME));
     sendmsg(get_lname_msg);
 
     ConstElementPtr routing, msg;
     recvmsg(routing, msg, false);
 
-    impl_->lname_ = msg->get("lname")->stringValue();
+    impl_->lname_ = msg->get(CC_PAYLOAD_LNAME)->stringValue();
     LOG_DEBUG(logger, DBG_TRACE_DETAILED, CC_LNAME_RECEIVED).arg(impl_->lname_);
 
     // At this point there's no risk of resource leak.
@@ -387,10 +387,10 @@ Session::recvmsg(ConstElementPtr& env, ConstElementPtr& msg,
         for (size_t i = 0; i < impl_->queue_->size(); i++) {
             q_el = impl_->queue_->get(i);
             if (( seq == -1 &&
-                  !q_el->get(0)->contains("reply")
+                  !q_el->get(0)->contains(CC_HEADER_REPLY)
                 ) || (
-                  q_el->get(0)->contains("reply") &&
-                  q_el->get(0)->get("reply")->intValue() == seq
+                  q_el->get(0)->contains(CC_HEADER_REPLY) &&
+                  q_el->get(0)->get(CC_HEADER_REPLY)->intValue() == seq
                 )
                ) {
                    env = q_el->get(0);
@@ -429,10 +429,10 @@ Session::recvmsg(ConstElementPtr& env, ConstElementPtr& msg,
     ConstElementPtr l_msg =
         Element::fromWire(body_wire_stream, length - header_length);
     if ((seq == -1 &&
-         !l_env->contains("reply")
+         !l_env->contains(CC_HEADER_REPLY)
         ) || (
-         l_env->contains("reply") &&
-         l_env->get("reply")->intValue() == seq
+         l_env->contains(CC_HEADER_REPLY) &&
+         l_env->get(CC_HEADER_REPLY)->intValue() == seq
         )
        ) {
         env = l_env;
@@ -453,9 +453,9 @@ Session::subscribe(std::string group, std::string instance) {
     LOG_DEBUG(logger, DBG_TRACE_DETAILED, CC_SUBSCRIBE).arg(group);
     ElementPtr env = Element::createMap();
 
-    env->set("type", Element::create("subscribe"));
-    env->set("group", Element::create(group));
-    env->set("instance", Element::create(instance));
+    env->set(CC_HEADER_TYPE, Element::create(CC_COMMAND_SUBSCRIBE));
+    env->set(CC_HEADER_GROUP, Element::create(group));
+    env->set(CC_HEADER_INSTANCE, Element::create(instance));
 
     sendmsg(env);
 }
@@ -465,9 +465,9 @@ Session::unsubscribe(std::string group, std::string instance) {
     LOG_DEBUG(logger, DBG_TRACE_DETAILED, CC_UNSUBSCRIBE).arg(group);
     ElementPtr env = Element::createMap();
 
-    env->set("type", Element::create("unsubscribe"));
-    env->set("group", Element::create(group));
-    env->set("instance", Element::create(instance));
+    env->set(CC_HEADER_TYPE, Element::create(CC_COMMAND_UNSUBSCRIBE));
+    env->set(CC_HEADER_GROUP, Element::create(group));
+    env->set(CC_HEADER_INSTANCE, Element::create(instance));
 
     sendmsg(env);
 }
@@ -498,7 +498,7 @@ bool
 Session::group_recvmsg(ConstElementPtr& envelope, ConstElementPtr& msg,
                        bool nonblock, int seq)
 {
-    LOG_DEBUG(logger, DBG_TRACE_DETAILED, CC_GROUP_RECEIVE);
+    LOG_DEBUG(logger, DBG_TRACE_DETAILED, CC_GROUP_RECEIVE).arg(seq);
     bool result(recvmsg(envelope, msg, nonblock, seq));
     if (result) {
         LOG_DEBUG(logger, DBG_TRACE_DETAILED, CC_GROUP_RECEIVED).
@@ -516,13 +516,17 @@ Session::reply(ConstElementPtr envelope, ConstElementPtr newmsg) {
     ElementPtr env = Element::createMap();
     long int nseq = ++impl_->sequence_;
 
-    env->set("type", Element::create("send"));
-    env->set("from", Element::create(impl_->lname_));
-    env->set("to", Element::create(envelope->get("from")->stringValue()));
-    env->set("group", Element::create(envelope->get("group")->stringValue()));
-    env->set("instance", Element::create(envelope->get("instance")->stringValue()));
-    env->set("seq", Element::create(nseq));
-    env->set("reply", Element::create(envelope->get("seq")->intValue()));
+    env->set(CC_HEADER_TYPE, Element::create(CC_COMMAND_SEND));
+    env->set(CC_HEADER_FROM, Element::create(impl_->lname_));
+    env->set(CC_HEADER_TO,
+             Element::create(envelope->get(CC_HEADER_FROM)->stringValue()));
+    env->set(CC_HEADER_GROUP,
+             Element::create(envelope->get(CC_HEADER_GROUP)->stringValue()));
+    env->set(CC_HEADER_INSTANCE,
+             Element::create(envelope->get(CC_HEADER_INSTANCE)->stringValue()));
+    env->set(CC_HEADER_SEQ, Element::create(nseq));
+    env->set(CC_HEADER_REPLY,
+             Element::create(envelope->get(CC_HEADER_SEQ)->intValue()));
 
     sendmsg(env, newmsg);
 

src/lib/cc/tests/session_unittests.cc

@@ -46,7 +46,7 @@ TEST(AsioSession, establish) {
     asio::io_service io_service_;
     Session sess(io_service_);
 
-    // can't return socket desciptor before session is established
+    // can't return socket descriptor before session is established
     EXPECT_THROW(sess.getSocketDesc(), isc::InvalidOperation);
 
     EXPECT_THROW(
@@ -293,7 +293,7 @@ TEST_F(SessionTest, run_with_handler_timeout) {
     msg = isc::data::Element::fromJSON("{ \"a third\": \"message\" }");
     tds->sendmsg(env, msg);
 
-    // No followup message, should time out.
+    // No follow-up message, should time out.
     ASSERT_THROW(my_io_service.run(), SessionTimeout);
 }
 

src/lib/config/ccsession.cc

@@ -32,7 +32,7 @@
 #include <boost/foreach.hpp>
 
 #include <cc/data.h>
-#include <module_spec.h>
+#include <config/module_spec.h>
 #include <cc/session.h>
 #include <exceptions/exceptions.h>
 
@@ -57,10 +57,10 @@ namespace config {
 /// Creates a standard config/command protocol answer message
 ConstElementPtr
 createAnswer() {
-    ElementPtr answer = Element::fromJSON("{\"result\": [] }");
+    ElementPtr answer = Element::createMap();
     ElementPtr answer_content = Element::createList();
-    answer_content->add(Element::create(0));
-    answer->set("result", answer_content);
+    answer_content->add(Element::create(isc::cc::CC_REPLY_SUCCESS));
+    answer->set(isc::cc::CC_PAYLOAD_RESULT, answer_content);
 
     return (answer);
 }
@@ -70,22 +70,22 @@ createAnswer(const int rcode, ConstElementPtr arg) {
     if (rcode != 0 && (!arg || arg->getType() != Element::string)) {
         isc_throw(CCSessionError, "Bad or no argument for rcode != 0");
     }
-    ElementPtr answer = Element::fromJSON("{\"result\": [] }");
+    ElementPtr answer = Element::createMap();
     ElementPtr answer_content = Element::createList();
     answer_content->add(Element::create(rcode));
     answer_content->add(arg);
-    answer->set("result", answer_content);
+    answer->set(isc::cc::CC_PAYLOAD_RESULT, answer_content);
 
     return (answer);
 }
 
 ConstElementPtr
 createAnswer(const int rcode, const std::string& arg) {
-    ElementPtr answer = Element::fromJSON("{\"result\": [] }");
+    ElementPtr answer = Element::createMap();
     ElementPtr answer_content = Element::createList();
     answer_content->add(Element::create(rcode));
     answer_content->add(Element::create(arg));
-    answer->set("result", answer_content);
+    answer->set(isc::cc::CC_PAYLOAD_RESULT, answer_content);
 
     return (answer);
 }
@@ -94,8 +94,8 @@ ConstElementPtr
 parseAnswer(int &rcode, ConstElementPtr msg) {
     if (msg &&
         msg->getType() == Element::map &&
-        msg->contains("result")) {
-        ConstElementPtr result = msg->get("result");
+        msg->contains(isc::cc::CC_PAYLOAD_RESULT)) {
+        ConstElementPtr result = msg->get(isc::cc::CC_PAYLOAD_RESULT);
         if (result->getType() != Element::list) {
             isc_throw(CCSessionError, "Result element in answer message is not a list");
         } else if (result->get(0)->getType() != Element::integer) {
@@ -133,7 +133,7 @@ createCommand(const std::string& command, ConstElementPtr arg) {
     if (arg) {
         cmd_parts->add(arg);
     }
-    cmd->set("command", cmd_parts);
+    cmd->set(isc::cc::CC_PAYLOAD_COMMAND, cmd_parts);
     return (cmd);
 }
 
@@ -141,8 +141,8 @@ std::string
 parseCommand(ConstElementPtr& arg, ConstElementPtr command) {
     if (command &&
         command->getType() == Element::map &&
-        command->contains("command")) {
-        ConstElementPtr cmd = command->get("command");
+        command->contains(isc::cc::CC_PAYLOAD_COMMAND)) {
+        ConstElementPtr cmd = command->get(isc::cc::CC_PAYLOAD_COMMAND);
         if (cmd->getType() == Element::list &&
             cmd->size() > 0 &&
             cmd->get(0)->getType() == Element::string) {
@@ -463,10 +463,13 @@ ModuleCCSession::ModuleCCSession(
         isc_throw(CCSessionInitError, answer->str());
     }
 
-    setLocalConfig(Element::fromJSON("{}"));
+    setLocalConfig(Element::createMap());
     // get any stored configuration from the manager
     if (config_handler_) {
-        ConstElementPtr cmd = Element::fromJSON("{ \"command\": [\"get_config\", {\"module_name\":\"" + module_name_ + "\"} ] }");
+        ConstElementPtr cmd =
+            createCommand("get_config",
+                          Element::fromJSON("{\"module_name\":\"" +
+                                            module_name_ + "\"}"));
         seq = session_.group_sendmsg(cmd, "ConfigManager");
         session_.group_recvmsg(env, answer, false, seq);
         ConstElementPtr new_config = parseAnswer(rcode, answer);
@@ -608,14 +611,16 @@ ModuleCCSession::checkCommand() {
 
         /* ignore result messages (in case we're out of sync, to prevent
          * pingpongs */
-        if (data->getType() != Element::map || data->contains("result")) {
+        if (data->getType() != Element::map ||
+            data->contains(isc::cc::CC_PAYLOAD_RESULT)) {
             return (0);
         }
         ConstElementPtr arg;
         ConstElementPtr answer;
         try {
             std::string cmd_str = parseCommand(arg, data);
-            std::string target_module = routing->get("group")->stringValue();
+            std::string target_module =
+                routing->get(isc::cc::CC_HEADER_GROUP)->stringValue();
             if (cmd_str == "config_update") {
                 answer = checkConfigUpdateCommand(target_module, arg);
             } else {
@@ -832,19 +837,19 @@ bool
 ModuleCCSession::requestMatch(const AsyncRecvRequest& request,
                               const ConstElementPtr& envelope) const
 {
-    if (request.is_reply != envelope->contains("reply")) {
+    if (request.is_reply != envelope->contains(isc::cc::CC_HEADER_REPLY)) {
         // Wrong type of message
         return (false);
     }
     if (request.is_reply &&
         (request.seq == -1 ||
-         request.seq == envelope->get("reply")->intValue())) {
+         request.seq == envelope->get(isc::cc::CC_HEADER_REPLY)->intValue())) {
         // This is the correct reply
         return (true);
     }
     if (!request.is_reply &&
-        (request.recipient.empty() ||
-         request.recipient == envelope->get("group")->stringValue())) {
+        (request.recipient.empty() || request.recipient ==
+         envelope->get(isc::cc::CC_HEADER_GROUP)->stringValue())) {
         // This is the correct command
         return (true);
     }
@@ -857,5 +862,27 @@ ModuleCCSession::cancelAsyncRecv(const AsyncRecvRequestID& id) {
     async_recv_requests_.erase(id);
 }
 
+ConstElementPtr
+ModuleCCSession::rpcCall(const std::string &command, const std::string &group,
+                         const std::string &instance, const std::string &to,
+                         const ConstElementPtr &params)
+{
+    ConstElementPtr command_el(createCommand(command, params));
+    const int seq = groupSendMsg(command_el, group, instance, to, true);
+    ConstElementPtr env, answer;
+    LOG_DEBUG(config_logger, DBGLVL_TRACE_DETAIL, CONFIG_RPC_SEQ).arg(command).
+        arg(group).arg(seq);
+    groupRecvMsg(env, answer, true, seq);
+    int rcode;
+    const ConstElementPtr result(parseAnswer(rcode, answer));
+    if (rcode == isc::cc::CC_REPLY_NO_RECPT) {
+        isc_throw(RPCRecipientMissing, result);
+    } else if (rcode != isc::cc::CC_REPLY_SUCCESS) {
+        isc_throw_1(RPCError, result, rcode);
+    } else {
+        return (result);
+    }
+}
+
 }
 }

src/lib/config/ccsession.h

@@ -20,6 +20,7 @@
 
 #include <cc/session.h>
 #include <cc/data.h>
+#include <cc/proto_defs.h>
 
 #include <string>
 #include <list>
@@ -146,6 +147,34 @@ public:
         isc::Exception(file, line, what) {}
 };
 
+/// \brief Exception thrown when there's a problem with the remote call.
+///
+/// This usually means either the command couldn't be called or the remote
+/// side sent an error as a response.
+class RPCError: public CCSessionError {
+public:
+    RPCError(const char* file, size_t line, const char* what, int rcode) :
+        CCSessionError(file, line, what),
+        rcode_(rcode)
+    {}
+
+    /// \brief The error code for the error.
+    int rcode() const {
+        return (rcode_);
+    }
+private:
+    const int rcode_;
+};
+
+/// \brief Specific version of RPCError for the case the recipient of command
+///     doesn't exist.
+class RPCRecipientMissing: public RPCError {
+public:
+    RPCRecipientMissing(const char* file, size_t line, const char* what) :
+        RPCError(file, line, what, isc::cc::CC_REPLY_NO_RECPT)
+    {}
+};
+
 ///
 /// \brief This module keeps a connection to the command channel,
 /// holds configuration information, and handles messages from
@@ -335,13 +364,15 @@ public:
      * \param group see isc::cc::Session::group_sendmsg()
      * \param instance see isc::cc::Session::group_sendmsg()
      * \param to see isc::cc::Session::group_sendmsg()
+     * \param want_answer see isc::cc::Session::group_sendmsg()
      * \return see isc::cc::Session::group_sendmsg()
      */
     int groupSendMsg(isc::data::ConstElementPtr msg,
                      std::string group,
-                     std::string instance = "*",
-                     std::string to = "*") {
-        return (session_.group_sendmsg(msg, group, instance, to));
+                     std::string instance = isc::cc::CC_INSTANCE_WILDCARD,
+                     std::string to = isc::cc::CC_TO_WILDCARD,
+                     bool want_answer = false) {
+        return (session_.group_sendmsg(msg, group, instance, to, want_answer));
     };
 
     /**
@@ -361,6 +392,50 @@ public:
         return (session_.group_recvmsg(envelope, msg, nonblock, seq));
     };
 
+    /// \brief Send a command message and wait for the answer.
+    ///
+    /// This is mostly a convenience wrapper around groupSendMsg
+    /// and groupRecvMsg, with some error handling.
+    ///
+    /// \param command Name of the command to call.
+    /// \param group Name of the remote module to call the command on.
+    /// \param instance Instance part of recipient address.
+    /// \param to The lname to send it to. Can be used to override the
+    ///     addressing and use a direct recipient.
+    /// \param params Parameters for the command. Can be left NULL if
+    ///     no parameters are needed.
+    /// \return Return value of the successfull remote call. It can be
+    ///     NULL if the remote command is void function (returns nothing).
+    /// \throw RPCError if the call fails (for example when the other
+    ///     side responds with error code).
+    /// \throw RPCRecipientMissing if the recipient doesn't exist.
+    /// \throw CCSessionError if some lower-level error happens (eg.
+    ///     the response was malformed).
+    isc::data::ConstElementPtr rpcCall(const std::string& command,
+                                       const std::string& group,
+                                       const std::string& instance =
+                                           isc::cc::CC_INSTANCE_WILDCARD,
+                                       const std::string& to =
+                                           isc::cc::CC_TO_WILDCARD,
+                                       const isc::data::ConstElementPtr&
+                                           params =
+                                           isc::data::ConstElementPtr());
+
+    /// \brief Convenience version of rpcCall
+    ///
+    /// This is exactly the same as the previous version of rpcCall, except
+    /// that the instance and to parameters are at their default. This
+    /// allows to sending a command with parameters to a named module
+    /// without long typing of the parameters.
+    isc::data::ConstElementPtr rpcCall(const std::string& command,
+                                       const std::string& group,
+                                       const isc::data::ConstElementPtr&
+                                           params)
+    {
+        return rpcCall(command, group, isc::cc::CC_INSTANCE_WILDCARD,
+                       isc::cc::CC_TO_WILDCARD, params);
+    }
+
     /// \brief Forward declaration of internal data structure.
     ///
     /// This holds information about one asynchronous request to receive

src/lib/config/config_messages.mes

@@ -94,3 +94,7 @@ manager.
 % CONFIG_OPEN_FAIL error opening %1: %2
 There was an error opening the given file. The reason for the failure
 is included in the message.
+
+% CONFIG_RPC_SEQ RPC call %1 to %2 with seq %3
+Debug message, saying there's a RPC call of given command to given module. It
+has internal sequence number as listed in the message.

src/lib/config/documentation.txt

@@ -18,7 +18,7 @@ In order for this to work, it is important that all modules have a way of dynami
 Overview
 --------
 
-Central to the configuration part is the Configuraion Manager b10-cfgmgr. The configuration manager loads any existing configuration, receives configuration updates from user interfaces, and notifies modules about configuration updates.
+Central to the configuration part is the Configuration Manager b10-cfgmgr. The configuration manager loads any existing configuration, receives configuration updates from user interfaces, and notifies modules about configuration updates.
 
 UI  <---UIAPI---> Configuration Manager <---ModuleAPI---> Module
                                         <---ModuleAPI---> Module

src/lib/config/tests/ccsession_unittests.cc

@@ -53,11 +53,33 @@ protected:
         // upon creation of a ModuleCCSession, the class
 
         // sends its specification to the config manager.
-        // it expects an ok answer back, so everytime we
+        // it expects an ok answer back, so every time we
         // create a ModuleCCSession, we must set an initial
         // ok answer.
         session.getMessages()->add(createAnswer());
     }
+    ConstElementPtr rpcCheck(const std::string& reply) {
+        ModuleCCSession mccs(ccspecfile("spec1.spec"), session, NULL, NULL,
+                             false, false);
+        // Prepare the answer beforehand, it'll block until it gets one
+        const ConstElementPtr reply_el(el(reply));
+        session.getMessages()->add(reply_el);
+        const ConstElementPtr
+            result(mccs.rpcCall("test", "Spec2",
+                                el("{\"param1\": \"Param 1\","
+                                   "\"param2\": \"Param 2\"}")));
+        const ConstElementPtr
+            request(el("[\"Spec2\", \"*\", {"
+                       "  \"command\": [\"test\", {"
+                       "    \"param1\": \"Param 1\","
+                       "    \"param2\": \"Param 2\""
+                       "}]}, -1, true]"));
+        // The 0th one is from the initialization, to ConfigManager.
+        // our is the 1st.
+        EXPECT_TRUE(request->equals(*session.getMsgQueue()->get(1))) <<
+            session.getMsgQueue()->get(1)->toWire();
+        return (result);
+    }
     ~CCSessionTest() {
         isc::log::setRootLoggerName(root_name);
     }
@@ -65,6 +87,36 @@ protected:
     const std::string root_name;
 };
 
+// Test we can send an RPC (command) and get an answer. The answer is success
+// in this case.
+TEST_F(CCSessionTest, rpcCallSuccess) {
+    const ConstElementPtr result =
+        rpcCheck("{\"result\": [0, {\"Hello\": \"a\"}]}");
+    EXPECT_TRUE(el("{\"Hello\": \"a\"}")->equals(*result));
+}
+
+// Test success of RPC, but the answer is empty (eg. a void function on the
+// remote side).
+TEST_F(CCSessionTest, rpcCallSuccessNone) {
+    EXPECT_FALSE(rpcCheck("{\"result\": [0]}"));
+}
+
+// Test it successfully raises CCSessionError if the answer is malformed.
+TEST_F(CCSessionTest, rpcCallMalformedAnswer) {
+    EXPECT_THROW(rpcCheck("[\"Nonsense\"]"), CCSessionError);
+}
+
+// Test it raises exception when the remote side reports an error
+TEST_F(CCSessionTest, rpcCallError) {
+    EXPECT_THROW(rpcCheck("{\"result\": [1, \"Error\"]}"), RPCError);
+}
+
+// Test it raises exception when the remote side doesn't exist
+TEST_F(CCSessionTest, rpcNoRecpt) {
+    EXPECT_THROW(rpcCheck("{\"result\": [-1, \"Error\"]}"),
+                 RPCRecipientMissing);
+}
+
 TEST_F(CCSessionTest, createAnswer) {
     ConstElementPtr answer;
     answer = createAnswer();