Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the request body limit configurable (and increase the default) #1762

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
4 changes: 4 additions & 0 deletions src/ServerMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ int main(int argc, char** argv) {
"variables that are unbound in the query throw an exception. These "
"queries technically are allowed by the SPARQL standard, but typically "
"are the result of typos and unintended by the user");
add("request-body-limit",
optionFactory.getProgramOption<"request-body-limit">(),
"Set the maximum size for the body of requests the server will process. "
"Set to zero to disable the limit.");
po::variables_map optionsMap;

try {
Expand Down
1 change: 1 addition & 0 deletions src/engine/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "util/ParseableDuration.h"
#include "util/TypeIdentity.h"
#include "util/TypeTraits.h"
#include "util/http/HttpServer.h"
#include "util/http/HttpUtils.h"
#include "util/http/websocket/MessageSender.h"

Expand Down
22 changes: 10 additions & 12 deletions src/engine/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
#include "util/MemorySize/MemorySize.h"
#include "util/ParseException.h"
#include "util/TypeTraits.h"
#include "util/http/HttpServer.h"
#include "util/http/HttpUtils.h"
#include "util/http/streamable_body.h"
#include "util/http/websocket/QueryHub.h"
#include "util/json.h"
Expand Down Expand Up @@ -84,11 +84,11 @@ class Server {
/// the `WebSocketHandler` created for `HttpServer`.
std::weak_ptr<ad_utility::websocket::QueryHub> queryHub_;

net::static_thread_pool queryThreadPool_;
net::static_thread_pool updateThreadPool_{1};
boost::asio::static_thread_pool queryThreadPool_;
boost::asio::static_thread_pool updateThreadPool_{1};

/// Executor with a single thread that is used to run timers asynchronously.
net::static_thread_pool timerExecutor_{1};
boost::asio::static_thread_pool timerExecutor_{1};

template <typename T>
using Awaitable = boost::asio::awaitable<T>;
Expand Down Expand Up @@ -186,12 +186,10 @@ class Server {
TimeLimit timeLimit);

// Plan a parsed query.
Awaitable<PlannedQuery> planQuery(net::static_thread_pool& thread_pool,
ParsedQuery&& operation,
const ad_utility::Timer& requestTimer,
TimeLimit timeLimit,
QueryExecutionContext& qec,
SharedCancellationHandle handle);
Awaitable<PlannedQuery> planQuery(
boost::asio::static_thread_pool& thread_pool, ParsedQuery&& operation,
const ad_utility::Timer& requestTimer, TimeLimit timeLimit,
QueryExecutionContext& qec, SharedCancellationHandle handle);
// Creates a `MessageSender` for the given operation.
CPP_template_2(typename RequestT)(
requires ad_utility::httpUtils::HttpRequest<RequestT>)
Expand All @@ -218,7 +216,7 @@ class Server {
/// its completion, wrapping the result.
template <std::invocable Function,
typename T = std::invoke_result_t<Function>>
Awaitable<T> computeInNewThread(net::static_thread_pool& threadPool,
Awaitable<T> computeInNewThread(boost::asio::static_thread_pool& threadPool,
Function function,
SharedCancellationHandle handle);

Expand Down Expand Up @@ -273,7 +271,7 @@ class Server {
/// Forbidden HTTP response if the change is not allowed. Return the new
/// timeout otherwise.
CPP_template_2(typename RequestT, typename ResponseT)(
requires ad_utility::httpUtils::HttpRequest<RequestT>) net::
requires ad_utility::httpUtils::HttpRequest<RequestT>) boost::asio::
awaitable<std::optional<Server::TimeLimit>> verifyUserSubmittedQueryTimeout(
std::optional<std::string_view> userTimeout, bool accessTokenOk,
const RequestT& request, ResponseT& send) const;
Expand Down
2 changes: 2 additions & 0 deletions src/global/RuntimeParameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ inline auto& RuntimeParameters() {
// Determines whether the cost estimate for a cached subtree should be
// set to zero in query planning.
Bool<"zero-cost-estimate-for-cached-subtree">{false},
// Maximum size for the body of requests that the server will process.
MemorySizeParameter<"request-body-limit">{100_MB},
};
}();
return params;
Expand Down
22 changes: 15 additions & 7 deletions src/util/http/HttpServer.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

// Copyright 2021, University of Freiburg,
// Chair of Algorithms and Data Structures.
// Author: Johannes Kalmbach <[email protected]>
// Copyright 2021-2025, University of Freiburg,
// Chair of Algorithms and Data Structures
// Authors: Johannes Kalmbach <[email protected]>
// Julian Mundhahs <mundhahj@t

#ifndef QLEVER_HTTPSERVER_H
#define QLEVER_HTTPSERVER_H
Expand All @@ -10,6 +10,7 @@
#include <future>

#include "absl/cleanup/cleanup.h"
#include "global/RuntimeParameters.h"
#include "util/Exception.h"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This header is very expensive to include. It would be nice if the implementation could be moved into a cpp file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for having a look. The HttpServer.h header is now only included in Server.cpp so that shouldn't be that much of a problem. The HttpServer unfortunately cannot be moved to a cpp, because the whole class is templated.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine too but I was thinking about adding a helper function in a cpp and using it to call the function indirectly

#include "util/Log.h"
#include "util/http/HttpUtils.h"
Expand Down Expand Up @@ -245,11 +246,18 @@ CPP_template(typename HttpHandler, typename WebSocketHandler)(
try {
// Set the timeout for reading the next request.
stream.expires_after(std::chrono::seconds(30));
http::request<http::string_body> req;

// Read a request
co_await http::async_read(stream, buffer, req,
// Read a request. Use a parser so that we can control the limit of the
// request size.
http::request_parser<http::string_body> requestParser;
auto bodyLimit =
RuntimeParameters().get<"request-body-limit">().getBytes();
requestParser.body_limit(bodyLimit == 0
? boost::none
: boost::optional<uint64_t>(bodyLimit));
co_await http::async_read(stream, buffer, requestParser,
boost::asio::use_awaitable);
http::request<http::string_body> req = requestParser.release();

// Let request be handled by `WebSocketSession` if the HTTP
// request is a WebSocket handshake
Expand Down
Loading