Remove middlewares
Middlewares, while kinda cool from an academic standpoint, make our
build times even worse than they already are. Given that we only really
use 1 real middleware today (token auth) and it needs to move into the
parser mode anyway (for security limiting buffer sizes), we might as well
use this as an opportunity to delete some code.
Some other things that happen:
1. Persistent data now moves out of the crow namespace
2. App is no longer a template
3. All request_routes implementations no longer become templates. This
should be a decent (unmeasured) win on compile times.
This commit was part of a commit previously called "various cleanups".
This separates ONLY the middleware deletion part of that.
Note, this also deletes about 400 lines of hard to understand code.
Change-Id: I4c19e25491a153a2aa2e4ef46fc797bcb5b3581a
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/http/app.h b/http/app.h
index ca871dc..dfd5304 100644
--- a/http/app.h
+++ b/http/app.h
@@ -3,7 +3,6 @@
#include "http_request.h"
#include "http_server.h"
#include "logging.h"
-#include "middleware_context.h"
#include "routing.h"
#include "utility.h"
@@ -25,25 +24,24 @@
#ifdef BMCWEB_ENABLE_SSL
using ssl_context_t = boost::asio::ssl::context;
#endif
-template <typename... Middlewares>
-class Crow
+class App
{
public:
- using self_t = Crow;
+ using self_t = App;
#ifdef BMCWEB_ENABLE_SSL
using ssl_socket_t = boost::beast::ssl_stream<boost::asio::ip::tcp::socket>;
- using ssl_server_t = Server<Crow, ssl_socket_t, Middlewares...>;
+ using ssl_server_t = Server<App, ssl_socket_t>;
#else
using socket_t = boost::asio::ip::tcp::socket;
- using server_t = Server<Crow, socket_t, Middlewares...>;
+ using server_t = Server<App, socket_t>;
#endif
- explicit Crow(std::shared_ptr<boost::asio::io_context> ioIn =
- std::make_shared<boost::asio::io_context>()) :
+ explicit App(std::shared_ptr<boost::asio::io_context> ioIn =
+ std::make_shared<boost::asio::io_context>()) :
io(std::move(ioIn))
{}
- ~Crow()
+ ~App()
{
this->stop();
}
@@ -100,12 +98,12 @@
if (-1 == socketFd)
{
sslServer = std::move(std::make_unique<ssl_server_t>(
- this, bindaddrStr, portUint, sslContext, &middlewares, io));
+ this, bindaddrStr, portUint, sslContext, io));
}
else
{
- sslServer = std::move(std::make_unique<ssl_server_t>(
- this, socketFd, sslContext, &middlewares, io));
+ sslServer = std::move(
+ std::make_unique<ssl_server_t>(this, socketFd, sslContext, io));
}
sslServer->setTickFunction(tickInterval, tickFunction);
sslServer->run();
@@ -115,12 +113,12 @@
if (-1 == socketFd)
{
server = std::move(std::make_unique<server_t>(
- this, bindaddrStr, portUint, nullptr, &middlewares, io));
+ this, bindaddrStr, portUint, nullptr, io));
}
else
{
- server = std::move(std::make_unique<server_t>(
- this, socketFd, nullptr, &middlewares, io));
+ server = std::move(
+ std::make_unique<server_t>(this, socketFd, nullptr, io));
}
server->setTickFunction(tickInterval, tickFunction);
server->run();
@@ -216,23 +214,6 @@
}
#endif
- // middleware
- using context_t = detail::Context<Middlewares...>;
- template <typename T>
- typename T::Context& getContext(const Request& req)
- {
- static_assert(black_magic::Contains<T, Middlewares...>::value,
- "App doesn't have the specified middleware type.");
- auto& ctx = *reinterpret_cast<context_t*>(req.middlewareContext);
- return ctx.template get<T>();
- }
-
- template <typename T>
- T& getMiddleware()
- {
- return utility::getElementByType<T, Middlewares...>(middlewares);
- }
-
template <typename Duration, typename Func>
self_t& tick(Duration d, Func f)
{
@@ -255,15 +236,11 @@
std::chrono::milliseconds tickInterval{};
std::function<void()> tickFunction;
- std::tuple<Middlewares...> middlewares;
-
#ifdef BMCWEB_ENABLE_SSL
std::unique_ptr<ssl_server_t> sslServer;
#else
std::unique_ptr<server_t> server;
#endif
};
-template <typename... Middlewares>
-using App = Crow<Middlewares...>;
-using SimpleApp = Crow<>;
} // namespace crow
+using App = crow::App;
diff --git a/http/http_connection.h b/http/http_connection.h
index 8dba3d6..609d4a1 100644
--- a/http/http_connection.h
+++ b/http/http_connection.h
@@ -3,7 +3,6 @@
#include "http_response.h"
#include "logging.h"
-#include "middleware_context.h"
#include "timer_queue.h"
#include "utility.h"
@@ -19,6 +18,7 @@
#include <boost/beast/http.hpp>
#include <boost/beast/ssl/ssl_stream.hpp>
#include <boost/beast/websocket.hpp>
+#include <security_headers.hpp>
#include <ssl_key_handler.hpp>
#include <atomic>
@@ -61,186 +61,6 @@
using namespace boost;
using tcp = asio::ip::tcp;
-namespace detail
-{
-template <typename MW>
-struct CheckBeforeHandleArity3Const
-{
- template <typename T,
- void (T::*)(Request&, Response&, typename MW::Context&) const =
- &T::beforeHandle>
- struct Get
- {};
-};
-
-template <typename MW>
-struct CheckBeforeHandleArity3
-{
- template <typename T, void (T::*)(Request&, Response&,
- typename MW::Context&) = &T::beforeHandle>
- struct Get
- {};
-};
-
-template <typename MW>
-struct CheckAfterHandleArity3Const
-{
- template <typename T,
- void (T::*)(Request&, Response&, typename MW::Context&) const =
- &T::afterHandle>
- struct Get
- {};
-};
-
-template <typename MW>
-struct CheckAfterHandleArity3
-{
- template <typename T, void (T::*)(Request&, Response&,
- typename MW::Context&) = &T::afterHandle>
- struct Get
- {};
-};
-
-template <typename T>
-struct IsBeforeHandleArity3Impl
-{
- template <typename C>
- static std::true_type
- f(typename CheckBeforeHandleArity3Const<T>::template Get<C>*);
-
- template <typename C>
- static std::true_type
- f(typename CheckBeforeHandleArity3<T>::template Get<C>*);
-
- template <typename C>
- static std::false_type f(...);
-
- public:
- static constexpr bool value = decltype(f<T>(nullptr))::value;
-};
-
-template <typename T>
-struct IsAfterHandleArity3Impl
-{
- template <typename C>
- static std::true_type
- f(typename CheckAfterHandleArity3Const<T>::template Get<C>*);
-
- template <typename C>
- static std::true_type
- f(typename CheckAfterHandleArity3<T>::template Get<C>*);
-
- template <typename C>
- static std::false_type f(...);
-
- public:
- static constexpr bool value = decltype(f<T>(nullptr))::value;
-};
-
-template <typename MW, typename Context, typename ParentContext>
-typename std::enable_if<!IsBeforeHandleArity3Impl<MW>::value>::type
- beforeHandlerCall(MW& mw, Request& req, Response& res, Context& ctx,
- ParentContext& /*parent_ctx*/)
-{
- mw.beforeHandle(req, res, ctx.template get<MW>(), ctx);
-}
-
-template <typename MW, typename Context, typename ParentContext>
-typename std::enable_if<IsBeforeHandleArity3Impl<MW>::value>::type
- beforeHandlerCall(MW& mw, Request& req, Response& res, Context& ctx,
- ParentContext& /*parent_ctx*/)
-{
- mw.beforeHandle(req, res, ctx.template get<MW>());
-}
-
-template <typename MW, typename Context, typename ParentContext>
-typename std::enable_if<!IsAfterHandleArity3Impl<MW>::value>::type
- afterHandlerCall(MW& mw, Request& req, Response& res, Context& ctx,
- ParentContext& /*parent_ctx*/)
-{
- mw.afterHandle(req, res, ctx.template get<MW>(), ctx);
-}
-
-template <typename MW, typename Context, typename ParentContext>
-typename std::enable_if<IsAfterHandleArity3Impl<MW>::value>::type
- afterHandlerCall(MW& mw, Request& req, Response& res, Context& ctx,
- ParentContext& /*parent_ctx*/)
-{
- mw.afterHandle(req, res, ctx.template get<MW>());
-}
-
-template <size_t N, typename Context, typename Container, typename CurrentMW,
- typename... Middlewares>
-bool middlewareCallHelper(Container& middlewares, Request& req, Response& res,
- Context& ctx)
-{
- using parent_context_t = typename Context::template partial<N - 1>;
- beforeHandlerCall<CurrentMW, Context, parent_context_t>(
- std::get<N>(middlewares), req, res, ctx,
- static_cast<parent_context_t&>(ctx));
-
- if (res.isCompleted())
- {
- afterHandlerCall<CurrentMW, Context, parent_context_t>(
- std::get<N>(middlewares), req, res, ctx,
- static_cast<parent_context_t&>(ctx));
- return true;
- }
-
- if (middlewareCallHelper<N + 1, Context, Container, Middlewares...>(
- middlewares, req, res, ctx))
- {
- afterHandlerCall<CurrentMW, Context, parent_context_t>(
- std::get<N>(middlewares), req, res, ctx,
- static_cast<parent_context_t&>(ctx));
- return true;
- }
-
- return false;
-}
-
-template <size_t N, typename Context, typename Container>
-bool middlewareCallHelper(Container& /*middlewares*/, Request& /*req*/,
- Response& /*res*/, Context& /*ctx*/)
-{
- return false;
-}
-
-template <size_t N, typename Context, typename Container>
-typename std::enable_if<(N < 0)>::type
- afterHandlersCallHelper(Container& /*middlewares*/, Context& /*Context*/,
- Request& /*req*/, Response& /*res*/)
-{}
-
-template <size_t N, typename Context, typename Container>
-typename std::enable_if<(N == 0)>::type
- afterHandlersCallHelper(Container& middlewares, Context& ctx, Request& req,
- Response& res)
-{
- using parent_context_t = typename Context::template partial<N - 1>;
- using CurrentMW = typename std::tuple_element<
- N, typename std::remove_reference<Container>::type>::type;
- afterHandlerCall<CurrentMW, Context, parent_context_t>(
- std::get<N>(middlewares), req, res, ctx,
- static_cast<parent_context_t&>(ctx));
-}
-
-template <size_t N, typename Context, typename Container>
-typename std::enable_if<(N > 0)>::type
- afterHandlersCallHelper(Container& middlewares, Context& ctx, Request& req,
- Response& res)
-{
- using parent_context_t = typename Context::template partial<N - 1>;
- using CurrentMW = typename std::tuple_element<
- N, typename std::remove_reference<Container>::type>::type;
- afterHandlerCall<CurrentMW, Context, parent_context_t>(
- std::get<N>(middlewares), req, res, ctx,
- static_cast<parent_context_t&>(ctx));
- afterHandlersCallHelper<N - 1, Context, Container>(middlewares, ctx, req,
- res);
-}
-} // namespace detail
-
#ifdef BMCWEB_ENABLE_DEBUG
static std::atomic<int> connectionCount;
#endif
@@ -261,21 +81,18 @@
static constexpr const size_t loggedOutAttempts =
(15 / timerQueueTimeoutSeconds);
-template <typename Adaptor, typename Handler, typename... Middlewares>
+template <typename Adaptor, typename Handler>
class Connection :
- public std::enable_shared_from_this<
- Connection<Adaptor, Handler, Middlewares...>>
+ public std::enable_shared_from_this<Connection<Adaptor, Handler>>
{
public:
Connection(boost::asio::io_context& ioService, Handler* handlerIn,
const std::string& ServerNameIn,
- std::tuple<Middlewares...>* middlewaresIn,
std::function<std::string()>& get_cached_date_str_f,
detail::TimerQueue& timerQueueIn, Adaptor adaptorIn) :
adaptor(std::move(adaptorIn)),
handler(handlerIn), serverName(ServerNameIn),
- middlewares(middlewaresIn), getCachedDateStr(get_cached_date_str_f),
- timerQueue(timerQueueIn)
+ getCachedDateStr(get_cached_date_str_f), timerQueue(timerQueueIn)
{
parser.emplace(std::piecewise_construct, std::make_tuple());
parser->body_limit(httpReqBodyLimit);
@@ -285,7 +102,7 @@
#ifdef BMCWEB_ENABLE_MUTUAL_TLS_AUTHENTICATION
auto ca_available = !std::filesystem::is_empty(
std::filesystem::path(ensuressl::trustStorePath));
- if (ca_available && crow::persistent_data::SessionStore::getInstance()
+ if (ca_available && persistent_data::SessionStore::getInstance()
.getAuthMethodsConfig()
.tls)
{
@@ -301,7 +118,7 @@
bool preverified,
boost::asio::ssl::verify_context& ctx) {
// do nothing if TLS is disabled
- if (!crow::persistent_data::SessionStore::getInstance()
+ if (!persistent_data::SessionStore::getInstance()
.getAuthMethodsConfig()
.tls)
{
@@ -444,10 +261,10 @@
}
sslUser.resize(lastChar);
- session = persistent_data::SessionStore::getInstance()
- .generateUserSession(
- sslUser,
- crow::persistent_data::PersistenceType::TIMEOUT);
+ session =
+ persistent_data::SessionStore::getInstance()
+ .generateUserSession(
+ sslUser, persistent_data::PersistenceType::TIMEOUT);
if (auto sp = session.lock())
{
BMCWEB_LOG_DEBUG << this
@@ -538,15 +355,9 @@
res.completeRequestHandler = [] {};
res.isAliveHelper = [this]() -> bool { return isAlive(); };
- ctx = detail::Context<Middlewares...>();
- req->middlewareContext = static_cast<void*>(&ctx);
req->ioService = static_cast<decltype(req->ioService)>(
&adaptor.get_executor().context());
- detail::middlewareCallHelper<
- 0U, decltype(ctx), decltype(*middlewares), Middlewares...>(
- *middlewares, *req, res, ctx);
-
if (!res.completed)
{
needToCallAfterHandlers = true;
@@ -618,16 +429,10 @@
BMCWEB_LOG_INFO << "Response: " << this << ' ' << req->url << ' '
<< res.resultInt() << " keepalive=" << req->keepAlive();
+ addSecurityHeaders(res);
+
if (needToCallAfterHandlers)
{
- needToCallAfterHandlers = false;
-
- // call all afterHandler of middlewares
- detail::afterHandlersCallHelper<sizeof...(Middlewares) - 1,
- decltype(ctx),
- decltype(*middlewares)>(
- *middlewares, ctx, *req, res);
-
crow::authorization::cleanupTempSession(*req);
}
@@ -949,7 +754,8 @@
std::optional<crow::Request> req;
crow::Response res;
- std::weak_ptr<crow::persistent_data::UserSession> session;
+
+ std::weak_ptr<persistent_data::UserSession> session;
const std::string& serverName;
@@ -958,13 +764,10 @@
bool needToCallAfterHandlers{};
bool needToStartReadAfterComplete{};
- std::tuple<Middlewares...>* middlewares;
- detail::Context<Middlewares...> ctx;
-
std::function<std::string()>& getCachedDateStr;
detail::TimerQueue& timerQueue;
using std::enable_shared_from_this<
- Connection<Adaptor, Handler, Middlewares...>>::shared_from_this;
+ Connection<Adaptor, Handler>>::shared_from_this;
};
} // namespace crow
diff --git a/http/http_request.h b/http/http_request.h
index 95f88c7..fa60f60 100644
--- a/http/http_request.h
+++ b/http/http_request.h
@@ -30,10 +30,9 @@
const std::string& body;
- void* middlewareContext{};
boost::asio::io_context* ioService{};
- std::shared_ptr<crow::persistent_data::UserSession> session;
+ std::shared_ptr<persistent_data::UserSession> session;
std::string userRole{};
std::function<Adaptor&()> socket;
diff --git a/http/http_response.h b/http/http_response.h
index d5d1e4b..7be6b09 100644
--- a/http/http_response.h
+++ b/http/http_response.h
@@ -11,12 +11,12 @@
namespace crow
{
-template <typename Adaptor, typename Handler, typename... Middlewares>
+template <typename Adaptor, typename Handler>
class Connection;
struct Response
{
- template <typename Adaptor, typename Handler, typename... Middlewares>
+ template <typename Adaptor, typename Handler>
friend class crow::Connection;
using response_type =
boost::beast::http::response<boost::beast::http::string_body>;
diff --git a/http/http_server.h b/http/http_server.h
index 0e8a702..c87ddd4 100644
--- a/http/http_server.h
+++ b/http/http_server.h
@@ -27,44 +27,39 @@
using namespace boost;
using tcp = asio::ip::tcp;
-template <typename Handler, typename Adaptor = boost::asio::ip::tcp::socket,
- typename... Middlewares>
+template <typename Handler, typename Adaptor = boost::asio::ip::tcp::socket>
class Server
{
public:
Server(Handler* handler, std::unique_ptr<tcp::acceptor>&& acceptor,
std::shared_ptr<boost::asio::ssl::context> adaptor_ctx,
- std::tuple<Middlewares...>* middlewares = nullptr,
std::shared_ptr<boost::asio::io_context> io =
std::make_shared<boost::asio::io_context>()) :
ioService(std::move(io)),
acceptor(std::move(acceptor)),
signals(*ioService, SIGINT, SIGTERM, SIGHUP), tickTimer(*ioService),
- timer(*ioService), handler(handler), middlewares(middlewares),
- adaptorCtx(adaptor_ctx)
+ timer(*ioService), handler(handler), adaptorCtx(adaptor_ctx)
{}
Server(Handler* handler, const std::string& bindaddr, uint16_t port,
std::shared_ptr<boost::asio::ssl::context> adaptor_ctx,
- std::tuple<Middlewares...>* middlewares = nullptr,
std::shared_ptr<boost::asio::io_context> io =
std::make_shared<boost::asio::io_context>()) :
Server(handler,
std::make_unique<tcp::acceptor>(
*io, tcp::endpoint(boost::asio::ip::make_address(bindaddr),
port)),
- adaptor_ctx, middlewares, io)
+ adaptor_ctx, io)
{}
Server(Handler* handler, int existing_socket,
std::shared_ptr<boost::asio::ssl::context> adaptor_ctx,
- std::tuple<Middlewares...>* middlewares = nullptr,
std::shared_ptr<boost::asio::io_context> io =
std::make_shared<boost::asio::io_context>()) :
Server(handler,
std::make_unique<tcp::acceptor>(*io, boost::asio::ip::tcp::v6(),
existing_socket),
- adaptor_ctx, middlewares, io)
+ adaptor_ctx, io)
{}
void setTickFunction(std::chrono::milliseconds d, std::function<void()> f)
@@ -223,11 +218,9 @@
boost::asio::ip::tcp::socket>>::value)
{
adaptorTemp = Adaptor(*ioService, *adaptorCtx);
- auto p =
- std::make_shared<Connection<Adaptor, Handler, Middlewares...>>(
- *ioService, handler, serverName, middlewares,
- getCachedDateStr, timerQueue,
- std::move(adaptorTemp.value()));
+ auto p = std::make_shared<Connection<Adaptor, Handler>>(
+ *ioService, handler, serverName, getCachedDateStr, timerQueue,
+ std::move(adaptorTemp.value()));
acceptor->async_accept(p->socket().next_layer(),
[this, p](boost::system::error_code ec) {
@@ -243,11 +236,9 @@
else
{
adaptorTemp = Adaptor(*ioService);
- auto p =
- std::make_shared<Connection<Adaptor, Handler, Middlewares...>>(
- *ioService, handler, serverName, middlewares,
- getCachedDateStr, timerQueue,
- std::move(adaptorTemp.value()));
+ auto p = std::make_shared<Connection<Adaptor, Handler>>(
+ *ioService, handler, serverName, getCachedDateStr, timerQueue,
+ std::move(adaptorTemp.value()));
acceptor->async_accept(
p->socket(), [this, p](boost::system::error_code ec) {
@@ -279,8 +270,6 @@
std::function<void()> tickFunction;
std::function<void(const boost::system::error_code& ec)> timerHandler;
- std::tuple<Middlewares...>* middlewares;
-
#ifdef BMCWEB_ENABLE_SSL
bool useSsl{false};
#endif
diff --git a/http/middleware_context.h b/http/middleware_context.h
deleted file mode 100644
index fa399d6..0000000
--- a/http/middleware_context.h
+++ /dev/null
@@ -1,72 +0,0 @@
-#pragma once
-
-#include "http_request.h"
-#include "http_response.h"
-#include "utility.h"
-
-namespace crow
-{
-namespace detail
-{
-template <typename... Middlewares>
-struct PartialContext :
- public black_magic::PopBack<Middlewares...>::template rebind<
- PartialContext>,
- public black_magic::LastElementType<Middlewares...>::type::Context
-{
- using parent_context = typename black_magic::PopBack<
- Middlewares...>::template rebind<::crow::detail::PartialContext>;
- template <size_t N>
- using partial = typename std::conditional<
- N == sizeof...(Middlewares) - 1, PartialContext,
- typename parent_context::template partial<N>>::type;
-
- template <typename T>
- typename T::Context& get()
- {
- return static_cast<typename T::Context&>(*this);
- }
-};
-
-template <>
-struct PartialContext<>
-{
- template <size_t>
- using partial = PartialContext;
-};
-
-template <size_t N, typename Context, typename Container, typename CurrentMW,
- typename... Middlewares>
-bool middlewareCallHelper(Container& middlewares, Request& req, Response& res,
- Context& ctx);
-
-template <typename... Middlewares>
-struct Context : private PartialContext<Middlewares...>
-// struct Context : private Middlewares::context... // simple but less type-safe
-{
- template <size_t N, typename Context, typename Container>
- friend typename std::enable_if<(N == 0)>::type
- afterHandlersCallHelper(Container& middlewares, Context& ctx,
- Request& req, Response& res);
- template <size_t N, typename Context, typename Container>
- friend typename std::enable_if<(N > 0)>::type
- afterHandlersCallHelper(Container& middlewares, Context& ctx,
- Request& req, Response& res);
-
- template <size_t N, typename Context, typename Container,
- typename CurrentMW, typename... Middlewares2>
- friend bool middlewareCallHelper(Container& middlewares, Request& req,
- Response& res, Context& ctx);
-
- template <typename T>
- typename T::Context& get()
- {
- return static_cast<typename T::Context&>(*this);
- }
-
- template <size_t N>
- using partial =
- typename PartialContext<Middlewares...>::template partial<N>;
-};
-} // namespace detail
-} // namespace crow
diff --git a/http/websocket.h b/http/websocket.h
index 91b537b..7670196 100644
--- a/http/websocket.h
+++ b/http/websocket.h
@@ -276,7 +276,7 @@
std::function<void(Connection&, const std::string&, bool)> messageHandler;
std::function<void(Connection&, const std::string&)> closeHandler;
std::function<void(Connection&)> errorHandler;
- std::shared_ptr<crow::persistent_data::UserSession> session;
+ std::shared_ptr<persistent_data::UserSession> session;
};
} // namespace websocket
} // namespace crow