diff --git a/src/HttpParser.h b/src/HttpParser.h index 0605e211f..71f651edb 100644 --- a/src/HttpParser.h +++ b/src/HttpParser.h @@ -533,15 +533,6 @@ struct HttpParser { const char *querySeparatorPtr = (const char *) memchr(req->headers->value.data(), '?', req->headers->value.length()); req->querySeparator = (unsigned int) ((querySeparatorPtr ? querySeparatorPtr : req->headers->value.data() + req->headers->value.length()) - req->headers->value.data()); - /* 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) { - /* We are upgraded to WebSocket or otherwise broken */ - return {consumedTotal, returnedUser}; - } - /* The rules at play here according to RFC 9112 for requests are essentially: * If both content-length and transfer-encoding then invalid message; must break. * If has transfer-encoding then must be chunked regardless of value. @@ -566,6 +557,28 @@ struct HttpParser { * This could be made stricter but makes no difference either way, unless forwarding the identical message as a proxy. */ remainingStreamingBytes = STATE_IS_CHUNKED; + } else if (contentLengthString.length()) { + remainingStreamingBytes = toUnsignedInteger(contentLengthString); + if (remainingStreamingBytes == UINT64_MAX) { + /* Parser error */ + return {HTTP_ERROR_400_BAD_REQUEST, FULLPTR}; + } + } else { + /* No body (e.g. GET requests); set to 0 to match this assumption */ + remainingStreamingBytes = 0; + } + + /* 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) { + /* We are upgraded to WebSocket or otherwise broken */ + return {consumedTotal, returnedUser}; + } + + /* Consume body data */ + if (transferEncodingString.length()) { /* If consume minimally, we do not want to consume anything but we want to mark this as being chunked */ if (!CONSUME_MINIMALLY) { /* Go ahead and parse it (todo: better heuristics for emitting FIN to the app level) */ @@ -582,16 +595,11 @@ struct HttpParser { consumedTotal += consumed; } } else if (contentLengthString.length()) { - remainingStreamingBytes = toUnsignedInteger(contentLengthString); - if (remainingStreamingBytes == UINT64_MAX) { - /* Parser error */ - return {HTTP_ERROR_400_BAD_REQUEST, FULLPTR}; - } - if (!CONSUME_MINIMALLY) { unsigned int emittable = (unsigned int) std::min(remainingStreamingBytes, length); - dataHandler(user, std::string_view(data, emittable), emittable == remainingStreamingBytes); + bool fin = (emittable == remainingStreamingBytes); remainingStreamingBytes -= emittable; + dataHandler(user, std::string_view(data, emittable), fin); data += emittable; length -= emittable; @@ -646,16 +654,17 @@ struct HttpParser { // this is exactly the same as below! // todo: refactor this if (remainingStreamingBytes >= length) { - void *returnedUser = dataHandler(user, std::string_view(data, length), remainingStreamingBytes == length); + bool fin = (remainingStreamingBytes == length); remainingStreamingBytes -= length; + void *returnedUser = dataHandler(user, std::string_view(data, length), fin); return {0, returnedUser}; } else { - void *returnedUser = dataHandler(user, std::string_view(data, remainingStreamingBytes), true); - - data += (unsigned int) remainingStreamingBytes; - length -= (unsigned int) remainingStreamingBytes; - + unsigned int toEmit = (unsigned int) remainingStreamingBytes; remainingStreamingBytes = 0; + void *returnedUser = dataHandler(user, std::string_view(data, toEmit), true); + + data += toEmit; + length -= toEmit; if (returnedUser != user) { return {0, returnedUser}; @@ -702,16 +711,17 @@ struct HttpParser { } else { // this is exactly the same as above! if (remainingStreamingBytes >= (unsigned int) length) { - void *returnedUser = dataHandler(user, std::string_view(data, length), remainingStreamingBytes == (unsigned int) length); + bool fin = (remainingStreamingBytes == (unsigned int) length); remainingStreamingBytes -= length; + void *returnedUser = dataHandler(user, std::string_view(data, length), fin); return {0, returnedUser}; } else { - void *returnedUser = dataHandler(user, std::string_view(data, remainingStreamingBytes), true); - - data += (unsigned int) remainingStreamingBytes; - length -= (unsigned int) remainingStreamingBytes; - + unsigned int toEmit = (unsigned int) remainingStreamingBytes; remainingStreamingBytes = 0; + void *returnedUser = dataHandler(user, std::string_view(data, toEmit), true); + + data += toEmit; + length -= toEmit; if (returnedUser != user) { return {0, returnedUser}; diff --git a/tests/.gitignore b/tests/.gitignore new file mode 100644 index 000000000..438402ecc --- /dev/null +++ b/tests/.gitignore @@ -0,0 +1,7 @@ +BloomFilter +ChunkedEncoding +ExtensionsNegotiator +HttpParser +HttpRouter +Query +TopicTree diff --git a/tests/HttpParser.cpp b/tests/HttpParser.cpp index a1b75317a..4428a6674 100644 --- a/tests/HttpParser.cpp +++ b/tests/HttpParser.cpp @@ -12,7 +12,7 @@ int main() { uWS::HttpParser httpParser; - auto [err, returnedUser] = httpParser.consumePostPadded((char *) data, size, user, reserved, [reserved](void *s, uWS::HttpRequest *httpRequest) -> void * { + auto [err, returnedUser] = httpParser.consumePostPadded((char *) data, size, user, reserved, [reserved, &httpParser](void *s, uWS::HttpRequest *httpRequest) -> void * { std::cout << httpRequest->getMethod() << std::endl; @@ -23,6 +23,9 @@ int main() { /* Since we did proper whitespace trimming this thing is there, but empty */ assert(httpRequest->getHeader("utf8").data()); + /* maxRemainingBodyLength() must be 0 for GET requests (no body) when called inside requestHandler */ + assert(httpParser.maxRemainingBodyLength() == 0); + /* Return ok */ return s; @@ -35,4 +38,30 @@ int main() { std::cout << "HTTP DONE" << std::endl; + /* Test that maxRemainingBodyLength() is set correctly before requestHandler for a POST with content-length */ + { + /* POST / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 5\r\n\r\nhello + 8 bytes padding */ + const char postData[] = "POST / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 5\r\n\r\nhelloEEEEEEEE"; + int postSize = (int)(sizeof(postData) - 1 - 8); + uWS::HttpParser postParser; + uint64_t bodyLengthInHandler = UINT64_MAX; + uint64_t bodyLengthInDataHandler = UINT64_MAX; + + postParser.consumePostPadded((char *) postData, postSize, user, reserved, [&postParser, &bodyLengthInHandler](void *s, uWS::HttpRequest *httpRequest) -> void * { + bodyLengthInHandler = postParser.maxRemainingBodyLength(); + return s; + }, [&postParser, &bodyLengthInDataHandler](void *user, std::string_view data, bool fin) -> void * { + /* maxRemainingBodyLength() must be decremented before dataHandler is called */ + bodyLengthInDataHandler = postParser.maxRemainingBodyLength(); + return user; + }); + + /* maxRemainingBodyLength() must return content-length (5) when called inside requestHandler */ + assert(bodyLengthInHandler == 5); + std::cout << "POST content-length in requestHandler test PASS" << std::endl; + /* maxRemainingBodyLength() must be 0 after all body is consumed (decremented before dataHandler) */ + assert(bodyLengthInDataHandler == 0); + std::cout << "POST content-length in dataHandler test PASS" << std::endl; + } + } \ No newline at end of file