From 1a1b64eebab1947652e802845e81105d8e006a9b Mon Sep 17 00:00:00 2001
From: Roel Standaert <roel@emweb.be>
Date: Wed, 8 Mar 2023 12:41:03 +0100
Subject: [PATCH 2/3] 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.
---
 ReleaseNotes.html                      |  9 +++++++++
 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 ++++---
 14 files changed, 90 insertions(+), 43 deletions(-)

diff --git a/ReleaseNotes.html b/ReleaseNotes.html
index 43eade75d..f7140da0f 100644
--- a/ReleaseNotes.html
+++ b/ReleaseNotes.html
@@ -71,6 +71,15 @@ behaviour.
     When the user tries to log in now, an info message will be shown saying they need to verify their email first along with a new button to resend the verification email. The user must then enter the email address again in a dialog before the email is sent.<br/>
     The virtual methods <a href="classWt_1_1Auth_1_1AuthWidget.html#a42f87508868746e5ccef5695fcbd8763">AuthWidget::letResendEmailVerification()</a> and <a href="classWt_1_1Auth_1_1AuthWidget.html#af45dea06ac5751e96499d62507fea69d">AuthWidget::createResendEmailVerificationView()</a> can be used to customize the UI.
   </li>
+  <li>
+    <a href="classWt_1_1WServer.html#aa17f85dc130b616b3fcf63aa857ff579">WServer::addResource()</a>
+    can now take a <code>shared_ptr</code> instead of a raw pointer,
+    enforcing that the resource lives at least as long as the server, unless removed with
+    <a href="classWt_1_1WServer.html#a9f5a82118198a1d2341fb89572fd8693">WServer::removeEntryPoint()</a>.
+    The overload that takes a raw pointer is now deprecated.
+    This also fixes <a href="https://redmine.webtoolkit.eu/issues/11301" target="_blank">issue #11301</a>, where there
+    could be a use-after-free in <code>test.http</code>.
+  </li>
 </ul>
 
 <h2>Release 4.9.2 (TBD)</h2>
diff --git a/examples/blog/blog.C b/examples/blog/blog.C
index 6b4bcc818..7e6dd1c4a 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 f02418580..8012d0181 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 11458e4da..88f6e91f6 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 3db3ee4c9..66682b03f 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 2bce66113..b64342d92 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 2fac16d31..6b61c197f 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 2097d7283..15de812c2 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 19d400ced..0c247c851 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 2d412c115..ac9af87ea 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 d71f3b01a..986f72175 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 3e1625b0e..ae2a4cf3e 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 a315a7152..4c698dcb0 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 c6a352faf..925431d2e 100644
--- a/test/http/HttpClientServerTest.C
+++ b/test/http/HttpClientServerTest.C
@@ -151,7 +151,8 @@ namespace {
             "--docroot", "." 
           };
       setServerConfiguration(argc, (char **)argv);
-      addResource(&resource_, "/test");
+      resource_ = std::make_shared<TestResource>();
+      addResource(resource_, "/test");
     }
 
     std::string address() 
@@ -159,10 +160,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.34.1

