[2472] Merge branch 'master' into trac2472

ChangeLog

@@ -1,3 +1,17 @@
+505.	[bug]		jelte
+	Fixed a bug in b10-xfrin where a wrong call was made during the
+	final check of a TSIG-signed transfer, incorrectly rejecting the
+	transfer.
+	(Trac #2464, git eac81c0cbebee72f6478bdb5cda915f5470d08e1)
+
+504.	[bug]*		naokikambe
+	Fixed an XML format viewed from b10-stats-httpd. Regarding
+	per-zone counters as zones of Xfrout, a part of the item values wasn't
+	an exact XML format. A zone name can be specified in URI as
+	/bind10/statistics/xml/Xfrout/zones/example.org/xfrreqdone. XSD and XSL
+	formats are also changed to constant ones due to these changes.
+	(Trac #2298, git 512d2d46f3cb431bcdbf8d90af27bff8874ba075)
+
 503.    [func]      Stephen
 	Add initial version of a MySQL backend for the DHCP code.  This
 	implements the basic IPv6 lease access functions - add lease, delete

src/bin/dhcp6/config_parser.cc

@@ -41,13 +41,13 @@ using namespace isc::asiolink;
 namespace isc {
 namespace dhcp {
 
-/// @brief auxiliary type used for storing element name and its parser
+/// @brief an auxiliary type used for storing an element name and its parser
 typedef pair<string, ConstElementPtr> ConfigPair;
 
 /// @brief a factory method that will create a parser for a given element name
 typedef DhcpConfigParser* ParserFactory(const std::string& config_id);
 
-/// @brief a collection of factories that creates parsers for specified element names
+/// @brief a collection of factories that create parsers for specified element names
 typedef std::map<std::string, ParserFactory*> FactoryMap;
 
 /// @brief a collection of elements that store uint32 values (e.g. renew-timer = 900)
@@ -58,12 +58,14 @@ typedef std::map<string, string> StringStorage;
 
 /// @brief a collection of pools
 ///
-/// That type is used as intermediate storage, when pools are parsed, but there is
+/// This type is used as intermediate storage, when pools are parsed, but there is
 /// no subnet object created yet to store them.
 typedef std::vector<Pool6Ptr> PoolStorage;
 
-/// @brief Collection of options.
-typedef std::vector<OptionPtr> OptionStorage;
+/// @brief Collection of option descriptors. This container allows searching for
+/// options using the option code or persistency flag. This is useful when merging
+/// existing options with newly configured options.
+typedef Subnet::OptionContainer OptionStorage;
 
 /// @brief Global uint32 parameters that will be used as defaults.
 Uint32Storage uint32_defaults;
@@ -76,9 +78,9 @@ OptionStorage option_defaults;
 
 /// @brief a dummy configuration parser
 ///
-/// It is a debugging parser. It does not configure anything,
+/// This is a debugging parser. It does not configure anything,
 /// will accept any configuration and will just print it out
-/// on commit. Useful for debugging existing configurations and
+/// on commit.  Useful for debugging existing configurations and
 /// adding new ones.
 class DebugParser : public DhcpConfigParser {
 public:
@@ -105,7 +107,7 @@ public:
 
     /// @brief pretends to apply the configuration
     ///
-    /// This is a method required by base class. It pretends to apply the
+    /// This is a method required by the base class. It pretends to apply the
     /// configuration, but in fact it only prints the parameter out.
     ///
     /// See \ref DhcpConfigParser class for details.
@@ -169,7 +171,7 @@ public:
             // Parsing the value as a int64 value allows to
             // check if the provided value is within the range
             // of uint32_t (is not negative or greater than
-            // maximal uint32_t value.
+            // maximal uint32_t value).
             int64value = boost::lexical_cast<int64_t>(value->str());
         } catch (const boost::bad_lexical_cast&) {
             parse_error = true;
@@ -193,19 +195,20 @@ public:
                       << " as unsigned 32-bit integer.");
         }
 
-        storage_->insert(pair<string, uint32_t>(param_name_, value_));
+        // If a given parameter already exists in the storage we override
+        // its value. If it doesn't we insert a new element.
+        (*storage_)[param_name_] = value_;
     }
 
     /// @brief does nothing
     ///
-    /// This method is required for all parser. The value itself
+    /// This method is required for all parsers. The value itself
     /// is not commited anywhere. Higher level parsers are expected to
     /// use values stored in the storage, e.g. renew-timer for a given
     /// subnet is stored in subnet-specific storage. It is not commited
     /// here, but is rather used by \ref Subnet6Parser when constructing
     /// the subnet.
-    virtual void commit() {
-    }
+    virtual void commit() { }
 
     /// @brief factory that constructs Uint32Parser objects
     ///
@@ -237,9 +240,9 @@ protected:
 /// @brief Configuration parser for string parameters
 ///
 /// This class is a generic parser that is able to handle any string
-/// parameter. By default it stores the value in external global container
+/// parameter. By default it stores the value in an external global container
 /// (string_defaults). If used in smaller scopes (e.g. to parse parameters
-/// in subnet config), it can be pointed to a different storage, using
+/// in subnet config), it can be pointed to a different storage, using the
 /// setStorage() method. This class follows the parser interface, laid out
 /// in its base class, \ref DhcpConfigParser.
 ///
@@ -256,26 +259,27 @@ public:
 
     /// @brief parses parameter value
     ///
-    /// Parses configuration entry and stored it in storage. See
+    /// Parses configuration entry and stores it in storage. See
     /// \ref setStorage() for details.
     ///
     /// @param value pointer to the content of parsed values
     virtual void build(ConstElementPtr value) {
         value_ = value->str();
         boost::erase_all(value_, "\"");
-        storage_->insert(pair<string, string>(param_name_, value_));
+        // If a given parameter already exists in the storage we override
+        // its value. If it doesn't we insert a new element.
+        (*storage_)[param_name_] = value_;
     }
 
     /// @brief does nothing
     ///
-    /// This method is required for all parser. The value itself
+    /// This method is required for all parsers. The value itself
     /// is not commited anywhere. Higher level parsers are expected to
     /// use values stored in the storage, e.g. renew-timer for a given
     /// subnet is stored in subnet-specific storage. It is not commited
     /// here, but is rather used by its parent parser when constructing
     /// an object, e.g. the subnet.
-    virtual void commit() {
-    }
+    virtual void commit() { }
 
     /// @brief factory that constructs StringParser objects
     ///
@@ -366,7 +370,7 @@ protected:
 /// and stored in chosen PoolStorage container.
 ///
 /// As there are no default values for pool, setStorage() must be called
-/// before build(). Otherwise exception will be thrown.
+/// before build(). Otherwise an exception will be thrown.
 ///
 /// It is useful for parsing Dhcp6/subnet6[X]/pool parameters.
 class PoolParser : public DhcpConfigParser {
@@ -393,7 +397,7 @@ public:
         BOOST_FOREACH(ConstElementPtr text_pool, pools_list->listValue()) {
 
             // That should be a single pool representation. It should contain
-            // text is form prefix/len or first - last. Note that spaces
+            // text in the form prefix/len or first - last. Note that spaces
             // are allowed
             string txt = text_pool->stringValue();
 
@@ -412,7 +416,7 @@ public:
                     // start with the first character after /
                     string prefix_len = txt.substr(pos + 1);
 
-                    // It is lexical cast to int and then downcast to uint8_t.
+                    // It is lexically cast to int and then downcast to uint8_t.
                     // Direct cast to uint8_t (which is really an unsigned char)
                     // will result in interpreting the first digit as output
                     // value and throwing exception if length is written on two
@@ -461,7 +465,7 @@ public:
 
     /// @brief does nothing.
     ///
-    /// This method is required for all parser. The value itself
+    /// This method is required for all parsers. The value itself
     /// is not commited anywhere. Higher level parsers (for subnet) are expected
     /// to use values stored in the storage.
     virtual void commit() {}
@@ -476,7 +480,7 @@ public:
 protected:
     /// @brief pointer to the actual Pools storage
     ///
-    /// That is typically a storage somewhere in Subnet parser
+    /// This is typically a storage somewhere in Subnet parser
     /// (an upper level parser).
     PoolStorage* pools_;
 };
@@ -485,11 +489,11 @@ protected:
 ///
 /// This parser parses configuration entries that specify value of
 /// a single option. These entries include option name, option code
-/// and data carried by the option. If parsing is successful than
+/// and data carried by the option. If parsing is successful than an
 /// instance of an option is created and added to the storage provided
 /// by the calling class.
 ///
-/// @todo This class parses and validates option name. However it is
+/// @todo This class parses and validates the option name. However it is
 /// not used anywhere util support for option spaces is implemented
 /// (see tickets #2319, #2314). When option spaces are implemented
 /// there will be a way to reference the particular option using
@@ -501,7 +505,9 @@ public:
     ///
     /// Class constructor.
     OptionDataParser(const std::string&)
-        : options_(NULL) { }
+        : options_(NULL),
+          // initialize option to NULL ptr
+          option_descriptor_(false) { }
 
     /// @brief Parses the single option data.
     ///
@@ -559,11 +565,37 @@ public:
         createOption();
     }
 
-    /// @brief Does nothing.
+    /// @brief Commits option value.
     ///
-    /// This function does noting because option data is committed
-    /// by a higher level parser.
-    virtual void commit() { }
+    /// This function adds a new option to the storage or replaces an existing option
+    /// with the same code.
+    ///
+    /// @throw isc::InvalidOperation if failed to set pointer to storage or failed
+    /// to call build() prior to commit. If that happens data in the storage
+    /// remain un-modified.
+    virtual void commit() {
+        if (options_ == NULL) {
+            isc_throw(isc::InvalidOperation, "Parser logic error: storage must be set before "
+                      "commiting option data.");
+        } else  if (!option_descriptor_.option) {
+            // Before we can commit the new option should be configured. If it is not
+            // than somebody must have called commit() before build().
+            isc_throw(isc::InvalidOperation, "Parser logic error: no option has been configured and"
+                      " thus there is nothing to commit. Has build() been called?");
+        }
+        uint16_t opt_type = option_descriptor_.option->getType();
+        Subnet::OptionContainerTypeIndex& idx = options_->get<1>();
+        // Try to find options with the particular option code in the main
+        // storage. If found, remove these options because they will be
+        // replaced with new one.
+        Subnet::OptionContainerTypeRange range =
+            idx.equal_range(opt_type);
+        if (std::distance(range.first, range.second) > 0) {
+            idx.erase(range.first, range.second);
+        }
+        // Append new option to the main storage.
+        options_->push_back(option_descriptor_);
+    }
 
     /// @brief Set storage for the parser.
     ///
@@ -584,11 +616,11 @@ private:
     ///
     /// Creates an instance of an option and adds it to the provided
     /// options storage. If the option data parsed by \ref build function
-    /// are invalid or insufficient it emits exception.
+    /// are invalid or insufficient this function emits an exception.
     ///
     /// @warning this function does not check if options_ storage pointer
-    /// is intitialized but this is not needed here because it is checked in
-    /// \ref build function.
+    /// is intitialized but this check is not needed here because it is done
+    /// in the \ref build function.
     ///
     /// @throw Dhcp6ConfigError if parameters provided in the configuration
     /// are invalid.
@@ -605,7 +637,7 @@ private:
             isc_throw(Dhcp6ConfigError, "Parser error: value of 'code' must not"
                       << " exceed " << std::numeric_limits<uint16_t>::max());
         }
-        // Check the option name has been specified, is non-empty and does not
+        // Check that the option name has been specified, is non-empty and does not
         // contain spaces.
         // @todo possibly some more restrictions apply here?
         std::string option_name = getStringParam("name");
@@ -649,23 +681,28 @@ private:
         } else if (num_defs == 0) {
             // @todo We have a limited set of option definitions intiialized at the moment.
             // In the future we want to initialize option definitions for all options.
-            // Consequently error will be issued if option definition does not exist
+            // Consequently an error will be issued if an option definition does not exist
             // for a particular option code. For now it is ok to create generic option
             // if definition does not exist.
             OptionPtr option(new Option(Option::V6, static_cast<uint16_t>(option_code),
                                         binary));
-            // If option is created succesfully, add it to the storage.
-            options_->push_back(option);
+            // The created option is stored in option_descriptor_ class member until the
+            // commit stage when it is inserted into the main storage. If an option with the
+            // same code exists in main storage already the old option is replaced.
+            option_descriptor_.option = option;
+            option_descriptor_.persistent = false;
         } else {
-            // We have exactly one option definition for the particular option code.
-            // use it to create option instance.
+            // We have exactly one option definition for the particular option code
+            // use it to create the option instance.
             const OptionDefinitionPtr& def = *(range.first);
             // getFactory should never return NULL pointer.
             Option::Factory* factory = def->getFactory();
             assert(factory != NULL);
             try {
                 OptionPtr option = factory(Option::V6, option_code, binary);
-                options_->push_back(option);
+                Subnet::OptionDescriptor desc(option, false);
+                option_descriptor_.option = option;
+                option_descriptor_.persistent = false;
             } catch (const isc::Exception& ex) {
                 isc_throw(Dhcp6ConfigError, "Parser error: option data does not match"
                           << " option definition (code " << option_code << "): "
@@ -707,9 +744,11 @@ private:
     /// Pointer to options storage. This storage is provided by
     /// the calling class and is shared by all OptionDataParser objects.
     OptionStorage* options_;
+    /// Option descriptor holds newly configured option.
+    Subnet::OptionDescriptor option_descriptor_;
 };
 
-/// @brief Parser for option data values with ina subnet.
+/// @brief Parser for option data values within a subnet.
 ///
 /// This parser iterates over all entries that define options
 /// data for a particular subnet and creates a collection of options.
@@ -721,10 +760,10 @@ public:
     /// @brief Constructor.
     ///
     /// Unless otherwise specified, parsed options will be stored in
-    /// a global option containers (option_default). That storage location
+    /// a global option container (option_default). That storage location
     /// is overriden on a subnet basis.
     OptionDataListParser(const std::string&)
-        : options_(&option_defaults) { }
+        : options_(&option_defaults), local_options_() { }
 
     /// @brief Parses entries that define options' data for a subnet.
     ///
@@ -738,9 +777,11 @@ public:
             boost::shared_ptr<OptionDataParser> parser(new OptionDataParser("option-data"));
             // options_ member will hold instances of all options thus
             // each OptionDataParser takes it as a storage.
-            parser->setStorage(options_);
-            // Build the instance of a singkle option.
+            parser->setStorage(&local_options_);
+            // Build the instance of a single option.
             parser->build(option_value);
+            // Store a parser as it will be used to commit.
+            parsers_.push_back(parser);
         }
     }
 
@@ -752,11 +793,18 @@ public:
     }
 
 
-    /// @brief Does nothing.
+    /// @brief Commit all option values.
     ///
-    /// @todo Currently this function does nothing but in the future
-    /// we may need to extend it to commit at this level.
-    void commit() { }
+    /// This function invokes commit for all option values.
+    void commit() {
+        BOOST_FOREACH(ParserPtr parser, parsers_) {
+            parser->commit();
+        }
+        // Parsing was successful and we have all configured
+        // options in local storage. We can now replace old values
+        // with new values.
+        std::swap(local_options_, *options_);
+    }
 
     /// @brief Create DhcpDataListParser object
     ///
@@ -767,8 +815,15 @@ public:
         return (new OptionDataListParser(param_name));
     }
 
+    /// Intermediate option storage. This storage is used by
+    /// lower level parsers to add new options.  Values held
+    /// in this storage are assigned to main storage (options_)
+    /// if overall parsing was successful.
+    OptionStorage local_options_;
     /// Pointer to options instances storage.
     OptionStorage* options_;
+    /// Collection of parsers;
+    ParserCollection parsers_;
 };
 
 /// @brief this class parses a single subnet
@@ -780,8 +835,8 @@ public:
 
     /// @brief constructor
     Subnet6ConfigParser(const std::string& ) {
-        // The parameter should always be "subnet", but we don't check here
-        // against it in case some wants to reuse this parser somewhere.
+        // The parameter should always be "subnet", but we don't check
+        // against that here in case some wants to reuse this parser somewhere.
     }
 
     /// @brief parses parameter value
@@ -792,8 +847,8 @@ public:
         BOOST_FOREACH(ConfigPair param, subnet->mapValue()) {
             ParserPtr parser(createSubnet6ConfigParser(param.first));
             // The actual type of the parser is unknown here. We have to discover
-            // parser type here to invoke corresponding setStorage function on it.
-            // We discover parser type by trying to cast the parser to various
+            // the parser type here to invoke the corresponding setStorage function
+            // on it.  We discover parser type by trying to cast the parser to various
             // parser types and checking which one was successful. For this one
             // a setStorage and build methods are invoked.
 
@@ -830,6 +885,10 @@ public:
     /// created in other parsers are used here and added to newly created Subnet6
     /// objects. Subnet6 are then added to DHCP CfgMgr.
     void commit() {
+        // Invoke commit on all sub-data parsers.
+        BOOST_FOREACH(ParserPtr parser, parsers_) {
+            parser->commit();
+        }
 
         StringStorage::const_iterator it = string_values_.find("subnet");
         if (it == string_values_.end()) {
@@ -871,33 +930,33 @@ public:
         const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
 
         // Add subnet specific options.
-        BOOST_FOREACH(OptionPtr option, options_) {
-            Subnet::OptionContainerTypeRange range = idx.equal_range(option->getType());
+        BOOST_FOREACH(Subnet::OptionDescriptor desc, options_) {
+            Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType());
             if (std::distance(range.first, range.second) > 0) {
                 LOG_WARN(dhcp6_logger, DHCP6_CONFIG_OPTION_DUPLICATE)
-                    .arg(option->getType()).arg(addr.toText());
+                    .arg(desc.option->getType()).arg(addr.toText());
             }
-            subnet->addOption(option);
+            subnet->addOption(desc.option);
         }
 
         // Check all global options and add them to the subnet object if
         // they have been configured in the global scope. If they have been
         // configured in the subnet scope we don't add global option because
-        // the one configured in the subnet scope always takes precedense.
-        BOOST_FOREACH(OptionPtr option, option_defaults) {
+        // the one configured in the subnet scope always takes precedence.
+        BOOST_FOREACH(Subnet::OptionDescriptor desc, option_defaults) {
             // Get all options specified locally in the subnet and having
             // code equal to global option's code.
-            Subnet::OptionContainerTypeRange range = idx.equal_range(option->getType());
+            Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType());
             // @todo: In the future we will be searching for options using either
-            // option code or namespace. Currently we have only the option
+            // an option code or namespace. Currently we have only the option
             // code available so if there is at least one option found with the
-            // specific code we don't add globally configured option.
+            // specific code we don't add the globally configured option.
             // @todo with this code the first globally configured option
             // with the given code will be added to a subnet. We may
-            // want to issue warning about dropping configuration of
-            // global option if one already exsist.
+            // want to issue a warning about dropping the configuration of
+            // a global option if one already exsists.
             if (std::distance(range.first, range.second) == 0) {
-                subnet->addOption(option);
+                subnet->addOption(desc.option);
             }
         }
 
@@ -908,9 +967,9 @@ private:
 
     /// @brief Set storage for a parser and invoke build.
     ///
-    /// This helper method casts the provided parser pointer to specified
-    /// type. If cast is successful it sets the corresponding storage for
-    /// this parser, invokes build on it and save the parser.
+    /// This helper method casts the provided parser pointer to the specified
+    /// type. If the cast is successful it sets the corresponding storage for
+    /// this parser, invokes build on it and saves the parser.
     ///
     /// @tparam T parser type to which parser argument should be cast.
     /// @tparam Y storage type for the specified parser type.
@@ -982,7 +1041,7 @@ private:
 
     /// @brief returns value for a given parameter (after using inheritance)
     ///
-    /// This method implements inheritance. For a given parameter name, it first
+    /// This method implements inheritance.  For a given parameter name, it first
     /// checks if there is a global value for it and overwrites it with specific
     /// value if such value was defined in subnet.
     ///
@@ -1028,7 +1087,7 @@ private:
     ParserCollection parsers_;
 };
 
-/// @brief this class parses list of subnets
+/// @brief this class parses a list of subnets
 ///
 /// This is a wrapper parser that handles the whole list of Subnet6
 /// definitions. It iterates over all entries and creates Subnet6ConfigParser
@@ -1044,7 +1103,7 @@ public:
 
     /// @brief parses contents of the list
     ///
-    /// Iterates over all entries on the list and creates Subnet6ConfigParser
+    /// Iterates over all entries on the list and creates a Subnet6ConfigParser
     /// for each entry.
     ///
     /// @param subnets_list pointer to a list of IPv6 subnets
@@ -1153,12 +1212,6 @@ configureDhcp6Server(Dhcpv6Srv& , ConstElementPtr config_set) {
                   "Null pointer is passed to configuration parser");
     }
 
-    /// Reset global storage. Containers being reset below may contain
-    /// data from the previous configuration attempts.
-    option_defaults.clear();
-    uint32_defaults.clear();
-    string_defaults.clear();
-
     /// @todo: append most essential info here (like "2 new subnets configured")
     string config_details;
 

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -53,6 +53,8 @@ public:
     }
 
     ~Dhcp6ParserTest() {
+        // Reset configuration database after each test.
+        resetConfiguration();
     };
 
     /// @brief Create the simple configuration with single option.
@@ -116,6 +118,51 @@ public:
         return (stream.str());
     }
 
+    /// @brief Reset configuration database.
+    ///
+    /// This function resets configuration data base by
+    /// removing all subnets and option-data. Reset must
+    /// be performed after each test to make sure that
+    /// contents of the database do not affect result of
+    /// subsequent tests.
+    void resetConfiguration() {
+        ConstElementPtr status;
+
+        string config = "{ \"interface\": [ \"all\" ],"
+            "\"preferred-lifetime\": 3000,"
+            "\"rebind-timer\": 2000, "
+            "\"renew-timer\": 1000, "
+            "\"valid-lifetime\": 4000, "
+            "\"subnet6\": [ ], "
+            "\"option-data\": [ ] }";
+
+        try {
+            ElementPtr json = Element::fromJSON(config);
+            status = configureDhcp6Server(srv_, json);
+        } catch (const std::exception& ex) {
+            FAIL() << "Fatal error: unable to reset configuration database"
+                   << " after the test. The following configuration was used"
+                   << " to reset database: " << std::endl
+                   << config << std::endl
+                   << " and the following error message was returned:"
+                   << ex.what() << std::endl;
+        }
+
+
+        // returned value should be 0 (configuration success)
+        if (!status) {
+            FAIL() << "Fatal error: unable to reset configuration database"
+                   << " after the test. Configuration function returned"
+                   << " NULL pointer" << std::endl;
+        }
+        comment_ = parseAnswer(rcode_, status);
+        if (rcode_ != 0) {
+            FAIL() << "Fatal error: unable to reset configuration database"
+                   << " after the test. Configuration function returned"
+                   << " error code " << rcode_ << std::endl;
+        }
+    }
+
     /// @brief Test invalid option parameter value.
     ///
     /// This test function constructs the simple configuration
@@ -691,7 +738,7 @@ TEST_F(Dhcp6ParserTest, stdOptionData) {
     // Option code 3 means OPTION_IA_NA.
     params["code"] = "3";
     params["data"] = "ABCDEF01 02030405 06070809";
-    
+
     std::string config = createConfigWithOption(params);
     ElementPtr json = Element::fromJSON(config);
 

src/bin/stats/stats-httpd-xsd.tpl

@@ -1,2 +1,97 @@
 <?xml version="1.0" encoding="UTF-8"?>
-$xsd_string
+<schema targetNamespace="$xsd_namespace"
+        xmlns="http://www.w3.org/2001/XMLSchema"
+        xmlns:bind10="$xsd_namespace">
+  <annotation>
+    <documentation>XML schema of statistics data in BIND 10</documentation>
+  </annotation>
+  <element name="statistics">
+    <annotation><documentation>A set of statistics data</documentation></annotation>
+    <complexType>
+      <sequence>
+        <element name="item" maxOccurs="unbounded" minOccurs="1">
+          <complexType>
+	    <attribute name="identifier" type="string" use="required">
+            <annotation>
+              <appinfo>Identifier</appinfo>
+              <documentation>Identifier of item</documentation>
+            </annotation>
+  	    </attribute>
+  	    <attribute name="value" type="string" use="optional">
+            <annotation>
+              <appinfo>Value</appinfo>
+              <documentation>Value of item</documentation>
+            </annotation>
+  	    </attribute>
+  	    <attribute name="owner" type="string" use="required">
+            <annotation>
+              <appinfo>Owner</appinfo>
+              <documentation>Owner module name</documentation>
+            </annotation>
+  	    </attribute>
+  	    <attribute name="uri" type="anyURI" use="required">
+            <annotation>
+              <appinfo>URI</appinfo>
+              <documentation>URI of item</documentation>
+            </annotation>
+  	    </attribute>
+  	    <attribute name="name" type="string" use="required">
+            <annotation>
+              <appinfo>Name</appinfo>
+              <documentation>Name of item</documentation>
+            </annotation>
+  	    </attribute>
+  	    <attribute name="type" use="required">
+  	    	<annotation>
+  	    	  <appinfo>Type</appinfo>
+  	    	  <documentation>Type of item</documentation>
+  	    	</annotation>
+  	    	<simpleType>
+  	    	  <restriction base="token">
+  	    	    <enumeration value="boolean"/>
+  	    	    <enumeration value="integer"/>
+  	    	    <enumeration value="real"/>
+  	    	    <enumeration value="string"/>
+  	    	    <enumeration value="map"/>
+  	    	    <enumeration value="list"/>
+  	    	    <enumeration value="named_set"/>
+  	    	    <enumeration value="any"/>
+  	    	  </restriction>
+  	    	</simpleType>
+  	    </attribute>
+  	    <attribute name="description" type="string" use="optional">
+            <annotation>
+              <appinfo>Description</appinfo>
+              <documentation>Description of item</documentation>
+            </annotation>
+  	    </attribute>
+  	    <attribute name="title" type="string" use="optional">
+            <annotation>
+              <appinfo>Title</appinfo>
+              <documentation>Title of item</documentation>
+            </annotation>
+  	    </attribute>
+  	    <attribute name="optional" type="boolean" use="optional">
+            <annotation>
+              <appinfo>Optional</appinfo>
+              <documentation>The item is optional or not</documentation>
+            </annotation>
+  	    </attribute>
+  	    <attribute name="default" type="string" use="optional">
+            <annotation>
+              <appinfo>Default</appinfo>
+              <documentation>Default of item</documentation>
+            </annotation>
+  	    </attribute>
+  	    <attribute name="format" type="string" use="optional">
+            <annotation>
+              <appinfo>Format</appinfo>
+              <documentation>Format of item value</documentation>
+            </annotation>
+  	    </attribute>
+          </complexType>
+        </element>
+      </sequence>
+    </complexType>
+  </element>
+</schema>

src/bin/stats/stats-httpd-xsl.tpl

@@ -30,5 +30,27 @@ td.title {
       </body>
     </html>
   </xsl:template>
-  $xsl_string
+  <xsl:template match="bind10:statistics">
+    <table>
+      <tr>
+	<th>Identifier</th><th>Value</th><th>Description</th>
+      </tr>
+      <xsl:for-each select="item">
+	<tr>
+	  <td>
+	    <xsl:element name="a">
+	      <xsl:attribute name="href"><xsl:value-of select="@uri" /></xsl:attribute>
+	      <xsl:value-of select="@identifier" />
+	    </xsl:element>
+	  </td>
+	  <td>
+	    <xsl:if test="@value"><xsl:value-of select="@value" /></xsl:if>
+	  </td>
+	  <td>
+	    <xsl:if test="@description"><xsl:value-of select="@description" /></xsl:if>
+	  </td>
+	</tr>
+      </xsl:for-each>
+    </table>
+  </xsl:template>
 </xsl:stylesheet>

src/bin/stats/stats_httpd.py.in

@@ -1,6 +1,6 @@
 #!@PYTHON@
 
-# Copyright (C) 2011  Internet Systems Consortium.
+# Copyright (C) 2011-2012  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
@@ -30,6 +30,7 @@ import socket
 import string
 import xml.etree.ElementTree
 import urllib.parse
+import re
 
 import isc.cc
 import isc.config
@@ -64,14 +65,70 @@ XSL_TEMPLATE_LOCATION = BASE_LOCATION + os.sep + "stats-httpd-xsl.tpl"
 # These variables are paths part of URL.
 # eg. "http://${address}" + XXX_URL_PATH
 XML_URL_PATH = '/bind10/statistics/xml'
-XSD_URL_PATH = '/bind10/statistics/xsd'
-XSL_URL_PATH = '/bind10/statistics/xsl'
+XSD_URL_PATH = '/bind10/statistics.xsd'
+XSL_URL_PATH = '/bind10/statistics.xsl'
 # TODO: This should be considered later.
 XSD_NAMESPACE = 'http://bind10.isc.org/bind10'
+XMLNS_XSI = "http://www.w3.org/2001/XMLSchema-instance"
+
+# constant parameter of XML
+XML_ROOT_ELEMENT = 'bind10:statistics'
+XML_ROOT_ATTRIB = { 'xsi:schemaLocation' : '%s %s' % (XSD_NAMESPACE, XSD_URL_PATH),
+                    'xmlns:bind10' : XSD_NAMESPACE,
+                    'xmlns:xsi' : XMLNS_XSI }
 
 # Assign this process name
 isc.util.process.rename()
 
+def item_name_list(element, identifier):
+    """Return a list of strings. The strings are string expression of
+    the first argument element which is dict type. The second argument
+    identifier is a string for specifying the strings which are
+    returned from this method as a list. For example, if we specify as
+
+      item_name_list({'a': {'aa': [0, 1]}, 'b': [0, 1]}, 'a/aa'),
+
+    then it returns
+
+      ['a/aa', 'a/aa[0]', 'a/aa[1]'].
+
+    If an empty string is specified in the second argument, all
+    possible strings are returned as a list.  In that example if we
+    specify an empty string in the second argument, then it returns
+
+      ['a', 'a/aa', 'a/aa[0]', 'a/aa[1]', 'b', 'b[0]', 'b[1]'].
+
+    The key name of element which is in the first argument is sorted.
+    Even if we specify as
+
+      item_name_list({'xx': 0, 'a': 1, 'x': 2}, ''),
+
+    then it returns
+
+      ['a', 'x', 'xx'].
+
+    This method internally invokes isc.cc.data.find(). The arguments
+    of this method are passed to isc.cc.data.find(). So an exception
+    DataNotFoundError or DataTypeError might be raised via
+    isc.cc.data.find() depending on the arguments. See details of
+    isc.cc.data.find() for more information about exceptions"""
+    elem = isc.cc.data.find(element, identifier)
+    ret = []
+    ident = identifier
+    if ident:
+        ret.append(ident)
+    if type(elem) is dict:
+        if ident:
+            ident = ident + '/'
+        for key in sorted(elem.keys()):
+            idstr = '%s%s' % (ident, key)
+            ret += item_name_list(element, idstr)
+    elif type(elem) is list:
+        for i in range(0, len(elem)):
+            idstr = '%s[%d]' % (ident, i)
+            ret += item_name_list(element, idstr)
+    return ret
+
 class HttpHandler(http.server.BaseHTTPRequestHandler):
     """HTTP handler class for HttpServer class. The class inhrits the super
     class http.server.BaseHTTPRequestHandler. It implemets do_GET()
@@ -89,31 +146,37 @@ class HttpHandler(http.server.BaseHTTPRequestHandler):
             req_path = self.path
             req_path = urllib.parse.urlsplit(req_path).path
             req_path = urllib.parse.unquote(req_path)
-            req_path = os.path.normpath(req_path)
-            path_dirs = req_path.split('/')
-            path_dirs = [ d for d in filter(None, path_dirs) ]
-            req_path = '/'+"/".join(path_dirs)
-            module_name = None
-            item_name = None
-            # in case of /bind10/statistics/xxx/YYY/zzz
-            if len(path_dirs) >= 5:
-                item_name = path_dirs[4]
-            # in case of /bind10/statistics/xxx/YYY ...
-            if len(path_dirs) >= 4:
-                module_name = path_dirs[3]
-            if req_path == '/'.join([XML_URL_PATH] + path_dirs[3:5]):
-                body = self.server.xml_handler(module_name, item_name)
-            elif req_path == '/'.join([XSD_URL_PATH] + path_dirs[3:5]):
-                body = self.server.xsd_handler(module_name, item_name)
-            elif req_path == '/'.join([XSL_URL_PATH] + path_dirs[3:5]):
-                body = self.server.xsl_handler(module_name, item_name)
+            body = None
+            # In case that the requested path (req_path),
+            # e.g. /bind10/statistics/Auth/, is started with
+            # XML_URL_PATH + '/'
+            if req_path.find('%s/' % XML_URL_PATH) == 0:
+                # remove './' from the path if there is
+                req_path = os.path.normpath(req_path)
+                # get the strings tailing after XML_URL_PATH
+                req_path = req_path.lstrip('%s/' % XML_URL_PATH)
+                # remove empty dir names from the path if there are
+                path_dirs = req_path.split('/')
+                path_dirs = [ d for d in filter(None, path_dirs) ]
+                req_path = '/'.join(path_dirs)
+                # pass the complete requested path,
+                # e.g. Auth/queries.upd, to xml_handler()
+                body = self.server.xml_handler(req_path)
+            # In case that the requested path (req_path) is exactly
+            # matched with XSD_URL_PATH
+            elif req_path == XSD_URL_PATH:
+                body = self.server.xsd_handler()
+            # In case that the requested path (req_path) is exactly
+            # matched with XSL_URL_PATH
+            elif req_path == XSL_URL_PATH:
+                body = self.server.xsl_handler()
             else:
-                if req_path == '/' and 'Host' in self.headers.keys():
-                    # redirect to XML URL only when requested with '/'
+                if 'Host' in self.headers.keys() and \
+                        ( req_path == '/' or req_path == XML_URL_PATH ):
+                    # redirect to XML URL only when requested with '/' or XML_URL_PATH
+                    toloc = "http://%s%s/" % (self.headers.get('Host'), XML_URL_PATH)
                     self.send_response(302)
-                    self.send_header(
-                        "Location",
-                        "http://" + self.headers.get('Host') + XML_URL_PATH)
+                    self.send_header("Location", toloc)
                     self.end_headers()
                     return None
                 else:
@@ -126,7 +189,7 @@ class HttpHandler(http.server.BaseHTTPRequestHandler):
             self.send_error(404)
             logger.error(STATHTTPD_SERVER_DATAERROR, err)
             return None
-        except StatsHttpdError as err:
+        except Exception as err:
             self.send_error(500)
             logger.error(STATHTTPD_SERVER_ERROR, err)
             return None
@@ -134,6 +197,7 @@ class HttpHandler(http.server.BaseHTTPRequestHandler):
             self.send_response(200)
             self.send_header("Content-type", "text/xml")
             self.send_header("Content-Length", len(body))
+            self.send_header("Last-Modified", self.date_time_string(time.time()))
             self.end_headers()
             return body
 
@@ -436,71 +500,66 @@ class StatsHttpd:
                                   (err.__class__.__name__, err))
 
 
-    def xml_handler(self, module_name=None, item_name=None):
+    def xml_handler(self, path=''):
         """Requests the specified statistics data and specification by
         using the functions get_stats_data and get_stats_spec
         respectively and loads the XML template file and returns the
-        string of the XML document.The first argument is the module
-        name which owns the statistics data, the second argument is
-        one name of the statistics items which the the module
-        owns. The second argument cannot be specified when the first
-        argument is not specified."""
-
-        # TODO: Separate the following recursive function by type of
-        # the parameter. Because we should be sure what type there is
-        # when we call it recursively.
-        def stats_data2xml(stats_spec, stats_data, xml_elem):
-            """Internal use for xml_handler. Reads stats_data and
-            stats_spec specified as first and second arguments, and
-            modify the xml object specified as third
-            argument. xml_elem must be modified and always returns
-            None."""
-            # assumed started with module_spec or started with
-            # item_spec in statistics
-            if type(stats_spec) is dict:
-                # assumed started with module_spec
-                if 'item_name' not in stats_spec \
-                        and 'item_type' not in stats_spec:
-                    for module_name in stats_spec.keys():
-                        elem = xml.etree.ElementTree.Element(module_name)
-                        stats_data2xml(stats_spec[module_name],
-                                       stats_data[module_name], elem)
-                        xml_elem.append(elem)
-                # started with item_spec in statistics
-                else:
-                    elem = xml.etree.ElementTree.Element(stats_spec['item_name'])
-                    if stats_spec['item_type'] == 'map':
-                        stats_data2xml(stats_spec['map_item_spec'],
-                                       stats_data,
-                                       elem)
-                    elif stats_spec['item_type'] == 'list':
-                        for item in stats_data:
-                            stats_data2xml(stats_spec['list_item_spec'],
-                                           item, elem)
-                    else:
-                        elem.text = str(stats_data)
-                    xml_elem.append(elem)
-            # assumed started with stats_spec
-            elif type(stats_spec) is list:
-                for item_spec in stats_spec:
-                    stats_data2xml(item_spec,
-                                   stats_data[item_spec['item_name']],
-                                   xml_elem)
-
+        string of the XML document.The argument is a path in the
+        requested URI. For example, the path is assumed to be like
+        ${module_name}/${top_level_item_name}/${second_level_item_name}/..."""
+
+        dirs = [ d for d in path.split("/") if len(d) > 0 ]
+        module_name = None
+        item_name = None
+        if len(dirs) > 0:
+            module_name = dirs[0]
+        if len(dirs) > 1:
+            item_name = dirs[1]
+            # removed an index string when list-type value is
+            # requested. Because such a item name can be accept by the
+            # stats module currently.
+            item_name = re.sub('\[\d+\]$', '', item_name)
         stats_spec = self.get_stats_spec(module_name, item_name)
         stats_data = self.get_stats_data(module_name, item_name)
-        # make the path xxx/module/item if specified respectively
-        path_info = ''
-        if module_name is not None and item_name is not None:
-            path_info = '/' + module_name + '/' + item_name
-        elif module_name is not None:
-            path_info = '/' + module_name
+        path_list = []
+        try:
+            path_list = item_name_list(stats_data, path)
+        except isc.cc.data.DataNotFoundError as err:
+            raise StatsHttpdDataError(
+                "%s: %s" % (err.__class__.__name__, err))
+        item_list = []
+        for path in path_list:
+            dirs = path.split("/")
+            if len(dirs) < 2: continue
+            item = {}
+            item['identifier'] = path
+            value = isc.cc.data.find(stats_data, path)
+            if type(value) is bool:
+                value = str(value).lower()
+            if type(value) is dict or type(value) is list:
+                value = None
+            if value is not None:
+                item['value'] = str(value)
+            owner = dirs[0]
+            item['owner'] = owner
+            item['uri'] = urllib.parse.quote('%s/%s' % (XML_URL_PATH, path))
+            item_path = '/'.join(dirs[1:])
+            spec = isc.config.find_spec_part(stats_spec[owner], item_path)
+            for key in ['name', 'type', 'description', 'title', \
+                            'optional', 'default', 'format']:
+                value = spec.get('item_%s' % key)
+                if type(value) is bool:
+                    value = str(value).lower()
+                if type(value) is dict or type(value) is list:
+                    value = None
+                if value is not None:
+                    item[key] = str(value)
+            item_list.append(item)
         xml_elem = xml.etree.ElementTree.Element(
-            'bind10:statistics',
-            attrib={ 'xsi:schemaLocation' : XSD_NAMESPACE + ' ' + XSD_URL_PATH + path_info,
-                     'xmlns:bind10' : XSD_NAMESPACE,
-                     'xmlns:xsi' : "http://www.w3.org/2001/XMLSchema-instance" })
-        stats_data2xml(stats_spec, stats_data, xml_elem)
+            XML_ROOT_ELEMENT, attrib=XML_ROOT_ATTRIB)
+        for item in item_list:
+            item_elem = xml.etree.ElementTree.Element('item', attrib=item)
+            xml_elem.append(item_elem)
         # The coding conversion is tricky. xml..tostring() of Python 3.2
         # returns bytes (not string) regardless of the coding, while
         # tostring() of Python 3.1 returns a string.  To support both
@@ -512,313 +571,34 @@ class StatsHttpd:
         xml_string = str(xml.etree.ElementTree.tostring(xml_elem, encoding='utf-8'),
                          encoding='us-ascii')
         self.xml_body = self.open_template(XML_TEMPLATE_LOCATION).substitute(
-            xml_string=xml_string,
-            xsl_url_path=XSL_URL_PATH + path_info)
-        assert self.xml_body is not None
+            xml_string=xml_string, xsl_url_path=XSL_URL_PATH)
         return self.xml_body
 
-    def xsd_handler(self, module_name=None, item_name=None):
-        """Requests the specified statistics specification by using
-        the function get_stats_spec respectively and loads the XSD
-        template file and returns the string of the XSD document.The
-        first argument is the module name which owns the statistics
-        data, the second argument is one name of the statistics items
-        which the the module owns. The second argument cannot be
-        specified when the first argument is not specified."""
-
-        # TODO: Separate the following recursive function by type of
-        # the parameter. Because we should be sure what type there is
-        # when we call it recursively.
-        def stats_spec2xsd(stats_spec, xsd_elem):
-            """Internal use for xsd_handler. Reads stats_spec
-            specified as first arguments, and modify the xml object
-            specified as second argument. xsd_elem must be
-            modified. Always returns None with no exceptions."""
-            # assumed module_spec or one stats_spec
-            if type(stats_spec) is dict:
-                # assumed module_spec
-                if 'item_name' not in stats_spec:
-                    for mod in stats_spec.keys():
-                        elem = xml.etree.ElementTree.Element(
-                            "element", { "name" : mod })
-                        complextype = xml.etree.ElementTree.Element("complexType")
-                        alltag = xml.etree.ElementTree.Element("all")
-                        stats_spec2xsd(stats_spec[mod], alltag)
-                        complextype.append(alltag)
-                        elem.append(complextype)
-                        xsd_elem.append(elem)
-                # assumed stats_spec
-                else:
-                    if stats_spec['item_type'] == 'map':
-                        alltag = xml.etree.ElementTree.Element("all")
-                        stats_spec2xsd(stats_spec['map_item_spec'], alltag)
-                        complextype = xml.etree.ElementTree.Element("complexType")
-                        complextype.append(alltag)
-                        elem = xml.etree.ElementTree.Element(
-                            "element", attrib={ "name" : stats_spec["item_name"],
-                                                "minOccurs": "0" \
-                                                    if stats_spec["item_optional"] \
-                                                    else "1",
-                                                "maxOccurs": "unbounded" })
-                        elem.append(complextype)
-                        xsd_elem.append(elem)
-                    elif stats_spec['item_type'] == 'list':
-                        alltag = xml.etree.ElementTree.Element("sequence")
-                        stats_spec2xsd(stats_spec['list_item_spec'], alltag)
-                        complextype = xml.etree.ElementTree.Element("complexType")
-                        complextype.append(alltag)
-                        elem = xml.etree.ElementTree.Element(
-                            "element", attrib={ "name" : stats_spec["item_name"],
-                                                "minOccurs": "0" \
-                                                    if stats_spec["item_optional"] \
-                                                    else "1",
-                                                "maxOccurs": "1" })
-                        elem.append(complextype)
-                        xsd_elem.append(elem)
-                    else:
-                        # determine the datatype of XSD
-                        # TODO: Should consider other item_format types
-                        datatype = stats_spec["item_type"] \
-                            if stats_spec["item_type"].lower() != 'real' \
-                            else 'float'
-                        if "item_format" in stats_spec:
-                            item_format = stats_spec["item_format"]
-                            if datatype.lower() == 'string' \
-                                    and item_format.lower() == 'date-time':
-                                 datatype = 'dateTime'
-                            elif datatype.lower() == 'string' \
-                                    and (item_format.lower() == 'date' \
-                                             or item_format.lower() == 'time'):
-                                 datatype = item_format.lower()
-                        elem = xml.etree.ElementTree.Element(
-                            "element",
-                            attrib={
-                                'name' : stats_spec["item_name"],
-                                'type' : datatype,
-                                'minOccurs' : "0" \
-                                    if stats_spec["item_optional"] \
-                                    else "1",
-                                'maxOccurs' : "1"
-                                }
-                            )
-                        annotation = xml.etree.ElementTree.Element("annotation")
-                        appinfo = xml.etree.ElementTree.Element("appinfo")
-                        documentation = xml.etree.ElementTree.Element("documentation")
-                        if "item_title" in stats_spec:
-                            appinfo.text = stats_spec["item_title"]
-                        if "item_description" in stats_spec:
-                            documentation.text = stats_spec["item_description"]
-                        annotation.append(appinfo)
-                        annotation.append(documentation)
-                        elem.append(annotation)
-                        xsd_elem.append(elem)
-            # multiple stats_specs
-            elif type(stats_spec) is list:
-                for item_spec in stats_spec:
-                    stats_spec2xsd(item_spec, xsd_elem)
-
-        # for XSD
-        stats_spec = self.get_stats_spec(module_name, item_name)
-        alltag = xml.etree.ElementTree.Element("all")
-        stats_spec2xsd(stats_spec, alltag)
-        complextype = xml.etree.ElementTree.Element("complexType")
-        complextype.append(alltag)
-        documentation = xml.etree.ElementTree.Element("documentation")
-        documentation.text = "A set of statistics data"
-        annotation = xml.etree.ElementTree.Element("annotation")
-        annotation.append(documentation)
-        elem = xml.etree.ElementTree.Element(
-            "element", attrib={ 'name' : 'statistics' })
-        elem.append(annotation)
-        elem.append(complextype)
-        documentation = xml.etree.ElementTree.Element("documentation")
-        documentation.text = "XML schema of the statistics data in BIND 10"
-        annotation = xml.etree.ElementTree.Element("annotation")
-        annotation.append(documentation)
-        xsd_root = xml.etree.ElementTree.Element(
-            "schema",
-            attrib={ 'xmlns' : "http://www.w3.org/2001/XMLSchema",
-                     'targetNamespace' : XSD_NAMESPACE,
-                     'xmlns:bind10' : XSD_NAMESPACE })
-        xsd_root.append(annotation)
-        xsd_root.append(elem)
-        # The coding conversion is tricky. xml..tostring() of Python 3.2
-        # returns bytes (not string) regardless of the coding, while
-        # tostring() of Python 3.1 returns a string.  To support both
-        # cases transparently, we first make sure tostring() returns
-        # bytes by specifying utf-8 and then convert the result to a
-        # plain string (code below assume it).
-        # FIXME: Non-ASCII characters might be lost here. Consider how
-        # the whole system should handle non-ASCII characters.
-        xsd_string = str(xml.etree.ElementTree.tostring(xsd_root, encoding='utf-8'),
-                         encoding='us-ascii')
+    def xsd_handler(self):
+        """Loads the XSD template file, replaces the variable strings,
+        and returns the string of the XSD document."""
         self.xsd_body = self.open_template(XSD_TEMPLATE_LOCATION).substitute(
-            xsd_string=xsd_string)
-        assert self.xsd_body is not None
+            xsd_namespace=XSD_NAMESPACE)
         return self.xsd_body
 
-    def xsl_handler(self, module_name=None, item_name=None):
-        """Requests the specified statistics specification by using
-        the function get_stats_spec respectively and loads the XSL
-        template file and returns the string of the XSL document.The
-        first argument is the module name which owns the statistics
-        data, the second argument is one name of the statistics items
-        which the the module owns. The second argument cannot be
-        specified when the first argument is not specified."""
-
-        # TODO: Separate the following recursive function by type of
-        # the parameter. Because we should be sure what type there is
-        # when we call it recursively.
-        def stats_spec2xsl(stats_spec, xsl_elem, path=XML_URL_PATH):
-            """Internal use for xsl_handler. Reads stats_spec
-            specified as first arguments, and modify the xml object
-            specified as second argument. xsl_elem must be
-            modified. The third argument is a base path used for
-            making anchor tag in XSL. Always returns None with no
-            exceptions."""
-            # assumed module_spec or one stats_spec
-            if type(stats_spec) is dict:
-                # assumed module_spec
-                if 'item_name' not in stats_spec:
-                    table = xml.etree.ElementTree.Element("table")
-                    tr = xml.etree.ElementTree.Element("tr")
-                    th = xml.etree.ElementTree.Element("th")
-                    th.text = "Module Name"
-                    tr.append(th)
-                    th = xml.etree.ElementTree.Element("th")
-                    th.text = "Module Item"
-                    tr.append(th)
-                    table.append(tr)
-                    for mod in stats_spec.keys():
-                        foreach = xml.etree.ElementTree.Element(
-                            "xsl:for-each", attrib={ "select" : mod })
-                        tr = xml.etree.ElementTree.Element("tr")
-                        td = xml.etree.ElementTree.Element("td")
-                        a = xml.etree.ElementTree.Element(
-                            "a", attrib={ "href": urllib.parse.quote(path + "/" + mod) })
-                        a.text = mod
-                        td.append(a)
-                        tr.append(td)
-                        td = xml.etree.ElementTree.Element("td")
-                        stats_spec2xsl(stats_spec[mod], td,
-                                       path + "/" + mod)
-                        tr.append(td)
-                        foreach.append(tr)
-                        table.append(foreach)
-                    xsl_elem.append(table)
-                # assumed stats_spec
-                else:
-                    if stats_spec['item_type'] == 'map':
-                        table = xml.etree.ElementTree.Element("table")
-                        tr = xml.etree.ElementTree.Element("tr")
-                        th = xml.etree.ElementTree.Element("th")
-                        th.text = "Item Name"
-                        tr.append(th)
-                        th = xml.etree.ElementTree.Element("th")
-                        th.text = "Item Value"
-                        tr.append(th)
-                        table.append(tr)
-                        foreach = xml.etree.ElementTree.Element(
-                            "xsl:for-each", attrib={ "select" : stats_spec['item_name'] })
-                        tr = xml.etree.ElementTree.Element("tr")
-                        td = xml.etree.ElementTree.Element(
-                            "td",
-                            attrib={ "class" : "title",
-                                     "title" : stats_spec["item_description"] \
-                                         if "item_description" in stats_spec \
-                                         else "" })
-                        # TODO: Consider whether we should always use
-                        # the identical name "item_name" for the
-                        # user-visible name in XSL.
-                        td.text = stats_spec[ "item_title" if "item_title" in stats_spec else "item_name" ]
-                        tr.append(td)
-                        td = xml.etree.ElementTree.Element("td")
-                        stats_spec2xsl(stats_spec['map_item_spec'], td,
-                                       path + "/" + stats_spec["item_name"])
-                        tr.append(td)
-                        foreach.append(tr)
-                        table.append(foreach)
-                        xsl_elem.append(table)
-                    elif stats_spec['item_type'] == 'list':
-                        stats_spec2xsl(stats_spec['list_item_spec'], xsl_elem,
-                                       path + "/" + stats_spec["item_name"])
-                    else:
-                        xsl_valueof = xml.etree.ElementTree.Element(
-                            "xsl:value-of",
-                            attrib={'select': stats_spec["item_name"]})
-                        xsl_elem.append(xsl_valueof)
-
-            # multiple stats_specs
-            elif type(stats_spec) is list:
-                table = xml.etree.ElementTree.Element("table")
-                tr = xml.etree.ElementTree.Element("tr")
-                th = xml.etree.ElementTree.Element("th")
-                th.text = "Item Name"
-                tr.append(th)
-                th = xml.etree.ElementTree.Element("th")
-                th.text = "Item Value"
-                tr.append(th)
-                table.append(tr)
-                for item_spec in stats_spec:
-                    tr = xml.etree.ElementTree.Element("tr")
-                    td = xml.etree.ElementTree.Element(
-                        "td",
-                        attrib={ "class" : "title",
-                                 "title" : item_spec["item_description"] \
-                                     if "item_description" in item_spec \
-                                     else "" })
-                    # if the path length is equal to or shorter than
-                    # XML_URL_PATH + /Module/Item, add the anchor tag.
-                    if len(path.split('/')) <= len((XML_URL_PATH + '/Module/Item').split('/')):
-                        a = xml.etree.ElementTree.Element(
-                            "a", attrib={ "href": urllib.parse.quote(path + "/" + item_spec["item_name"]) })
-                        a.text = item_spec[ "item_title" if "item_title" in item_spec else "item_name" ]
-                        td.append(a)
-                    else:
-                        td.text = item_spec[ "item_title" if "item_title" in item_spec else "item_name" ]
-                    tr.append(td)
-                    td = xml.etree.ElementTree.Element("td")
-                    stats_spec2xsl(item_spec, td, path)
-                    tr.append(td)
-                    if item_spec['item_type'] == 'list':
-                        foreach = xml.etree.ElementTree.Element(
-                            "xsl:for-each", attrib={ "select" : item_spec['item_name'] })
-                        foreach.append(tr)
-                        table.append(foreach)
-                    else:
-                        table.append(tr)
-                xsl_elem.append(table)
-
-        # for XSL
-        stats_spec = self.get_stats_spec(module_name, item_name)
-        xsd_root = xml.etree.ElementTree.Element( # started with xml:template tag
-            "xsl:template",
-            attrib={'match': "bind10:statistics"})
-        stats_spec2xsl(stats_spec, xsd_root)
-        # The coding conversion is tricky. xml..tostring() of Python 3.2
-        # returns bytes (not string) regardless of the coding, while
-        # tostring() of Python 3.1 returns a string.  To support both
-        # cases transparently, we first make sure tostring() returns
-        # bytes by specifying utf-8 and then convert the result to a
-        # plain string (code below assume it).
-        # FIXME: Non-ASCII characters might be lost here. Consider how
-        # the whole system should handle non-ASCII characters.
-        xsl_string = str(xml.etree.ElementTree.tostring(xsd_root, encoding='utf-8'),
-                         encoding='us-ascii')
+    def xsl_handler(self):
+        """Loads the XSL template file, replaces the variable strings,
+        and returns the string of the XSL document."""
         self.xsl_body = self.open_template(XSL_TEMPLATE_LOCATION).substitute(
-            xsl_string=xsl_string,
             xsd_namespace=XSD_NAMESPACE)
-        assert self.xsl_body is not None
         return self.xsl_body
 
     def open_template(self, file_name):
         """It opens a template file, and it loads all lines to a
         string variable and returns string. Template object includes
-        the variable. Limitation of a file size isn't needed there."""
-        f = open(file_name, 'r')
-        lines = "".join(f.readlines())
-        f.close()
-        assert lines is not None
+        the variable. Limitation of a file size isn't needed there. XXXX"""
+        lines = None
+        try:
+            with open(file_name, 'r') as f:
+                lines = "".join(f.readlines())
+        except IOError as err:
+            raise StatsHttpdDataError(
+                "%s: %s" % (err.__class__.__name__, err))
         return string.Template(lines)
 
 if __name__ == "__main__":

src/bin/stats/tests/test_utils.py

@@ -1,3 +1,18 @@
+# Copyright (C) 2011-2012  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.
+
 """
 Utilities and mock modules for unittests of statistics modules
 
@@ -16,6 +31,8 @@ import isc.config.cfgmgr
 import stats
 import stats_httpd
 
+CONST_BASETIME = (2011, 6, 22, 8, 14, 8, 2, 173, 0)
+
 class SignalHandler():
     """A signal handler class for deadlock in unittest"""
     def __init__(self, fail_handler, timeout=20):
@@ -222,7 +239,7 @@ class MockBoss:
   }
 }
 """
-    _BASETIME = (2011, 6, 22, 8, 14, 8, 2, 173, 0)
+    _BASETIME = CONST_BASETIME
 
     def __init__(self):
         self._started = threading.Event()
@@ -457,6 +474,11 @@ class MockAuth:
         return isc.config.create_answer(1, "Unknown Command")
 
 class MyStats(stats.Stats):
+
+    stats._BASETIME = CONST_BASETIME
+    stats.get_timestamp = lambda: time.mktime(CONST_BASETIME)
+    stats.get_datetime = lambda x=None: time.strftime("%Y-%m-%dT%H:%M:%SZ", CONST_BASETIME)
+
     def __init__(self):
         self._started = threading.Event()
         stats.Stats.__init__(self)

src/bin/xfrin/tests/xfrin_test.py

@@ -570,7 +570,7 @@ class TestXfrinIXFRAdd(TestXfrinState):
         # difference, starting with removing that SOA.
         self.conn._diff.add_data(self.ns_rrset) # put some dummy change
         self.conn._tsig_ctx = MockTSIGContext(TSIG_KEY)
-        self.conn._tsig_ctx.last_has_signature = lambda: False
+        self.conn._tsig_ctx.last_had_signature = lambda: False
         # First, push a starting SOA inside. This should be OK, nothing checked
         # yet.
         self.state.handle_rr(self.conn, self.begin_soa)
@@ -821,7 +821,7 @@ class TestAXFR(TestXfrinConnection):
         mock_ctx = MockTSIGContext(key)
         mock_ctx.error = error
         if not has_last_signature:
-            mock_ctx.last_has_signature = lambda: False
+            mock_ctx.last_had_signature = lambda: False
         return mock_ctx
 
     def __match_exception(self, expected_exception, expected_msg, expression):

src/bin/xfrin/xfrin.py.in

@@ -797,7 +797,7 @@ class XfrinConnection(asyncore.dispatcher):
         Check there's a signature at the last message.
         """
         if self._tsig_ctx is not None:
-            if not self._tsig_ctx.last_has_signature():
+            if not self._tsig_ctx.last_had_signature():
                 raise XfrinProtocolError('TSIG verify fail: no TSIG on last '+
                                          'message')
 

src/lib/dhcp/subnet.h

@@ -69,6 +69,12 @@ public:
         /// @param persist if true option is always sent.
         OptionDescriptor(OptionPtr& opt, bool persist)
             : option(opt), persistent(persist) {};
+
+        /// @brief Constructor
+        ///
+        /// @param persist if true option is always sent.
+        OptionDescriptor(bool persist)
+            : option(OptionPtr()), persistent(persist) {};
     };
 
     /// @brief Extractor class to extract key with another key.

src/lib/dns/rrttl.cc

@@ -92,7 +92,7 @@ RRTTL::RRTTL(const std::string& ttlstr) {
             // There's a unit now.
             units_mode = true;
             // Find the unit and get the size.
-            uint32_t multiply;
+            uint32_t multiply = 1;  // initialize to silence compiler warnings
             bool found = false;
             for (size_t i = 0; i < sizeof(units) / sizeof(*units); ++i) {
                 if (toupper(*unit) == units[i].unit) {

src/lib/python/isc/testutils/tsigctx_mock.py

@@ -52,5 +52,5 @@ class MockTSIGContext(TSIGContext):
             return self.error(self)
         return self.error
 
-    def last_has_signature(self):
+    def last_had_signature(self):
         return True

tests/lettuce/features/terrain/terrain.py

@@ -61,6 +61,8 @@ copylist = [
      "configurations/ddns/noddns.config"],
     ["configurations/xfrin/retransfer_master.conf.orig",
      "configurations/xfrin/retransfer_master.conf"],
+    ["configurations/xfrin/retransfer_slave.conf.orig",
+     "configurations/xfrin/retransfer_slave.conf"],
     ["data/inmem-xfrin.sqlite3.orig",
      "data/inmem-xfrin.sqlite3"],
     ["data/xfrin-notify.sqlite3.orig",

tests/lettuce/features/xfrin_bind10.feature

@@ -1,6 +1,6 @@
-Feature: Xfrin 
+Feature: Xfrin
     Tests for Xfrin, specific for BIND 10 behaviour.
-    
+
     Scenario: Retransfer command
     # Standard check to test (non-)existence of a file.
     # This file is actually automatically created.
@@ -37,3 +37,52 @@ Feature: Xfrin
     # We don't have to specify the address/port here; the defaults will work.
     When I do an AXFR transfer of example.org
     Then transfer result should have 13 rrs
+
+
+
+    Scenario: Transfer with TSIG
+    # Similar setup to the test above, but this time, we add TSIG configuration
+
+    # In order to check that the tests don't give false positives because config
+    # happens to be right (like no TSIG on either side), we take an existing
+    # non-TSIG config, add TSIG on the master side, see it fail, add TSIG
+    # on the slave side, then check again.
+
+    Given I have bind10 running with configuration xfrin/retransfer_master.conf with cmdctl port 47804 as master
+    And wait for master stderr message AUTH_SERVER_STARTED
+    And wait for master stderr message XFROUT_STARTED
+
+    And I have bind10 running with configuration xfrin/retransfer_slave.conf
+    And wait for bind10 stderr message CMDCTL_STARTED
+    And wait for bind10 stderr message XFRIN_STARTED
+
+    # Set slave config for 'automatic' xfrin
+    When I set bind10 configuration Xfrin/zones to [{"master_port": 47806, "name": "example.org", "master_addr": "::1"}]
+
+    # Make sure it is fully open
+    When I send bind10 the command Xfrin retransfer example.org
+    Then wait for new bind10 stderr message XFRIN_TRANSFER_SUCCESS not XFRIN_XFR_PROCESS_FAILURE
+    And wait for new bind10 stderr message ZONEMGR_RECEIVE_XFRIN_SUCCESS
+
+    # First to master, a transfer should then fail
+    When I send bind10 the following commands with cmdctl port 47804:
+    """
+    config add tsig_keys/keys "example.key.:c2VjcmV0"
+    config set Xfrout/zone_config[0]/transfer_acl [{"action": "ACCEPT", "from": "::1", "key": "example.key."}]
+    config commit
+    """
+
+    # Transfer should fail
+    When I send bind10 the command Xfrin retransfer example.org
+    Then wait for new bind10 stderr message XFRIN_XFR_TRANSFER_PROTOCOL_ERROR not XFRIN_TRANSFER_SUCCESS
+    # Set client to use TSIG as well
+    When I send bind10 the following commands:
+    """
+    config add tsig_keys/keys "example.key.:c2VjcmV0"
+    config set Xfrin/zones[0]/tsig_key  "example.key.:c2VjcmV0"
+    config commit
+    """
+
+    # Transwer should succeed now
+    When I send bind10 the command Xfrin retransfer example.org
+    Then wait for new bind10 stderr message XFRIN_TRANSFER_SUCCESS not XFRIN_XFR_PROCESS_FAILURE