diff --git a/src/util/http/HttpClient.cpp b/src/util/http/HttpClient.cpp index e09808c3c3..1998d0f947 100644 --- a/src/util/http/HttpClient.cpp +++ b/src/util/http/HttpClient.cpp @@ -119,7 +119,7 @@ HttpOrHttpsResponse HttpClientImpl::sendRequest( beast::flat_buffer buffer; auto responseParser = std::make_unique>(); - responseParser->body_limit(std::numeric_limits::max()); + responseParser->body_limit(boost::none); wait(http::async_read_header(*(client->stream_), buffer, *responseParser, net::use_awaitable)); diff --git a/test/HttpTest.cpp b/test/HttpTest.cpp index 217d912f4b..e04e7c76ad 100644 --- a/test/HttpTest.cpp +++ b/test/HttpTest.cpp @@ -8,6 +8,8 @@ #include #include "HttpTestHelpers.h" +#include "global/RuntimeParameters.h" +#include "util/GTestHelpers.h" #include "util/http/HttpClient.h" #include "util/http/HttpServer.h" #include "util/http/HttpUtils.h" @@ -275,3 +277,103 @@ TEST(HttpServer, ErrorHandlingInSession) { EXPECT_THAT(s, HasSubstr("Weird exception not inheriting from std::exception")); } + +// Test the request body size limit +TEST(HttpServer, RequestBodySizeLimit) { + ad_utility::SharedCancellationHandle handle = + std::make_shared>(); + + TestHttpServer httpServer([](auto request, + auto&& send) -> boost::asio::awaitable { + std::string methodName; + switch (request.method()) { + case verb::get: + methodName = "GET"; + break; + case verb::post: + methodName = "POST"; + break; + default: + methodName = "OTHER"; + } + + auto response = [](std::string methodName, + std::string target) -> cppcoro::generator { + co_yield methodName; + co_yield "\n"; + co_yield target; + }(methodName, std::string(toStd(request.target()))); + + // Send a response. + co_await send(createOkResponse(std::move(response), request, + ad_utility::MediaType::textPlain)); + }); + + httpServer.runInOwnThread(); + + auto ResponseMetadata = [](const status status, string_view content_type) { + return AllOf( + AD_FIELD(HttpOrHttpsResponse, status_, testing::Eq(status)), + AD_FIELD(HttpOrHttpsResponse, contentType_, testing::Eq(content_type))); + }; + auto expect_ = [&httpServer](const ad_utility::MemorySize& requestBodySize, + const std::string& expectedBody, + const auto& responseMatcher) { + ad_utility::SharedCancellationHandle handle = + std::make_shared>(); + + auto httpClient = std::make_unique( + "localhost", std::to_string(httpServer.getPort())); + + const std::string body(requestBodySize.getBytes(), 'f'); + + auto response = HttpClient::sendRequest( + std::move(httpClient), verb::post, "localhost", "target", handle, body); + EXPECT_THAT(response, responseMatcher); + EXPECT_THAT(toString(std::move(response.body_)), expectedBody); + }; + + auto expectRequestFails = [&ResponseMetadata, &expect_]( + const ad_utility::MemorySize& requestBodySize) { + const ad_utility::MemorySize currentLimit = + RuntimeParameters().get<"request-body-limit">(); + // For large requests we get an exception while writing to the request + // stream when going over the limit. For small requests we get the response + // normally. We would need the HttpClient to return the response even + // if it couldn't send the request fully in that case. + expect_( + requestBodySize, + R"({"error": "Request body size exceeds the allowed size ()" + + currentLimit.asString() + + R"(). Send a smaller request or set the allowed size via the "request-body-limit" run-time parameter."})", + ResponseMetadata(status::payload_too_large, "application/json")); + }; + auto expectRequest = [&expect_, &ResponseMetadata]( + const ad_utility::MemorySize requestBodySize) { + expect_(requestBodySize, "POST\ntarget", + ResponseMetadata(status::ok, "text/plain")); + }; + constexpr auto testingRequestBodyLimit = 50_kB; + + // Set a smaller limit for testing. The default of 100 MB is quite large. + RuntimeParameters().set("request-body-limit", 50_kB .asString()); + // Requests with bodies smaller than the request body limit are processed. + expectRequest(3_B); + // Exactly the limit is allowed. + expectRequest(testingRequestBodyLimit); + // Larger than the limit is forbidden. + expectRequestFails(testingRequestBodyLimit + 1_B); + + // Setting a smaller request-body limit. + RuntimeParameters().set("request-body-limit", 1_B .asString()); + expectRequestFails(3_B); + // Only the request body size counts. The empty body is allowed even if the + // body is limited to 1 byte. + expectRequest(0_B); + + // Disable the request body limit, by setting it to 0. + RuntimeParameters().set("request-body-limit", 0_B .asString()); + // Arbitrarily large requests are now allowed. + expectRequest(10_kB); + expectRequest(5_MB); +}