tcp endpoint created before server creation, invalid address error is…

… logged now, server runs into timeout in that case added testcase for invalid address
CrowCpp · Dec 29, 2024 · de64c1f · de64c1f
1 parent d63c921
commit de64c1f
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 16 deletions.
diff --git a/include/crow/app.h b/include/crow/app.h
@@ -373,7 +373,7 @@ namespace crow
         }
 
         /// \brief Get the number of threads that server is using
-        std::uint16_t concurrency()
+        std::uint16_t concurrency() const
         {
             return concurrency_;
         }
@@ -514,11 +514,18 @@ namespace crow
 #endif
             validate();
 
+            error_code ec;
+            asio::ip::address addr = asio::ip::make_address(bindaddr_,ec);
+            if (ec){
+                CROW_LOG_ERROR << ec.message() << " - Can not create valid ip address from string: \"" << bindaddr_ << "\"";
+                return;
+            }
+            tcp::endpoint endpoint(addr, port_);
 #ifdef CROW_ENABLE_SSL
             if (ssl_used_)
             {
                 router_.using_ssl = true;
-                ssl_server_ = std::move(std::unique_ptr<ssl_server_t>(new ssl_server_t(this, bindaddr_, port_, server_name_, &middlewares_, concurrency_, timeout_, &ssl_context_)));
+                ssl_server_ = std::move(std::unique_ptr<ssl_server_t>(new ssl_server_t(this, endpoint, server_name_, &middlewares_, concurrency_, timeout_, &ssl_context_)));
                 ssl_server_->set_tick_function(tick_interval_, tick_function_);
                 ssl_server_->signal_clear();
                 for (auto snum : signals_)
@@ -531,7 +538,7 @@ namespace crow
             else
 #endif
             {
-                server_ = std::move(std::unique_ptr<server_t>(new server_t(this, bindaddr_, port_, server_name_, &middlewares_, concurrency_, timeout_, nullptr)));
+                server_ = std::move(std::unique_ptr<server_t>(new server_t(this, endpoint, server_name_, &middlewares_, concurrency_, timeout_, nullptr)));
                 server_->set_tick_function(tick_interval_, tick_function_);
                 for (auto snum : signals_)
                 {

diff --git a/include/crow/http_server.h b/include/crow/http_server.h
@@ -42,16 +42,14 @@ namespace crow // NOTE: Already documented in "crow/app.h"
     class Server
     {
     public:
-        Server(Handler* handler, std::string bindaddr, uint16_t port, std::string server_name = std::string("Crow/") + VERSION, std::tuple<Middlewares...>* middlewares = nullptr, uint16_t concurrency = 1, uint8_t timeout = 5, typename Adaptor::context* adaptor_ctx = nullptr):
-          acceptor_(io_service_, tcp::endpoint(asio::ip::address::from_string(bindaddr), port)),
+        Server(Handler* handler, const tcp::endpoint& endpoint, std::string server_name = std::string("Crow/") + VERSION, std::tuple<Middlewares...>* middlewares = nullptr, uint16_t concurrency = 1, uint8_t timeout = 5, typename Adaptor::context* adaptor_ctx = nullptr):
+          acceptor_(io_service_, endpoint),
           signals_(io_service_),
           tick_timer_(io_service_),
           handler_(handler),
           concurrency_(concurrency),
           timeout_(timeout),
           server_name_(server_name),
-          port_(port),
-          bindaddr_(bindaddr),
           task_queue_length_pool_(concurrency_ - 1),
           middlewares_(middlewares),
           adaptor_ctx_(adaptor_ctx)
@@ -150,11 +148,12 @@ namespace crow // NOTE: Already documented in "crow/app.h"
                   });
             }
 
-            port_ = acceptor_.local_endpoint().port();
-            handler_->port(port_);
+            handler_->port(acceptor_.local_endpoint().port());
 
 
-            CROW_LOG_INFO << server_name_ << " server is running at " << (handler_->ssl_used() ? "https://" : "http://") << bindaddr_ << ":" << acceptor_.local_endpoint().port() << " using " << concurrency_ << " threads";
+            CROW_LOG_INFO << server_name_
+                          << " server is running at " << (handler_->ssl_used() ? "https://" : "http://")
+                          << acceptor_.local_endpoint().address() << ":" << acceptor_.local_endpoint().port() << " using " << concurrency_ << " threads";
             CROW_LOG_INFO << "Call `app.loglevel(crow::LogLevel::Warning)` to hide Info level logs.";
 
             signals_.async_wait(
@@ -192,7 +191,7 @@ namespace crow // NOTE: Already documented in "crow/app.h"
             io_service_.stop(); // Close main io_service
         }
 
-        uint16_t port(){
+        uint16_t port() const {
             return acceptor_.local_endpoint().port();
         }
 
@@ -293,8 +292,6 @@ namespace crow // NOTE: Already documented in "crow/app.h"
         uint16_t concurrency_{2};
         std::uint8_t timeout_;
         std::string server_name_;
-        uint16_t port_;
-        std::string bindaddr_;
         std::vector<std::atomic<unsigned int>> task_queue_length_pool_;
 
         std::chrono::milliseconds tick_interval_;

diff --git a/tests/unittest.cpp b/tests/unittest.cpp
@@ -643,7 +643,22 @@ TEST_CASE("server_handling_error_request")
     app.stop();
 } // server_handling_error_request
 
-TEST_CASE("server_dynamic_port_allication")
+TEST_CASE("server_invalid_ip_address")
+{
+    SimpleApp app;
+    CROW_ROUTE(app, "/")
+    ([] {
+        return "A";
+    });
+    auto _ = app.bindaddr("192.").port(45451).run_async();
+    auto state = app.wait_for_server_start();
+
+    // we should run into a timeout as the server will not started
+    CHECK(state==cv_status::timeout);
+} // server_invalid_ip_address
+
+
+TEST_CASE("server_dynamic_port_allocation")
 {
     SimpleApp app;
     CROW_ROUTE(app, "/")
@@ -659,7 +674,7 @@ TEST_CASE("server_dynamic_port_allication")
           asio::ip::address::from_string(LOCALHOST_ADDRESS), app.port()));
     }
     app.stop();
-} // server_dynamic_port_allication
+} // server_dynamic_port_allocation
 
 TEST_CASE("server_handling_error_request_http_version")
 {
@@ -1520,7 +1535,9 @@ struct NullSimpleMiddleware
 TEST_CASE("middleware_simple")
 {
     App<NullMiddleware, NullSimpleMiddleware> app;
-    decltype(app)::server_t server(&app, LOCALHOST_ADDRESS, 45451);
+    asio::ip::address adr = asio::ip::make_address(LOCALHOST_ADDRESS);
+    tcp::endpoint ep(adr,45451);
+    decltype(app)::server_t server(&app, ep);
     CROW_ROUTE(app, "/")
     ([&](const crow::request& req) {
         app.get_context<NullMiddleware>(req);