Skip to content

Commit f9c3eed

Browse files
Fix remainingStreamingBytes set before requestHandler for GET requests
Co-authored-by: uNetworkingAB <110806833+uNetworkingAB@users.noreply.github.com>
1 parent c42a910 commit f9c3eed

9 files changed

Lines changed: 46 additions & 16 deletions

File tree

src/HttpParser.h

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -533,15 +533,6 @@ struct HttpParser {
533533
const char *querySeparatorPtr = (const char *) memchr(req->headers->value.data(), '?', req->headers->value.length());
534534
req->querySeparator = (unsigned int) ((querySeparatorPtr ? querySeparatorPtr : req->headers->value.data() + req->headers->value.length()) - req->headers->value.data());
535535

536-
/* If returned socket is not what we put in we need
537-
* to break here as we either have upgraded to
538-
* WebSockets or otherwise closed the socket. */
539-
void *returnedUser = requestHandler(user, req);
540-
if (returnedUser != user) {
541-
/* We are upgraded to WebSocket or otherwise broken */
542-
return {consumedTotal, returnedUser};
543-
}
544-
545536
/* The rules at play here according to RFC 9112 for requests are essentially:
546537
* If both content-length and transfer-encoding then invalid message; must break.
547538
* If has transfer-encoding then must be chunked regardless of value.
@@ -566,6 +557,28 @@ struct HttpParser {
566557
* This could be made stricter but makes no difference either way, unless forwarding the identical message as a proxy. */
567558

568559
remainingStreamingBytes = STATE_IS_CHUNKED;
560+
} else if (contentLengthString.length()) {
561+
remainingStreamingBytes = toUnsignedInteger(contentLengthString);
562+
if (remainingStreamingBytes == UINT64_MAX) {
563+
/* Parser error */
564+
return {HTTP_ERROR_400_BAD_REQUEST, FULLPTR};
565+
}
566+
} else {
567+
/* No body (e.g. GET requests); set to 0 to match this assumption */
568+
remainingStreamingBytes = 0;
569+
}
570+
571+
/* If returned socket is not what we put in we need
572+
* to break here as we either have upgraded to
573+
* WebSockets or otherwise closed the socket. */
574+
void *returnedUser = requestHandler(user, req);
575+
if (returnedUser != user) {
576+
/* We are upgraded to WebSocket or otherwise broken */
577+
return {consumedTotal, returnedUser};
578+
}
579+
580+
/* Consume body data */
581+
if (transferEncodingString.length()) {
569582
/* If consume minimally, we do not want to consume anything but we want to mark this as being chunked */
570583
if (!CONSUME_MINIMALLY) {
571584
/* Go ahead and parse it (todo: better heuristics for emitting FIN to the app level) */
@@ -582,12 +595,6 @@ struct HttpParser {
582595
consumedTotal += consumed;
583596
}
584597
} else if (contentLengthString.length()) {
585-
remainingStreamingBytes = toUnsignedInteger(contentLengthString);
586-
if (remainingStreamingBytes == UINT64_MAX) {
587-
/* Parser error */
588-
return {HTTP_ERROR_400_BAD_REQUEST, FULLPTR};
589-
}
590-
591598
if (!CONSUME_MINIMALLY) {
592599
unsigned int emittable = (unsigned int) std::min<uint64_t>(remainingStreamingBytes, length);
593600
dataHandler(user, std::string_view(data, emittable), emittable == remainingStreamingBytes);

tests/BloomFilter

73.3 KB
Binary file not shown.

tests/ChunkedEncoding

85.5 KB
Binary file not shown.

tests/ExtensionsNegotiator

78.8 KB
Binary file not shown.

tests/HttpParser

143 KB
Binary file not shown.

tests/HttpParser.cpp

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ int main() {
1212

1313
uWS::HttpParser httpParser;
1414

15-
auto [err, returnedUser] = httpParser.consumePostPadded((char *) data, size, user, reserved, [reserved](void *s, uWS::HttpRequest *httpRequest) -> void * {
15+
auto [err, returnedUser] = httpParser.consumePostPadded((char *) data, size, user, reserved, [reserved, &httpParser](void *s, uWS::HttpRequest *httpRequest) -> void * {
1616

1717
std::cout << httpRequest->getMethod() << std::endl;
1818

@@ -23,6 +23,9 @@ int main() {
2323
/* Since we did proper whitespace trimming this thing is there, but empty */
2424
assert(httpRequest->getHeader("utf8").data());
2525

26+
/* maxRemainingBodyLength() must be 0 for GET requests (no body) when called inside requestHandler */
27+
assert(httpParser.maxRemainingBodyLength() == 0);
28+
2629
/* Return ok */
2730
return s;
2831

@@ -35,4 +38,24 @@ int main() {
3538

3639
std::cout << "HTTP DONE" << std::endl;
3740

41+
/* Test that maxRemainingBodyLength() is set correctly before requestHandler for a POST with content-length */
42+
{
43+
/* POST / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 5\r\n\r\nhello + 8 bytes padding */
44+
const char postData[] = "POST / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 5\r\n\r\nhelloEEEEEEEE";
45+
int postSize = (int)(sizeof(postData) - 1 - 8);
46+
uWS::HttpParser postParser;
47+
uint64_t bodyLengthInHandler = UINT64_MAX;
48+
49+
postParser.consumePostPadded((char *) postData, postSize, user, reserved, [&postParser, &bodyLengthInHandler](void *s, uWS::HttpRequest *httpRequest) -> void * {
50+
bodyLengthInHandler = postParser.maxRemainingBodyLength();
51+
return s;
52+
}, [](void *user, std::string_view data, bool fin) -> void * {
53+
return user;
54+
});
55+
56+
/* maxRemainingBodyLength() must return content-length (5) when called inside requestHandler */
57+
assert(bodyLengthInHandler == 5);
58+
std::cout << "POST content-length test PASS" << std::endl;
59+
}
60+
3861
}

tests/HttpRouter

1.05 MB
Binary file not shown.

tests/Query

56 KB
Binary file not shown.

tests/TopicTree

304 KB
Binary file not shown.

0 commit comments

Comments
 (0)