[357] configuration for tcp_recv_timeout in auth

src/bin/auth/auth.spec.pre.in

@@ -90,6 +90,11 @@
             }
           ]
         }
+      },
+      { "item_name": "tcp_recv_timeout",
+        "item_type": "integer",
+        "item_optional": false,
+        "item_default": 1000
       }
     ],
     "commands": [

src/bin/auth/auth_config.cc

@@ -116,6 +116,24 @@ private:
      */
     AddrListPtr rollbackAddresses_;
 };
+
+class TCPRecvTimeoutConfig : public AuthConfigParser {
+public:
+    TCPRecvTimeoutConfig(AuthSrv& server) : server_(server)
+    {}
+
+    virtual void build(ConstElementPtr config) {
+        timeout_ = config->intValue();
+    }
+
+    virtual void commit() {
+        server_.setTCPRecvTimeout(timeout_);
+    }
+private:
+    AuthSrv& server_;
+    size_t timeout_;
+};
+
 } // end of unnamed namespace
 
 AuthConfigParser*
@@ -147,6 +165,8 @@ createAuthConfigParser(AuthSrv& server, const std::string& config_id) {
         // We need to return something. The VersionConfig is empty now,
         // so we may abuse that one, as it is a short-term solution only.
         return (new VersionConfig());
+    } else if (config_id == "tcp_recv_timeout") {
+        return (new TCPRecvTimeoutConfig(server));
     } else {
         isc_throw(AuthConfigError, "Unknown configuration identifier: " <<
                   config_id);

src/bin/auth/auth_srv.cc

@@ -927,3 +927,8 @@ AuthSrv::getClientListClasses() const {
     }
     return (result);
 }
+
+void
+AuthSrv::setTCPRecvTimeout(size_t timeout) {
+    dnss_->setTCPRecvTimeout(timeout);
+}

src/bin/auth/auth_srv.h

@@ -319,6 +319,9 @@ public:
     ///     has been set by setClientList.
     std::vector<isc::dns::RRClass> getClientListClasses() const;
 
+    /// \brief Sets the timeout for incoming TCP connections
+    void setTCPRecvTimeout(size_t timeout);
+
 private:
     AuthSrvImpl* impl_;
     isc::asiolink::SimpleCallback* checkin_;

src/bin/auth/tests/config_syntax_unittest.cc

@@ -36,7 +36,8 @@ TEST_F(AuthConfigSyntaxTest, inmemoryDefaultFileType) {
     EXPECT_TRUE(
         mspec_.validateConfig(
             Element::fromJSON(
-                "{\"listen_on\": [], \"datasources\": "
+                "{\"tcp_recv_timeout\": 1000,"
+                " \"listen_on\": [], \"datasources\": "
                 "  [{\"type\": \"memory\", \"class\": \"IN\", "
                 "    \"zones\": [{\"origin\": \"example.com\","
                 "                 \"file\": \""
@@ -48,7 +49,8 @@ TEST_F(AuthConfigSyntaxTest, inmemorySQLite3Backend) {
     EXPECT_TRUE(
         mspec_.validateConfig(
             Element::fromJSON(
-                "{\"datasources\": "
+                "{\"tcp_recv_timeout\": 1000,"
+                " \"datasources\": "
                 "  [{\"type\": \"memory\","
                 "    \"zones\": [{\"origin\": \"example.com\","
                 "                 \"file\": \""
@@ -58,14 +60,30 @@ TEST_F(AuthConfigSyntaxTest, inmemorySQLite3Backend) {
 
 TEST_F(AuthConfigSyntaxTest, badInmemoryFileType) {
     // filetype must be a string
-    EXPECT_FALSE(
+    ASSERT_FALSE(
         mspec_.validateConfig(
             Element::fromJSON(
-                "{\"datasources\": "
+                "{\"tcp_recv_timeout\": 1000,"
+                " \"datasources\": "
                 "  [{\"type\": \"memory\","
                 "    \"zones\": [{\"origin\": \"example.com\","
                 "                 \"file\": \""
                 TEST_DATA_DIR "/example.zone\","
                 "                 \"filetype\": 42}]}]}"), false));
 }
+
+TEST_F(AuthConfigSyntaxTest, badTCPRecvTimeout) {
+    // tcp_recv_timeout must be int
+    EXPECT_FALSE(
+        mspec_.validateConfig(
+            Element::fromJSON(
+                "{\"tcp_recv_timeout\": \"foo\","
+                " \"datasources\": "
+                "  [{\"type\": \"memory\","
+                "    \"zones\": [{\"origin\": \"example.com\","
+                "                 \"file\": \""
+                TEST_DATA_DIR "/example.zone\","
+                "                 \"filetype\": \"sqlite3\"}]}]}"), false));
+}
+
 }

src/bin/auth/tests/config_unittest.cc

@@ -143,4 +143,13 @@ TEST_F(AuthConfigTest, listenAddressConfig) {
     EXPECT_EQ(DNSService::SERVER_SYNC_OK, dnss_.getUDPFdParams().at(1).options);
 }
 
+TEST_F(AuthConfigTest, tcpRecvTimeoutConfig) {
+    configureAuthServer(server, Element::fromJSON(
+    "{ \"tcp_recv_timeout\": 123 }"));
+    EXPECT_EQ(123, dnss_.getTCPRecvTimeout());
+    configureAuthServer(server, Element::fromJSON(
+    "{ \"tcp_recv_timeout\": 2000 }"));
+    EXPECT_EQ(2000, dnss_.getTCPRecvTimeout());
+}
+
 }

src/lib/asiodns/dns_server.h

@@ -81,7 +81,7 @@ public:
     /// \brief Stop current running server
     virtual void stop() { self_->stop();}
 
-    /// \brief Resume processing of the server coroutine after an 
+    /// \brief Resume processing of the server coroutine after an
     /// asynchronous call (e.g., to the DNS Lookup provider) has completed.
     ///
     /// \param done If true, this signals the system there is an answer
@@ -99,6 +99,8 @@ public:
     virtual DNSServer* clone() { return (self_->clone()); }
     //@}
 
+    virtual void setTCPRecvTimeout(size_t) {}
+
 protected:
     /// \brief Lookup handler object.
     ///

src/lib/asiodns/dns_service.cc

@@ -53,13 +53,25 @@ public:
     SimpleCallback* checkin_;
     DNSLookup* lookup_;
     DNSAnswer* answer_;
+    size_t tcp_recv_timeout_;
 
     template<class Ptr, class Server> void addServerFromFD(int fd, int af) {
         Ptr server(new Server(io_service_.get_io_service(), fd, af, checkin_,
                               lookup_, answer_));
+        server->setTCPRecvTimeout(tcp_recv_timeout_);
         (*server)();
         servers_.push_back(server);
     }
+
+    void setTCPRecvTimeout(size_t timeout) {
+        // Store it for future tcp connections
+        tcp_recv_timeout_ = timeout;
+        // Update existing (TCP) Servers
+        std::vector<DNSServerPtr>::iterator it = servers_.begin();
+        for (; it != servers_.end(); ++it) {
+            (*it)->setTCPRecvTimeout(timeout);
+        }
+    }
 };
 
 DNSService::DNSService(IOService& io_service, SimpleCallback* checkin,
@@ -99,5 +111,10 @@ DNSService::clearServers() {
     impl_->servers_.clear();
 }
 
+void
+DNSService::setTCPRecvTimeout(size_t timeout) {
+    impl_->setTCPRecvTimeout(timeout);
+}
+
 } // namespace asiodns
 } // namespace isc

src/lib/asiodns/dns_service.h

@@ -84,6 +84,9 @@ public:
                                     ServerFlag options = SERVER_DEFAULT) = 0;
     virtual void clearServers() = 0;
 
+    /// Sets the timeout TODO
+    virtual void setTCPRecvTimeout(size_t timeout) = 0;
+
     virtual asiolink::IOService& getIOService() = 0;
 };
 
@@ -187,6 +190,7 @@ public:
     /// \return IOService object for this DNS service.
     virtual asiolink::IOService& getIOService() { return (io_service_);}
 
+    virtual void setTCPRecvTimeout(size_t timeout);
 private:
     DNSServiceImpl* impl_;
     asiolink::IOService& io_service_;

src/lib/asiodns/tcp_server.cc

@@ -47,6 +47,8 @@ namespace asiodns {
 /// The following functions implement the \c TCPServer class.
 ///
 /// The constructor
+// timeout is initialized to be sure, but it should be updated
+// quite immediately anyway
 TCPServer::TCPServer(io_service& io_service, int fd, int af,
                      const SimpleCallback* checkin,
                      const DNSLookup* lookup,
@@ -70,6 +72,9 @@ TCPServer::TCPServer(io_service& io_service, int fd, int af,
         // it
         isc_throw(IOError, exception.what());
     }
+    // Set it to some value. It should be set to the right one
+    // immediately, but set something non-zero just in case.
+    tcp_recv_timeout_.reset(new size_t(1000));
 }
 
 namespace {
@@ -82,7 +87,6 @@ namespace {
     {
         if (error != asio::error::operation_aborted) {
             socket.cancel();
-        } else {
         }
     }
 }
@@ -130,7 +134,8 @@ TCPServer::operator()(asio::error_code ec, size_t length) {
         data_.reset(new char[MAX_LENGTH]);
 
         timeout_.reset(new asio::deadline_timer(io_));
-        timeout_->expires_from_now(boost::posix_time::milliseconds(1000));
+        timeout_->expires_from_now(
+            boost::posix_time::milliseconds(*tcp_recv_timeout_));
         timeout_->async_wait(boost::bind(&do_timeout, boost::ref(*socket_),
                              asio::placeholders::error));
 

src/lib/asiodns/tcp_server.h

@@ -61,6 +61,16 @@ public:
         return (s);
     }
 
+    /// \brief Set the read timeout
+    ///
+    /// If the client does not send (all) query data within this
+    /// timeframe, the connection is dropped
+    ///
+    /// \param timeout in milliseconds
+    virtual void setTCPRecvTimeout(size_t timeout) {
+        *tcp_recv_timeout_ = timeout;
+    }
+
 private:
     enum { MAX_LENGTH = 65535 };
     static const size_t TCP_MESSAGE_LENGTHSIZE = 2;
@@ -128,6 +138,11 @@ private:
     // that outlives the operator() call and is copyable (for CORO_FORK)
     // even though it is only set after fork
     boost::shared_ptr<asio::deadline_timer> timeout_;
+
+    // Timeout value to use in the timer;
+    // this, too, is a pointer, so that it can be updated whithout restarting
+    // the server
+    boost::shared_ptr<size_t> tcp_recv_timeout_;
 };
 
 } // namespace asiodns

src/lib/asiodns/tests/dns_server_unittest.cc

@@ -259,7 +259,7 @@ class TCPClient : public SimpleClient {
     static const unsigned int SERVER_TIME_OUT = 2;
     TCPClient(asio::io_service& service, const ip::tcp::endpoint& server)
         : SimpleClient(service, SERVER_TIME_OUT),
-          send_data_(true), send_data_len_(true)
+          send_data_delay_(0), send_data_len_delay_(0)
     {
         server_ = server;
         socket_.reset(new ip::tcp::socket(service));
@@ -281,14 +281,12 @@ class TCPClient : public SimpleClient {
                                 std::string(received_data_ + 2));
     }
 
-    // if set to false, does not actually send data length
-    void setSendDataLen(bool send_data_len) {
-        send_data_len_ = send_data_len;
+    void setSendDataLenDelay(size_t send_data_len_delay) {
+        send_data_len_delay_ = send_data_len_delay;
     }
 
-    // if set to false, does not actually send data
-    void setSendData(bool send_data) {
-        send_data_ = send_data;
+    void setSendDataDelay(size_t send_data_delay) {
+        send_data_delay_ = send_data_delay;
     }
 
     private:
@@ -297,8 +295,9 @@ class TCPClient : public SimpleClient {
     }
 
     void connectHandler(const asio::error_code& error) {
-        if (!error && send_data_len_) {
+        if (!error) {
             data_to_send_len_ = htons(data_to_send_len_);
+            sleep(send_data_len_delay_);
             socket_->async_send(buffer(&data_to_send_len_, 2),
                                 boost::bind(&TCPClient::sendMessageBodyHandler,
                                             this, _1, _2));
@@ -308,7 +307,8 @@ class TCPClient : public SimpleClient {
     void sendMessageBodyHandler(const asio::error_code& error,
                                 size_t send_bytes)
     {
-        if (!error && send_bytes == 2 && send_data_) {
+        if (!error && send_bytes == 2 && send_data_len_delay_ == 0) {
+            sleep(send_data_delay_);
             socket_->async_send(buffer(data_to_send_.c_str(),
                                        data_to_send_.size() + 1),
                     boost::bind(&TCPClient::finishSendHandler, this, _1, _2));
@@ -327,8 +327,11 @@ class TCPClient : public SimpleClient {
     ip::tcp::endpoint server_;
     std::string data_to_send_;
     uint16_t data_to_send_len_;
-    bool send_data_;
-    bool send_data_len_;
+
+    // if 0, send body immediately
+    // if >0, send after the delay (in seconds)
+    size_t send_data_delay_;
+    size_t send_data_len_delay_;
 };
 
 // \brief provide the context which including two clients and
@@ -579,7 +582,8 @@ TYPED_TEST(DNSServerTest, stopTCPServerAfterOneQuery) {
 }
 
 TYPED_TEST(DNSServerTest, TCPTimeoutOnLen) {
-    this->tcp_client_->setSendDataLen(false);
+    this->tcp_server_->setTCPRecvTimeout(100);
+    this->tcp_client_->setSendDataLenDelay(2);
     this->testStopServerByStopper(this->tcp_server_, this->tcp_client_,
                                   this->tcp_client_);
     EXPECT_EQ("", this->tcp_client_->getReceivedData());
@@ -587,11 +591,23 @@ TYPED_TEST(DNSServerTest, TCPTimeoutOnLen) {
 }
 
 TYPED_TEST(DNSServerTest, TCPTimeout) {
-    this->tcp_client_->setSendData(false);
+    // set delay higher than timeout
+    this->tcp_server_->setTCPRecvTimeout(100);
+    this->tcp_client_->setSendDataDelay(2);
     this->testStopServerByStopper(this->tcp_server_, this->tcp_client_,
                                   this->tcp_client_);
     EXPECT_EQ("", this->tcp_client_->getReceivedData());
-    EXPECT_FALSE(this->serverStopSucceed());
+    EXPECT_TRUE(this->serverStopSucceed());
+}
+
+TYPED_TEST(DNSServerTest, TCPNoTimeout) {
+    // set delay lower than timeout
+    this->tcp_server_->setTCPRecvTimeout(3000);
+    this->tcp_client_->setSendDataDelay(1);
+    this->testStopServerByStopper(this->tcp_server_, this->tcp_client_,
+                                  this->tcp_client_);
+    EXPECT_EQ("BIND10 is awesome", this->tcp_client_->getReceivedData());
+    EXPECT_TRUE(this->serverStopSucceed());
 }
 
 // Test whether tcp server stopped successfully before server start to serve

src/lib/testutils/mockups.h

@@ -138,9 +138,18 @@ public:
         return (udp_fd_params_);
     }
 
+    virtual void setTCPRecvTimeout(size_t timeout) {
+        tcp_recv_timeout_ = timeout;
+    }
+
+    size_t getTCPRecvTimeout() {
+        return tcp_recv_timeout_;
+    }
+
 private:
     std::vector<std::pair<int, int> > tcp_fd_params_;
     std::vector<UDPFdParams> udp_fd_params_;
+    size_t tcp_recv_timeout_;
 };
 
 // A nonoperative DNSServer object to be used in calls to processMessage().