Fix (probably) GET && POST working, extend unit test3 with continued truncated GET request

Author: clauspruefer

Debug.cpp

@@ -11,10 +11,10 @@ using namespace std;
 
 #define DBG(level, debugmsg) \
         if (LOG_LEVEL > level) { \
-            cout << __func__ << "(), " << __LINE__ << " - " << debugmsg << endl; \
+            cout << dec << __func__ << "(), " << __LINE__ << " - " << debugmsg << endl; \
         }
 
 #define ERR(errmsg) \
-        cerr << __func__ << "(), " << __LINE__ << " - " << errmsg << endl; \
+        cerr << dec << __func__ << "(), " << __LINE__ << " - " << errmsg << endl; \
 
 #endif

Helper.hpp

@@ -95,6 +95,7 @@ class String {
         for (auto i = String.begin(); i != String.end(); ++i) {
             std::cout << "0x" << std::hex << std::setw(2) << std::setfill('0') << static_cast<int>(*i) << ' ';
         }
+        std::cout << endl;
     }
 };
 

ThreadHandler.hpp

@@ -39,7 +39,7 @@ typedef struct {
 } ThreadHandlerGlobals_t;
 
 
-class ClientThread: private HTTPParser, private SHMPythonAS
+class ClientThread: private HTTPParser
 {
 
 public:

lib/http/httpparser.cpp

@@ -9,7 +9,7 @@ HTTPParser::HTTPParser(ClientFD_t ClientFD) :
     _RequestCountGet(0),
     _RequestCountPost(0),
     _RequestCountPostAS(0),
-    _HTTPRequest("")
+    _HTTPRequestBuffer("")
 {
     DBG(120, "Constructor");
 }
@@ -21,121 +21,193 @@ HTTPParser::~HTTPParser()
 
 void HTTPParser::appendBuffer(const char* BufferRef, const uint16_t BufferSize)
 {
-    _HTTPRequest = _HTTPRequest + string(&BufferRef[0], BufferSize);
-    String::hexout(_HTTPRequest);
-    //DBG(250, "HTTPRequest:'" << _HTTPRequest << "'");
-    _splitRequests();
+    _HTTPRequestBuffer = _HTTPRequestBuffer + string(&BufferRef[0], BufferSize);
+    //String::hexout(_HTTPRequestBuffer);
+    DBG(250, "_HTTPRequestBuffer:'" << _HTTPRequestBuffer << "'");
+
+    //-> only process on min 1 valid request
+    const size_t EndMarkerFound = _HTTPRequestBuffer.find("\r\n\r\n");
+
+    if (EndMarkerFound != string::npos) {
+        _splitRequests();
+    }
 }
 
 void HTTPParser::_splitRequests()
 {
-    //DBG(180, "splitRequests Buffer:'" << _HTTPRequest << "'");
+    //DBG(180, "splitRequests Buffer:'" << _HTTPRequestBuffer << "'");
 
     //- reset request counters
     _RequestCountGet = 0;
     _RequestCountPost = 0;
     _RequestCountPostAS = 0;
 
-    //-> reset incomplete request string
-    string incompleteRequest("");
-
-    //-> check for existing valid request end marker
-    size_t LastDelimiterPos = _HTTPRequest.rfind("\r\n\r\n");
-    DBG(120, "LastDelimiterPos:" << LastDelimiterPos);
-
-    //-> if min 1 full valid request && rest without end marker
-    if (LastDelimiterPos != string::npos && LastDelimiterPos != _HTTPRequest.length()-4) {
-
-        //-> put incomplete last request into tmp string
-        incompleteRequest = _HTTPRequest.substr(LastDelimiterPos+4);
-
-        //-> remove incomplete last request from _HTTPRequest
-        _HTTPRequest.replace(_HTTPRequest.begin()+LastDelimiterPos+4, _HTTPRequest.end(), "");
-    }
+    //-> reset _SplittedRequests vector
+    _SplittedRequests.clear();
 
     //-> split requests into _SplittedRequests vector
-    _SplittedRequests.clear();
-    String::split(_HTTPRequest, "\r\n\r\n", _SplittedRequests);
+    String::split(_HTTPRequestBuffer, "\r\n\r\n", _SplittedRequests);
     _RequestCount = _SplittedRequests.size();
     DBG(120, "splitRequests count after splitted into Vector:" << _RequestCount);
-
-    //-> "restore" incomplete last request buffer
-    _HTTPRequest = incompleteRequest;
 }
 
-uint HTTPParser::processRequests(SharedMemAddress_t SHMGetRequests, const ASRequestHandlerRef_t ASRequestHandlerRef)
+size_t HTTPParser::processRequests(SharedMemAddress_t SHMGetRequests, const ASRequestHandlerRef_t ASRequestHandlerRef)
 {
+    //- set get requests SHM base
     setBaseAddress(SHMGetRequests);
 
-    for (auto &Request:_SplittedRequests) {
-        _processRequestProperties(Request, ASRequestHandlerRef);
+    //- iterate over splitted requests
+    for(size_t i=0; i<_SplittedRequests.size(); ++i) {
+        _processRequestProperties(i, ASRequestHandlerRef);
+    }
+
+    //- restore truncated requests
+    const auto LastElementIndex = _SplittedRequests.size();
+
+    if (LastElementIndex > 0) {
+        const auto LastElementVal = _SplittedRequests.at(LastElementIndex-1);
+
+        if (!LastElementVal.empty()) {
+            _HTTPRequestBuffer.append(LastElementVal);
+        }
+    }
+
+    if (LastElementIndex > 1) {
+        const auto LastElementPrevVal = _SplittedRequests.at(LastElementIndex-2);
+
+        if (!LastElementPrevVal.empty()) {
+            _HTTPRequestBuffer.append(LastElementPrevVal);
+        }
     }
 
     return _RequestCountGet;
 }
 
-void HTTPParser::_processRequestProperties(string& Request, const ASRequestHandlerRef_t ASRequestHandlerRef)
+void HTTPParser::_processRequestProperties(const size_t Index, const ASRequestHandlerRef_t ASRequestHandlerRef)
 {
-    //DBG(180, "HTTP Request:'" << Request << "'");
+    //- get request ref at vector index
+    auto Request = _SplittedRequests.at(Index);
+
+    //- on empty request return
+    if (Request.empty()) { return; }
 
     BasePropsResult_t BasePropsFound;
     this->_parseRequestProperties(Request, BasePropsFound);
 
     DBG(140, "HTTP Version:" << BasePropsFound.at(0) << " File:" << BasePropsFound.at(1) << " Method:" << BasePropsFound.at(2));
-    DBG(140, "HTTP Payload (c_str):" << Request.c_str());
+    DBG(140, "Complete Request:" << Request.c_str());
 
-    //- check HTTP/1.2 (currently unimplemented)
-    const size_t HTTPVersion1_2Found = BasePropsFound.at(0).find("HTTP/1.2");
+    //- temp hardcode HTTPVersion
+    uint16_t HTTPVersion = 1;
 
-    //- if not HTTP/1.1, do not process further
+    //- check HTTP/1.2 or HTTP/1.2 (currently unimplemented)
     const size_t HTTPVersion1_1Found = BasePropsFound.at(0).find("HTTP/1.1");
+    const size_t HTTPVersion1_2Found = BasePropsFound.at(2).find("HTTP/1.2");
 
-    const uint16_t HTTPMethod = (BasePropsFound.at(2).find("POST") != string::npos) ? 2 : 1;
-    const uint16_t HTTPVersion = (BasePropsFound.at(0).find("HTTP/1.1") != string::npos) ? 1 : 2;
+    //- if not HTTP/1.1 set request to "" in vector element, return
+    if (HTTPVersion1_1Found == string::npos) {
+        _SplittedRequests.at(Index) = "";
+        return;
+    }
 
-    //- check if POST request is an AS request
-    const size_t PythonReqFound = BasePropsFound.at(1).find("/python/");
+    //- check for method GET || POST
+    const size_t HTTPMethodPOST = BasePropsFound.at(2).find("POST");
+    const size_t HTTPMethodGET = BasePropsFound.at(2).find("GET");
 
-    //- get unique request nr
-    uint16_t RequestNr = getNextReqNr();
+    //- set numerical http method (GET: 1, POST: 2)
+    uint16_t HTTPMethod = 0;
 
-    DBG(140, "HTTP RequestNr:" << RequestNr << " HTTPVersion:" << HTTPVersion << " HTTPMethod:" << HTTPMethod);
+    if (HTTPMethodGET != string::npos) { HTTPMethod = 1; }
+    if (HTTPMethodPOST != string::npos) { HTTPMethod = 2; }
+
+    DBG(140, "HTTPMethod:" << HTTPMethod);
+
+    //- if not GET || POST, set request to "" in vector element, return
+    if (HTTPMethod == 0) {
+        _SplittedRequests.at(Index) = "";
+        return;
+    }
 
-    if (HTTPMethod == 2) {
+    //- check if POST request is a POSTAS request
+    const size_t PythonReqFound = BasePropsFound.at(1).find("/python/");
+
+    //- if not POSTAS, set request to "" in vector element, return
+    if (HTTPMethod == 2 && PythonReqFound == string::npos) {
         ++this->_RequestCountPost;
+        _SplittedRequests.at(Index) = "";
+        return;
     }
 
-    if (HTTPMethod == 2 && PythonReqFound != BasePropsFound.at(1).npos) {
+    //- get unique request nr
+    const uint16_t RequestNr = getNextReqNr();
+
+    DBG(140, "HTTP RequestNr:" << RequestNr);
+
+    if (HTTPMethod == 2 && PythonReqFound != string::npos) {
 
-        DBG(140, "Python Request:" << Request);
+        DBG(140, "Request Type PythonAS:" << Request);
 
-        ++this->_RequestCountPostAS;
+        //-> check first line end exists
+        size_t FirstLineEndMarker = Request.find("\r\n");
+
+        //-> if not: truncated
+        if (FirstLineEndMarker == string::npos) {
+            DBG(200, "Truncated POST Request - no First Line end");
+            return;
+        }
 
         //-> cut first properties line from request
-        size_t FirstLineEndMarker = Request.find("\n\r");
-        Request.replace(0, FirstLineEndMarker+2, "");
+        if (FirstLineEndMarker != string::npos) {
+            Request.replace(0, FirstLineEndMarker+2, "");
+        }
 
         RequestHeaderResult_t Headers;
         this->_parseRequestHeaders(Request, Headers);
 
-        auto ContentBytes = stoi(Headers.at("Content-Length"));
-        string Payload = Request.substr(Request.length()-ContentBytes, ContentBytes);
+        uint ContentBytes = 0;
 
-        DBG(140, "HTTP POST-AS payload:" << Payload);
+        //- try get content length header
+        try {
+            ContentBytes = stoi(Headers.at("Content-Length"));
+        }
+        catch(const std::exception& e) {
+            DBG(200, "Truncated POST Request - no Content-Length");
+            return;
+        }
 
-        //- add ASRequestHandler request
-        ASRequestHandlerRef->addRequest({
-            Headers.at("Host"),
-            _ClientFD,
-            HTTPMethod,
-            HTTPVersion,
-            RequestNr,
-            Payload
-        });
+        //- try get next request +payload at vector index +1
+        try {
+            auto NextRequest = _SplittedRequests.at(Index+1);
+
+            if (NextRequest.length() >= ContentBytes) {
+                string Payload = NextRequest.substr(0, ContentBytes);
+                NextRequest.replace(0, ContentBytes, "");
+                DBG(140, "HTTP POST-AS Payload:" << Payload);
+
+                //- increment request count
+                ++this->_RequestCountPostAS;
+
+                //- add ASRequestHandler request
+                ASRequestHandlerRef->addRequest({
+                    Headers.at("Host"),
+                    _ClientFD,
+                    HTTPMethod,
+                    HTTPVersion,
+                    RequestNr,
+                    Payload
+                });
+            }
+        }
+        catch(const std::exception& e) {
+            DBG(200, "Truncated POST Request - no Payload");
+            return;
+        }
     }
 
     if (HTTPMethod == 1) {
 
+        DBG(140, "Request Type GET:" << Request);
+
         ++this->_RequestCountGet;
 
         //- set values in get requests shared memory
@@ -168,7 +240,7 @@ void HTTPParser::_parseRequestProperties(string& Request, BasePropsResultRef_t R
     //- find first line endline
     size_t StartPos = Request.find("\r\n");
 
-    //-> if no headers (no \n), set start pos to end of string
+    //-> if no headers (no \r\n), set start pos to end of string
     if (StartPos == string::npos) {
         StartPos = Request.length();
     }
@@ -187,13 +259,19 @@ void HTTPParser::_parseRequestHeaders(string& Request, RequestHeaderResultRef_t
     vector<string> Lines;
     String::split(Request, "\r\n", Lines);
 
+    //- add last line (unsplitted)
+    const size_t LastDelimiter = Request.rfind("\r\n");
+    if (LastDelimiter != string::npos) {
+        Lines.push_back(Request.substr(LastDelimiter+2, Request.length()));
+    }
+
     //- loop over lines, split, put into result map
     for (auto &Line:Lines) {
 
         DBG(120, "Line:'" << Line << "'");
 
         vector<string> HeaderPair;
-        if (Line.find(':') != string::npos) {
+        if (Line.find(":") != string::npos) {
 
             String::rsplit(Line, Line.length(), ": ", HeaderPair);
             string HeaderID = HeaderPair.at(1);

lib/http/httpparser.hpp

@@ -40,12 +40,12 @@ class HTTPParser: private Client, private SHMStaticFS, private SHMPythonAS
     ~HTTPParser();
 
     void appendBuffer(const char*, const uint16_t);
-    uint processRequests(SharedMemAddress_t, const ASRequestHandlerRef_t);
+    size_t processRequests(SharedMemAddress_t, const ASRequestHandlerRef_t);
 
 private:
 
     void _splitRequests();
-    void _processRequestProperties(string&, const ASRequestHandlerRef_t);
+    void _processRequestProperties(const size_t, const ASRequestHandlerRef_t);
 
     RequestHeader_t _RequestHeaders;
     vector<string> _SplittedRequests;
@@ -55,7 +55,7 @@ class HTTPParser: private Client, private SHMStaticFS, private SHMPythonAS
     size_t _RequestCountPost;
     size_t _RequestCountPostAS;
 
-    string _HTTPRequest;
+    string _HTTPRequestBuffer;
 
 protected:
 

test/unit/http-parser/test-parser-requests

@@ -146,7 +146,12 @@ BOOST_AUTO_TEST_CASE( test_multiple_get_request_truncated )
 
     std::string Request("GET /t/tA.png HTTP/1.1\r\nCustomHeader: a\r\n\r\nGET /t/tB.png HTTP/1.1\r\nCustomHeader: b\r\n\r\nGET /t/tC.png HT");
     ClientObj->appendBuffer(Request.c_str(), Request.length());
-    auto r = ClientObj->processRequests(SHMGetRequests, ASRequestHandlerRef);
+    auto r1 = ClientObj->processRequests(SHMGetRequests, ASRequestHandlerRef);
 
-    BOOST_TEST(r == 2);
+    std::string Request2("TP/1.1\r\nCustomHeader: c\r\n\r\n");
+    ClientObj->appendBuffer(Request2.c_str(), Request2.length());
+    auto r2 = ClientObj->processRequests(SHMGetRequests, ASRequestHandlerRef);
+
+    BOOST_TEST(r1 == 2);
+    BOOST_TEST(r2 == 1);
 }