Batch of bugfixes for new RestAPI

common/include/RESTHttpsServer.h

@@ -120,7 +120,7 @@ public:
             return;
         }
         ServerLOG(info) << "Starting...";
-        _serverThread = std::thread(RESTHttpsServer::run, this);
+        _serverThread = std::thread(&RESTHttpsServer::run, this);
 
         _isRunning = true;
         ServerLOG(info) << "Started!";
@@ -190,16 +190,13 @@ private:
      *        On a connection attempt handle_request() is called.
      */
     void startAccept() {
-        // This will receive the new connection
-        tcp::socket socket{_io};
-
         // Start asynchronous wait until we get a connection.
         // On connection start launch the session, transferring ownership of the socket
-        _acceptor->async_accept(socket, _remoteEndpoint,
+        _acceptor->async_accept(*_socket, _remoteEndpoint,
                                 std::bind(&RESTHttpsServer::handle_session,
                                           this,
-                                          std::move(socket),
-                                          std::ref(_ctx)));
+                                          std::ref(*_socket),
+                                          std::ref(*_ctx)));
     }
 
     /**
@@ -283,6 +280,7 @@ private:
 
     std::unique_ptr<boost::asio::io_context> _io; /**< Central io_context for all I/O */
     std::unique_ptr<ssl::context> _ctx; /**< SSL context hold the certificates and is required for https support */
+    std::unique_ptr<tcp::socket> _socket; /** Socket object used for connections */
     std::unique_ptr<tcp::acceptor> _acceptor; /**< Acceptor receives incoming connections */
 
     tcp::endpoint _remoteEndpoint; /**< Used to store information about the connecting client endpoint */

common/src/RESTHttpsServer.cpp

@@ -19,9 +19,9 @@ template<class Stream>
 struct send_lambda {
     Stream& stream_;
     bool& close_;
-    beast::error_code& ec_;
+    boost::beast::error_code& ec_;
 
-    explicit send_lambda(Stream& stream, bool& close, beast::error_code& ec) :
+    explicit send_lambda(Stream& stream, bool& close, boost::beast::error_code& ec) :
             stream_(stream),
             close_(close),
             ec_(ec) {
@@ -42,8 +42,6 @@ struct send_lambda {
 
 RESTHttpsServer::RESTHttpsServer(serverSettings_t settings) :
     _isRunning(false) {
-    auto const address = boost::asio::ip::make_address(settings.host);
-    auto const port    = static_cast<unsigned short>(std::stoul(settings.port));
 
     _io = std::unique_ptr<boost::asio::io_context>(
             new boost::asio::io_context(1));
@@ -67,8 +65,19 @@ RESTHttpsServer::RESTHttpsServer(serverSettings_t settings) :
     _ctx->use_private_key_file(settings.privateKey, ssl::context::pem);
     _ctx->use_tmp_dh_file(settings.dhFile);
 
-    _acceptor = std::unique_ptr<tcp::acceptor>(new tcp::acceptor(_io, {address, port}));
-    _acceptor->set_option(tcp::acceptor::reuse_address(true));
+    // This will receive the new connection
+    _socket = std::unique_ptr<tcp::socket>(new tcp::socket(*_io));
+
+    try {
+        auto const address = boost::asio::ip::make_address(settings.host);
+        auto const port    = static_cast<unsigned short>(std::stoul(settings.port));
+
+        _acceptor = std::unique_ptr<tcp::acceptor>(new tcp::acceptor(*_io, {address, port}));
+        _acceptor->set_option(tcp::acceptor::reuse_address(true));
+    } catch (const std::exception& e) {
+        LOG(fatal) << "RestAPI address invalid! Please make sure IP address and port are valid!";
+        throw;
+    }
 }
 
 
@@ -89,51 +98,54 @@ void RESTHttpsServer::handle_session(tcp::socket& socket, ssl::context& ctx) {
         goto serverError;
     }
 
-    // This buffer is required to persist across reads
-    boost::beast::flat_buffer buffer;
-
-    // This lambda is used to send messages
-    send_lambda<boost::beast::ssl_stream<tcp::socket&>> lambda{stream, close, ec};
-
-    while(true) {
-        // Read a request
-        http::request<http::string_body> req;
-        http::read(stream, buffer, req, ec);
-        if(ec == http::error::end_of_stream) {
-            break;
-        }
-        if(ec) {
-            ServerLOG(error) << "read: " << ec.message();
-            goto serverError;
-        }
-
-        if(!validateUser(req, lambda)) {
-            break;
-        }
-
-        // Send the response
-        handle_request(req, lambda);
-
-        if(ec) {
-            ServerLOG(error) << "write: " << ec.message();
-            goto serverError;
-        }
-        if(close) {
-            // This means we should close the connection, usually because
-            // the response indicated the "Connection: close" semantic.
-            break;
+    {//scope, so any goto before does not cross variable initialization
+        // This buffer is required to persist across reads
+        boost::beast::flat_buffer buffer;
+
+        // This lambda is used to send messages
+        send_lambda<boost::beast::ssl_stream<tcp::socket&>> lambda{stream, close, ec};
+
+        while(true) {
+            // Read a request
+            http::request<http::string_body> req;
+            http::read(stream, buffer, req, ec);
+            if(ec == http::error::end_of_stream) {
+                break;
+            }
+            if(ec) {
+                ServerLOG(error) << "read: " << ec.message();
+                goto serverError;
+            }
+
+            if(!validateUser(req, lambda)) {
+                break;
+            }
+
+            // Send the response
+            handle_request(req, lambda);
+
+            if(ec) {
+                ServerLOG(error) << "write: " << ec.message();
+                goto serverError;
+            }
+            if(close) {
+                // This means we should close the connection, usually because
+                // the response indicated the "Connection: close" semantic.
+                break;
+            }
         }
     }
 
     // Perform the SSL shutdown
     stream.shutdown(ec);
-    if(ec) {
-        ServerLOG(error) << "shutdown: " << ec.message();
-        goto serverError;
-    }
-    // At this point the connection is closed gracefully
+    if(ec) { ServerLOG(error) << "stream shutdown: " << ec.message(); }
+
+serverError:
+    socket.shutdown(tcp::socket::shutdown_both, ec);
+    if(ec) { ServerLOG(error) << "socket shutdown: " << ec.message(); }
 
-    serverError:
+    socket.close(ec);
+    if(ec) { ServerLOG(error) << "socket close: " << ec.message(); }
     startAccept();
 }
 
@@ -148,7 +160,7 @@ void RESTHttpsServer::handle_request(http::request<Body>& req, Send&& send) {
 
     //split target and find matching endpoint handler
     queries_t queries;
-    const std::string endpointName = splitUri(req.target(), queries);
+    const std::string endpointName = splitUri(req.target().to_string(), queries);
 
     //Look up the endpoint
     try {
@@ -159,8 +171,8 @@ void RESTHttpsServer::handle_request(http::request<Body>& req, Send&& send) {
             ServerLOG(info) << req.method_string() << " " << endpointName << " requested";
             endpoint.second(res, queries);
         } else {
-            const std::string msg = "Request method " + req.method_string() +
-                                    " does not match endpoint " + endpointName;
+            const std::string msg = "Request method " + req.method_string().to_string() +
+                                    " does not match endpoint " + endpointName + "\n";
             ServerLOG(info) << msg;
             res.result(http::status::bad_request);
             res.body() = msg;
@@ -168,7 +180,7 @@ void RESTHttpsServer::handle_request(http::request<Body>& req, Send&& send) {
     } catch (const std::out_of_range& e) {
         ServerLOG(info) << "Requested endpoint " << endpointName << " not found";
         res.result(http::status::not_implemented);
-        res.body() = "Invalid endpoint";
+        res.body() = "Invalid endpoint\n";
     }
 
 #ifdef DEBUG
@@ -182,38 +194,12 @@ void RESTHttpsServer::handle_request(http::request<Body>& req, Send&& send) {
 template<class Body, class Send>
 bool RESTHttpsServer::validateUser(const http::request<Body>& req, Send&& send) {
 
-    // Returns a unauthorized response
-    auto const unauthorized = [&req]() {
-        http::response<http::string_body> res {http::status::unauthorized, req.version()};
-        res.set(http::field::server, SERVER_STRING);
-        res.set(http::field::content_type, "text/plain");
-        res.keep_alive(req.keep_alive());
-        res.body() = "Unauthorized access!\n";
-        res.prepare_payload();
-        return res;
-    };
-
-    // Returns a forbidden response
-    auto const forbidden = [&req]() {
-        http::response<http::string_body> res {http::status::forbidden, req.version()};
-        res.set(http::field::server, SERVER_STRING);
-        res.set(http::field::content_type, "text/plain");
-        res.keep_alive(req.keep_alive());
-        res.body() = "Insufficient permissions!\n";
-        res.prepare_payload();
-        return res;
-    };
-
-    // Returns a not implemented response
-    auto const not_implemented = [&req]() {
-        http::response<http::string_body> res {http::status::not_implemented, req.version()};
-        res.set(http::field::server, SERVER_STRING);
-        res.set(http::field::content_type, "text/plain");
-        res.keep_alive(req.keep_alive());
-        res.body() = "Request method not supported!\n";
-        res.prepare_payload();
-        return res;
-    };
+    http::response<http::string_body> res {http::status::unauthorized, req.version()};
+    res.set(http::field::server, SERVER_STRING);
+    res.set(http::field::content_type, "text/plain");
+    res.keep_alive(req.keep_alive());
+    res.body() = "Unauthorized access!\n";
+    res.prepare_payload();
 
     //GET /help does not need any authorization
     if (req.target() == "/help" && req.method() == http::verb::get) {
@@ -224,10 +210,10 @@ bool RESTHttpsServer::validateUser(const http::request<Body>& req, Send&& send)
     std::string credentials;
 
     try {
-        auth = req.base().at(http::field::authorization);
+        auth = req.base().at(http::field::authorization).to_string();
     } catch (const std::out_of_range& e) {
         ServerLOG(info) << "No credentials were provided";
-        send(unauthorized);
+        send(std::move(res));
         return false;
     }
 
@@ -261,13 +247,13 @@ bool RESTHttpsServer::validateUser(const http::request<Body>& req, Send&& send)
         userData = _users.at(usr);
     } catch (const std::out_of_range& e) {
         ServerLOG(warning) << "User does not exist: " << usr;
-        send(unauthorized);
+        send(std::move(res));
         return false;
     }
 
     if (pwd != userData.first) {
         ServerLOG(warning) << "Invalid password provided: " << usr << ":" << pwd;
-        send(unauthorized);
+        send(std::move(res));
         return false;
     }
 
@@ -294,12 +280,18 @@ bool RESTHttpsServer::validateUser(const http::request<Body>& req, Send&& send)
     try {
         if (!userData.second.test(perm)) {
             ServerLOG(warning) << "User " << usr << " has insufficient permissions";
-            send(forbidden);
+            res.result(http::status::forbidden);
+            res.body() = "Insufficient permissions\n";
+            res.prepare_payload();
+            send(std::move(res));
             return false;
         }
     } catch (const std::out_of_range& e) {
         ServerLOG(error) << "Permission out of range (method not supported)";
-        send(not_implemented);
+        res.result(http::status::not_implemented);
+        res.body() = "Request method not supported!\n";
+        res.prepare_payload();
+        send(std::move(res));
         return false;
     }
 

dcdbpusher/RestAPI.h

@@ -334,7 +334,7 @@ private:
     // Return true if plugin was given, false otherwise.
     inline bool hasPlugin(const std::string& plugin, http::response<http::string_body>& res) {
         if (plugin == "") {
-            const std::string err = "Request malformed: plugin query missing";
+            const std::string err = "Request malformed: plugin query missing\n";
             RESTAPILOG(error) << err;
             res.body() = err;
             res.result(http::status::bad_request);
@@ -350,7 +350,7 @@ private:
     // Return true if loaded, false otherwise.
     inline bool managerLoaded(http::response<http::string_body>& res) {
         if (_manager->getStatus() != AnalyticsManager::LOADED) {
-            const std::string err = "AnalyticsManager is not loaded!";
+            const std::string err = "AnalyticsManager is not loaded!\n";
             RESTAPILOG(error) << err;
             res.body() = err;
             res.result(http::status::internal_server_error);

dcdbpusher/config/dcdbpusher.conf

@@ -11,7 +11,7 @@ global {
 }
 
 restAPI {
-	address	localhost:8000
+	address	127.0.0.1:8000
 	certificate	../../deps/openssl-1.0.2l/certs/demo/ca-cert.pem
 	privateKey	../../deps/openssl-1.0.2l/certs/demo/ca-cert.pem
 	dhFile		../../deps/openssl-1.0.2l/crypto/dh/dh2048.pem