From 7972368faf578f72b05c7224a4c9db38b68ba5b0 Mon Sep 17 00:00:00 2001
From: Roel Standaert <roel@emweb.be>
Date: Wed, 8 Mar 2023 12:41:03 +0100
Subject: [PATCH 1/5] WT-11301: add WServer::addResource taking a shared_ptr

This deprecates the version of addResource that takes a raw pointer.
Using the shared_ptr we can enforce that resources outlive the server
instance, so requests are not at risk of being routed to a deleted
resource.

All of the examples and tests have been changed to use the new version
of addResource.

====================================================================
The original patch with test case is available at:
https://redmine.webtoolkit.eu/attachments/download/5517/0002-WT-11301-add-WServer-addResource-taking-a-shared_ptr.patch
from: https://redmine.webtoolkit.eu/issues/11301

NOTE: This is not exactly the same commit as supplied by Roel. The
patch to ReleaseNotes.html did not apply cleanly and was omitted.
Also two hunks in test/http/HttpClientServerTest.C were offset.
---
 examples/blog/blog.C                   |  4 ++--
 examples/feature/oidc/Oidc.C           |  8 ++++----
 examples/feature/urlparams/urlparams.C | 12 ++++++------
 examples/te-benchmark/benchmark.cpp    | 18 ++++++------------
 examples/wt-homepage/main.C            |  9 +++++----
 src/Wt/Auth/OAuthService.C             | 10 ++++++----
 src/Wt/Auth/Saml/Service.C             | 10 ++++------
 src/Wt/Auth/Saml/Service.h             |  2 --
 src/Wt/WServer.C                       | 13 +++++++++++++
 src/Wt/WServer.h                       | 21 +++++++++++++++++++++
 src/web/EntryPoint.C                   |  8 ++++++++
 src/web/EntryPoint.h                   |  2 ++
 test/http/HttpClientServerTest.C       |  7 ++++---
 13 files changed, 81 insertions(+), 43 deletions(-)

diff --git a/examples/blog/blog.C b/examples/blog/blog.C
index 6b4bcc81..7e6dd1c4 100644
--- a/examples/blog/blog.C
+++ b/examples/blog/blog.C
@@ -44,9 +44,9 @@ int main(int argc, char **argv)
     std::unique_ptr<Wt::Dbo::SqlConnectionPool> blogDb
       = BlogSession::createConnectionPool(server.appRoot() + "blog.db");
 
-    BlogRSSFeed rssFeed(*blogDb, "Wt blog example", "", "It's just an example.");
+    auto rssFeed = std::make_shared<BlogRSSFeed>(*blogDb, "Wt blog example", "", "It's just an example.");
 
-    server.addResource(&rssFeed, FeedUrl);
+    server.addResource(rssFeed, FeedUrl);
     //When the blog application is deployed in ISAPI on the path "/blog"
     //the resources (css+images) are not fetched correctly
     server.addEntryPoint(Wt::EntryPointType::Application,
diff --git a/examples/feature/oidc/Oidc.C b/examples/feature/oidc/Oidc.C
index f0241858..8012d018 100644
--- a/examples/feature/oidc/Oidc.C
+++ b/examples/feature/oidc/Oidc.C
@@ -103,12 +103,12 @@ int main(int argc, char** argv)
   });
 
   Session tokenSession(dbPath);
-  Wt::Auth::OAuthTokenEndpoint tokenEndpoint{tokenSession.users(), deployUrl};
-  server.addResource(&tokenEndpoint, "/oauth2/token");
+  auto tokenEndpoint = std::make_shared<Wt::Auth::OAuthTokenEndpoint>(tokenSession.users(), deployUrl);
+  server.addResource(tokenEndpoint, "/oauth2/token");
 
   Session userInfoSession(dbPath);
-  Wt::Auth::OidcUserInfoEndpoint userInfoEndpoint{userInfoSession.users()};
-  server.addResource(&userInfoEndpoint, "/oidc/userinfo");
+  auto userInfoEndpoint = std::make_shared<Wt::Auth::OidcUserInfoEndpoint>(userInfoSession.users());
+  server.addResource(userInfoEndpoint, "/oidc/userinfo");
 
   Session::configureAuth();
 
diff --git a/examples/feature/urlparams/urlparams.C b/examples/feature/urlparams/urlparams.C
index 11458e4d..88f6e91f 100644
--- a/examples/feature/urlparams/urlparams.C
+++ b/examples/feature/urlparams/urlparams.C
@@ -45,15 +45,15 @@ public:
 
 int main(int argc, char *argv[])
 {
-  Resource resource;
-
   try {
     Wt::WServer server{argc, argv, WTHTTP_CONFIGURATION};
 
-    server.addResource(&resource, "/users");
-    server.addResource(&resource, "/users/${user}");
-    server.addResource(&resource, "/users/${user}/posts");
-    server.addResource(&resource, "/users/${user}/posts/${post}");
+    auto resource = std::make_shared<Resource>();
+
+    server.addResource(resource, "/users");
+    server.addResource(resource, "/users/${user}");
+    server.addResource(resource, "/users/${user}/posts");
+    server.addResource(resource, "/users/${user}/posts/${post}");
 
     server.run();
   } catch (const Wt::WServer::Exception &e) {
diff --git a/examples/te-benchmark/benchmark.cpp b/examples/te-benchmark/benchmark.cpp
index 3db3ee4c..66682b03 100644
--- a/examples/te-benchmark/benchmark.cpp
+++ b/examples/te-benchmark/benchmark.cpp
@@ -337,23 +337,17 @@ int main(int argc, char** argv) {
     bundle->use(server.appRoot() + "fortunes");
     server.setLocalizedStrings(bundle);
 
-    JsonResource jsonResource;
-    server.addResource(&jsonResource, "/json");
+    server.addResource(std::make_shared<JsonResource>(), "/json");
 
-    DbResource dbResource;
-    server.addResource(&dbResource, "/db");
+    server.addResource(std::make_shared<DbResource>(), "/db");
 
-    QueriesResource queriesResource;
-    server.addResource(&queriesResource, "/queries");
+    server.addResource(std::make_shared<QueriesResource>(), "/queries");
 
-    FortuneResource fortuneResource;
-    server.addResource(&fortuneResource, "/fortune");
+    server.addResource(std::make_shared<FortuneResource>(), "/fortune");
 
-    UpdateResource updateResource;
-    server.addResource(&updateResource, "/updates");
+    server.addResource(std::make_shared<UpdateResource>(), "/updates");
 
-    PlaintextResource plaintextResource;
-    server.addResource(&plaintextResource, "/plaintext");
+    server.addResource(std::make_shared<PlaintextResource>(), "/plaintext");
 
     if (server.start()) {
       int sig = Wt::WServer::waitForShutdown();
diff --git a/examples/wt-homepage/main.C b/examples/wt-homepage/main.C
index 2bce6611..b64342d9 100644
--- a/examples/wt-homepage/main.C
+++ b/examples/wt-homepage/main.C
@@ -26,11 +26,12 @@ int main(int argc, char **argv)
     std::unique_ptr<Dbo::SqlConnectionPool> blogDb
       = BlogSession::createConnectionPool(server.appRoot() + "blog.db");
 
-    BlogRSSFeed rssFeed(*blogDb, "Wt and JWt blog",
-                        "http://www.webtoolkit.eu/wt/blog",
-                        "We care about our webtoolkits.");
+    auto rssFeed = std::make_shared<BlogRSSFeed>(*blogDb,
+                                                 "Wt and JWt blog",
+                                                 "http://www.webtoolkit.eu/wt/blog",
+                                                 "We care about our webtoolkits.");
 
-    server.addResource(&rssFeed, "/wt/blog/feed/");
+    server.addResource(rssFeed, "/wt/blog/feed/");
 
     server.addEntryPoint(EntryPointType::Application,
                          std::bind(&createJWtHomeApplication, std::placeholders::_1, blogDb.get()),
diff --git a/src/Wt/Auth/OAuthService.C b/src/Wt/Auth/OAuthService.C
index 2fac16d3..6b61c197 100644
--- a/src/Wt/Auth/OAuthService.C
+++ b/src/Wt/Auth/OAuthService.C
@@ -31,6 +31,8 @@
 #include "WebSession.h"
 #include "WebRequest.h"
 
+#include <memory>
+
 #ifdef WT_THREADED
 #include <mutex>
 #endif // WT_THREADED
@@ -608,7 +610,7 @@ struct OAuthService::Impl
     const OAuthService& service_;
   };
 
-  std::unique_ptr<RedirectEndpoint> redirectResource_;
+  std::shared_ptr<RedirectEndpoint> redirectResource_;
   std::string secret_;
 };
 
@@ -693,7 +695,7 @@ void OAuthService::configureRedirectEndpoint() const
     std::unique_lock<std::mutex> guard(impl_->mutex_);
 #endif
     if (!impl_->redirectResource_) {
-      auto r = std::unique_ptr<Impl::RedirectEndpoint>(new Impl::RedirectEndpoint(*this));
+      auto r = std::make_shared<Impl::RedirectEndpoint>(*this);
       std::string path = redirectEndpointPath();
 
       LOG_INFO("deploying endpoint at " << path);
@@ -704,9 +706,9 @@ void OAuthService::configureRedirectEndpoint() const
       else
         server = WServer::instance();
 
-      server->addResource(r.get(), path);
+      server->addResource(r, path);
 
-      impl_->redirectResource_ = std::move(r);
+      impl_->redirectResource_ = r;
     }
   }
 }
diff --git a/src/Wt/Auth/Saml/Service.C b/src/Wt/Auth/Saml/Service.C
index 2097d728..15de812c 100644
--- a/src/Wt/Auth/Saml/Service.C
+++ b/src/Wt/Auth/Saml/Service.C
@@ -391,7 +391,7 @@ std::unique_ptr<Process> Service::createProcess() const
 
 void Service::generateAcsEndpoint()
 {
-  auto resource = std::make_unique<StaticAcsResource>(*this);
+  auto resource = std::make_shared<StaticAcsResource>(*this);
   std::string path = acsPath();
 
   LOG_INFO("deploying endpoint at " << path);
@@ -403,9 +403,7 @@ void Service::generateAcsEndpoint()
     server = WServer::instance();
   }
 
-  server->addResource(resource.get(), path);
-
-  staticAcsResource_ = std::move(resource);
+  server->addResource(resource, path);
 }
 
 void Service::generateMetadataEndpoint()
@@ -413,7 +411,7 @@ void Service::generateMetadataEndpoint()
   if (metadataPath_.empty())
     return;
 
-  metadataResource_ = std::make_unique<MetadataResource>(*this);
+  auto resource = std::make_shared<MetadataResource>(*this);
   WApplication *app = WApplication::instance();
   WServer *server = nullptr;
   if (app) {
@@ -422,7 +420,7 @@ void Service::generateMetadataEndpoint()
     server = WServer::instance();
   }
 
-  server->addResource(metadataResource_.get(), metadataPath_);
+  server->addResource(resource, metadataPath_);
 }
 
 std::string Service::metadata() const
diff --git a/src/Wt/Auth/Saml/Service.h b/src/Wt/Auth/Saml/Service.h
index 19d400ce..0c247c85 100644
--- a/src/Wt/Auth/Saml/Service.h
+++ b/src/Wt/Auth/Saml/Service.h
@@ -750,8 +750,6 @@ private:
 
   // state
   std::unique_ptr<ServiceImpl> impl_;
-  std::unique_ptr<StaticAcsResource> staticAcsResource_;
-  std::unique_ptr<MetadataResource> metadataResource_;
 
   // configuration
   std::string secret_;
diff --git a/src/Wt/WServer.C b/src/Wt/WServer.C
index 2d412c11..ac9af87e 100644
--- a/src/Wt/WServer.C
+++ b/src/Wt/WServer.C
@@ -279,6 +279,19 @@ void WServer::addResource(WResource *resource, const std::string& path)
   }
 }
 
+void WServer::addResource(const std::shared_ptr<WResource> &resource,
+                          const std::string &path)
+{
+  bool success = configuration().tryAddResource(EntryPoint(resource, prependDefaultPath(path)));
+  if (success) {
+    resource->setInternalPath(path);
+  } else {
+    WString error(Wt::utf8("WServer::addResource() error: "
+                           "a static resource was already deployed on path '{1}'"));
+    throw WServer::Exception(error.arg(path).toUTF8());
+  }
+}
+
 void WServer::removeEntryPoint(const std::string& path){
   configuration().removeEntryPoint(path);
 }
diff --git a/src/Wt/WServer.h b/src/Wt/WServer.h
index d71f3b01..986f7217 100644
--- a/src/Wt/WServer.h
+++ b/src/Wt/WServer.h
@@ -300,8 +300,29 @@ public:
    * public. Use this method to add a public resource with a fixed
    * path.
    *
+   * \throw Exception if an entrypoint was already registered at the given path
+   *
    * \sa removeEntryPoint()
    */
+  WT_API void addResource(const std::shared_ptr<WResource>& resource, const std::string& path);
+
+  /*! \brief Binds a resource to a fixed path.
+   *
+   * Resources may either be private to a single session or
+   * public. Use this method to add a public resource with a fixed
+   * path.
+   *
+   * \note Ownership of the resource is external to %WServer. The resource first needs
+   *       to be \link removeEntryPoint() removed\endlink (while the server is
+   *       \link stop() stopped\endlink) before being destroyed, or has to outlive the %WServer.
+   *
+   * \throw Exception if an entrypoint was already registered at the given path
+   *
+   * \sa removeEntryPoint()
+   *
+   * \deprecated Use addResource(const std::shared_ptr<WResource>&, const std::string&) instead.
+   */
+  WT_DEPRECATED("Use addResource(const std::shared_ptr<WResource>&, const std::string&) instead")
   WT_API void addResource(WResource *resource, const std::string& path);
 
   /*! \brief Removes an entry point.
diff --git a/src/web/EntryPoint.C b/src/web/EntryPoint.C
index 3e1625b0..ae2a4cf3 100644
--- a/src/web/EntryPoint.C
+++ b/src/web/EntryPoint.C
@@ -18,6 +18,14 @@ EntryPoint::EntryPoint(WResource *resource, const std::string& path)
     path_(path)
 { }
 
+EntryPoint::EntryPoint(const std::shared_ptr<WResource>& resource, const std::string& path)
+  : type_(EntryPointType::StaticResource),
+    resource_(resource.get()),
+    ownedResource_(resource),
+    appCallback_(nullptr),
+    path_(path)
+{ }
+
 EntryPoint::~EntryPoint()
 {
 }
diff --git a/src/web/EntryPoint.h b/src/web/EntryPoint.h
index a315a715..4c698dcb 100644
--- a/src/web/EntryPoint.h
+++ b/src/web/EntryPoint.h
@@ -20,6 +20,7 @@ public:
              const std::string& path,
              const std::string& favicon);
   EntryPoint(WResource *resource, const std::string& path);
+  EntryPoint(const std::shared_ptr<WResource>& resource, const std::string& path);
   ~EntryPoint();
 
   void setPath(const std::string& path);
@@ -33,6 +34,7 @@ public:
 private:
   EntryPointType type_;
   WResource *resource_;
+  std::shared_ptr<WResource> ownedResource_;
   ApplicationCreator appCallback_;
   std::string path_;
   std::string favicon_;
diff --git a/test/http/HttpClientServerTest.C b/test/http/HttpClientServerTest.C
index dfc00a33..5b814242 100644
--- a/test/http/HttpClientServerTest.C
+++ b/test/http/HttpClientServerTest.C
@@ -139,7 +139,8 @@ namespace {
             "--docroot", "." 
           };
       setServerConfiguration(argc, (char **)argv);
-      addResource(&resource_, "/test");
+      resource_ = std::make_shared<TestResource>();
+      addResource(resource_, "/test");
     }
 
     std::string address() 
@@ -147,10 +148,10 @@ namespace {
       return "127.0.0.1:" + std::to_string(httpPort());
     }
 
-    TestResource& resource() { return resource_; }
+    TestResource& resource() { return *resource_; }
 
   private:
-    TestResource resource_;
+    std::shared_ptr<TestResource> resource_;
   };
 
   class Client : public Wt::WObject {
-- 
2.38.3


From 3e1976ad49a7122883aa00f312c489c09aeb824d Mon Sep 17 00:00:00 2001
From: Roel Standaert <roel@emweb.be>
Date: Wed, 8 Mar 2023 16:52:56 +0100
Subject: [PATCH 2/5] WT-11302: avoid use-after-free with closed listeners

When we call handleResume() or handleStop() all acceptors are closed
and all listeners deleted. However, closing the acceptor causes
handleTcpAccept or handleSslAccept to be called with an error code.
Previously, this would mean that the listener argument to these
functions would be a dangling pointer and dereferencing it is undefined
behavior. By letting the callback take a weak_ptr we can ensure that
we don't dereference a dangling pointer.

====================================================================
The original patch from Roel is available at:
https://redmine.webtoolkit.eu/attachments/download/5527/0002-WT-11302-avoid-use-after-free-with-closed-listeners.patch
from: https://redmine.webtoolkit.eu/issues/11301

note: the attachment was intended for issue 11032, but was actually attached to issue 11301
---
 src/http/Server.C | 90 ++++++++++++++++++++++++++---------------------
 src/http/Server.h | 12 ++++---
 2 files changed, 57 insertions(+), 45 deletions(-)

diff --git a/src/http/Server.C b/src/http/Server.C
index b26a4447..6ef0149e 100644
--- a/src/http/Server.C
+++ b/src/http/Server.C
@@ -393,8 +393,8 @@ void Server::addTcpEndpoint(const asio::ip::tcp::endpoint &endpoint,
                             const std::string &address,
                             Wt::AsioWrapper::error_code &errc)
 {
-  tcp_listeners_.push_back(TcpListener(asio::ip::tcp::acceptor(wt_.ioService()), TcpConnectionPtr()));
-  asio::ip::tcp::acceptor &tcp_acceptor = tcp_listeners_.back().acceptor;
+  tcp_listeners_.push_back(std::make_shared<TcpListener>(asio::ip::tcp::acceptor(wt_.ioService()), TcpConnectionPtr()));
+  asio::ip::tcp::acceptor &tcp_acceptor = tcp_listeners_.back()->acceptor;
   tcp_acceptor.open(endpoint.protocol());
   tcp_acceptor.set_option(asio::ip::tcp::acceptor::reuse_address(true));
 #ifndef WT_WIN32
@@ -406,7 +406,7 @@ void Server::addTcpEndpoint(const asio::ip::tcp::endpoint &endpoint,
 
     LOG_INFO_S(&wt_, "started server: " << addressString("http", endpoint, address));
 
-    tcp_listeners_.back().new_connection.reset
+    tcp_listeners_.back()->new_connection.reset
       (new TcpConnection(wt_.ioService(), this, connection_manager_,
                          request_handler_));
   } else {
@@ -450,8 +450,8 @@ void Server::addSslEndpoint(const asio::ip::tcp::endpoint &endpoint,
                             const std::string &address,
                             Wt::AsioWrapper::error_code &errc)
 {
-  ssl_listeners_.push_back(SslListener(asio::ip::tcp::acceptor(wt_.ioService()), SslConnectionPtr()));
-  asio::ip::tcp::acceptor &ssl_acceptor = ssl_listeners_.back().acceptor;
+  ssl_listeners_.push_back(std::make_shared<SslListener>(asio::ip::tcp::acceptor(wt_.ioService()), SslConnectionPtr()));
+  asio::ip::tcp::acceptor &ssl_acceptor = ssl_listeners_.back()->acceptor;
   ssl_acceptor.open(endpoint.protocol());
   ssl_acceptor.set_option(asio::ip::tcp::acceptor::reuse_address(true));
 #ifndef WT_WIN32
@@ -463,7 +463,7 @@ void Server::addSslEndpoint(const asio::ip::tcp::endpoint &endpoint,
 
     LOG_INFO_S(&wt_, "started server: " << addressString("https", endpoint, address));
 
-    ssl_listeners_.back().new_connection.reset
+    ssl_listeners_.back()->new_connection.reset
       (new SslConnection(wt_.ioService(), this, ssl_context_, connection_manager_,
                          request_handler_));
   } else {
@@ -481,13 +481,13 @@ int Server::httpPort() const
     if (ssl_listeners_.empty())
       return -1;
     else
-      return ssl_listeners_.front().acceptor.local_endpoint().port();
+      return ssl_listeners_.front()->acceptor.local_endpoint().port();
 #else // HTTP_WITH_SSL
     return -1;
 #endif // HTTP_WITH_SSL
   }
 
-  return tcp_listeners_.front().acceptor.local_endpoint().port();
+  return tcp_listeners_.front()->acceptor.local_endpoint().port();
 }
 
 void Server::startAccept()
@@ -502,23 +502,23 @@ void Server::startAccept()
    * need to access the ConnectionManager mutex in any case).
    */
   for (std::size_t i = 0; i < tcp_listeners_.size(); ++i) {
-    asio::ip::tcp::acceptor &acceptor = tcp_listeners_[i].acceptor;
-    TcpConnectionPtr &new_connection = tcp_listeners_[i].new_connection;
+    asio::ip::tcp::acceptor &acceptor = tcp_listeners_[i]->acceptor;
+    TcpConnectionPtr &new_connection = tcp_listeners_[i]->new_connection;
     acceptor.async_accept(new_connection->socket(),
                           accept_strand_.wrap(
                             std::bind(&Server::handleTcpAccept, this,
-                                        &tcp_listeners_[i],
+                                        tcp_listeners_[i],
                                         std::placeholders::_1)));
   }
 
 #ifdef HTTP_WITH_SSL
   for (std::size_t i = 0; i < ssl_listeners_.size(); ++i) {
-    asio::ip::tcp::acceptor &acceptor = ssl_listeners_[i].acceptor;
-    SslConnectionPtr &new_connection = ssl_listeners_[i].new_connection;
+    asio::ip::tcp::acceptor &acceptor = ssl_listeners_[i]->acceptor;
+    SslConnectionPtr &new_connection = ssl_listeners_[i]->new_connection;
     acceptor.async_accept(new_connection->socket(),
                           accept_strand_.wrap(
                             std::bind(&Server::handleSslAccept, this,
-                                        &ssl_listeners_[i],
+                                        ssl_listeners_[i],
                                         std::placeholders::_1)));
   }
 #endif // HTTP_WITH_SSL
@@ -539,7 +539,7 @@ void Server
   if (!err) {
     // tcp_listeners_.front() should be fine here:
     // it should be the only acceptor when this is a session process
-    auto port = tcp_listeners_.front().acceptor.local_endpoint().port();
+    auto port = tcp_listeners_.front()->acceptor.local_endpoint().port();
     Wt::WStringStream ss;
     ss << "port:" << port << "\n";
     auto buf = std::make_shared<std::string>(ss.str());
@@ -595,57 +595,65 @@ void Server::resume()
 void Server::handleResume()
 {
   for (std::size_t i = 0; i < tcp_listeners_.size(); ++i)
-    tcp_listeners_[i].acceptor.close();
+    tcp_listeners_[i]->acceptor.close();
   tcp_listeners_.clear();
 
 #ifdef HTTP_WITH_SSL
   for (std::size_t i = 0; i < ssl_listeners_.size(); ++i)
-    ssl_listeners_[i].acceptor.close();
+    ssl_listeners_[i]->acceptor.close();
   ssl_listeners_.clear();
 #endif // HTTP_WITH_SSL
 
   start();
 }
 
-void Server::handleTcpAccept(TcpListener *listener, const Wt::AsioWrapper::error_code& e)
+void Server::handleTcpAccept(const std::weak_ptr<TcpListener>& listener,
+                             const Wt::AsioWrapper::error_code& e)
 {
-  if (!e) {
-    connection_manager_.start(listener->new_connection);
-    listener->new_connection.reset(new TcpConnection(wt_.ioService(), this,
-                                                     connection_manager_, request_handler_));
-  } else if (!listener->acceptor.is_open()) {
+  auto l = listener.lock();
+  if (!l || (e && !l->acceptor.is_open())) {
     // server shutdown
-    LOG_DEBUG("handleTcpAccept: async_accept error (acceptor closed, probably server shutdown): " << e.message());
+    LOG_DEBUG("handleTcpAccept: async_accept error (no listener, probably server shutdown): " << e.message());
     return;
+  }
+
+  if (!e) {
+    connection_manager_.start(l->new_connection);
+    l->new_connection.reset(new TcpConnection(wt_.ioService(), this,
+                                              connection_manager_, request_handler_));
   } else {
     LOG_ERROR("handleTcpAccept: async_accept error: " << e.message());
   }
 
-  listener->acceptor.async_accept(listener->new_connection->socket(),
-                                  accept_strand_.wrap(
-                                          std::bind(&Server::handleTcpAccept, this,
-                                                    listener, std::placeholders::_1)));
+  l->acceptor.async_accept(l->new_connection->socket(),
+                           accept_strand_.wrap(
+                                   std::bind(&Server::handleTcpAccept, this,
+                                             listener, std::placeholders::_1)));
 }
 
 #ifdef HTTP_WITH_SSL
-void Server::handleSslAccept(SslListener *listener, const Wt::AsioWrapper::error_code& e)
+void Server::handleSslAccept(const std::weak_ptr<SslListener>& listener,
+                             const Wt::AsioWrapper::error_code& e)
 {
-  if (!e) {
-    connection_manager_.start(listener->new_connection);
-    listener->new_connection.reset(new SslConnection(wt_.ioService(), this,
-                                                     ssl_context_, connection_manager_, request_handler_));
-  } else if (!listener->acceptor.is_open()) {
+  auto l = listener.lock();
+  if (!l || (e && !l->acceptor.is_open())) {
     // server shutdown
-    LOG_DEBUG("handleSslAccept: async_accept error (acceptor closed, probably server shutdown): " << e.message());
+    LOG_DEBUG("handleSslAccept: async_accept error (no listener, probably server shutdown): " << e.message());
     return;
+  }
+
+  if (!e) {
+    connection_manager_.start(l->new_connection);
+    l->new_connection.reset(new SslConnection(wt_.ioService(), this,
+                                              ssl_context_, connection_manager_, request_handler_));
   } else {
     LOG_ERROR("handleSslAccept: async_accept error: " << e.message());
   }
 
-  listener->acceptor.async_accept(listener->new_connection->socket(),
-                                  accept_strand_.wrap(
-                                          std::bind(&Server::handleSslAccept, this,
-                                                    listener, std::placeholders::_1)));
+  l->acceptor.async_accept(l->new_connection->socket(),
+                           accept_strand_.wrap(
+                                   std::bind(&Server::handleSslAccept, this,
+                                             listener, std::placeholders::_1)));
 }
 #endif // HTTP_WITH_SSL
 
@@ -663,12 +671,12 @@ void Server::handleStop()
   // operations. Once all operations have finished the io_service::run() call
   // will exit.
   for (std::size_t i = 0; i < tcp_listeners_.size(); ++i)
-    tcp_listeners_[i].acceptor.close();
+    tcp_listeners_[i]->acceptor.close();
   tcp_listeners_.clear();
 
 #ifdef HTTP_WITH_SSL
   for (std::size_t i = 0; i < ssl_listeners_.size(); ++i)
-    ssl_listeners_[i].acceptor.close();
+    ssl_listeners_[i]->acceptor.close();
   ssl_listeners_.clear();
 #endif // HTTP_WITH_SSL
 
diff --git a/src/http/Server.h b/src/http/Server.h
index 36758d4d..317eded4 100644
--- a/src/http/Server.h
+++ b/src/http/Server.h
@@ -31,6 +31,8 @@
 
 #include "Wt/WLogger.h"
 
+#include <memory>
+
 namespace http {
 namespace server {
 
@@ -110,7 +112,8 @@ private:
   void closeParentConnection();
 
   /// Handle completion of an asynchronous accept operation.
-  void handleTcpAccept(TcpListener *listener, const Wt::AsioWrapper::error_code& e);
+  void handleTcpAccept(const std::weak_ptr<TcpListener>& listener,
+                       const Wt::AsioWrapper::error_code& e);
 
   /// Handle a request to stop the server.
   void handleStop();
@@ -134,7 +137,7 @@ private:
   Wt::AsioWrapper::strand accept_strand_;
 
   /// Acceptors used to listen for incoming http connections.
-  std::vector<TcpListener> tcp_listeners_;
+  std::vector<std::shared_ptr<TcpListener>> tcp_listeners_;
 
 #ifdef HTTP_WITH_SSL
   struct SslListener {
@@ -149,7 +152,7 @@ private:
   asio::ssl::context ssl_context_;
 
   /// Acceptors used to listen for incoming https connections
-  std::vector<SslListener> ssl_listeners_;
+  std::vector<std::shared_ptr<SslListener>> ssl_listeners_;
 
   /// Add new SSL listener, called from start()
   void addSslListener(asio::ip::tcp::resolver &resolver,
@@ -162,7 +165,8 @@ private:
                       Wt::AsioWrapper::error_code &errc);
 
   /// Handle completion of an asynchronous SSL accept operation.
-  void handleSslAccept(SslListener *listener, const Wt::AsioWrapper::error_code& e);
+  void handleSslAccept(const std::weak_ptr<SslListener>& listener,
+                       const Wt::AsioWrapper::error_code& e);
 #endif // HTTP_WITH_SSL
 
   void handleTimeout(asio::steady_timer *timer,
-- 
2.38.3


From 9905884c0dcec64abaa30e4a26cfaeaff7a6b376 Mon Sep 17 00:00:00 2001
From: Bruce Toll <4109762+tollb@users.noreply.github.com>
Date: Wed, 8 Mar 2023 18:00:27 -0500
Subject: [PATCH 3/5] Add test case for clean client server shutdown

Add a test case that will shutdown the server with an outstanding
request to a static resource running on a thread.

If the resource is destructed before the server is shutdown,
this can result in a use after free when the resource's request
handler accesses the destructed resource object, e.g. to create a
continuation.

====================================================================
The original patch with test case is available at:
https://redmine.webtoolkit.eu/attachments/download/5438/0001-Add-test-case-for-clean-client-server-shutdown.patch
from: https://redmine.webtoolkit.eu/issues/11301
---
 test/http/HttpClientServerTest.C | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/test/http/HttpClientServerTest.C b/test/http/HttpClientServerTest.C
index 5b814242..04214223 100644
--- a/test/http/HttpClientServerTest.C
+++ b/test/http/HttpClientServerTest.C
@@ -35,6 +35,8 @@ namespace {
     Exception,
   };
 
+  constexpr auto SimulatedWorkTime = std::chrono::milliseconds{500};
+
   class TestResource : public WResource
   {
   public:
@@ -42,6 +44,7 @@ namespace {
       : delaySendingBody_(false),
         haveEverMoreData_(false),
         haveRandomMoreData_(false),
+        simulateWork_(false),
         aborted_(0)
     { }
 
@@ -69,9 +72,16 @@ namespace {
       return aborted_;
     }
 
+    void simulateWork() {
+      simulateWork_ = true;
+    }
+
     virtual void handleRequest(const Http::Request& request,
                                Http::Response& response) override
     {
+      if (simulateWork_)
+        std::this_thread::sleep_for(SimulatedWorkTime);
+
       switch (type_) {
       case TestType::Simple:
         return handleSimple(request, response);
@@ -93,6 +103,7 @@ namespace {
     bool delaySendingBody_;
     bool haveEverMoreData_;
     bool haveRandomMoreData_;
+    bool simulateWork_;
     int aborted_;
     TestType type_ = TestType::Simple;
 
@@ -646,4 +657,23 @@ BOOST_AUTO_TEST_CASE( http_wresource_exception )
   }
 }
 
+BOOST_AUTO_TEST_CASE( http_client_server_clean_shutdown )
+{
+  Server server;
+
+  server.resource().setType(TestType::Continuation);
+  server.resource().simulateWork();
+
+  if (server.start()) {
+    Client client;
+    client.get("http://" + server.address() + "/test");
+
+    std::this_thread::sleep_for(SimulatedWorkTime / 2);
+
+    client.abort();
+    client.waitDone();
+  }
+}
+
+
 #endif // WT_THREADED
-- 
2.38.3


From 4a48d69b13bc08a0448700b6bcc4e451e682604b Mon Sep 17 00:00:00 2001
From: Bruce Toll <4109762+tollb@users.noreply.github.com>
Date: Wed, 8 Mar 2023 18:10:44 -0500
Subject: [PATCH 4/5] Add test for use after free race on listener close

Add a test to drive a use after free race when the server
listener(s) are closed.

====================================================================
The original patch with test case is available at:
https://redmine.webtoolkit.eu/attachments/download/5440/0003-Add-test-for-use-after-free-race-on-listener-close.patch
from: https://redmine.webtoolkit.eu/issues/11302

The patch was modified from the original to just use server.stop() from
the base class.
---
 test/http/HttpClientServerTest.C | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/test/http/HttpClientServerTest.C b/test/http/HttpClientServerTest.C
index 04214223..e2da7ae4 100644
--- a/test/http/HttpClientServerTest.C
+++ b/test/http/HttpClientServerTest.C
@@ -675,5 +675,31 @@ BOOST_AUTO_TEST_CASE( http_client_server_clean_shutdown )
   }
 }
 
+BOOST_AUTO_TEST_CASE( http_server_clean_close )
+{
+  constexpr unsigned ClientCount {1000};
+  Server server;
+
+  server.resource().setType(TestType::Continuation);
+  server.resource().simulateWork();
+
+  if (server.start()) {
+    Client client;
+    std::vector<Client *> clients;
+
+    for (unsigned i = 0; i < ClientCount; ++i) {
+      Client *client = new Client();
+      client->get("http://" + server.address() + "/test");
+      clients.push_back(client);
+    }
+
+    server.stop();
+
+    for (unsigned i = 0; i < ClientCount; ++i) {
+      clients[i]->waitDone();
+      delete clients[i];
+    }
+  }
+}
 
 #endif // WT_THREADED
-- 
2.38.3


From 10d77ab01057fb65ddd24fcb6785d00cef486522 Mon Sep 17 00:00:00 2001
From: Bruce Toll <4109762+tollb@users.noreply.github.com>
Date: Thu, 9 Mar 2023 14:32:21 -0500
Subject: [PATCH 5/5] Updated test case for tracking client close

Updated the http_server_clean_close test with a lower number of clients
to avoid running out of file descriptors and to stop the server after
waiting for ten clients to complete.
---
 test/http/HttpClientServerTest.C | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/test/http/HttpClientServerTest.C b/test/http/HttpClientServerTest.C
index e2da7ae4..8013eae1 100644
--- a/test/http/HttpClientServerTest.C
+++ b/test/http/HttpClientServerTest.C
@@ -677,7 +677,7 @@ BOOST_AUTO_TEST_CASE( http_client_server_clean_shutdown )
 
 BOOST_AUTO_TEST_CASE( http_server_clean_close )
 {
-  constexpr unsigned ClientCount {1000};
+  constexpr unsigned ClientCount {500};
   Server server;
 
   server.resource().setType(TestType::Continuation);
@@ -693,9 +693,10 @@ BOOST_AUTO_TEST_CASE( http_server_clean_close )
       clients.push_back(client);
     }
 
-    server.stop();
 
     for (unsigned i = 0; i < ClientCount; ++i) {
+      if (i == 10)
+        server.stop();
       clients[i]->waitDone();
       delete clients[i];
     }
-- 
2.38.3

