[#3609] Addressed comments

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``

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:
 

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``

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``

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);
         }
     }
 

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;

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)

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());

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());

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.

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.

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());