diff --git a/include/seastar/http/client.hh b/include/seastar/http/client.hh index 8a2b6f3d37b..2da612a245e 100644 --- a/include/seastar/http/client.hh +++ b/include/seastar/http/client.hh @@ -127,8 +127,7 @@ public: future<> close(); private: - future do_make_request(request& rq); - void setup_request(request& rq); + future do_make_request(const request& rq); future<> send_request_head(const request& rq); future maybe_wait_for_continue(const request& req); future<> write_body(const request& rq); @@ -182,7 +181,7 @@ private: requires std::invocable auto with_new_connection(Fn&& fn, abort_source*); - future<> do_make_request(connection& con, request& req, reply_handler& handle, abort_source*, std::optional expected); + future<> do_make_request(connection& con, const request& req, reply_handler& handle, abort_source*, std::optional expected); public: /** @@ -275,7 +274,7 @@ public: * `request and the `handle`, it caller's responsibility the make sure they * are referencing valid instances */ - future<> make_request(request& req, reply_handler& handle, std::optional expected = std::nullopt, abort_source* as = nullptr); + future<> make_request(const request& req, reply_handler& handle, std::optional expected = std::nullopt, abort_source* as = nullptr); /** * \brief Updates the maximum number of connections a client may have diff --git a/src/http/client.cc b/src/http/client.cc index 40088407b5d..092d957d30a 100644 --- a/src/http/client.cc +++ b/src/http/client.cc @@ -111,15 +111,12 @@ future connection::maybe_wait_for_continue(const request& }); } -void connection::setup_request(request& req) { +static void validate_request(const request& req) { if (req._version.empty()) { - req._version = "1.1"; + throw std::runtime_error("HTTP version not set"); } - if (req.content_length != 0) { - if (!req.body_writer && req.content.empty()) { - throw std::runtime_error("Request body writer not set and content is empty"); - } - req._headers["Content-Length"] = to_sstring(req.content_length); + if (req.content_length != 0 && !req.body_writer && req.content.empty()) { + throw std::runtime_error("Request body writer not set and content is empty"); } } @@ -156,8 +153,7 @@ future connection::recv_reply() { }); } -future connection::do_make_request(request& req) { - setup_request(req); +future connection::do_make_request(const request& req) { return send_request_head(req).then([this, &req] { return maybe_wait_for_continue(req).then([this, &req] (reply_ptr cont) { if (cont) { @@ -174,6 +170,11 @@ future connection::do_make_request(request& req) { } future connection::make_request(request req) { + try { + validate_request(req); + } catch (...) { + return current_exception_as_future(); + } return do_with(std::move(req), [this] (auto& req) { return do_make_request(req).then([] (reply_ptr rep) { return make_ready_future(std::move(*rep)); @@ -353,7 +354,12 @@ static bool is_retryable_exception(std::exception_ptr ex) { return false; } -future<> client::make_request(request& req, reply_handler& handle, std::optional expected, abort_source* as) { +future<> client::make_request(const request& req, reply_handler& handle, std::optional expected, abort_source* as) { + try { + validate_request(req); + } catch (...) { + return current_exception_as_future(); + } return with_connection([this, &req, &handle, as, expected] (connection& con) { return do_make_request(con, req, handle, as, expected); }, as).handle_exception([this, &req, &handle, as, expected] (std::exception_ptr ex) { @@ -391,7 +397,7 @@ class skip_body_source : public data_source_impl { } }; -future<> client::do_make_request(connection& con, request& req, reply_handler& handle, abort_source* as, std::optional expected) { +future<> client::do_make_request(connection& con, const request& req, reply_handler& handle, abort_source* as, std::optional expected) { auto sub = as ? as->subscribe([&con] () noexcept { con.shutdown(); }) : std::nullopt; return con.do_make_request(req).then([this, &con, &req, &handle, expected] (connection::reply_ptr reply) mutable { auto& rep = *reply; diff --git a/src/http/request.cc b/src/http/request.cc index 8a9fde51ff7..7b238efb7bf 100644 --- a/src/http/request.cc +++ b/src/http/request.cc @@ -101,6 +101,7 @@ sstring request::parse_query_param() { void request::write_body(const sstring& content_type, sstring content) { set_content_type(content_type); content_length = content.size(); + _headers["Content-Length"] = to_sstring(content_length); this->content = std::move(content); } @@ -113,6 +114,7 @@ void request::write_body(const sstring& content_type, body_writer_type&& body_wr void request::write_body(const sstring& content_type, size_t len, body_writer_type&& body_writer) { set_content_type(content_type); content_length = len; + _headers["Content-Length"] = to_sstring(content_length); if (len > 0) { // At the time of this writing, connection::write_body() // assumes that `body_writer` is unset if `content_length` is 0. @@ -126,6 +128,7 @@ void request::set_expects_continue() { request request::make(sstring method, sstring host, sstring path) { request rq; + rq._version = "1.1"; rq._method = std::move(method); rq._url = std::move(path); rq._headers["Host"] = std::move(host); diff --git a/tests/unit/httpd_test.cc b/tests/unit/httpd_test.cc index f094262e071..7fa5f3f2b36 100644 --- a/tests/unit/httpd_test.cc +++ b/tests/unit/httpd_test.cc @@ -2099,7 +2099,7 @@ SEASTAR_THREAD_TEST_CASE(test_http_with_broken_wire) { http::experimental::client c(addr, creds); http::request req; - + req._version = "1.1"; req.write_body("html", std::string(5134, 'a')); auto sa = server.accept();