Parcourir la source

[5099] Protect against exceptions in the HttpListener and HttpConnection.

Marcin Siodelski il y a 8 ans
Parent
commit
d61c85b7f1

+ 65 - 14
src/lib/http/connection.cc

@@ -49,7 +49,13 @@ void
 HttpConnection::asyncAccept() {
     HttpAcceptorCallback cb = boost::bind(&HttpConnection::acceptorCallback,
                                           this, _1);
-    acceptor_.asyncAccept(socket_, cb);
+    try {
+        acceptor_.asyncAccept(socket_, cb);
+
+    } catch (const std::exception& ex) {
+        isc_throw(HttpConnectionError, "unable to start accepting TCP "
+                  "connections: " << ex.what());
+    }
 }
 
 void
@@ -59,17 +65,29 @@ HttpConnection::close() {
 
 void
 HttpConnection::doRead() {
-    TCPEndpoint endpoint;
-    socket_.asyncReceive(static_cast<void*>(buf_.data()), buf_.size(), 0, &endpoint,
-                         socket_callback_);
+    try {
+        TCPEndpoint endpoint;
+        socket_.asyncReceive(static_cast<void*>(buf_.data()), buf_.size(),
+                             0, &endpoint, socket_callback_);
+
+    } catch (const std::exception& ex) {
+        isc_throw(HttpConnectionError, "unable to start asynchronous HTTP message"
+                  " receive over TCP socket: " << ex.what());
+    }
 }
 
 void
 HttpConnection::doWrite() {
     if (!output_buf_.empty()) {
-        socket_.asyncSend(output_buf_.data(),
-                          output_buf_.length(),
-                          socket_write_callback_);
+        try {
+            socket_.asyncSend(output_buf_.data(),
+                              output_buf_.length(),
+                              socket_write_callback_);
+
+        } catch (const std::exception& ex) {
+            isc_throw(HttpConnectionError, "unable to start asynchronous HTTP"
+                      " message write over TCP socket: " << ex.what());
+        }
     }
 }
 
@@ -79,14 +97,24 @@ HttpConnection::acceptorCallback(const boost::system::error_code& ec) {
         return;
     }
 
-    if (ec) {
-        connection_pool_.stop(shared_from_this());
+    try {
+        if (ec) {
+            connection_pool_.stop(shared_from_this());
+        }
+    } catch (...) {
     }
 
     acceptor_callback_(ec);
 
-    if (!ec) {
-        doRead();
+    try {
+        if (!ec) {
+            doRead();
+        }
+    } catch (const std::exception& ex) {
+        try {
+            connection_pool_.stop(shared_from_this());
+        } catch (...) {
+        }
     }
 
 }
@@ -97,7 +125,14 @@ HttpConnection::socketReadCallback(boost::system::error_code ec, size_t length)
     parser_->postBuffer(static_cast<void*>(buf_.data()), length);
     parser_->poll();
     if (parser_->needData()) {
-        doRead();
+        try {
+            doRead();
+        } catch (const std::exception& ex) {
+            try {
+                connection_pool_.stop(shared_from_this());
+            } catch (...) {
+            }
+        }
 
     } else {
         try {
@@ -106,7 +141,15 @@ HttpConnection::socketReadCallback(boost::system::error_code ec, size_t length)
         }
         HttpResponsePtr response = response_creator_->createHttpResponse(request_);
         output_buf_ = response->toString();
-        doWrite();
+        try {
+            doWrite();
+
+        } catch (const std::exception& ex) {
+            try {
+                connection_pool_.stop(shared_from_this());
+            } catch (...) {
+            }
+        }
     }
 }
 
@@ -115,7 +158,15 @@ HttpConnection::socketWriteCallback(boost::system::error_code ec,
                                     size_t length) {
     if (length <= output_buf_.size()) {
         output_buf_.erase(0, length);
-        doWrite();
+        try {
+            doWrite();
+
+        } catch (const std::exception& ex) {
+            try {
+                connection_pool_.stop(shared_from_this());
+            } catch (...) {
+            }
+        }
 
     } else {
         output_buf_.clear();

+ 7 - 0
src/lib/http/connection.h

@@ -20,6 +20,13 @@
 namespace isc {
 namespace http {
 
+class HttpConnectionError : public Exception {
+public:
+    HttpConnectionError(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
+
 class HttpConnectionPool;
 
 class HttpConnection;

+ 23 - 9
src/lib/http/listener.cc

@@ -17,8 +17,18 @@ HttpListener::HttpListener(IOService& io_service,
                            const unsigned short server_port,
                            const HttpResponseCreatorFactoryPtr& creator_factory)
     : io_service_(io_service), acceptor_(io_service),
-      endpoint_(server_address, server_port),
-      creator_factory_(creator_factory) {
+      endpoint_(), creator_factory_(creator_factory) {
+    try {
+        endpoint_.reset(new TCPEndpoint(server_address, server_port));
+
+    } catch (...) {
+        isc_throw(HttpListenerError, "unable to create TCP endpoint for "
+                  << server_address << ":" << server_port);
+    }
+    if (!creator_factory_) {
+        isc_throw(HttpListenerError, "HttpResponseCreatorFactory must not"
+                  " be null");
+    }
 }
 
 HttpListener::~HttpListener() {
@@ -27,10 +37,16 @@ HttpListener::~HttpListener() {
 
 void
 HttpListener::start() {
-    acceptor_.open(endpoint_);
-    acceptor_.setOption(HttpAcceptor::ReuseAddress(true));
-    acceptor_.bind(endpoint_);
-    acceptor_.listen();
+    try {
+        acceptor_.open(*endpoint_);
+        acceptor_.setOption(HttpAcceptor::ReuseAddress(true));
+        acceptor_.bind(*endpoint_);
+        acceptor_.listen();
+
+    } catch (const boost::system::system_error& ex) {
+        isc_throw(HttpListenerError, "unable to setup TCP acceptor for "
+                  "listening to the incoming HTTP requests: " << ex.what());
+    } 
 
     accept();
 }
@@ -55,9 +71,7 @@ HttpListener::accept() {
 
 void
 HttpListener::acceptHandler(const boost::system::error_code& ec) {
-    if (!ec) {
-        accept();
-    }
+    accept();
 }
 
 

+ 9 - 1
src/lib/http/listener.h

@@ -10,14 +10,22 @@
 #include <asiolink/io_address.h>
 #include <asiolink/io_service.h>
 #include <asiolink/tcp_endpoint.h>
+#include <exceptions/exceptions.h>
 #include <http/connection.h>
 #include <http/connection_pool.h>
 #include <http/http_acceptor.h>
 #include <http/response_creator_factory.h>
+#include <boost/scoped_ptr.hpp>
 
 namespace isc {
 namespace http {
 
+class HttpListenerError : public Exception {
+public:
+    HttpListenerError(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
 class HttpListener {
 public:
 
@@ -40,7 +48,7 @@ private:
 
     asiolink::IOService& io_service_;
     HttpAcceptor acceptor_;
-    asiolink::TCPEndpoint endpoint_;
+    boost::scoped_ptr<asiolink::TCPEndpoint> endpoint_;
     HttpConnectionPool connections_;
     HttpResponseCreatorFactoryPtr creator_factory_;
 

+ 20 - 0
src/lib/http/tests/listener_unittests.cc

@@ -14,6 +14,7 @@
 #include <http/response_json.h>
 #include <http/tests/response_test.h>
 #include <boost/asio/buffer.hpp>
+#include <boost/asio/ip/tcp.hpp>
 #include <boost/bind.hpp>
 #include <gtest/gtest.h>
 #include <list>
@@ -299,4 +300,23 @@ TEST_F(HttpListenerTest, badRequest) {
               client->getResponse());
 }
 
+TEST_F(HttpListenerTest, invalidFactory) {
+    EXPECT_THROW(HttpListener(io_service_, IOAddress(SERVER_ADDRESS),
+                              SERVER_PORT, HttpResponseCreatorFactoryPtr()),
+                 HttpListenerError);
+}
+
+TEST_F(HttpListenerTest, addressInUse) {
+    boost::asio::ip::tcp::acceptor acceptor(io_service_.get_io_service());;
+    boost::asio::ip::tcp::endpoint
+        endpoint(boost::asio::ip::address::from_string(SERVER_ADDRESS),
+                 SERVER_PORT + 1);
+    acceptor.open(endpoint.protocol());
+    acceptor.bind(endpoint);
+
+    HttpListener listener(io_service_, IOAddress(SERVER_ADDRESS),
+                          SERVER_PORT + 1, factory_);
+    EXPECT_THROW(listener.start(), HttpListenerError);
+}
+
 }