[#3609] Addressed comments

diff --git a/doc/sphinx/arm/agent.rst b/doc/sphinx/arm/agent.rst
index 4862d46a926a40a8215f12b0cd8a75e7bb24aa0d..3519cb097de1b36398686f47136f5fc25d944858 100644 (file)
--- a/doc/sphinx/arm/agent.rst
+++ b/doc/sphinx/arm/agent.rst
@@ -120,7 +120,7 @@ different from the HA peer URLs, which are strictly
 for internal HA traffic between the peers. User commands should
 still be sent via the CA.
 
-Since Kea 1.7.5 the ``http-headers`` parameter specifies a list of
+Since Kea 2.7.5 the ``http-headers`` parameter specifies a list of
 extra HTTP headers to add to HTTP responses.
 
 The ``trust-anchor``, ``cert-file``, ``key-file``, and ``cert-required``

diff --git a/doc/sphinx/arm/ctrl-channel.rst b/doc/sphinx/arm/ctrl-channel.rst
index 2a1c2f6f4bb5c6b27a2ce19e81272eb79ad07823..594074358609707369f0ba4d452d9bac1fefbb0c 100644 (file)
--- a/doc/sphinx/arm/ctrl-channel.rst
+++ b/doc/sphinx/arm/ctrl-channel.rst
@@ -210,7 +210,21 @@ depends on the specific command.
 .. note::
 
    Since Kea 2.7.5 it is possible to specify extra HTTP headers which
-   are added to HTTP responses.
+   are added to HTTP responses. Each header is specified by its name
+   and value with optionally a user context. For instance:
+
+::
+
+   ...,
+   "http-headers": [
+       {
+           "name": "Strict-Transport-Security",
+           "value": "max-age=31536000"
+       }
+    ],
+    ...
+
+adds a HSTS header declaring that HTTPS (vs HTTP) must be used for one year.
 
 .. _ctrl-channel-control-agent-command-response-format:
 

diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst
index 50283a240853fcb129ff1859f2a8631c8854ba00..cc46612479294c7e6593a15c43a06a1f2899cffc 100644 (file)
--- a/doc/sphinx/arm/dhcp4-srv.rst
+++ b/doc/sphinx/arm/dhcp4-srv.rst
@@ -7777,7 +7777,7 @@ TLS is required). The ``socket-address`` (default ``127.0.0.1``) and
 ``socket-port`` (default 8000) specify an IP address and port to which
 the HTTP service will be bound.
 
-Since Kea 1.7.5 the ``http-headers`` parameter specifies a list of
+Since Kea 2.7.5 the ``http-headers`` parameter specifies a list of
 extra HTTP headers to add to HTTP responses.
 
 The ``trust-anchor``, ``cert-file``, ``key-file``, and ``cert-required``

diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst
index 14c89c70eb15e62cb97fd03513cead6a7eec27a3..a1e0e8cf453a9cc04253c0c8629ab2dbb35cbaf3 100644 (file)
--- a/doc/sphinx/arm/dhcp6-srv.rst
+++ b/doc/sphinx/arm/dhcp6-srv.rst
@@ -7591,7 +7591,7 @@ TLS is required). The ``socket-address`` (default ``::1``) and
 ``socket-port`` (default 8000) specify an IP address and port to which
 the HTTP service will be bound.
 
-Since Kea 1.7.5 the ``http-headers`` parameter specifies a list of
+Since Kea 2.7.5 the ``http-headers`` parameter specifies a list of
 extra HTTP headers to add to HTTP responses.
 
 The ``trust-anchor``, ``cert-file``, ``key-file``, and ``cert-required``

diff --git a/src/bin/agent/ca_response_creator.cc b/src/bin/agent/ca_response_creator.cc
index ddc010acbf8aefe64348783f6ccb725907f69084..ff0e3b5383ee3ff23ad1a7d80d4979445cd53ec5 100644 (file)
--- a/src/bin/agent/ca_response_creator.cc
+++ b/src/bin/agent/ca_response_creator.cc
@@ -63,6 +63,31 @@ createStockHttpResponse(const HttpRequestPtr& request,
     return (response);
 }
 
+namespace {
+
+/// Getting the config context.
+CtrlAgentCfgContextPtr getCtrlAgentCfgContext() {
+    // There is a hierarchy of the objects through which we need to pass to get
+    // the configuration context. We may simplify this at some point but since
+    // we're in the singleton we want to make sure that we're using most current
+    // configuration.
+    CtrlAgentCfgContextPtr ctx;
+    boost::shared_ptr<CtrlAgentController> controller =
+        boost::dynamic_pointer_cast<CtrlAgentController>(CtrlAgentController::instance());
+    if (controller) {
+        CtrlAgentProcessPtr process = controller->getCtrlAgentProcess();
+        if (process) {
+            CtrlAgentCfgMgrPtr cfgmgr = process->getCtrlAgentCfgMgr();
+            if (cfgmgr) {
+                ctx = cfgmgr->getCtrlAgentCfgContext();
+            }
+        }
+    }
+    return (ctx);
+}
+
+} // end of anonymous namespace.
+
 HttpResponsePtr
 CtrlAgentResponseCreator::
 createStockHttpResponseInternal(const HttpRequestPtr& request,
@@ -81,20 +106,9 @@ createStockHttpResponseInternal(const HttpRequestPtr& request,
     }
     // This will generate the response holding JSON content.
     HttpResponsePtr response(new HttpResponseJson(http_version, status_code));
-    // See the comment below.
-    boost::shared_ptr<CtrlAgentController> controller =
-        boost::dynamic_pointer_cast<CtrlAgentController>(CtrlAgentController::instance());
-    if (controller) {
-        CtrlAgentProcessPtr process = controller->getCtrlAgentProcess();
-        if (process) {
-            CtrlAgentCfgMgrPtr cfgmgr = process->getCtrlAgentCfgMgr();
-            if (cfgmgr) {
-                CtrlAgentCfgContextPtr ctx = cfgmgr->getCtrlAgentCfgContext();
-                if (ctx) {
-                    copyHttpHeaders(ctx->getHttpHeaders(), *response);
-                }
-            }
-        }
+    CtrlAgentCfgContextPtr ctx = getCtrlAgentCfgContext();
+    if (ctx) {
+        copyHttpHeaders(ctx->getHttpHeaders(), *response);
     }
     return (response);
 }
@@ -109,29 +123,13 @@ createDynamicHttpResponse(HttpRequestPtr request) {
     HttpResponseJsonPtr http_response;
 
     // Context will hold the server configuration.
-    CtrlAgentCfgContextPtr ctx;
-
-    // There is a hierarchy of the objects through which we need to pass to get
-    // the configuration context. We may simplify this at some point but since
-    // we're in the singleton we want to make sure that we're using most current
-    // configuration.
-    boost::shared_ptr<CtrlAgentController> controller =
-        boost::dynamic_pointer_cast<CtrlAgentController>(CtrlAgentController::instance());
-    if (controller) {
-        CtrlAgentProcessPtr process = controller->getCtrlAgentProcess();
-        if (process) {
-            CtrlAgentCfgMgrPtr cfgmgr = process->getCtrlAgentCfgMgr();
-            if (cfgmgr) {
-                ctx = cfgmgr->getCtrlAgentCfgContext();
-                if (ctx) {
-                    headers = ctx->getHttpHeaders();
-                    const HttpAuthConfigPtr& auth = ctx->getAuthConfig();
-                    if (auth) {
-                        // Check authentication.
-                        http_response = auth->checkAuth(*this, request);
-                    }
-                }
-            }
+    CtrlAgentCfgContextPtr ctx = getCtrlAgentCfgContext();
+    if (ctx) {
+        headers = ctx->getHttpHeaders();
+        const HttpAuthConfigPtr& auth = ctx->getAuthConfig();
+        if (auth) {
+            // Check authentication.
+            http_response = auth->checkAuth(*this, request);
         }
     }
 

diff --git a/src/bin/agent/simple_parser.cc b/src/bin/agent/simple_parser.cc
index e3018730b85fb8ef0f9d1766fdf26a4af0c003ef..b50cda14409c488b99af5b4edc620fb4aa984079 100644 (file)
--- a/src/bin/agent/simple_parser.cc
+++ b/src/bin/agent/simple_parser.cc
@@ -153,7 +153,7 @@ AgentSimpleParser::parse(const CtrlAgentCfgContextPtr& ctx,
         }
     }
 
-    // Basic HTTP authentications are forth.
+    // Basic HTTP authentications are fourth.
     ConstElementPtr auth_config = config->get("authentication");
     if (auth_config) {
         using namespace isc::http;

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index bd2d56e8ab54067a4954ac0de26fb6af54c9afcf..8c85e98897aa533abf76de426f627d83056c7847 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -2894,7 +2894,7 @@ Dhcpv4Srv::createNameChangeRequests(const Lease4Ptr& lease,
     }
 
     if ((lease->reuseable_valid_lft_ == 0) &&
-        (!old_lease || ddns_params.getUpdateOnRenew() || 
+        (!old_lease || ddns_params.getUpdateOnRenew() ||
          !lease->hasIdenticalFqdn(*old_lease))) {
         if (old_lease) {
             // Queue's up a remove of the old lease's DNS (if needed)

diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index d178dce589f74394959761c71ce31099b616b784..8ebfe37be838e14d72ccd572073bf961c19e3b2d 100644 (file)
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -7121,6 +7121,12 @@ TEST_F(Dhcp4ParserTest, comments) {
     ASSERT_EQ(1, headers->size());
     ConstElementPtr header = headers->get(0);
     ASSERT_TRUE(header);
+    ASSERT_TRUE(header->get("name"));
+    EXPECT_EQ("\"Strict-Transport-Security\"", header->get("name")->str());
+    ASSERT_TRUE(header->get("value"));
+    EXPECT_EQ("\"max-age=31536000\"", header->get("value")->str());
+
+    // Check HTTP header user context.
     ConstElementPtr ctx_header = header->get("user-context");
     ASSERT_TRUE(ctx_header);
     ASSERT_EQ(1, ctx_header->size());

diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc
index c3f20bf3f45d369303c9cc443f342b7cf42d141a..51ba912d5dc3dee1b830d0533b1fc82669c093e8 100644 (file)
--- a/src/bin/dhcp6/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp6/tests/config_parser_unittest.cc
@@ -7935,6 +7935,12 @@ TEST_F(Dhcp6ParserTest, comments) {
     ASSERT_EQ(1, headers->size());
     ConstElementPtr header = headers->get(0);
     ASSERT_TRUE(header);
+    ASSERT_TRUE(header->get("name"));
+    EXPECT_EQ("\"Strict-Transport-Security\"", header->get("name")->str());
+    ASSERT_TRUE(header->get("value"));
+    EXPECT_EQ("\"max-age=31536000\"", header->get("value")->str());
+
+    // Check HTTP header user context.
     ConstElementPtr ctx_header = header->get("user-context");
     ASSERT_TRUE(ctx_header);
     ASSERT_EQ(1, ctx_header->size());

diff --git a/src/lib/http/cfg_http_header.h b/src/lib/http/cfg_http_header.h
index ae3301e453a3d8aa3160572732c746533350cc66..2b6e1993a2c6cd63b9c572514941d5c24e18c6aa 100644 (file)
--- a/src/lib/http/cfg_http_header.h
+++ b/src/lib/http/cfg_http_header.h
@@ -17,9 +17,16 @@ namespace isc {
 namespace http {
 
 /// @brief Config HTTP header.
+///
+/// Extra headers to include in a message are configured as a list of
+/// objects of this class. At the difference of other HTTP header classes
+/// there is no numeric value.
 class CfgHttpHeader : public isc::data::UserContext, public isc::data::CfgToElement {
 public:
+    /// @brief Header name.
     std::string name_;
+
+    /// @brief Header value.
     std::string value_;
 
     /// @brief Constructor.

diff --git a/src/lib/http/header_context.h b/src/lib/http/header_context.h
index 9baf48201a9454d5d9c71b66548fa9dfcc4b331e..49819a39391a39b6f656bd48f40736bbf5b2d6a4 100644 (file)
--- a/src/lib/http/header_context.h
+++ b/src/lib/http/header_context.h
@@ -17,7 +17,10 @@ namespace http {
 
 /// @brief HTTP header context.
 struct HttpHeaderContext {
+    /// @brief Header name.
     std::string name_;
+
+    /// @brief Header value.
     std::string value_;
 
     /// @brief Constructor.

diff --git a/src/lib/http/tests/response_unittests.cc b/src/lib/http/tests/response_unittests.cc
index f04a7ff74e03d8a4d14c127f4ebb0fd6d1c8888c..e60f20da89ea3cfe5a3000b0f7ce71d315d33214 100644 (file)
--- a/src/lib/http/tests/response_unittests.cc
+++ b/src/lib/http/tests/response_unittests.cc
@@ -180,7 +180,7 @@ TEST_F(HttpResponseTest, addHeader) {
         "<head><title>Kea page title</title></head>"
         "<body><h1>Some header</h1></body>"
         "</html>";
-        response.context()->headers_.push_back(HttpHeaderContext("Content-Type", "text/html"));
+    response.context()->headers_.push_back(HttpHeaderContext("Content-Type", "text/html"));
     response.context()->headers_.push_back(HttpHeaderContext("Host", "kea.example.org"));
     response.context()->body_ = sample_body;
     ASSERT_NO_THROW(response.finalize());