From f1bdbc968bc8f9628527b6b89b6ffd0326933c5f Mon Sep 17 00:00:00 2001 From: Alex Hultman Date: Sun, 10 Feb 2019 00:17:13 +0100 Subject: [PATCH] First pass of overall cleanups --- src/AsyncSocket.h | 22 ++++++---------------- src/HttpContext.h | 20 +++++++++----------- src/HttpContextData.h | 2 -- src/HttpParser.h | 29 +++++++++++------------------ src/HttpResponse.h | 2 +- src/HttpResponseData.h | 1 - src/HttpRouter.h | 3 ++- src/PerMessageDeflate.h | 4 +--- src/TopicTree.h | 6 ++++++ src/WebSocket.h | 2 +- src/WebSocketContext.h | 2 ++ 11 files changed, 39 insertions(+), 54 deletions(-) diff --git a/src/AsyncSocket.h b/src/AsyncSocket.h index 52032c4..add34b8 100644 --- a/src/AsyncSocket.h +++ b/src/AsyncSocket.h @@ -20,7 +20,6 @@ /* This class implements async socket memory management strategies */ - #include "LoopData.h" #include "AsyncSocketData.h" @@ -35,15 +34,12 @@ protected: /* Get loop data for socket */ LoopData *getLoopData() { - return (LoopData *) us_loop_ext( - us_new_socket_context_loop(SSL, - us_new_socket_context(SSL, (us_new_socket_t *) this)) - ); + return (LoopData *) us_loop_ext(us_new_socket_context_loop(SSL, us_new_socket_context(SSL, (us_new_socket_t *) this))); } /* Get socket extension */ - void *getExt() { - return us_new_socket_ext(SSL, (us_new_socket_t *) this); + AsyncSocketData *getAsyncSocketData() { + return (AsyncSocketData *) us_new_socket_ext(SSL, (us_new_socket_t *) this); } /* Socket timeout */ @@ -81,20 +77,14 @@ protected: loopData->corkOffset += size; return {sendBuffer, false}; } else { - // slow path for now - + /* Slow path for now, we want to always be corked if possible */ return {(char *) malloc(size), true}; - - // if we are out of buffer, fail this completely? - } } /* Returns the user space backpressure. */ int getBufferedAmount() { - AsyncSocketData *asyncSocketData = (AsyncSocketData *) getExt(); - - return asyncSocketData->buffer.size(); + return getAsyncSocketData()->buffer.size(); } /* Write in three levels of prioritization: cork-buffer, syscall, socket-buffer. Always drain if possible. @@ -107,7 +97,7 @@ protected: } LoopData *loopData = getLoopData(); - AsyncSocketData *asyncSocketData = (AsyncSocketData *) getExt(); + AsyncSocketData *asyncSocketData = getAsyncSocketData(); /* We are limited if we have a per-socket buffer */ if (asyncSocketData->buffer.length()) { diff --git a/src/HttpContext.h b/src/HttpContext.h index 859f0db..ea6147d 100644 --- a/src/HttpContext.h +++ b/src/HttpContext.h @@ -22,13 +22,10 @@ #include "Loop.h" #include "HttpContextData.h" - #include "HttpResponseData.h" #include "AsyncSocket.h" #include -#include - #include "f2/function2.hpp" namespace uWS { @@ -36,6 +33,7 @@ template struct HttpResponse; template struct HttpContext { + template friend struct TemplatedApp; private: HttpContext() = delete; @@ -220,7 +218,7 @@ private: us_new_socket_context_on_writable(SSL, getSocketContext(), [](auto *s) { AsyncSocket *asyncSocket = (AsyncSocket *) s; - HttpResponseData *httpResponseData = (HttpResponseData *) asyncSocket->getExt(); + HttpResponseData *httpResponseData = (HttpResponseData *) asyncSocket->getAsyncSocketData(); /* Ask the developer to write data and return success (true) or failure (false), OR skip sending anything and return success (true). */ if (httpResponseData->onWritable) { @@ -272,6 +270,13 @@ private: return this; } + /* Used by App in its WebSocket handler */ + void upgradeToWebSocket(void *newSocket) { + HttpContextData *httpContextData = getSocketContextData(); + + httpContextData->upgradedWebSocket = newSocket; + } + public: /* Construct a new HttpContext using specified loop */ static HttpContext *create(Loop *loop, us_new_socket_context_options_t options = {}) { @@ -319,13 +324,6 @@ public: }); } - // this should not be public - void upgradeToWebSocket(void *newSocket) { - HttpContextData *httpContextData = getSocketContextData(); - - httpContextData->upgradedWebSocket = newSocket; - } - /* Listen to port using this HttpContext */ us_listen_socket *listen(const char *host, int port, int options) { return us_new_socket_context_listen(SSL, getSocketContext(), host, port, options, sizeof(HttpResponseData)); diff --git a/src/HttpContextData.h b/src/HttpContextData.h index d09a816..5b57ddd 100644 --- a/src/HttpContextData.h +++ b/src/HttpContextData.h @@ -20,9 +20,7 @@ #include "HttpRouter.h" -#include #include - #include "f2/function2.hpp" namespace uWS { diff --git a/src/HttpParser.h b/src/HttpParser.h index dba55ee..c19e33f 100644 --- a/src/HttpParser.h +++ b/src/HttpParser.h @@ -18,13 +18,13 @@ #ifndef HTTPPARSER_H #define HTTPPARSER_H +// todo: HttpParser is in need of a few clean-ups and refactorings + /* The HTTP parser is an independent module subject to unit testing / fuzz testing */ #include -#include #include #include - #include "f2/function2.hpp" namespace uWS { @@ -62,11 +62,6 @@ public: return std::string_view(nullptr, 0); } - // todo: implement this - /*int getHeader(std::string_view header) { - return 0; - }*/ - std::string_view getUrl() { return std::string_view(headers->value.data(), querySeparator); } @@ -151,22 +146,20 @@ private: req->headers->value = std::string_view(req->headers->value.data(), std::max(0, req->headers->value.length() - 9)); - // querySeparator is untested, todo: go through this + /* Parse query */ const char *querySeparatorPtr = (const char *) memchr(req->headers->value.data(), '?', req->headers->value.length()); req->querySeparator = (querySeparatorPtr ? querySeparatorPtr : req->headers->value.data() + req->headers->value.length()) - req->headers->value.data(); - // this one should return socket and exit on closed - // what happens with data left for websockets? + /* If returned socket is not what we put in we need + * to break here as we either have upgraded to + * WebSockets or otherwise closed the socket. */ void *returnedUser = requestHandler(user, req); if (returnedUser != user) { - // upgraded socket, or otherwise broken - - // return pair of consumed and user + /* We are upgraded to WebSocket or otherwise broken */ return {consumedTotal, returnedUser}; } - // do not check this for GET! - + // todo: do not check this for GET (get should not have a body) // todo: also support reading chunked streams std::string_view contentLengthString = req->getHeader("content-length"); if (contentLengthString.length()) { @@ -192,7 +185,6 @@ private: public: - // todo: what can we do with the socket inside the handlers? we need to check on return from any handler if we closed or terminated or upgraded the socket void *consumePostPadded(char *data, int length, void *user, fu2::unique_function &&requestHandler, fu2::unique_function &&dataHandler, fu2::unique_function &&errorHandler) { HttpRequest req; @@ -200,6 +192,7 @@ public: if (remainingStreamingBytes) { // this is exactly the same as below! + // todo: refactor this if (remainingStreamingBytes >= length) { void *returnedUser = dataHandler(user, std::string_view(data, length), remainingStreamingBytes == length); remainingStreamingBytes -= length; @@ -222,7 +215,7 @@ public: int maxCopyDistance = std::min(MAX_FALLBACK_SIZE - fallback.length(), (size_t) length); - fallback.reserve(maxCopyDistance + 32); // padding should be same as libus + fallback.reserve(maxCopyDistance + 32); // todo: padding should be same as libus fallback.append(data, maxCopyDistance); // break here on break @@ -260,7 +253,7 @@ public: } else { if (fallback.length() == MAX_FALLBACK_SIZE) { - // you don't really need error handler, just return something strange! + // note: you don't really need error handler, just return something strange! // we could have it return a constant pointer to denote error! return errorHandler(user); } diff --git a/src/HttpResponse.h b/src/HttpResponse.h index 7f57398..a07d19d 100644 --- a/src/HttpResponse.h +++ b/src/HttpResponse.h @@ -44,7 +44,7 @@ struct HttpResponse : public AsyncSocket { typedef AsyncSocket Super; private: HttpResponseData *getHttpResponseData() { - return (HttpResponseData *) Super::getExt(); + return (HttpResponseData *) Super::getAsyncSocketData(); } /* Write an unsigned 32-bit integer in hex */ diff --git a/src/HttpResponseData.h b/src/HttpResponseData.h index 99694ec..21834fa 100644 --- a/src/HttpResponseData.h +++ b/src/HttpResponseData.h @@ -22,7 +22,6 @@ #include "HttpParser.h" #include "AsyncSocketData.h" -#include #include "f2/function2.hpp" diff --git a/src/HttpRouter.h b/src/HttpRouter.h index abba76b..cd0e314 100644 --- a/src/HttpRouter.h +++ b/src/HttpRouter.h @@ -18,11 +18,12 @@ #ifndef HTTPROUTER_HPP #define HTTPROUTER_HPP +// todo: this module also needs a few clean-ups and simplifications + /* HTTP router is an independent module subject to unit testing and fuzz testing */ /* This module is not fully optimized yet, waiting for more features before doing so */ #include -#include #include #include #include diff --git a/src/PerMessageDeflate.h b/src/PerMessageDeflate.h index 792748a..cc14873 100644 --- a/src/PerMessageDeflate.h +++ b/src/PerMessageDeflate.h @@ -35,11 +35,9 @@ struct DeflationStream { #else #include - #include -#include -#define LARGE_BUFFER_SIZE 1024 * 16 // fix this +#define LARGE_BUFFER_SIZE 1024 * 16 // todo: fix this struct ZlibContext { /* Any returned data is valid until next same-class call. diff --git a/src/TopicTree.h b/src/TopicTree.h index 44a1b3c..20bd242 100644 --- a/src/TopicTree.h +++ b/src/TopicTree.h @@ -27,6 +27,8 @@ #include #include +// todo: obviously this module is WIP + namespace uWS { // publishing to a node, then another node, then another node should prioritize draining that way @@ -71,6 +73,10 @@ private: public: + ~TopicTree() { + /* We have a few leaks here, I think */ + } + TopicTree() { /* Dynamically hook us up with the Loop post handler */ Loop::defaultLoop()->addPostHandler([this](Loop *loop) { diff --git a/src/WebSocket.h b/src/WebSocket.h index b0130fb..1b47580 100644 --- a/src/WebSocket.h +++ b/src/WebSocket.h @@ -56,7 +56,7 @@ public: bool send(std::string_view message, uWS::OpCode opCode = uWS::OpCode::BINARY, bool compress = false) { /* Transform the message to compressed domain if requested */ if (compress) { - WebSocketData *webSocketData = (WebSocketData *) Super::getExt(); + WebSocketData *webSocketData = (WebSocketData *) Super::getAsyncSocketData(); /* Check and correct the compress hint */ if (opCode < 3 && webSocketData->compressionStatus == WebSocketData::ENABLED) { diff --git a/src/WebSocketContext.h b/src/WebSocketContext.h index a5c6011..aed4f3a 100644 --- a/src/WebSocketContext.h +++ b/src/WebSocketContext.h @@ -23,6 +23,8 @@ #include "WebSocketData.h" #include "WebSocket.h" +// todo: this module needs fixing! see below! + namespace uWS { template