From cda23d867cd2e93aca7eb1c9b1c9d56ca94282af Mon Sep 17 00:00:00 2001 From: Huang-Ming Huang Date: Mon, 20 Jul 2026 20:29:13 +0000 Subject: [PATCH 1/7] Fix SEC-130 bound snapshot bootstrap downloads --- docs/snapshot-serving-plan.md | 12 +- .../include/fc/network/http/http_client.hpp | 30 +- .../libfc/src/network/http/http_client.cpp | 169 +++++++-- libraries/libfc/test/CMakeLists.txt | 1 + .../libfc/test/network/test_http_client.cpp | 336 ++++++++++++++++++ plugins/chain_plugin/src/chain_plugin.cpp | 96 ++++- plugins/chain_plugin/test/CMakeLists.txt | 2 + .../chain_plugin/test/plugin_config_test.cpp | 95 +++++ plugins/snapshot_api_plugin/README.md | 22 ++ tests/snapshot_api_test.py | 12 +- 10 files changed, 729 insertions(+), 46 deletions(-) create mode 100644 libraries/libfc/test/network/test_http_client.cpp diff --git a/docs/snapshot-serving-plan.md b/docs/snapshot-serving-plan.md index 9b46d556c0..5aaa348816 100644 --- a/docs/snapshot-serving-plan.md +++ b/docs/snapshot-serving-plan.md @@ -252,7 +252,7 @@ The peers endpoint is a separate feature and was not implemented in this phase. ### Configuration -A single CLI-only option (not config file — single-use bootstrap): +The endpoint and its resource limits are CLI-only because bootstrap is a single-use operation: ``` --snapshot-endpoint URL Fetch snapshot from URL and bootstrap. @@ -261,6 +261,12 @@ A single CLI-only option (not config file — single-use bootstrap): https://snap.example.com/50000 → fetches block 50000 ``` +The bounded download options are `--snapshot-endpoint-connect-timeout-ms`, +`--snapshot-endpoint-header-timeout-ms`, `--snapshot-endpoint-idle-timeout-ms`, +`--snapshot-endpoint-total-timeout-ms`, `--snapshot-endpoint-max-download-size-mb`, and +`--snapshot-endpoint-min-disk-free-mb`. They enforce finite phase/total deadlines, fixed-length and chunked response +ceilings, and reserved filesystem headroom while retaining atomic temporary-file cleanup. + The block number is encoded as a trailing path component of the URL. If the last path segment is a decimal number, it's treated as a specific block request (POST to `/v1/snapshot/by_block`); otherwise POST to `/v1/snapshot/latest`. ### Files @@ -281,7 +287,9 @@ The block number is encoded as a trailing path component of the URL. If the last ``` This works naturally with `--delete-all-blocks` which clears state before snapshot handling. 3. **Fetch metadata:** POST to `/v1/snapshot/latest` or `/v1/snapshot/by_block` depending on URL format. -4. **Download snapshot:** Uses `fc::http_client::post_to_file()` to POST to `/v1/snapshot/download` and save binary response to local snapshots directory. +4. **Download snapshot:** Uses bounded `fc::http_client::post_to_file()` streaming to POST to + `/v1/snapshot/download`, enforce deadlines/size/disk headroom, and atomically save the response to the local + snapshots directory. 5. **Root hash verification:** Uses `threaded_snapshot_reader::load_index()` to read the footer and compare the stored root hash against the advertised `root_hash`. This is a fast metadata-only check that catches download corruption. Full integrity verification (re-hashing all sections) happens during snapshot loading, and on-chain attestation verification happens after syncing. 6. **Continue normal loading:** Sets `snapshot_path` to downloaded file and `snapshot_auto_fetched = true`. No `--genesis-json` needed — snapshot contains genesis. diff --git a/libraries/libfc/include/fc/network/http/http_client.hpp b/libraries/libfc/include/fc/network/http/http_client.hpp index e34b9d6727..3dc1f0e3e9 100644 --- a/libraries/libfc/include/fc/network/http/http_client.hpp +++ b/libraries/libfc/include/fc/network/http/http_client.hpp @@ -11,10 +11,27 @@ #include #include +#include #include namespace fc { +/** Resource and deadline limits for a streamed HTTP file download. */ +struct http_file_download_options { + /// Maximum time allowed to establish the connection. + microseconds connect_timeout; + /// Maximum time allowed to receive the response header after sending the request. + microseconds response_header_timeout; + /// Maximum time allowed without decoded response-body progress. + microseconds idle_read_timeout; + /// Maximum time allowed for the complete request and download. + microseconds total_timeout; + /// Maximum accepted response-body size in bytes. + uint64_t max_response_body_bytes; + /// Free bytes that must remain on the destination filesystem. + uint64_t min_free_disk_space_bytes; +}; + class http_client { public: http_client(); @@ -37,6 +54,17 @@ class http_client { const std::filesystem::path& output, const time_point& deadline = time_point::maximum()); + /** + * Download a binary POST response using explicit phase deadlines and resource limits. + * + * The response is written to a temporary sibling of @p output and renamed only after + * the complete bounded body has been persisted successfully. + */ + void post_to_file(const url& dest, + const variant& payload, + const std::filesystem::path& output, + const http_file_download_options& options); + void add_cert(const std::string& cert_pem_string); void set_verify_peers(bool enabled); @@ -44,4 +72,4 @@ class http_client { std::unique_ptr _my; }; -} \ No newline at end of file +} diff --git a/libraries/libfc/src/network/http/http_client.cpp b/libraries/libfc/src/network/http/http_client.cpp index 20d1594ac4..296f9d0e1e 100644 --- a/libraries/libfc/src/network/http/http_client.cpp +++ b/libraries/libfc/src/network/http/http_client.cpp @@ -3,7 +3,11 @@ #include #include +#include #include +#include +#include +#include #include #include @@ -19,6 +23,13 @@ namespace local = boost::asio::local; namespace fc { +namespace { + +constexpr size_t download_read_buffer_bytes = 1024 * 1024; +constexpr size_t download_parser_buffer_bytes = 64 * 1024; + +} // namespace + /** * mapping of protocols to their standard ports */ @@ -146,16 +157,49 @@ class http_client_impl { }); } - /// Read the remainder of the response (the body) into @p parser, honoring @p deadline. + /// Read one increment of a streaming response body into @p parser, honoring @p deadline. template - error_code sync_read_parser_with_timeout(SyncReadStream& s, boost::beast::flat_buffer& buffer, Parser& parser, const deadline_type& deadline ) { - return sync_do_with_deadline(s, deadline, [&s, &buffer, &parser](std::optional& final_ec){ - http::async_read(s, buffer, parser, [&final_ec]( const error_code& ec, std::size_t ) { + error_code sync_read_parser_some_with_timeout(SyncReadStream& s, boost::beast::flat_buffer& buffer, + Parser& parser, const deadline_type& deadline) { + return sync_do_with_deadline(s, deadline, [&s, &buffer, &parser](std::optional& final_ec) { + http::async_read_some(s, buffer, parser, [&final_ec](const error_code& ec, std::size_t) { final_ec.emplace(ec); }); }); } + /// Return the earlier of the total deadline and a relative phase timeout from now. + static deadline_type phase_deadline(const fc::microseconds& timeout, const deadline_type& total_deadline) { + auto deadline = fc::time_point::now(); + deadline.safe_add(timeout); + return std::min(deadline.to_system_clock(), total_deadline); + } + + /// Return a path on the destination filesystem that can be queried for free space. + static std::filesystem::path space_query_path(const std::filesystem::path& destination) { + if (destination.has_parent_path()) { + return destination.parent_path(); + } + + std::error_code ec; + auto current = std::filesystem::current_path(ec); + FC_ASSERT(!ec, "Failed to resolve current directory for download disk-space check: {}", ec.message()); + return current; + } + + /// Require enough free space for @p next_write_bytes while retaining @p reserved_bytes. + static void require_disk_space(const std::filesystem::path& destination, uint64_t next_write_bytes, + uint64_t reserved_bytes) { + const auto query_path = space_query_path(destination); + std::error_code ec; + const auto space = std::filesystem::space(query_path, ec); + FC_ASSERT(!ec, "Failed to query free disk space at {}: {}", query_path.string(), ec.message()); + FC_ASSERT(space.available >= reserved_bytes && next_write_bytes <= space.available - reserved_bytes, + "Insufficient disk space at {} for HTTP download: {} bytes available, {} bytes required for the " + "next write, and {} bytes reserved as headroom", + query_path.string(), space.available, next_write_bytes, reserved_bytes); + } + host_key url_to_host_key( const url& dest ) { FC_ASSERT(dest.host(), "Provided URL has no host"); uint16_t port = 80; @@ -388,8 +432,9 @@ class http_client_impl { return _unix_url_paths.emplace(full_url, fc::url("unix", socket_file.string(), ostring(), ostring(), url_path.string(), ostring(), ovariant_object(), std::optional())).first->second; } - void post_to_file(const url& dest, const variant& payload, const std::filesystem::path& final_dest, const fc::time_point& deadline_time) { - auto deadline = deadline_time.to_system_clock(); + void post_to_file(const url& dest, const variant& payload, const std::filesystem::path& final_dest, + const http_file_download_options& options, const fc::time_point& total_deadline_time) { + const auto total_deadline = total_deadline_time.to_system_clock(); FC_ASSERT(dest.host(), "No host set on URL"); std::string path = dest.path() ? dest.path()->generic_string() : "/"; @@ -411,29 +456,30 @@ class http_client_impl { req.set(http::field::user_agent, BOOST_BEAST_VERSION_STRING); req.set(http::field::content_type, "application/json"); req.keep_alive(false); - req.body() = json::to_string(payload, deadline_time); + req.body() = json::to_string(payload, total_deadline_time); req.prepare_payload(); - auto conn_iter = get_connection(dest, deadline); + auto conn_iter = get_connection(dest, phase_deadline(options.connect_timeout, total_deadline)); auto eraser = make_scoped_exit([this, &conn_iter](){ _connections.erase(conn_iter); }); - error_code ec = std::visit(write_request_visitor(this, req, deadline), conn_iter->second); + error_code ec = std::visit(write_request_visitor(this, req, total_deadline), conn_iter->second); FC_ASSERT(!ec, "Failed to send POST request: {}", ec.message()); - // Stream the response body straight to disk through a file_body parser: snapshot - // downloads are multi-GB, so buffering the payload in a string_body response could - // exhaust memory long before the data reaches its hash check. - boost::beast::flat_buffer buffer; - http::response_parser parser; - // The download exceeds the parser's default body-size limit; the transfer is bounded - // by the caller's deadline and by available disk space instead. - parser.body_limit(boost::none); + // Parse into a bounded caller-owned buffer so every write can enforce the byte ceiling, + // idle deadline, and filesystem headroom before bytes reach disk. + boost::beast::flat_buffer buffer(download_parser_buffer_bytes); + http::response_parser parser; + parser.body_limit(options.max_response_body_bytes); ec = std::visit([&](auto& stream) { - return sync_read_header_with_timeout(*stream, buffer, parser, deadline); + return sync_read_header_with_timeout( + *stream, buffer, parser, phase_deadline(options.response_header_timeout, total_deadline)); }, conn_iter->second); + if (ec == http::error::body_limit) { + FC_THROW("HTTP response body exceeds configured maximum of {} bytes", options.max_response_body_bytes); + } FC_ASSERT(!ec, "Failed to read response header: {}", ec.message()); // Require exactly 200 OK: this client never sends Range requests, so 206 Partial Content @@ -441,15 +487,16 @@ class http_client_impl { // the final destination would hand the caller a truncated file. const auto status = parser.get().result(); if (status != http::status::ok) { - // Error responses carry small diagnostic bodies; read into memory for the message. - // Best effort: on a read failure the throw below reports whatever body arrived. - http::response_parser err_parser{std::move(parser)}; - if (!err_parser.is_done()) { - std::visit([&](auto& stream) { - return sync_read_parser_with_timeout(*stream, buffer, err_parser, deadline); - }, conn_iter->second); - } - FC_THROW("HTTP POST failed with status {}: {}", (int)status, err_parser.get().body().substr(0, 200)); + FC_THROW("HTTP POST failed with status {}", parser.get().result_int()); + } + + if (const auto content_length = parser.content_length()) { + FC_ASSERT(*content_length <= options.max_response_body_bytes, + "HTTP response Content-Length {} exceeds configured maximum of {} bytes", + *content_length, options.max_response_body_bytes); + require_disk_space(final_dest, *content_length, options.min_free_disk_space_bytes); + } else { + require_disk_space(final_dest, 0, options.min_free_disk_space_bytes); } // Write to temp file then rename @@ -460,18 +507,43 @@ class http_client_impl { std::filesystem::remove(temp_path, rm_ec); }); - http::file_body::value_type file; - file.open(temp_path.c_str(), boost::beast::file_mode::write, ec); - FC_ASSERT(!ec, "Failed to open temp file {} for writing: {}", temp_path.string(), ec.message()); - parser.get().body() = std::move(file); + std::ofstream file(temp_path, std::ios::binary | std::ios::trunc); + FC_ASSERT(file.is_open(), "Failed to open temp file {} for writing", temp_path.string()); - if (!parser.is_done()) { + std::vector read_buffer(download_read_buffer_bytes); + uint64_t downloaded_bytes = 0; + auto idle_deadline = phase_deadline(options.idle_read_timeout, total_deadline); + while (!parser.is_done()) { + parser.get().body().data = read_buffer.data(); + parser.get().body().size = read_buffer.size(); ec = std::visit([&](auto& stream) { - return sync_read_parser_with_timeout(*stream, buffer, parser, deadline); + return sync_read_parser_some_with_timeout(*stream, buffer, parser, idle_deadline); }, conn_iter->second); + if (ec == http::error::need_buffer) { + ec = {}; + } + if (ec == http::error::body_limit) { + FC_THROW("HTTP response body exceeds configured maximum of {} bytes", options.max_response_body_bytes); + } FC_ASSERT(!ec, "Failed to read response body: {}", ec.message()); + + const auto chunk_bytes = read_buffer.size() - parser.get().body().size; + FC_ASSERT(chunk_bytes <= options.max_response_body_bytes && + downloaded_bytes <= options.max_response_body_bytes - chunk_bytes, + "HTTP response body exceeds configured maximum of {} bytes", options.max_response_body_bytes); + if (chunk_bytes == 0) { + continue; + } + + require_disk_space(final_dest, chunk_bytes, options.min_free_disk_space_bytes); + file.write(read_buffer.data(), static_cast(chunk_bytes)); + FC_ASSERT(file.good(), "Failed to write {} bytes to temp file {}", chunk_bytes, temp_path.string()); + downloaded_bytes += chunk_bytes; + idle_deadline = phase_deadline(options.idle_read_timeout, total_deadline); } - parser.get().body().close(); + + file.close(); + FC_ASSERT(file.good(), "Failed to close temp file {} after HTTP download", temp_path.string()); std::error_code rename_ec; std::filesystem::rename(temp_path, final_dest, rename_ec); @@ -479,6 +551,30 @@ class http_client_impl { cleanup.cancel(); } + void post_to_file(const url& dest, const variant& payload, const std::filesystem::path& final_dest, + const fc::time_point& deadline_time) { + const auto now = fc::time_point::now(); + const auto remaining = deadline_time == fc::time_point::maximum() + ? fc::microseconds::maximum() + : deadline_time - now; + const http_file_download_options options{ + .connect_timeout = remaining, + .response_header_timeout = remaining, + .idle_read_timeout = remaining, + .total_timeout = remaining, + .max_response_body_bytes = std::numeric_limits::max(), + .min_free_disk_space_bytes = 0, + }; + post_to_file(dest, payload, final_dest, options, deadline_time); + } + + void post_to_file(const url& dest, const variant& payload, const std::filesystem::path& final_dest, + const http_file_download_options& options) { + auto total_deadline = fc::time_point::now(); + total_deadline.safe_add(options.total_timeout); + post_to_file(dest, payload, final_dest, options, total_deadline); + } + boost::asio::io_context _ioc; connection_map _connections; unix_url_split_map _unix_url_paths; @@ -495,6 +591,11 @@ void http_client::post_to_file(const url& dest, const variant& payload, const st _my->post_to_file(dest, payload, output, deadline); } +void http_client::post_to_file(const url& dest, const variant& payload, const std::filesystem::path& output, + const http_file_download_options& options) { + _my->post_to_file(dest, payload, output, options); +} + variant http_client::post_sync(const url& dest, const variant& payload, const fc::time_point& deadline) { if(dest.proto() == "unix") return _my->post_sync(_my->get_unix_url(*dest.host()), payload, deadline); diff --git a/libraries/libfc/test/CMakeLists.txt b/libraries/libfc/test/CMakeLists.txt index ec1cc0e211..a0205e553b 100644 --- a/libraries/libfc/test/CMakeLists.txt +++ b/libraries/libfc/test/CMakeLists.txt @@ -18,6 +18,7 @@ add_executable( test_fc io/test_raw.cpp io/test_tracked_storage.cpp network/ethereum/test_ethereum_client_and_rlp.cpp + network/test_http_client.cpp network/test_json_rpc_client.cpp network/solana/test_solana_client.cpp network/test_message_buffer.cpp diff --git a/libraries/libfc/test/network/test_http_client.cpp b/libraries/libfc/test/network/test_http_client.cpp new file mode 100644 index 0000000000..f54eaac601 --- /dev/null +++ b/libraries/libfc/test/network/test_http_client.cpp @@ -0,0 +1,336 @@ +/** + * @file test_http_client.cpp + * @brief Regression tests for bounded streamed HTTP file downloads. + */ + +#include +#include + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace std::chrono_literals; + +namespace { + +using tcp = boost::asio::ip::tcp; + +constexpr int64_t short_timeout_ms = 100; +constexpr int64_t normal_timeout_ms = 2'000; +constexpr int64_t max_test_elapsed_ms = 1'500; +constexpr int64_t framing_delay_ms = 70; +constexpr size_t exact_body_bytes = 8; +constexpr size_t oversized_chunk_extension_bytes = 128 * 1024; +constexpr std::string_view exact_body = "12345678"; + +/** A one-request loopback HTTP server driven by a caller-supplied response script. */ +class scripted_http_server { +public: + using handler_type = std::function; + + /** Start a loopback listener and execute @p handler after receiving one request header. */ + explicit scripted_http_server(handler_type handler) + : _acceptor(_io, tcp::endpoint(boost::asio::ip::make_address("127.0.0.1"), 0)) + , _socket(_io) + , _port(_acceptor.local_endpoint().port()) + , _handler(std::move(handler)) + , _worker([this] { serve(); }) {} + + scripted_http_server(const scripted_http_server&) = delete; + scripted_http_server& operator=(const scripted_http_server&) = delete; + + /** Stop the response script and release its worker. */ + ~scripted_http_server() { + _stop = true; + boost::system::error_code ec; + _acceptor.close(ec); + _socket.cancel(ec); + _socket.close(ec); + unblock_accept(); + if (_worker.joinable()) { + _worker.join(); + } + } + + /** Return the ephemeral loopback port assigned to this server. */ + uint16_t port() const { return _port; } + +private: + /** Connect to the listener so a platform that does not cancel synchronous accept can exit. */ + void unblock_accept() { + boost::asio::io_context io; + tcp::socket socket(io); + boost::system::error_code ec; + socket.connect(tcp::endpoint(boost::asio::ip::make_address("127.0.0.1"), _port), ec); + } + + /** Accept one request, consume its header, and run the configured response script. */ + void serve() { + boost::system::error_code ec; + _acceptor.accept(_socket, ec); + if (ec) { + return; + } + + boost::asio::streambuf request; + boost::asio::read_until(_socket, request, "\r\n\r\n", ec); + if (!ec) { + _handler(_socket, _stop); + } + } + + boost::asio::io_context _io; + tcp::acceptor _acceptor; + tcp::socket _socket; + uint16_t _port; + handler_type _handler; + std::atomic_bool _stop{false}; + std::thread _worker; +}; + +/** Write all of @p bytes unless the peer has already disconnected. */ +bool write_bytes(tcp::socket& socket, std::string_view bytes) { + boost::system::error_code ec; + boost::asio::write(socket, boost::asio::buffer(bytes.data(), bytes.size()), ec); + return !ec; +} + +/** Return a complete 200 response header for a fixed-length body. */ +std::string fixed_length_header(uint64_t body_bytes) { + std::ostringstream response; + response << "HTTP/1.1 200 OK\r\n" + << "Content-Type: application/octet-stream\r\n" + << "Content-Length: " << body_bytes << "\r\n" + << "Connection: close\r\n\r\n"; + return response.str(); +} + +/** Return a complete 200 response header for a chunked body. */ +std::string chunked_header() { + return "HTTP/1.1 200 OK\r\n" + "Content-Type: application/octet-stream\r\n" + "Transfer-Encoding: chunked\r\n" + "Connection: close\r\n\r\n"; +} + +/** Return finite test limits with a caller-selected response-body maximum. */ +fc::http_file_download_options download_options(uint64_t max_body_bytes) { + return fc::http_file_download_options{ + .connect_timeout = fc::milliseconds(normal_timeout_ms), + .response_header_timeout = fc::milliseconds(normal_timeout_ms), + .idle_read_timeout = fc::milliseconds(normal_timeout_ms), + .total_timeout = fc::milliseconds(normal_timeout_ms), + .max_response_body_bytes = max_body_bytes, + .min_free_disk_space_bytes = 1, + }; +} + +/** Return the URL for @p server. */ +fc::url server_url(const scripted_http_server& server) { + return fc::url("http://127.0.0.1:" + std::to_string(server.port()) + "/download"); +} + +/** Invoke a bounded empty-payload download into @p output. */ +void download(const scripted_http_server& server, const std::filesystem::path& output, + const fc::http_file_download_options& options) { + fc::http_client client; + client.post_to_file(server_url(server), fc::variant(fc::mutable_variant_object()), output, options); +} + +/** Return the temporary sibling used while @p output is being downloaded. */ +std::filesystem::path partial_path(const std::filesystem::path& output) { + auto partial = output; + partial += ".downloading"; + return partial; +} + +/** Verify that neither the final output nor its temporary sibling exists. */ +void check_download_files_removed(const std::filesystem::path& output) { + BOOST_CHECK(!std::filesystem::exists(output)); + BOOST_CHECK(!std::filesystem::exists(partial_path(output))); +} + +/** Return the complete binary contents of @p path. */ +std::string read_file(const std::filesystem::path& path) { + std::ifstream input(path, std::ios::binary); + return std::string(std::istreambuf_iterator(input), std::istreambuf_iterator()); +} + +} // namespace + +BOOST_AUTO_TEST_SUITE(http_client_file_download_tests) + +/// A peer that never sends a response header must hit the configured header deadline. +BOOST_AUTO_TEST_CASE(response_header_timeout_removes_partial_file) { + scripted_http_server server([](tcp::socket&, const std::atomic_bool& stop) { + while (!stop.load()) { + std::this_thread::sleep_for(10ms); + } + }); + fc::temp_directory temp; + const auto output = temp.path() / "header-timeout.bin"; + auto options = download_options(exact_body_bytes); + options.response_header_timeout = fc::milliseconds(short_timeout_ms); + + const auto start = std::chrono::steady_clock::now(); + BOOST_CHECK_THROW(download(server, output, options), fc::exception); + const auto elapsed = std::chrono::steady_clock::now() - start; + + BOOST_CHECK_LT(std::chrono::duration_cast(elapsed).count(), max_test_elapsed_ms); + check_download_files_removed(output); +} + +/// A body that stops after partial progress must hit the reset-on-progress idle deadline. +BOOST_AUTO_TEST_CASE(idle_body_timeout_removes_partial_file) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool& stop) { + if (!write_bytes(socket, fixed_length_header(2)) || !write_bytes(socket, "1")) { + return; + } + while (!stop.load()) { + std::this_thread::sleep_for(10ms); + } + }); + fc::temp_directory temp; + const auto output = temp.path() / "idle-timeout.bin"; + auto options = download_options(exact_body_bytes); + options.idle_read_timeout = fc::milliseconds(short_timeout_ms); + + BOOST_CHECK_THROW(download(server, output, options), fc::exception); + check_download_files_removed(output); +} + +/// Chunk framing without decoded body bytes must not refresh the response-progress deadline. +BOOST_AUTO_TEST_CASE(chunk_framing_does_not_reset_idle_timeout) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + if (!write_bytes(socket, chunked_header())) { + return; + } + std::this_thread::sleep_for(std::chrono::milliseconds(framing_delay_ms)); + if (!write_bytes(socket, "1\r\n")) { + return; + } + std::this_thread::sleep_for(std::chrono::milliseconds(framing_delay_ms)); + write_bytes(socket, "1\r\n0\r\n\r\n"); + }); + fc::temp_directory temp; + const auto output = temp.path() / "framing-idle-timeout.bin"; + auto options = download_options(exact_body_bytes); + options.idle_read_timeout = fc::milliseconds(short_timeout_ms); + + BOOST_CHECK_THROW(download(server, output, options), fc::exception); + check_download_files_removed(output); +} + +/// Continuous sub-idle progress must still stop at the independent total deadline. +BOOST_AUTO_TEST_CASE(total_timeout_stops_trickled_body_and_removes_partial_file) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool& stop) { + if (!write_bytes(socket, fixed_length_header(1'000))) { + return; + } + while (!stop.load()) { + if (!write_bytes(socket, "1")) { + return; + } + std::this_thread::sleep_for(40ms); + } + }); + fc::temp_directory temp; + const auto output = temp.path() / "total-timeout.bin"; + auto options = download_options(1'000); + options.idle_read_timeout = fc::milliseconds(short_timeout_ms); + options.total_timeout = fc::milliseconds(250); + + BOOST_CHECK_THROW(download(server, output, options), fc::exception); + check_download_files_removed(output); +} + +/// Oversized Content-Length is rejected while parsing the header, before a temp file is opened. +BOOST_AUTO_TEST_CASE(oversized_fixed_length_response_is_rejected_before_write) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + write_bytes(socket, fixed_length_header(exact_body_bytes + 1)); + }); + fc::temp_directory temp; + const auto output = temp.path() / "oversized-fixed.bin"; + + BOOST_CHECK_THROW(download(server, output, download_options(exact_body_bytes)), fc::exception); + check_download_files_removed(output); +} + +/// A chunked response that exceeds the configured maximum is aborted and its temp file is removed. +BOOST_AUTO_TEST_CASE(oversized_chunked_response_is_rejected_and_removed) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + write_bytes(socket, chunked_header() + "9\r\n123456789\r\n0\r\n\r\n"); + }); + fc::temp_directory temp; + const auto output = temp.path() / "oversized-chunked.bin"; + + BOOST_CHECK_THROW(download(server, output, download_options(exact_body_bytes)), fc::exception); + check_download_files_removed(output); +} + +/// An unterminated oversized chunk extension must fail within bounded parser memory and time. +BOOST_AUTO_TEST_CASE(oversized_chunk_extension_is_bounded_and_removed) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + if (!write_bytes(socket, chunked_header())) { + return; + } + write_bytes(socket, "1;" + std::string(oversized_chunk_extension_bytes, 'a')); + }); + fc::temp_directory temp; + const auto output = temp.path() / "oversized-chunk-extension.bin"; + auto options = download_options(exact_body_bytes); + options.idle_read_timeout = fc::milliseconds(short_timeout_ms); + + const auto start = std::chrono::steady_clock::now(); + BOOST_CHECK_EXCEPTION(download(server, output, options), fc::exception, + [](const fc::exception& error) { + return error.to_detail_string().find("buffer overflow") != std::string::npos; + }); + const auto elapsed = std::chrono::steady_clock::now() - start; + + BOOST_CHECK_LT(std::chrono::duration_cast(elapsed).count(), max_test_elapsed_ms); + check_download_files_removed(output); +} + +/// A response exactly equal to the configured byte maximum must complete and be atomically renamed. +BOOST_AUTO_TEST_CASE(exact_maximum_response_succeeds) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + write_bytes(socket, fixed_length_header(exact_body_bytes) + std::string(exact_body)); + }); + fc::temp_directory temp; + const auto output = temp.path() / "exact-maximum.bin"; + + BOOST_CHECK_NO_THROW(download(server, output, download_options(exact_body_bytes))); + BOOST_CHECK(std::filesystem::exists(output)); + BOOST_CHECK(!std::filesystem::exists(partial_path(output))); + BOOST_CHECK_EQUAL(read_file(output), exact_body); +} + +/// A reserved-headroom requirement larger than the filesystem capacity must fail before writing. +BOOST_AUTO_TEST_CASE(insufficient_disk_headroom_is_rejected_before_write) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + write_bytes(socket, fixed_length_header(1) + "1"); + }); + fc::temp_directory temp; + const auto output = temp.path() / "disk-headroom.bin"; + auto options = download_options(exact_body_bytes); + options.min_free_disk_space_bytes = std::numeric_limits::max(); + + BOOST_CHECK_THROW(download(server, output, options), fc::exception); + check_download_files_removed(output); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/plugins/chain_plugin/src/chain_plugin.cpp b/plugins/chain_plugin/src/chain_plugin.cpp index 080b2cfcbb..f39241077e 100644 --- a/plugins/chain_plugin/src/chain_plugin.cpp +++ b/plugins/chain_plugin/src/chain_plugin.cpp @@ -38,6 +38,8 @@ #include #include #include +#include +#include const std::string deep_mind_logger_name("dmlog"); @@ -63,6 +65,37 @@ namespace snapshot_attest { /// provider snapshot interval (_snapshot_provider_block_spacing). constexpr uint32_t snapshot_attestation_grace_blocks = 12500; +constexpr std::string_view snapshot_connect_timeout_option = "snapshot-endpoint-connect-timeout-ms"; +constexpr std::string_view snapshot_header_timeout_option = "snapshot-endpoint-header-timeout-ms"; +constexpr std::string_view snapshot_idle_timeout_option = "snapshot-endpoint-idle-timeout-ms"; +constexpr std::string_view snapshot_total_timeout_option = "snapshot-endpoint-total-timeout-ms"; +constexpr std::string_view snapshot_max_download_size_option = "snapshot-endpoint-max-download-size-mb"; +constexpr std::string_view snapshot_min_disk_free_option = "snapshot-endpoint-min-disk-free-mb"; + +constexpr uint64_t default_snapshot_connect_timeout_ms = 10'000; +constexpr uint64_t default_snapshot_header_timeout_ms = 30'000; +constexpr uint64_t default_snapshot_idle_timeout_ms = 60'000; +constexpr uint64_t default_snapshot_total_timeout_ms = 24 * 60 * 60 * 1000; +constexpr uint64_t bytes_per_mebibyte = 1024 * 1024; + +/// Read a positive millisecond option and convert it without overflowing fc::microseconds. +fc::microseconds checked_milliseconds(const boost::program_options::variables_map& options, + std::string_view option_name) { + const auto value = options.at(std::string(option_name)).as(); + constexpr auto max_milliseconds = static_cast(std::numeric_limits::max() / 1000); + SYS_ASSERT(value > 0 && value <= max_milliseconds, sysio::chain::plugin_config_exception, + "{} must be between 1 and {} milliseconds", option_name, max_milliseconds); + return fc::milliseconds(static_cast(value)); +} + +/// Convert a positive MiB option value to bytes without overflowing uint64_t. +uint64_t checked_mebibytes(uint64_t value, std::string_view option_name) { + constexpr auto max_mebibytes = std::numeric_limits::max() / bytes_per_mebibyte; + SYS_ASSERT(value > 0 && value <= max_mebibytes, sysio::chain::plugin_config_exception, + "{} must be between 1 and {} MiB", option_name, max_mebibytes); + return value * bytes_per_mebibyte; +} + } // anonymous namespace namespace std { @@ -268,9 +301,10 @@ class chain_plugin_impl { /// irreversible-block handler with the just-finalized block; retries on later finalized /// blocks until the attestation record appears or the node catches up to the chain tip. void verify_snapshot_attestation(const signed_block_ptr& lib_block); - /// Download and hash-check a snapshot from a remote provider, leaving snapshot_path + /// Download and hash-check a bounded snapshot from a remote provider, leaving snapshot_path /// pointing at the downloaded file for the normal snapshot-load path to consume. - void fetch_snapshot_from_endpoint(const std::string& endpoint_url); + void fetch_snapshot_from_endpoint(const std::string& endpoint_url, + const fc::http_file_download_options& download_options); private: static void log_guard_exception(const chain::guard_exception& e); @@ -478,6 +512,22 @@ void chain_plugin::set_program_options(options_description& cli, options_descrip " http://host:port - fetches latest snapshot\n" " http://host:port/50000 - fetches snapshot at block 50000\n" "Requires empty database (use --delete-all-blocks to clear existing data).") + (snapshot_connect_timeout_option.data(), + bpo::value()->default_value(default_snapshot_connect_timeout_ms), + "Maximum time in milliseconds to establish the snapshot download connection.") + (snapshot_header_timeout_option.data(), + bpo::value()->default_value(default_snapshot_header_timeout_ms), + "Maximum time in milliseconds to receive snapshot endpoint response headers.") + (snapshot_idle_timeout_option.data(), + bpo::value()->default_value(default_snapshot_idle_timeout_ms), + "Maximum time in milliseconds without snapshot response-body progress.") + (snapshot_total_timeout_option.data(), + bpo::value()->default_value(default_snapshot_total_timeout_ms), + "Maximum total time in milliseconds for the snapshot file download.") + (snapshot_max_download_size_option.data(), bpo::value(), + "Maximum snapshot response size in MiB. Defaults to chain-state-db-size-mb.") + (snapshot_min_disk_free_option.data(), bpo::value(), + "Free disk space in MiB that must remain during download. Defaults to chain-state-db-guard-size-mb.") ; } @@ -882,7 +932,30 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) { if (options.count("snapshot-endpoint")) { SYS_ASSERT(!options.contains("snapshot"), plugin_config_exception, "--snapshot-endpoint is incompatible with --snapshot; use one or the other"); - fetch_snapshot_from_endpoint(options.at("snapshot-endpoint").as()); + + const auto max_download_bytes = checked_mebibytes( + options.contains(std::string(snapshot_max_download_size_option)) + ? options.at(std::string(snapshot_max_download_size_option)).as() + : options.at("chain-state-db-size-mb").as(), + options.contains(std::string(snapshot_max_download_size_option)) + ? snapshot_max_download_size_option + : std::string_view{"chain-state-db-size-mb"}); + const auto min_free_disk_bytes = checked_mebibytes( + options.contains(std::string(snapshot_min_disk_free_option)) + ? options.at(std::string(snapshot_min_disk_free_option)).as() + : options.at("chain-state-db-guard-size-mb").as(), + options.contains(std::string(snapshot_min_disk_free_option)) + ? snapshot_min_disk_free_option + : std::string_view{"chain-state-db-guard-size-mb"}); + const fc::http_file_download_options download_options{ + .connect_timeout = checked_milliseconds(options, snapshot_connect_timeout_option), + .response_header_timeout = checked_milliseconds(options, snapshot_header_timeout_option), + .idle_read_timeout = checked_milliseconds(options, snapshot_idle_timeout_option), + .total_timeout = checked_milliseconds(options, snapshot_total_timeout_option), + .max_response_body_bytes = max_download_bytes, + .min_free_disk_space_bytes = min_free_disk_bytes, + }; + fetch_snapshot_from_endpoint(options.at("snapshot-endpoint").as(), download_options); } std::optional chain_id; @@ -1515,7 +1588,8 @@ bool chain_plugin::accept_transactions() const { return my->accept_transactions; } -void chain_plugin_impl::fetch_snapshot_from_endpoint(const std::string& endpoint_url) { +void chain_plugin_impl::fetch_snapshot_from_endpoint( + const std::string& endpoint_url, const fc::http_file_download_options& download_options) { // Check for existing chain data auto shared_mem_path = chain_config->state_dir / "shared_memory.bin"; auto chain_head_path = chain_config->state_dir / chain_head_filename; @@ -1545,16 +1619,20 @@ void chain_plugin_impl::fetch_snapshot_from_endpoint(const std::string& endpoint ilog("Fetching snapshot metadata from endpoint: {}", endpoint_url); - fc::http_client http_client; + fc::http_client metadata_http_client; + auto metadata_deadline = fc::time_point::now(); + metadata_deadline.safe_add(download_options.connect_timeout); + metadata_deadline.safe_add(download_options.response_header_timeout); fc::variant metadata_response; if (request_block_num) { auto url = fc::url(base_url + "/v1/snapshot/by_block"); fc::mutable_variant_object payload; payload("block_num", *request_block_num); - metadata_response = http_client.post_sync(url, fc::variant(payload)); + metadata_response = metadata_http_client.post_sync(url, fc::variant(payload), metadata_deadline); } else { auto url = fc::url(base_url + "/v1/snapshot/latest"); - metadata_response = http_client.post_sync(url, fc::variant(fc::mutable_variant_object())); + metadata_response = metadata_http_client.post_sync( + url, fc::variant(fc::mutable_variant_object()), metadata_deadline); } auto snap_block_num = metadata_response["block_num"].as(); @@ -1576,7 +1654,9 @@ void chain_plugin_impl::fetch_snapshot_from_endpoint(const std::string& endpoint auto download_url = fc::url(base_url + "/v1/snapshot/download"); fc::mutable_variant_object download_payload; download_payload("block_num", snap_block_num); - http_client.post_to_file(download_url, fc::variant(download_payload), download_dest); + fc::http_client download_http_client; + download_http_client.post_to_file( + download_url, fc::variant(download_payload), download_dest, download_options); } // Quick check: compare the root hash stored in the snapshot footer against diff --git a/plugins/chain_plugin/test/CMakeLists.txt b/plugins/chain_plugin/test/CMakeLists.txt index 9e30f3f891..c77bea0764 100644 --- a/plugins/chain_plugin/test/CMakeLists.txt +++ b/plugins/chain_plugin/test/CMakeLists.txt @@ -8,6 +8,8 @@ add_executable( test_chain_plugin target_link_libraries( test_chain_plugin chain_plugin sysio_testing sysio_chain_wrap version ) set(CHAIN_PLUGIN_TEST_CASES chain_plugin_default_tests + chain_plugin_snapshot_endpoint_option_registration + chain_plugin_snapshot_endpoint_option_validation account_query_db_tests trx_finality_status_processing_test trx_retry_db_test diff --git a/plugins/chain_plugin/test/plugin_config_test.cpp b/plugins/chain_plugin/test/plugin_config_test.cpp index ab05aa42ec..801fe872c2 100644 --- a/plugins/chain_plugin/test/plugin_config_test.cpp +++ b/plugins/chain_plugin/test/plugin_config_test.cpp @@ -1,8 +1,44 @@ #include +#include #include #include #include #include +#include +#include +#include +#include + +namespace { + +/** Initialize chain_plugin with one snapshot-endpoint limit override. */ +sysio::chain::exit_code::exit_code initialize_with_snapshot_limit(std::string_view option_name, + std::string_view option_value) { + fc::temp_directory tmp; + sysio::chain::application exe({.enable_resource_monitor = false}); + + const auto tmp_path = tmp.path().string(); + std::vector arguments{ + "test_chain_plugin", + "--snapshot-endpoint", + "http://127.0.0.1:1", + "--config-dir", + tmp_path, + "--data-dir", + tmp_path, + "--" + std::string(option_name), + std::string(option_value), + }; + std::vector argv; + argv.reserve(arguments.size()); + for (auto& argument : arguments) { + argv.push_back(argument.data()); + } + + return exe.init(static_cast(argv.size()), argv.data()); +} + +} // anonymous namespace BOOST_AUTO_TEST_CASE(chain_plugin_default_tests) { fc::temp_directory tmp; @@ -25,6 +61,65 @@ BOOST_AUTO_TEST_CASE(chain_plugin_default_tests) { } +/** Verify that every bounded snapshot-download command-line option is registered and parsed. */ +BOOST_AUTO_TEST_CASE(chain_plugin_snapshot_endpoint_option_registration) { + sysio::chain::application exe({.enable_resource_monitor = false}); + sysio::chain_plugin plugin; + boost::program_options::options_description cli; + boost::program_options::options_description cfg; + boost::program_options::options_description options; + plugin.set_program_options(cli, cfg); + options.add(cli).add(cfg); + + std::array arguments{ + "test_chain_plugin", + "--snapshot-endpoint", "http://127.0.0.1:1", + "--snapshot-endpoint-connect-timeout-ms", "11", + "--snapshot-endpoint-header-timeout-ms", "12", + "--snapshot-endpoint-idle-timeout-ms", "13", + "--snapshot-endpoint-total-timeout-ms", "14", + "--snapshot-endpoint-max-download-size-mb", "15", + "--snapshot-endpoint-min-disk-free-mb", "16", + }; + boost::program_options::variables_map variables; + boost::program_options::store( + boost::program_options::parse_command_line(arguments.size(), const_cast(arguments.data()), options), + variables); + boost::program_options::notify(variables); + + BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint").as(), "http://127.0.0.1:1"); + BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint-connect-timeout-ms").as(), 11); + BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint-header-timeout-ms").as(), 12); + BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint-idle-timeout-ms").as(), 13); + BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint-total-timeout-ms").as(), 14); + BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint-max-download-size-mb").as(), 15); + BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint-min-disk-free-mb").as(), 16); +} + +/** Verify that snapshot endpoint limits reject zero and overflow-prone values before connecting. */ +BOOST_AUTO_TEST_CASE(chain_plugin_snapshot_endpoint_option_validation) { + constexpr std::array invalid_options{ + std::pair{"snapshot-endpoint-connect-timeout-ms", "0"}, + std::pair{"snapshot-endpoint-header-timeout-ms", "0"}, + std::pair{"snapshot-endpoint-idle-timeout-ms", "0"}, + std::pair{"snapshot-endpoint-total-timeout-ms", "0"}, + std::pair{"snapshot-endpoint-max-download-size-mb", "0"}, + std::pair{"snapshot-endpoint-min-disk-free-mb", "0"}, + std::pair{"snapshot-endpoint-total-timeout-ms", "9223372036854776"}, + std::pair{"snapshot-endpoint-max-download-size-mb", "17592186044416"}, + std::pair{"chain-state-db-size-mb", "0"}, + std::pair{"chain-state-db-size-mb", "17592186044416"}, + std::pair{"chain-state-db-guard-size-mb", "0"}, + std::pair{"chain-state-db-guard-size-mb", "17592186044416"}, + }; + + for (const auto& [option_name, option_value] : invalid_options) { + BOOST_TEST_CONTEXT(option_name << '=' << option_value) { + BOOST_CHECK(initialize_with_snapshot_limit(option_name, option_value) != sysio::chain::exit_code::SUCCESS); + } + } +} + #ifdef SYSIO_SYS_VM_OC_RUNTIME_ENABLED /** Verify the default SYS VM OC whitelist when the OC runtime is compiled into this build. */ BOOST_AUTO_TEST_CASE(chain_plugin_default_sys_vm_oc_whitelist) { diff --git a/plugins/snapshot_api_plugin/README.md b/plugins/snapshot_api_plugin/README.md index 9a8aa4fc31..fb9bf711cd 100644 --- a/plugins/snapshot_api_plugin/README.md +++ b/plugins/snapshot_api_plugin/README.md @@ -247,6 +247,24 @@ The bootstrap process: `--delete-all-blocks` is required when existing chain data is present. The `--snapshot-endpoint` option is incompatible with `--snapshot` (local file). +### Bootstrap download limits + +Snapshot metadata and file transfers use finite deadlines and a streaming byte ceiling. The following CLI-only options +can be tuned for the expected provider latency, snapshot size, and available storage: + +| Option | Default | Purpose | +|---|---:|---| +| `--snapshot-endpoint-connect-timeout-ms` | `10000` | Maximum time to establish the download connection | +| `--snapshot-endpoint-header-timeout-ms` | `30000` | Maximum time to receive response headers | +| `--snapshot-endpoint-idle-timeout-ms` | `60000` | Maximum time without response-body progress | +| `--snapshot-endpoint-total-timeout-ms` | `86400000` | Maximum total snapshot file download time (24 hours) | +| `--snapshot-endpoint-max-download-size-mb` | `chain-state-db-size-mb` | Maximum accepted snapshot response size | +| `--snapshot-endpoint-min-disk-free-mb` | `chain-state-db-guard-size-mb` | Free space retained on the snapshots filesystem | + +All limits must be positive. A fixed-length response that exceeds the maximum is rejected from its `Content-Length` +before a temporary file is opened. Chunked or lengthless responses are stopped at the same byte ceiling. Available +disk space is checked before and throughout the transfer, and a failed transfer removes its `.downloading` file. + ## Reverse Proxy Considerations For production deployments, consider placing a reverse proxy (nginx, caddy) in front of the snapshot endpoint: @@ -256,4 +274,8 @@ For production deployments, consider placing a reverse proxy (nginx, caddy) in f - **Caching / CDN** — offload download bandwidth from the node - **Access control** — restrict by IP or authentication +Configure the proxy with response-size, idle, and total-transfer limits that are no weaker than the origin node's +bootstrap limits. Proxy authentication and TLS reduce interception and unauthorized access risk, but they do not +replace the origin-side resource bounds. + The snapshot files are served uncompressed via zero-copy `sendfile()`. If bandwidth is a concern, configure compression at the proxy layer (e.g., `gzip_static` or `zstd_static` in nginx with pre-compressed files). diff --git a/tests/snapshot_api_test.py b/tests/snapshot_api_test.py index 8c08b837fc..72bb0a21a3 100755 --- a/tests/snapshot_api_test.py +++ b/tests/snapshot_api_test.py @@ -453,11 +453,21 @@ def recordPresent(): endpointUrl = node0.endpointHttp Print(f"Restart bootstrap node with --snapshot-endpoint {endpointUrl}") + # Exercise the bounded-download CLI options with a finite cap just above the + # snapshot being bootstrapped and a small deterministic test headroom. + snap2FileSize = os.path.getsize(node0.getLatestSnapshot()) + bytesPerMiB = 1024 * 1024 + downloadLimitMiB = (snap2FileSize + bytesPerMiB - 1) // bytesPerMiB + 1 + downloadLimits = ( + f"--snapshot-endpoint-max-download-size-mb {downloadLimitMiB} " + "--snapshot-endpoint-min-disk-free-mb 1" + ) + # Fetches latest snapshot (snap2). The attestation is NOT in the snapshot — # it's in blocks after snap2BlockNum. The bootstrap node syncs forward and # finds the attestation record once it reaches those blocks. isRelaunchSuccess = bootstrapNode.relaunch( - chainArg=f"--delete-all-blocks --snapshot-endpoint {endpointUrl}") + chainArg=f"--delete-all-blocks --snapshot-endpoint {endpointUrl} {downloadLimits}") assert isRelaunchSuccess, "Failed to relaunch bootstrap node from snapshot endpoint" # The attestation is in blocks after the snapshot height, so wait for the bootstrap From 538197a4f56f18af8847c29fe7bbe30ef1a8bf8d Mon Sep 17 00:00:00 2001 From: Huang-Ming Huang Date: Mon, 20 Jul 2026 21:37:29 +0000 Subject: [PATCH 2/7] Address SEC-130 review feedback --- .../include/fc/network/http/http_client.hpp | 12 +- .../libfc/src/network/http/http_client.cpp | 190 ++++++++---- .../libfc/test/network/test_http_client.cpp | 286 +++++++++++++++++- plugins/chain_plugin/src/chain_plugin.cpp | 32 +- plugins/snapshot_api_plugin/README.md | 4 +- 5 files changed, 432 insertions(+), 92 deletions(-) diff --git a/libraries/libfc/include/fc/network/http/http_client.hpp b/libraries/libfc/include/fc/network/http/http_client.hpp index 3dc1f0e3e9..dfd8e4227f 100644 --- a/libraries/libfc/include/fc/network/http/http_client.hpp +++ b/libraries/libfc/include/fc/network/http/http_client.hpp @@ -20,7 +20,7 @@ namespace fc { struct http_file_download_options { /// Maximum time allowed to establish the connection. microseconds connect_timeout; - /// Maximum time allowed to receive the response header after sending the request. + /// Maximum time allowed for each of the request-write and response-header phases. microseconds response_header_timeout; /// Maximum time allowed without decoded response-body progress. microseconds idle_read_timeout; @@ -30,6 +30,8 @@ struct http_file_download_options { uint64_t max_response_body_bytes; /// Free bytes that must remain on the destination filesystem. uint64_t min_free_disk_space_bytes; + /// Retry a failed request once when it used a cached connection. Only enable for idempotent requests. + bool retry_failed_reused_connection = false; }; class http_client { @@ -46,14 +48,6 @@ class http_client { return post_sync(dest, payload_v, deadline); } - /// Download binary response from a POST request to a file. - /// Uses a temp file during download and renames on completion. - /// Clean up on failure. - void post_to_file(const url& dest, - const variant& payload, - const std::filesystem::path& output, - const time_point& deadline = time_point::maximum()); - /** * Download a binary POST response using explicit phase deadlines and resource limits. * diff --git a/libraries/libfc/src/network/http/http_client.cpp b/libraries/libfc/src/network/http/http_client.cpp index 296f9d0e1e..d580439df7 100644 --- a/libraries/libfc/src/network/http/http_client.cpp +++ b/libraries/libfc/src/network/http/http_client.cpp @@ -4,9 +4,11 @@ #include #include +#include #include #include #include +#include #include #include @@ -27,6 +29,9 @@ namespace { constexpr size_t download_read_buffer_bytes = 1024 * 1024; constexpr size_t download_parser_buffer_bytes = 64 * 1024; +constexpr uint64_t disk_space_check_interval_bytes = 64 * 1024 * 1024; +constexpr uint64_t disk_space_concurrency_margin_bytes = 64 * 1024 * 1024; +constexpr auto disk_space_check_interval = std::chrono::seconds(5); } // namespace @@ -175,6 +180,17 @@ class http_client_impl { return std::min(deadline.to_system_clock(), total_deadline); } + /// Return whether @p ec can indicate that a cached peer connection closed before reuse. + static bool is_stale_connection_error(const error_code& ec) { + return ec == boost::asio::error::eof || + ec == boost::asio::error::connection_reset || + ec == boost::asio::error::connection_aborted || + ec == boost::asio::error::broken_pipe || + ec == boost::asio::error::not_connected || + ec == boost::asio::error::shut_down || + ec == http::error::end_of_stream; + } + /// Return a path on the destination filesystem that can be queried for free space. static std::filesystem::path space_query_path(const std::filesystem::path& destination) { if (destination.has_parent_path()) { @@ -187,9 +203,13 @@ class http_client_impl { return current; } - /// Require enough free space for @p next_write_bytes while retaining @p reserved_bytes. - static void require_disk_space(const std::filesystem::path& destination, uint64_t next_write_bytes, - uint64_t reserved_bytes) { + /** + * Require enough free space for @p next_write_bytes while retaining @p reserved_bytes. + * + * @return Bytes currently available for writes after retaining the requested reserve. + */ + static uint64_t require_disk_space(const std::filesystem::path& destination, uint64_t next_write_bytes, + uint64_t reserved_bytes) { const auto query_path = space_query_path(destination); std::error_code ec; const auto space = std::filesystem::space(query_path, ec); @@ -198,6 +218,27 @@ class http_client_impl { "Insufficient disk space at {} for HTTP download: {} bytes available, {} bytes required for the " "next write, and {} bytes reserved as headroom", query_path.string(), space.available, next_write_bytes, reserved_bytes); + return space.available - reserved_bytes; + } + + /** + * Verify and return the next cached write budget. + * + * Each budget is capped at 64 MiB and requires a separate 64 MiB concurrency margin. + * The margin bounds headroom exposure if another process consumes space between probes; + * slow transfers also refresh the probe every five seconds. + */ + static uint64_t refresh_disk_space_write_budget(const std::filesystem::path& destination, + uint64_t remaining_body_bytes, + uint64_t reserved_bytes) { + const auto write_budget = std::min(remaining_body_bytes, disk_space_check_interval_bytes); + if (write_budget == 0) { + require_disk_space(destination, 0, reserved_bytes); + return 0; + } + + require_disk_space(destination, write_budget + disk_space_concurrency_margin_bytes, reserved_bytes); + return write_budget; } host_key url_to_host_key( const url& dest ) { @@ -268,12 +309,19 @@ class http_client_impl { } } - connection_map::iterator get_connection( const url& dest, const deadline_type& deadline ) { + connection_map::iterator get_connection(const url& dest, const deadline_type& deadline, + bool* reused_connection = nullptr) { auto key = url_to_host_key(dest); auto conn_itr = _connections.find(key); if (conn_itr == _connections.end() || check_closed(conn_itr)) { + if (reused_connection) { + *reused_connection = false; + } return create_connection(dest, deadline); } else { + if (reused_connection) { + *reused_connection = true; + } return conn_itr; } } @@ -459,45 +507,88 @@ class http_client_impl { req.body() = json::to_string(payload, total_deadline_time); req.prepare_payload(); - auto conn_iter = get_connection(dest, phase_deadline(options.connect_timeout, total_deadline)); - auto eraser = make_scoped_exit([this, &conn_iter](){ - _connections.erase(conn_iter); + bool reused_connection = false; + auto conn_iter = get_connection( + dest, phase_deadline(options.connect_timeout, total_deadline), &reused_connection); + const auto connection_key = url_to_host_key(dest); + auto eraser = make_scoped_exit([this, connection_key](){ + _connections.erase(connection_key); }); - error_code ec = std::visit(write_request_visitor(this, req, total_deadline), conn_iter->second); - FC_ASSERT(!ec, "Failed to send POST request: {}", ec.message()); - - // Parse into a bounded caller-owned buffer so every write can enforce the byte ceiling, - // idle deadline, and filesystem headroom before bytes reach disk. - boost::beast::flat_buffer buffer(download_parser_buffer_bytes); - http::response_parser parser; - parser.body_limit(options.max_response_body_bytes); - - ec = std::visit([&](auto& stream) { - return sync_read_header_with_timeout( - *stream, buffer, parser, phase_deadline(options.response_header_timeout, total_deadline)); - }, conn_iter->second); - if (ec == http::error::body_limit) { - FC_THROW("HTTP response body exceeds configured maximum of {} bytes", options.max_response_body_bytes); + std::unique_ptr buffer; + std::unique_ptr> parser; + error_code ec; + bool retried_stale_connection = false; + while (true) { + ec = std::visit( + write_request_visitor(this, req, phase_deadline(options.response_header_timeout, total_deadline)), + conn_iter->second); + if (ec) { + if (options.retry_failed_reused_connection && reused_connection && !retried_stale_connection && + is_stale_connection_error(ec)) { + _connections.erase(conn_iter); + conn_iter = get_connection( + dest, phase_deadline(options.connect_timeout, total_deadline), &reused_connection); + retried_stale_connection = true; + continue; + } + FC_ASSERT(false, "Failed to send POST request: {}", ec.message()); + } + + // Parse into a bounded caller-owned buffer so every write can enforce the byte ceiling, + // idle deadline, and filesystem headroom before bytes reach disk. + buffer = std::make_unique(download_parser_buffer_bytes); + parser = std::make_unique>(); + parser->body_limit(options.max_response_body_bytes); + + ec = std::visit([&](auto& stream) { + return sync_read_header_with_timeout( + *stream, *buffer, *parser, phase_deadline(options.response_header_timeout, total_deadline)); + }, conn_iter->second); + if (!ec) { + break; + } + if (ec == http::error::body_limit) { + FC_THROW("HTTP response body exceeds configured maximum of {} bytes", options.max_response_body_bytes); + } + if (options.retry_failed_reused_connection && reused_connection && !retried_stale_connection && + is_stale_connection_error(ec)) { + _connections.erase(conn_iter); + conn_iter = get_connection( + dest, phase_deadline(options.connect_timeout, total_deadline), &reused_connection); + retried_stale_connection = true; + continue; + } + FC_ASSERT(false, "Failed to read response header: {}", ec.message()); } - FC_ASSERT(!ec, "Failed to read response header: {}", ec.message()); // Require exactly 200 OK: this client never sends Range requests, so 206 Partial Content // can only come from a broken or hostile endpoint/proxy, and renaming a partial body into // the final destination would hand the caller a truncated file. - const auto status = parser.get().result(); + const auto status = parser->get().result(); if (status != http::status::ok) { - FC_THROW("HTTP POST failed with status {}", parser.get().result_int()); + FC_THROW("HTTP POST failed with status {}", parser->get().result_int()); } - if (const auto content_length = parser.content_length()) { + const auto content_length = parser->content_length(); + uint64_t disk_space_write_budget = 0; + if (content_length) { FC_ASSERT(*content_length <= options.max_response_body_bytes, "HTTP response Content-Length {} exceeds configured maximum of {} bytes", *content_length, options.max_response_body_bytes); - require_disk_space(final_dest, *content_length, options.min_free_disk_space_bytes); + FC_ASSERT(*content_length <= std::numeric_limits::max() - disk_space_concurrency_margin_bytes, + "HTTP response Content-Length {} is too large for the {}-byte disk safety margin", + *content_length, disk_space_concurrency_margin_bytes); + const auto preflight_bytes = *content_length == 0 + ? 0 + : *content_length + disk_space_concurrency_margin_bytes; + require_disk_space(final_dest, preflight_bytes, options.min_free_disk_space_bytes); + disk_space_write_budget = std::min(*content_length, disk_space_check_interval_bytes); } else { - require_disk_space(final_dest, 0, options.min_free_disk_space_bytes); + disk_space_write_budget = refresh_disk_space_write_budget( + final_dest, options.max_response_body_bytes, options.min_free_disk_space_bytes); } + auto next_disk_space_check = std::chrono::steady_clock::now() + disk_space_check_interval; // Write to temp file then rename auto temp_path = final_dest; @@ -513,11 +604,11 @@ class http_client_impl { std::vector read_buffer(download_read_buffer_bytes); uint64_t downloaded_bytes = 0; auto idle_deadline = phase_deadline(options.idle_read_timeout, total_deadline); - while (!parser.is_done()) { - parser.get().body().data = read_buffer.data(); - parser.get().body().size = read_buffer.size(); + while (!parser->is_done()) { + parser->get().body().data = read_buffer.data(); + parser->get().body().size = read_buffer.size(); ec = std::visit([&](auto& stream) { - return sync_read_parser_some_with_timeout(*stream, buffer, parser, idle_deadline); + return sync_read_parser_some_with_timeout(*stream, *buffer, *parser, idle_deadline); }, conn_iter->second); if (ec == http::error::need_buffer) { ec = {}; @@ -527,7 +618,7 @@ class http_client_impl { } FC_ASSERT(!ec, "Failed to read response body: {}", ec.message()); - const auto chunk_bytes = read_buffer.size() - parser.get().body().size; + const auto chunk_bytes = read_buffer.size() - parser->get().body().size; FC_ASSERT(chunk_bytes <= options.max_response_body_bytes && downloaded_bytes <= options.max_response_body_bytes - chunk_bytes, "HTTP response body exceeds configured maximum of {} bytes", options.max_response_body_bytes); @@ -535,10 +626,20 @@ class http_client_impl { continue; } - require_disk_space(final_dest, chunk_bytes, options.min_free_disk_space_bytes); + // Amortize filesystem-space queries while bounding concurrent-consumption exposure with + // a separate safety margin. Recheck after 64 MiB or five seconds, whichever comes first. + if (disk_space_write_budget < chunk_bytes || + std::chrono::steady_clock::now() >= next_disk_space_check) { + const auto response_size_limit = content_length.value_or(options.max_response_body_bytes); + const auto remaining_body_bytes = response_size_limit - downloaded_bytes; + disk_space_write_budget = refresh_disk_space_write_budget( + final_dest, remaining_body_bytes, options.min_free_disk_space_bytes); + next_disk_space_check = std::chrono::steady_clock::now() + disk_space_check_interval; + } file.write(read_buffer.data(), static_cast(chunk_bytes)); FC_ASSERT(file.good(), "Failed to write {} bytes to temp file {}", chunk_bytes, temp_path.string()); downloaded_bytes += chunk_bytes; + disk_space_write_budget -= chunk_bytes; idle_deadline = phase_deadline(options.idle_read_timeout, total_deadline); } @@ -551,23 +652,6 @@ class http_client_impl { cleanup.cancel(); } - void post_to_file(const url& dest, const variant& payload, const std::filesystem::path& final_dest, - const fc::time_point& deadline_time) { - const auto now = fc::time_point::now(); - const auto remaining = deadline_time == fc::time_point::maximum() - ? fc::microseconds::maximum() - : deadline_time - now; - const http_file_download_options options{ - .connect_timeout = remaining, - .response_header_timeout = remaining, - .idle_read_timeout = remaining, - .total_timeout = remaining, - .max_response_body_bytes = std::numeric_limits::max(), - .min_free_disk_space_bytes = 0, - }; - post_to_file(dest, payload, final_dest, options, deadline_time); - } - void post_to_file(const url& dest, const variant& payload, const std::filesystem::path& final_dest, const http_file_download_options& options) { auto total_deadline = fc::time_point::now(); @@ -587,10 +671,6 @@ http_client::http_client() } -void http_client::post_to_file(const url& dest, const variant& payload, const std::filesystem::path& output, const fc::time_point& deadline) { - _my->post_to_file(dest, payload, output, deadline); -} - void http_client::post_to_file(const url& dest, const variant& payload, const std::filesystem::path& output, const http_file_download_options& options) { _my->post_to_file(dest, payload, output, options); diff --git a/libraries/libfc/test/network/test_http_client.cpp b/libraries/libfc/test/network/test_http_client.cpp index f54eaac601..c30d33482c 100644 --- a/libraries/libfc/test/network/test_http_client.cpp +++ b/libraries/libfc/test/network/test_http_client.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -33,20 +34,26 @@ constexpr int64_t normal_timeout_ms = 2'000; constexpr int64_t max_test_elapsed_ms = 1'500; constexpr int64_t framing_delay_ms = 70; constexpr size_t exact_body_bytes = 8; +constexpr size_t blocked_request_body_bytes = 16 * 1024 * 1024; +constexpr size_t disk_space_budget_bytes = 64 * 1024 * 1024; +constexpr size_t large_body_chunk_bytes = 1024 * 1024; constexpr size_t oversized_chunk_extension_bytes = 128 * 1024; constexpr std::string_view exact_body = "12345678"; -/** A one-request loopback HTTP server driven by a caller-supplied response script. */ +/** A loopback HTTP server driven by a caller-supplied script for each accepted connection. */ class scripted_http_server { public: using handler_type = std::function; - /** Start a loopback listener and execute @p handler after receiving one request header. */ - explicit scripted_http_server(handler_type handler) + /** Start a listener and execute @p handler for each requested connection. */ + explicit scripted_http_server(handler_type handler, bool consume_request_header = true, + size_t connections_to_accept = 1) : _acceptor(_io, tcp::endpoint(boost::asio::ip::make_address("127.0.0.1"), 0)) , _socket(_io) , _port(_acceptor.local_endpoint().port()) , _handler(std::move(handler)) + , _consume_request_header(consume_request_header) + , _connections_to_accept(connections_to_accept) , _worker([this] { serve(); }) {} scripted_http_server(const scripted_http_server&) = delete; @@ -68,6 +75,9 @@ class scripted_http_server { /** Return the ephemeral loopback port assigned to this server. */ uint16_t port() const { return _port; } + /** Return whether the configured server script has completed. */ + bool finished() const { return _finished.load(); } + private: /** Connect to the listener so a platform that does not cancel synchronous accept can exit. */ void unblock_accept() { @@ -77,19 +87,37 @@ class scripted_http_server { socket.connect(tcp::endpoint(boost::asio::ip::make_address("127.0.0.1"), _port), ec); } - /** Accept one request, consume its header, and run the configured response script. */ + /** Accept configured requests, consume each header, and run the response script. */ void serve() { boost::system::error_code ec; - _acceptor.accept(_socket, ec); - if (ec) { - return; - } + for (size_t connection_index = 0; connection_index < _connections_to_accept; ++connection_index) { + if (connection_index != 0) { + _socket = tcp::socket(_io); + } + _acceptor.accept(_socket, ec); + if (ec) { + break; + } - boost::asio::streambuf request; - boost::asio::read_until(_socket, request, "\r\n\r\n", ec); - if (!ec) { + if (_consume_request_header) { + boost::asio::streambuf request; + boost::asio::read_until(_socket, request, "\r\n\r\n", ec); + if (ec) { + break; + } + } else { + _socket.set_option(tcp::socket::receive_buffer_size(1'024), ec); + if (ec) { + break; + } + } _handler(_socket, _stop); + if (connection_index + 1 < _connections_to_accept) { + _socket.close(ec); + } } + _acceptor.close(ec); + _finished = true; } boost::asio::io_context _io; @@ -97,7 +125,10 @@ class scripted_http_server { tcp::socket _socket; uint16_t _port; handler_type _handler; + bool _consume_request_header; + size_t _connections_to_accept; std::atomic_bool _stop{false}; + std::atomic_bool _finished{false}; std::thread _worker; }; @@ -126,6 +157,44 @@ std::string chunked_header() { "Connection: close\r\n\r\n"; } +/** Return a JSON metadata response that invites a second request on the same connection. */ +std::string keep_alive_metadata_response() { + return "HTTP/1.1 200 OK\r\n" + "Content-Type: application/json\r\n" + "Content-Length: 2\r\n" + "Connection: keep-alive\r\n\r\n{}"; +} + +/** Write @p body_bytes bytes in bounded blocks. */ +bool write_repeated_body(tcp::socket& socket, uint64_t body_bytes) { + const std::string block(large_body_chunk_bytes, 'x'); + while (body_bytes != 0) { + const auto write_bytes_count = std::min(body_bytes, block.size()); + if (!write_bytes(socket, std::string_view(block.data(), write_bytes_count))) { + return false; + } + body_bytes -= write_bytes_count; + } + return true; +} + +/** Write a chunked body in one-MiB decoded blocks. */ +bool write_chunked_body(tcp::socket& socket, uint64_t body_bytes) { + const std::string block(large_body_chunk_bytes, 'x'); + while (body_bytes != 0) { + const auto write_bytes_count = std::min(body_bytes, block.size()); + std::ostringstream chunk_size; + chunk_size << std::hex << write_bytes_count << "\r\n"; + if (!write_bytes(socket, chunk_size.str()) || + !write_bytes(socket, std::string_view(block.data(), write_bytes_count)) || + !write_bytes(socket, "\r\n")) { + return false; + } + body_bytes -= write_bytes_count; + } + return write_bytes(socket, "0\r\n\r\n"); +} + /** Return finite test limits with a caller-selected response-body maximum. */ fc::http_file_download_options download_options(uint64_t max_body_bytes) { return fc::http_file_download_options{ @@ -135,6 +204,7 @@ fc::http_file_download_options download_options(uint64_t max_body_bytes) { .total_timeout = fc::milliseconds(normal_timeout_ms), .max_response_body_bytes = max_body_bytes, .min_free_disk_space_bytes = 1, + .retry_failed_reused_connection = false, }; } @@ -173,6 +243,121 @@ std::string read_file(const std::filesystem::path& path) { BOOST_AUTO_TEST_SUITE(http_client_file_download_tests) +/// A peer that accepts but does not read the request must hit the request-write phase deadline. +BOOST_AUTO_TEST_CASE(request_write_timeout_precedes_total_timeout) { + scripted_http_server server([](tcp::socket&, const std::atomic_bool& stop) { + while (!stop.load()) { + std::this_thread::sleep_for(10ms); + } + }, false); + fc::temp_directory temp; + const auto output = temp.path() / "request-write-timeout.bin"; + auto options = download_options(exact_body_bytes); + options.response_header_timeout = fc::milliseconds(short_timeout_ms); + + const auto start = std::chrono::steady_clock::now(); + fc::http_client client; + BOOST_CHECK_EXCEPTION( + client.post_to_file(server_url(server), fc::variant(std::string(blocked_request_body_bytes, 'a')), + output, options), + fc::exception, + [](const fc::exception& error) { + return error.to_detail_string().find("Failed to send POST request") != std::string::npos; + }); + const auto elapsed = std::chrono::steady_clock::now() - start; + + BOOST_CHECK_LT(std::chrono::duration_cast(elapsed).count(), max_test_elapsed_ms); + check_download_files_removed(output); +} + +/// Metadata and download requests should reuse one healthy keep-alive connection. +BOOST_AUTO_TEST_CASE(healthy_metadata_connection_is_reused_for_download) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + if (!write_bytes(socket, keep_alive_metadata_response())) { + return; + } + boost::asio::streambuf request; + boost::system::error_code ec; + boost::asio::read_until(socket, request, "\r\n\r\n", ec); + if (!ec) { + write_bytes(socket, fixed_length_header(exact_body_bytes) + std::string(exact_body)); + } + }); + fc::temp_directory temp; + const auto output = temp.path() / "reused-connection.bin"; + fc::http_client client; + auto metadata_deadline = fc::time_point::now(); + metadata_deadline.safe_add(fc::milliseconds(normal_timeout_ms)); + + BOOST_REQUIRE_NO_THROW( + client.post_sync(server_url(server), fc::variant(fc::mutable_variant_object()), metadata_deadline)); + auto options = download_options(exact_body_bytes); + options.retry_failed_reused_connection = true; + BOOST_REQUIRE_NO_THROW( + client.post_to_file(server_url(server), fc::variant(fc::mutable_variant_object()), output, options)); + BOOST_CHECK_EQUAL(read_file(output), exact_body); +} + +/// A cached connection closed after metadata should retry the idempotent download once. +BOOST_AUTO_TEST_CASE(stale_metadata_connection_retries_download_on_fresh_connection) { + std::atomic_size_t connection_index{0}; + scripted_http_server server([&](tcp::socket& socket, const std::atomic_bool&) { + if (connection_index.fetch_add(1) == 0) { + if (write_bytes(socket, keep_alive_metadata_response())) { + boost::system::error_code ec; + socket.shutdown(tcp::socket::shutdown_both, ec); + } + return; + } + write_bytes(socket, fixed_length_header(exact_body_bytes) + std::string(exact_body)); + }, true, 2); + fc::temp_directory temp; + const auto output = temp.path() / "retried-connection.bin"; + fc::http_client client; + auto metadata_deadline = fc::time_point::now(); + metadata_deadline.safe_add(fc::milliseconds(normal_timeout_ms)); + + BOOST_REQUIRE_NO_THROW( + client.post_sync(server_url(server), fc::variant(fc::mutable_variant_object()), metadata_deadline)); + auto options = download_options(exact_body_bytes); + options.retry_failed_reused_connection = true; + BOOST_REQUIRE_NO_THROW( + client.post_to_file(server_url(server), fc::variant(fc::mutable_variant_object()), output, options)); + BOOST_CHECK_EQUAL(connection_index.load(), 2U); + BOOST_CHECK_EQUAL(read_file(output), exact_body); +} + +/// A failed reconnect after stale reuse must unwind without touching an invalid connection iterator. +BOOST_AUTO_TEST_CASE(stale_metadata_reconnect_failure_cleans_up_safely) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + if (write_bytes(socket, keep_alive_metadata_response())) { + boost::system::error_code ec; + socket.shutdown(tcp::socket::shutdown_both, ec); + } + }); + fc::temp_directory temp; + const auto output = temp.path() / "failed-reconnect.bin"; + fc::http_client client; + auto metadata_deadline = fc::time_point::now(); + metadata_deadline.safe_add(fc::milliseconds(normal_timeout_ms)); + + BOOST_REQUIRE_NO_THROW( + client.post_sync(server_url(server), fc::variant(fc::mutable_variant_object()), metadata_deadline)); + for (size_t wait_count = 0; wait_count < 1'000 && !server.finished(); ++wait_count) { + std::this_thread::sleep_for(1ms); + } + BOOST_REQUIRE(server.finished()); + auto options = download_options(exact_body_bytes); + options.retry_failed_reused_connection = true; + BOOST_CHECK_EXCEPTION( + client.post_to_file(server_url(server), fc::variant(fc::mutable_variant_object()), output, options), + fc::exception, + [](const fc::exception& error) { + return error.to_detail_string().find("Failed to connect") != std::string::npos; + }); + check_download_files_removed(output); +} + /// A peer that never sends a response header must hit the configured header deadline. BOOST_AUTO_TEST_CASE(response_header_timeout_removes_partial_file) { scripted_http_server server([](tcp::socket&, const std::atomic_bool& stop) { @@ -319,6 +504,41 @@ BOOST_AUTO_TEST_CASE(exact_maximum_response_succeeds) { BOOST_CHECK_EQUAL(read_file(output), exact_body); } +/// A fixed-length response exactly one disk-check budget wide should not require a refill. +BOOST_AUTO_TEST_CASE(exact_disk_space_budget_response_succeeds) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + if (write_bytes(socket, fixed_length_header(disk_space_budget_bytes))) { + write_repeated_body(socket, disk_space_budget_bytes); + } + }); + fc::temp_directory temp; + const auto output = temp.path() / "exact-disk-budget.bin"; + auto options = download_options(disk_space_budget_bytes); + options.idle_read_timeout = fc::milliseconds(30'000); + options.total_timeout = fc::milliseconds(30'000); + + BOOST_REQUIRE_NO_THROW(download(server, output, options)); + BOOST_CHECK_EQUAL(std::filesystem::file_size(output), disk_space_budget_bytes); +} + +/// A chunked response beyond one disk-check budget must refill and complete successfully. +BOOST_AUTO_TEST_CASE(chunked_response_refills_disk_space_budget) { + constexpr auto response_bytes = disk_space_budget_bytes + large_body_chunk_bytes; + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + if (write_bytes(socket, chunked_header())) { + write_chunked_body(socket, response_bytes); + } + }); + fc::temp_directory temp; + const auto output = temp.path() / "refilled-disk-budget.bin"; + auto options = download_options(response_bytes); + options.idle_read_timeout = fc::milliseconds(30'000); + options.total_timeout = fc::milliseconds(30'000); + + BOOST_REQUIRE_NO_THROW(download(server, output, options)); + BOOST_CHECK_EQUAL(std::filesystem::file_size(output), response_bytes); +} + /// A reserved-headroom requirement larger than the filesystem capacity must fail before writing. BOOST_AUTO_TEST_CASE(insufficient_disk_headroom_is_rejected_before_write) { scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { @@ -333,4 +553,48 @@ BOOST_AUTO_TEST_CASE(insufficient_disk_headroom_is_rejected_before_write) { check_download_files_removed(output); } +/// A write that only barely fits must still leave room for the concurrent-consumer margin. +BOOST_AUTO_TEST_CASE(disk_space_budget_requires_concurrency_margin) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + write_bytes(socket, fixed_length_header(exact_body_bytes) + std::string(exact_body)); + }); + fc::temp_directory temp; + const auto output = temp.path() / "disk-concurrency-margin.bin"; + std::error_code ec; + const auto space = std::filesystem::space(temp.path(), ec); + BOOST_REQUIRE(!ec); + BOOST_REQUIRE_GT(space.available, exact_body_bytes); + auto options = download_options(exact_body_bytes); + options.min_free_disk_space_bytes = space.available - exact_body_bytes; + + BOOST_CHECK_THROW(download(server, output, options), fc::exception); + check_download_files_removed(output); +} + +/// Fixed-length preflight must include the margin before opening a temporary file. +BOOST_AUTO_TEST_CASE(fixed_length_preflight_includes_concurrency_margin) { + constexpr auto response_bytes = disk_space_budget_bytes + 1; + constexpr auto headroom_without_full_margin = disk_space_budget_bytes / 2; + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + write_bytes(socket, fixed_length_header(response_bytes)); + }); + fc::temp_directory temp; + const auto output = temp.path() / "fixed-preflight-margin.bin"; + std::error_code ec; + const auto space = std::filesystem::space(temp.path(), ec); + BOOST_REQUIRE(!ec); + BOOST_REQUIRE_GT(space.available, response_bytes + headroom_without_full_margin); + auto options = download_options(response_bytes); + options.min_free_disk_space_bytes = + space.available - response_bytes - headroom_without_full_margin; + + BOOST_CHECK_EXCEPTION( + download(server, output, options), + fc::exception, + [](const fc::exception& error) { + return error.to_detail_string().find("Insufficient disk space") != std::string::npos; + }); + check_download_files_removed(output); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/plugins/chain_plugin/src/chain_plugin.cpp b/plugins/chain_plugin/src/chain_plugin.cpp index f39241077e..72a46341c5 100644 --- a/plugins/chain_plugin/src/chain_plugin.cpp +++ b/plugins/chain_plugin/src/chain_plugin.cpp @@ -517,7 +517,7 @@ void chain_plugin::set_program_options(options_description& cli, options_descrip "Maximum time in milliseconds to establish the snapshot download connection.") (snapshot_header_timeout_option.data(), bpo::value()->default_value(default_snapshot_header_timeout_ms), - "Maximum time in milliseconds to receive snapshot endpoint response headers.") + "Maximum time in milliseconds for each snapshot request-write and response-header phase.") (snapshot_idle_timeout_option.data(), bpo::value()->default_value(default_snapshot_idle_timeout_ms), "Maximum time in milliseconds without snapshot response-body progress.") @@ -933,20 +933,20 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) { SYS_ASSERT(!options.contains("snapshot"), plugin_config_exception, "--snapshot-endpoint is incompatible with --snapshot; use one or the other"); + const std::string max_download_size_option_name{snapshot_max_download_size_option}; + const bool has_max_download_size = options.contains(max_download_size_option_name); const auto max_download_bytes = checked_mebibytes( - options.contains(std::string(snapshot_max_download_size_option)) - ? options.at(std::string(snapshot_max_download_size_option)).as() + has_max_download_size + ? options.at(max_download_size_option_name).as() : options.at("chain-state-db-size-mb").as(), - options.contains(std::string(snapshot_max_download_size_option)) - ? snapshot_max_download_size_option - : std::string_view{"chain-state-db-size-mb"}); + has_max_download_size ? snapshot_max_download_size_option : std::string_view{"chain-state-db-size-mb"}); + const std::string min_disk_free_option_name{snapshot_min_disk_free_option}; + const bool has_min_disk_free = options.contains(min_disk_free_option_name); const auto min_free_disk_bytes = checked_mebibytes( - options.contains(std::string(snapshot_min_disk_free_option)) - ? options.at(std::string(snapshot_min_disk_free_option)).as() + has_min_disk_free + ? options.at(min_disk_free_option_name).as() : options.at("chain-state-db-guard-size-mb").as(), - options.contains(std::string(snapshot_min_disk_free_option)) - ? snapshot_min_disk_free_option - : std::string_view{"chain-state-db-guard-size-mb"}); + has_min_disk_free ? snapshot_min_disk_free_option : std::string_view{"chain-state-db-guard-size-mb"}); const fc::http_file_download_options download_options{ .connect_timeout = checked_milliseconds(options, snapshot_connect_timeout_option), .response_header_timeout = checked_milliseconds(options, snapshot_header_timeout_option), @@ -954,6 +954,7 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) { .total_timeout = checked_milliseconds(options, snapshot_total_timeout_option), .max_response_body_bytes = max_download_bytes, .min_free_disk_space_bytes = min_free_disk_bytes, + .retry_failed_reused_connection = true, }; fetch_snapshot_from_endpoint(options.at("snapshot-endpoint").as(), download_options); } @@ -1619,7 +1620,7 @@ void chain_plugin_impl::fetch_snapshot_from_endpoint( ilog("Fetching snapshot metadata from endpoint: {}", endpoint_url); - fc::http_client metadata_http_client; + fc::http_client http_client; auto metadata_deadline = fc::time_point::now(); metadata_deadline.safe_add(download_options.connect_timeout); metadata_deadline.safe_add(download_options.response_header_timeout); @@ -1628,10 +1629,10 @@ void chain_plugin_impl::fetch_snapshot_from_endpoint( auto url = fc::url(base_url + "/v1/snapshot/by_block"); fc::mutable_variant_object payload; payload("block_num", *request_block_num); - metadata_response = metadata_http_client.post_sync(url, fc::variant(payload), metadata_deadline); + metadata_response = http_client.post_sync(url, fc::variant(payload), metadata_deadline); } else { auto url = fc::url(base_url + "/v1/snapshot/latest"); - metadata_response = metadata_http_client.post_sync( + metadata_response = http_client.post_sync( url, fc::variant(fc::mutable_variant_object()), metadata_deadline); } @@ -1654,8 +1655,7 @@ void chain_plugin_impl::fetch_snapshot_from_endpoint( auto download_url = fc::url(base_url + "/v1/snapshot/download"); fc::mutable_variant_object download_payload; download_payload("block_num", snap_block_num); - fc::http_client download_http_client; - download_http_client.post_to_file( + http_client.post_to_file( download_url, fc::variant(download_payload), download_dest, download_options); } diff --git a/plugins/snapshot_api_plugin/README.md b/plugins/snapshot_api_plugin/README.md index fb9bf711cd..fcce4a1046 100644 --- a/plugins/snapshot_api_plugin/README.md +++ b/plugins/snapshot_api_plugin/README.md @@ -255,7 +255,7 @@ can be tuned for the expected provider latency, snapshot size, and available sto | Option | Default | Purpose | |---|---:|---| | `--snapshot-endpoint-connect-timeout-ms` | `10000` | Maximum time to establish the download connection | -| `--snapshot-endpoint-header-timeout-ms` | `30000` | Maximum time to receive response headers | +| `--snapshot-endpoint-header-timeout-ms` | `30000` | Maximum time for each request-write and response-header phase | | `--snapshot-endpoint-idle-timeout-ms` | `60000` | Maximum time without response-body progress | | `--snapshot-endpoint-total-timeout-ms` | `86400000` | Maximum total snapshot file download time (24 hours) | | `--snapshot-endpoint-max-download-size-mb` | `chain-state-db-size-mb` | Maximum accepted snapshot response size | @@ -264,6 +264,8 @@ can be tuned for the expected provider latency, snapshot size, and available sto All limits must be positive. A fixed-length response that exceeds the maximum is rejected from its `Content-Length` before a temporary file is opened. Chunked or lengthless responses are stopped at the same byte ceiling. Available disk space is checked before and throughout the transfer, and a failed transfer removes its `.downloading` file. +Long transfers recheck after at most 64 MiB or five seconds of progress and retain an additional 64 MiB safety margin +to bound interference from concurrent disk consumers between probes. ## Reverse Proxy Considerations From 9f25e68dcf6fb4ac99b101868ea218d837f274fa Mon Sep 17 00:00:00 2001 From: Huang-Ming Huang Date: Wed, 22 Jul 2026 18:20:29 +0000 Subject: [PATCH 3/7] Replace snapshot timeouts with attended progress --- docs/snapshot-serving-plan.md | 10 +- .../include/fc/network/http/http_client.hpp | 45 ++- .../libfc/src/network/http/http_client.cpp | 291 ++++++++++++------ .../libfc/test/network/test_http_client.cpp | 172 +++++++---- plugins/chain_plugin/src/chain_plugin.cpp | 149 ++++++--- .../chain_plugin/test/plugin_config_test.cpp | 32 +- plugins/snapshot_api_plugin/README.md | 20 +- tests/snapshot_api_test.py | 12 +- 8 files changed, 477 insertions(+), 254 deletions(-) diff --git a/docs/snapshot-serving-plan.md b/docs/snapshot-serving-plan.md index 5aaa348816..52ec0eee38 100644 --- a/docs/snapshot-serving-plan.md +++ b/docs/snapshot-serving-plan.md @@ -261,11 +261,11 @@ The endpoint and its resource limits are CLI-only because bootstrap is a single- https://snap.example.com/50000 → fetches block 50000 ``` -The bounded download options are `--snapshot-endpoint-connect-timeout-ms`, -`--snapshot-endpoint-header-timeout-ms`, `--snapshot-endpoint-idle-timeout-ms`, -`--snapshot-endpoint-total-timeout-ms`, `--snapshot-endpoint-max-download-size-mb`, and -`--snapshot-endpoint-min-disk-free-mb`. They enforce finite phase/total deadlines, fixed-length and chunked response -ceilings, and reserved filesystem headroom while retaining atomic temporary-file cleanup. +Snapshot bootstrap is an attended operation against an operator-selected endpoint. It reports connection/request phases +and, every five seconds during transfer, downloaded bytes, percentage, rate, and ETA when `Content-Length` is available. +SIGINT cancels pending resolver or socket work and retains atomic temporary-file cleanup. The existing +`chain-state-db-size-mb` setting bounds both fixed-length and chunked response bodies; the only snapshot-specific +resource override is `--snapshot-endpoint-min-disk-free-mb` for reserved filesystem headroom. The block number is encoded as a trailing path component of the URL. If the last path segment is a decimal number, it's treated as a specific block request (POST to `/v1/snapshot/by_block`); otherwise POST to `/v1/snapshot/latest`. diff --git a/libraries/libfc/include/fc/network/http/http_client.hpp b/libraries/libfc/include/fc/network/http/http_client.hpp index dfd8e4227f..5dccf7f3c2 100644 --- a/libraries/libfc/include/fc/network/http/http_client.hpp +++ b/libraries/libfc/include/fc/network/http/http_client.hpp @@ -13,25 +13,42 @@ #include #include +#include +#include namespace fc { -/** Resource and deadline limits for a streamed HTTP file download. */ +/** Current phase of a streamed HTTP file download. */ +enum class http_file_download_phase { + connecting, + sending_request, + waiting_for_response, + downloading, + complete, +}; + +/** Periodic status for a streamed HTTP file download. */ +struct http_file_download_status { + /// Current request or transfer phase. + http_file_download_phase phase; + /// Decoded response-body bytes persisted so far. + uint64_t downloaded_bytes; + /// Expected response-body bytes when the peer supplied Content-Length. + std::optional total_bytes; + /// Time elapsed since the download request started. + microseconds elapsed; +}; + +/** Resource limits and status reporting for a streamed HTTP file download. */ struct http_file_download_options { - /// Maximum time allowed to establish the connection. - microseconds connect_timeout; - /// Maximum time allowed for each of the request-write and response-header phases. - microseconds response_header_timeout; - /// Maximum time allowed without decoded response-body progress. - microseconds idle_read_timeout; - /// Maximum time allowed for the complete request and download. - microseconds total_timeout; /// Maximum accepted response-body size in bytes. uint64_t max_response_body_bytes; /// Free bytes that must remain on the destination filesystem. uint64_t min_free_disk_space_bytes; /// Retry a failed request once when it used a cached connection. Only enable for idempotent requests. bool retry_failed_reused_connection = false; + /// Receive phase transitions and periodic transfer status. The callback must not throw. + std::function status_callback; }; class http_client { @@ -49,7 +66,7 @@ class http_client { } /** - * Download a binary POST response using explicit phase deadlines and resource limits. + * Download a binary POST response using explicit resource limits. * * The response is written to a temporary sibling of @p output and renamed only after * the complete bounded body has been persisted successfully. @@ -59,6 +76,14 @@ class http_client { const std::filesystem::path& output, const http_file_download_options& options); + /** + * Set a predicate used to cancel synchronous HTTP operations. + * + * The predicate is polled while resolver and socket operations are pending. Returning true + * cancels the active operation so callers can interrupt otherwise unbounded requests. + */ + void set_cancel_check(std::function cancel_check); + void add_cert(const std::string& cert_pem_string); void set_verify_peers(bool enabled); diff --git a/libraries/libfc/src/network/http/http_client.cpp b/libraries/libfc/src/network/http/http_client.cpp index d580439df7..6503e2d3ba 100644 --- a/libraries/libfc/src/network/http/http_client.cpp +++ b/libraries/libfc/src/network/http/http_client.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -32,6 +33,8 @@ constexpr size_t download_parser_buffer_bytes = 64 * 1024; constexpr uint64_t disk_space_check_interval_bytes = 64 * 1024 * 1024; constexpr uint64_t disk_space_concurrency_margin_bytes = 64 * 1024 * 1024; constexpr auto disk_space_check_interval = std::chrono::seconds(5); +constexpr auto operation_monitor_interval = std::chrono::milliseconds(100); +constexpr auto download_status_interval = std::chrono::seconds(5); } // namespace @@ -52,6 +55,7 @@ class http_client_impl { using unix_url_split_map = std::map; using error_code = boost::system::error_code; using deadline_type = std::chrono::system_clock::time_point; + using operation_monitor = std::function; http_client_impl() :_ioc() @@ -63,121 +67,175 @@ class http_client_impl { void set_verify_peers(bool enabled) {} + void set_cancel_check(std::function cancel_check) { + _cancel_check = std::move(cancel_check); + } + + /** Return whether the caller requested cancellation of the active HTTP operation. */ + bool is_cancelled() const { + return _cancel_check && _cancel_check(); + } + template - error_code sync_do_with_deadline( SyncReadStream& s, deadline_type deadline, Fn f, CancelFn cf ) { - bool timer_expired = false; - boost::asio::system_timer timer(_ioc); - - timer.expires_at(deadline); - bool timer_cancelled = false; - timer.async_wait([&timer_expired, &timer_cancelled] (const error_code&) { - // the only non-success error_code this is called with is operation_aborted but since - // we could have queued "success" when we cancelled the timer, we set a flag at the - // safer scope and only respect that. - if (!timer_cancelled) { - timer_expired = true; - } - }); + error_code sync_do_with_deadline_impl(SyncReadStream& s, deadline_type deadline, Fn f, CancelFn cf, + const operation_monitor& monitor = {}) { + if (is_cancelled()) { + return boost::asio::error::operation_aborted; + } + const bool has_deadline = deadline != deadline_type::max(); + bool deadline_expired = false; + bool deadline_cancelled = false; + boost::asio::system_timer deadline_timer(_ioc); + if (has_deadline) { + deadline_timer.expires_at(deadline); + deadline_timer.async_wait([&deadline_expired, &deadline_cancelled](const error_code&) { + // The only non-success error_code expected here is operation_aborted. A success + // completion may already be queued when cancellation occurs, so use the explicit + // flag to decide whether the deadline still applies. + if (!deadline_cancelled) { + deadline_expired = true; + } + }); + } + + bool operation_cancelled = false; + boost::asio::steady_timer monitor_timer(_ioc); std::optional f_result; + std::function arm_monitor; + if (_cancel_check || monitor) { + arm_monitor = [&]() { + monitor_timer.expires_after(operation_monitor_interval); + monitor_timer.async_wait([&](const error_code& ec) { + if (ec == boost::asio::error::operation_aborted || f_result) { + return; + } + if (is_cancelled()) { + operation_cancelled = true; + cf(); + return; + } + if (monitor) { + monitor(); + } + arm_monitor(); + }); + }; + arm_monitor(); + } + f(f_result); _ioc.restart(); - while (_ioc.run_one()) - { + while (_ioc.run_one()) { if (f_result) { - timer_cancelled = true; - timer.cancel(); - } else if (timer_expired) { + deadline_cancelled = true; + if (has_deadline) { + deadline_timer.cancel(); + } + monitor_timer.cancel(); + } else if (deadline_expired) { cf(); } } - if (!timer_expired) { - return *f_result; - } else { + if (operation_cancelled) { + return boost::asio::error::operation_aborted; + } + if (deadline_expired) { return error_code(boost::system::errc::timed_out, boost::system::system_category()); } + return *f_result; } template - error_code sync_do_with_deadline( SyncReadStream& s, deadline_type deadline, Fn f) { - return sync_do_with_deadline(s, deadline, f, [&s](){ - s.lowest_layer().cancel(); - }); - }; + error_code sync_do_with_deadline(SyncReadStream& s, deadline_type deadline, Fn f, + const operation_monitor& monitor = {}) { + return sync_do_with_deadline_impl(s, deadline, f, [&s]() { + error_code ec; + s.lowest_layer().cancel(ec); + }, monitor); + } template - error_code sync_connect_with_timeout( SyncReadStream& s, const std::string& host, const std::string& port, const deadline_type& deadline ) { + error_code sync_connect_with_timeout(SyncReadStream& s, const std::string& host, const std::string& port, + const deadline_type& deadline, const operation_monitor& monitor = {}) { tcp::resolver local_resolver(_ioc); bool cancelled = false; - auto res = sync_do_with_deadline(s, deadline, [&local_resolver, &cancelled, &s, &host, &port](std::optional& final_ec){ - local_resolver.async_resolve(host, port, [&cancelled, &s, &final_ec](const error_code& ec, tcp::resolver::results_type resolved ){ - if (ec) { - final_ec.emplace(ec); - return; - } - - if (!cancelled) { - boost::asio::async_connect(s, resolved.begin(), resolved.end(), [&final_ec](const error_code& ec, auto ){ - final_ec.emplace(ec); + auto res = sync_do_with_deadline_impl( + s, deadline, + [&local_resolver, &cancelled, &s, &host, &port](std::optional& final_ec) { + local_resolver.async_resolve( + host, port, + [&cancelled, &s, &final_ec](const error_code& ec, tcp::resolver::results_type resolved) { + if (ec) { + final_ec.emplace(ec); + return; + } + + if (!cancelled) { + boost::asio::async_connect( + s, resolved.begin(), resolved.end(), + [&final_ec](const error_code& ec, auto) { final_ec.emplace(ec); }); + } }); - } - }); - },[&local_resolver, &cancelled](){ - cancelled = true; - local_resolver.cancel(); - }); + }, + [&local_resolver, &cancelled, &s]() { + cancelled = true; + local_resolver.cancel(); + error_code ec; + s.lowest_layer().cancel(ec); + }, + monitor); return res; }; template - error_code sync_write_with_timeout(SyncReadStream& s, http::request& req, const deadline_type& deadline ) { + error_code sync_write_with_timeout(SyncReadStream& s, http::request& req, + const deadline_type& deadline, const operation_monitor& monitor = {}) { return sync_do_with_deadline(s, deadline, [&s, &req](std::optional& final_ec){ http::async_write(s, req, [&final_ec]( const error_code& ec, std::size_t ) { final_ec.emplace(ec); }); - }); + }, monitor); } template - error_code sync_read_with_timeout(SyncReadStream& s, boost::beast::flat_buffer& buffer, http::response& res, const deadline_type& deadline ) { + error_code sync_read_with_timeout(SyncReadStream& s, boost::beast::flat_buffer& buffer, + http::response& res, const deadline_type& deadline, + const operation_monitor& monitor = {}) { return sync_do_with_deadline(s, deadline, [&s, &buffer, &res](std::optional& final_ec){ http::async_read(s, buffer, res, [&final_ec]( const error_code& ec, std::size_t ) { final_ec.emplace(ec); }); - }); + }, monitor); } /// Read only the response header into @p parser, honoring @p deadline. Used by the /// streaming download path to inspect the status line before committing the body to a file. template - error_code sync_read_header_with_timeout(SyncReadStream& s, boost::beast::flat_buffer& buffer, Parser& parser, const deadline_type& deadline ) { + error_code sync_read_header_with_timeout(SyncReadStream& s, boost::beast::flat_buffer& buffer, Parser& parser, + const deadline_type& deadline, + const operation_monitor& monitor = {}) { return sync_do_with_deadline(s, deadline, [&s, &buffer, &parser](std::optional& final_ec){ http::async_read_header(s, buffer, parser, [&final_ec]( const error_code& ec, std::size_t ) { final_ec.emplace(ec); }); - }); + }, monitor); } /// Read one increment of a streaming response body into @p parser, honoring @p deadline. template error_code sync_read_parser_some_with_timeout(SyncReadStream& s, boost::beast::flat_buffer& buffer, - Parser& parser, const deadline_type& deadline) { + Parser& parser, const deadline_type& deadline, + const operation_monitor& monitor = {}) { return sync_do_with_deadline(s, deadline, [&s, &buffer, &parser](std::optional& final_ec) { http::async_read_some(s, buffer, parser, [&final_ec](const error_code& ec, std::size_t) { final_ec.emplace(ec); }); - }); - } - - /// Return the earlier of the total deadline and a relative phase timeout from now. - static deadline_type phase_deadline(const fc::microseconds& timeout, const deadline_type& total_deadline) { - auto deadline = fc::time_point::now(); - deadline.safe_add(timeout); - return std::min(deadline.to_system_clock(), total_deadline); + }, monitor); } /// Return whether @p ec can indicate that a cached peer connection closed before reuse. @@ -266,11 +324,13 @@ class http_client_impl { return res.first; } - connection_map::iterator create_raw_connection( const url& dest, const deadline_type& deadline ) { + connection_map::iterator create_raw_connection(const url& dest, const deadline_type& deadline, + const operation_monitor& monitor = {}) { auto key = url_to_host_key(dest); auto socket = std::make_unique(_ioc); - error_code ec = sync_connect_with_timeout(*socket, *dest.host(), dest.port() ? std::to_string(*dest.port()) : "80", deadline); + error_code ec = sync_connect_with_timeout( + *socket, *dest.host(), dest.port() ? std::to_string(*dest.port()) : "80", deadline, monitor); FC_ASSERT(!ec, "Failed to connect: {}", ec.message()); auto res = _connections.emplace(std::piecewise_construct, @@ -280,9 +340,10 @@ class http_client_impl { return res.first; } - connection_map::iterator create_connection( const url& dest, const deadline_type& deadline ) { + connection_map::iterator create_connection(const url& dest, const deadline_type& deadline, + const operation_monitor& monitor = {}) { if (dest.proto() == "http") { - return create_raw_connection(dest, deadline); + return create_raw_connection(dest, deadline, monitor); } else if (dest.proto() == "unix") { return create_unix_connection(dest, deadline); } else { @@ -310,14 +371,15 @@ class http_client_impl { } connection_map::iterator get_connection(const url& dest, const deadline_type& deadline, - bool* reused_connection = nullptr) { + bool* reused_connection = nullptr, + const operation_monitor& monitor = {}) { auto key = url_to_host_key(dest); auto conn_itr = _connections.find(key); if (conn_itr == _connections.end() || check_closed(conn_itr)) { if (reused_connection) { *reused_connection = false; } - return create_connection(dest, deadline); + return create_connection(dest, deadline, monitor); } else { if (reused_connection) { *reused_connection = true; @@ -327,39 +389,46 @@ class http_client_impl { } struct write_request_visitor : visitor { - write_request_visitor(http_client_impl* that, http::request& req, const deadline_type& deadline) + write_request_visitor(http_client_impl* that, http::request& req, + const deadline_type& deadline, const operation_monitor& monitor = {}) :that(that) ,req(req) ,deadline(deadline) + ,monitor(monitor) {} template error_code operator() ( S& stream ) const { - return that->sync_write_with_timeout(*stream, req, deadline); + return that->sync_write_with_timeout(*stream, req, deadline, monitor); } http_client_impl* that; http::request& req; const deadline_type& deadline; + operation_monitor monitor; }; struct read_response_visitor : visitor { - read_response_visitor(http_client_impl* that, boost::beast::flat_buffer& buffer, http::response& res, const deadline_type& deadline) + read_response_visitor(http_client_impl* that, boost::beast::flat_buffer& buffer, + http::response& res, const deadline_type& deadline, + const operation_monitor& monitor = {}) :that(that) ,buffer(buffer) ,res(res) ,deadline(deadline) + ,monitor(monitor) {} template error_code operator() ( S& stream ) const { - return that->sync_read_with_timeout(*stream, buffer, res, deadline); + return that->sync_read_with_timeout(*stream, buffer, res, deadline, monitor); } http_client_impl* that; boost::beast::flat_buffer& buffer; http::response& res; const deadline_type& deadline; + operation_monitor monitor; }; variant post_sync(const url& dest, const variant& payload, const fc::time_point& _deadline) { @@ -481,8 +550,7 @@ class http_client_impl { } void post_to_file(const url& dest, const variant& payload, const std::filesystem::path& final_dest, - const http_file_download_options& options, const fc::time_point& total_deadline_time) { - const auto total_deadline = total_deadline_time.to_system_clock(); + const http_file_download_options& options) { FC_ASSERT(dest.host(), "No host set on URL"); std::string path = dest.path() ? dest.path()->generic_string() : "/"; @@ -504,12 +572,44 @@ class http_client_impl { req.set(http::field::user_agent, BOOST_BEAST_VERSION_STRING); req.set(http::field::content_type, "application/json"); req.keep_alive(false); - req.body() = json::to_string(payload, total_deadline_time); + req.body() = json::to_string(payload, fc::time_point::maximum()); req.prepare_payload(); + const auto request_started = std::chrono::steady_clock::now(); + auto next_status_report = request_started; + auto download_phase = http_file_download_phase::connecting; + uint64_t downloaded_bytes = 0; + std::optional response_body_bytes; + auto report_status = [&](bool force) { + if (!options.status_callback) { + return; + } + const auto now = std::chrono::steady_clock::now(); + if (!force && now < next_status_report) { + return; + } + const auto elapsed = std::chrono::duration_cast(now - request_started); + options.status_callback(http_file_download_status{ + .phase = download_phase, + .downloaded_bytes = downloaded_bytes, + .total_bytes = response_body_bytes, + .elapsed = fc::microseconds(elapsed.count()), + }); + next_status_report = now + download_status_interval; + }; + auto transition_to = [&](http_file_download_phase phase) { + download_phase = phase; + report_status(true); + }; + operation_monitor monitor; + if (options.status_callback) { + monitor = [&]() { report_status(false); }; + } + const auto no_deadline = deadline_type::max(); + bool reused_connection = false; - auto conn_iter = get_connection( - dest, phase_deadline(options.connect_timeout, total_deadline), &reused_connection); + transition_to(http_file_download_phase::connecting); + auto conn_iter = get_connection(dest, no_deadline, &reused_connection, monitor); const auto connection_key = url_to_host_key(dest); auto eraser = make_scoped_exit([this, connection_key](){ _connections.erase(connection_key); @@ -520,15 +620,15 @@ class http_client_impl { error_code ec; bool retried_stale_connection = false; while (true) { + transition_to(http_file_download_phase::sending_request); ec = std::visit( - write_request_visitor(this, req, phase_deadline(options.response_header_timeout, total_deadline)), - conn_iter->second); + write_request_visitor(this, req, no_deadline, monitor), conn_iter->second); if (ec) { if (options.retry_failed_reused_connection && reused_connection && !retried_stale_connection && is_stale_connection_error(ec)) { _connections.erase(conn_iter); - conn_iter = get_connection( - dest, phase_deadline(options.connect_timeout, total_deadline), &reused_connection); + transition_to(http_file_download_phase::connecting); + conn_iter = get_connection(dest, no_deadline, &reused_connection, monitor); retried_stale_connection = true; continue; } @@ -536,14 +636,14 @@ class http_client_impl { } // Parse into a bounded caller-owned buffer so every write can enforce the byte ceiling, - // idle deadline, and filesystem headroom before bytes reach disk. + // cancellation checks, and filesystem headroom before bytes reach disk. buffer = std::make_unique(download_parser_buffer_bytes); parser = std::make_unique>(); parser->body_limit(options.max_response_body_bytes); + transition_to(http_file_download_phase::waiting_for_response); ec = std::visit([&](auto& stream) { - return sync_read_header_with_timeout( - *stream, *buffer, *parser, phase_deadline(options.response_header_timeout, total_deadline)); + return sync_read_header_with_timeout(*stream, *buffer, *parser, no_deadline, monitor); }, conn_iter->second); if (!ec) { break; @@ -554,8 +654,8 @@ class http_client_impl { if (options.retry_failed_reused_connection && reused_connection && !retried_stale_connection && is_stale_connection_error(ec)) { _connections.erase(conn_iter); - conn_iter = get_connection( - dest, phase_deadline(options.connect_timeout, total_deadline), &reused_connection); + transition_to(http_file_download_phase::connecting); + conn_iter = get_connection(dest, no_deadline, &reused_connection, monitor); retried_stale_connection = true; continue; } @@ -571,6 +671,9 @@ class http_client_impl { } const auto content_length = parser->content_length(); + if (content_length) { + response_body_bytes = *content_length; + } uint64_t disk_space_write_budget = 0; if (content_length) { FC_ASSERT(*content_length <= options.max_response_body_bytes, @@ -602,13 +705,12 @@ class http_client_impl { FC_ASSERT(file.is_open(), "Failed to open temp file {} for writing", temp_path.string()); std::vector read_buffer(download_read_buffer_bytes); - uint64_t downloaded_bytes = 0; - auto idle_deadline = phase_deadline(options.idle_read_timeout, total_deadline); + transition_to(http_file_download_phase::downloading); while (!parser->is_done()) { parser->get().body().data = read_buffer.data(); parser->get().body().size = read_buffer.size(); ec = std::visit([&](auto& stream) { - return sync_read_parser_some_with_timeout(*stream, *buffer, *parser, idle_deadline); + return sync_read_parser_some_with_timeout(*stream, *buffer, *parser, no_deadline, monitor); }, conn_iter->second); if (ec == http::error::need_buffer) { ec = {}; @@ -623,6 +725,7 @@ class http_client_impl { downloaded_bytes <= options.max_response_body_bytes - chunk_bytes, "HTTP response body exceeds configured maximum of {} bytes", options.max_response_body_bytes); if (chunk_bytes == 0) { + report_status(false); continue; } @@ -640,9 +743,10 @@ class http_client_impl { FC_ASSERT(file.good(), "Failed to write {} bytes to temp file {}", chunk_bytes, temp_path.string()); downloaded_bytes += chunk_bytes; disk_space_write_budget -= chunk_bytes; - idle_deadline = phase_deadline(options.idle_read_timeout, total_deadline); + report_status(false); } + FC_ASSERT(!is_cancelled(), "HTTP file download cancelled"); file.close(); FC_ASSERT(file.good(), "Failed to close temp file {} after HTTP download", temp_path.string()); @@ -650,18 +754,13 @@ class http_client_impl { std::filesystem::rename(temp_path, final_dest, rename_ec); FC_ASSERT(!rename_ec, "Failed to rename downloaded file: {}", rename_ec.message()); cleanup.cancel(); - } - - void post_to_file(const url& dest, const variant& payload, const std::filesystem::path& final_dest, - const http_file_download_options& options) { - auto total_deadline = fc::time_point::now(); - total_deadline.safe_add(options.total_timeout); - post_to_file(dest, payload, final_dest, options, total_deadline); + transition_to(http_file_download_phase::complete); } boost::asio::io_context _ioc; connection_map _connections; unix_url_split_map _unix_url_paths; + std::function _cancel_check; }; @@ -676,6 +775,10 @@ void http_client::post_to_file(const url& dest, const variant& payload, const st _my->post_to_file(dest, payload, output, options); } +void http_client::set_cancel_check(std::function cancel_check) { + _my->set_cancel_check(std::move(cancel_check)); +} + variant http_client::post_sync(const url& dest, const variant& payload, const fc::time_point& deadline) { if(dest.proto() == "unix") return _my->post_sync(_my->get_unix_url(*dest.host()), payload, deadline); diff --git a/libraries/libfc/test/network/test_http_client.cpp b/libraries/libfc/test/network/test_http_client.cpp index c30d33482c..d9add38eeb 100644 --- a/libraries/libfc/test/network/test_http_client.cpp +++ b/libraries/libfc/test/network/test_http_client.cpp @@ -22,6 +22,7 @@ #include #include #include +#include using namespace std::chrono_literals; @@ -29,10 +30,9 @@ namespace { using tcp = boost::asio::ip::tcp; -constexpr int64_t short_timeout_ms = 100; constexpr int64_t normal_timeout_ms = 2'000; +constexpr int64_t cancellation_delay_ms = 100; constexpr int64_t max_test_elapsed_ms = 1'500; -constexpr int64_t framing_delay_ms = 70; constexpr size_t exact_body_bytes = 8; constexpr size_t blocked_request_body_bytes = 16 * 1024 * 1024; constexpr size_t disk_space_budget_bytes = 64 * 1024 * 1024; @@ -198,10 +198,6 @@ bool write_chunked_body(tcp::socket& socket, uint64_t body_bytes) { /** Return finite test limits with a caller-selected response-body maximum. */ fc::http_file_download_options download_options(uint64_t max_body_bytes) { return fc::http_file_download_options{ - .connect_timeout = fc::milliseconds(normal_timeout_ms), - .response_header_timeout = fc::milliseconds(normal_timeout_ms), - .idle_read_timeout = fc::milliseconds(normal_timeout_ms), - .total_timeout = fc::milliseconds(normal_timeout_ms), .max_response_body_bytes = max_body_bytes, .min_free_disk_space_bytes = 1, .retry_failed_reused_connection = false, @@ -215,11 +211,42 @@ fc::url server_url(const scripted_http_server& server) { /** Invoke a bounded empty-payload download into @p output. */ void download(const scripted_http_server& server, const std::filesystem::path& output, - const fc::http_file_download_options& options) { + const fc::http_file_download_options& options, + std::function cancel_check = {}) { fc::http_client client; + client.set_cancel_check(std::move(cancel_check)); client.post_to_file(server_url(server), fc::variant(fc::mutable_variant_object()), output, options); } +/** Request cancellation after a short deterministic delay. */ +class delayed_cancellation { +public: + delayed_cancellation() + : _worker([this]() { + std::this_thread::sleep_for(std::chrono::milliseconds(cancellation_delay_ms)); + _requested = true; + }) {} + + delayed_cancellation(const delayed_cancellation&) = delete; + delayed_cancellation& operator=(const delayed_cancellation&) = delete; + + /** Join the worker that publishes the cancellation request. */ + ~delayed_cancellation() { + if (_worker.joinable()) { + _worker.join(); + } + } + + /** Return a predicate suitable for http_client::set_cancel_check(). */ + std::function check() { + return [this]() { return _requested.load(); }; + } + +private: + std::atomic_bool _requested{false}; + std::thread _worker; +}; + /** Return the temporary sibling used while @p output is being downloaded. */ std::filesystem::path partial_path(const std::filesystem::path& output) { auto partial = output; @@ -243,23 +270,26 @@ std::string read_file(const std::filesystem::path& path) { BOOST_AUTO_TEST_SUITE(http_client_file_download_tests) -/// A peer that accepts but does not read the request must hit the request-write phase deadline. -BOOST_AUTO_TEST_CASE(request_write_timeout_precedes_total_timeout) { - scripted_http_server server([](tcp::socket&, const std::atomic_bool& stop) { +/// An attended caller can cancel while a peer accepts but does not read the request. +BOOST_AUTO_TEST_CASE(request_write_can_be_cancelled) { + std::atomic_bool cancellation_requested{false}; + scripted_http_server server([&cancellation_requested](tcp::socket&, const std::atomic_bool& stop) { + std::this_thread::sleep_for(std::chrono::milliseconds(cancellation_delay_ms)); + cancellation_requested = true; while (!stop.load()) { std::this_thread::sleep_for(10ms); } }, false); fc::temp_directory temp; - const auto output = temp.path() / "request-write-timeout.bin"; + const auto output = temp.path() / "request-write-cancelled.bin"; auto options = download_options(exact_body_bytes); - options.response_header_timeout = fc::milliseconds(short_timeout_ms); + const fc::variant payload(std::string(blocked_request_body_bytes, 'a')); const auto start = std::chrono::steady_clock::now(); fc::http_client client; + client.set_cancel_check([&cancellation_requested]() { return cancellation_requested.load(); }); BOOST_CHECK_EXCEPTION( - client.post_to_file(server_url(server), fc::variant(std::string(blocked_request_body_bytes, 'a')), - output, options), + client.post_to_file(server_url(server), payload, output, options), fc::exception, [](const fc::exception& error) { return error.to_detail_string().find("Failed to send POST request") != std::string::npos; @@ -270,6 +300,29 @@ BOOST_AUTO_TEST_CASE(request_write_timeout_precedes_total_timeout) { check_download_files_removed(output); } +/// Metadata requests using the default unlimited deadline remain interruptible. +BOOST_AUTO_TEST_CASE(unbounded_post_sync_can_be_cancelled) { + scripted_http_server server([](tcp::socket&, const std::atomic_bool& stop) { + while (!stop.load()) { + std::this_thread::sleep_for(10ms); + } + }); + delayed_cancellation cancellation; + fc::http_client client; + client.set_cancel_check(cancellation.check()); + + const auto start = std::chrono::steady_clock::now(); + BOOST_CHECK_EXCEPTION( + client.post_sync(server_url(server), fc::variant(fc::mutable_variant_object())), + fc::exception, + [](const fc::exception& error) { + return error.to_detail_string().find("Failed to read response") != std::string::npos; + }); + const auto elapsed = std::chrono::steady_clock::now() - start; + + BOOST_CHECK_LT(std::chrono::duration_cast(elapsed).count(), max_test_elapsed_ms); +} + /// Metadata and download requests should reuse one healthy keep-alive connection. BOOST_AUTO_TEST_CASE(healthy_metadata_connection_is_reused_for_download) { scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { @@ -358,28 +411,28 @@ BOOST_AUTO_TEST_CASE(stale_metadata_reconnect_failure_cleans_up_safely) { check_download_files_removed(output); } -/// A peer that never sends a response header must hit the configured header deadline. -BOOST_AUTO_TEST_CASE(response_header_timeout_removes_partial_file) { +/// An attended caller can cancel while a peer never sends a response header. +BOOST_AUTO_TEST_CASE(response_header_wait_can_be_cancelled) { scripted_http_server server([](tcp::socket&, const std::atomic_bool& stop) { while (!stop.load()) { std::this_thread::sleep_for(10ms); } }); fc::temp_directory temp; - const auto output = temp.path() / "header-timeout.bin"; + const auto output = temp.path() / "header-cancelled.bin"; auto options = download_options(exact_body_bytes); - options.response_header_timeout = fc::milliseconds(short_timeout_ms); + delayed_cancellation cancellation; const auto start = std::chrono::steady_clock::now(); - BOOST_CHECK_THROW(download(server, output, options), fc::exception); + BOOST_CHECK_THROW(download(server, output, options, cancellation.check()), fc::exception); const auto elapsed = std::chrono::steady_clock::now() - start; BOOST_CHECK_LT(std::chrono::duration_cast(elapsed).count(), max_test_elapsed_ms); check_download_files_removed(output); } -/// A body that stops after partial progress must hit the reset-on-progress idle deadline. -BOOST_AUTO_TEST_CASE(idle_body_timeout_removes_partial_file) { +/// An attended caller can cancel a body that stops after partial progress. +BOOST_AUTO_TEST_CASE(stalled_body_can_be_cancelled_and_removes_partial_file) { scripted_http_server server([](tcp::socket& socket, const std::atomic_bool& stop) { if (!write_bytes(socket, fixed_length_header(2)) || !write_bytes(socket, "1")) { return; @@ -389,38 +442,16 @@ BOOST_AUTO_TEST_CASE(idle_body_timeout_removes_partial_file) { } }); fc::temp_directory temp; - const auto output = temp.path() / "idle-timeout.bin"; + const auto output = temp.path() / "body-cancelled.bin"; auto options = download_options(exact_body_bytes); - options.idle_read_timeout = fc::milliseconds(short_timeout_ms); + delayed_cancellation cancellation; - BOOST_CHECK_THROW(download(server, output, options), fc::exception); + BOOST_CHECK_THROW(download(server, output, options, cancellation.check()), fc::exception); check_download_files_removed(output); } -/// Chunk framing without decoded body bytes must not refresh the response-progress deadline. -BOOST_AUTO_TEST_CASE(chunk_framing_does_not_reset_idle_timeout) { - scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { - if (!write_bytes(socket, chunked_header())) { - return; - } - std::this_thread::sleep_for(std::chrono::milliseconds(framing_delay_ms)); - if (!write_bytes(socket, "1\r\n")) { - return; - } - std::this_thread::sleep_for(std::chrono::milliseconds(framing_delay_ms)); - write_bytes(socket, "1\r\n0\r\n\r\n"); - }); - fc::temp_directory temp; - const auto output = temp.path() / "framing-idle-timeout.bin"; - auto options = download_options(exact_body_bytes); - options.idle_read_timeout = fc::milliseconds(short_timeout_ms); - - BOOST_CHECK_THROW(download(server, output, options), fc::exception); - check_download_files_removed(output); -} - -/// Continuous sub-idle progress must still stop at the independent total deadline. -BOOST_AUTO_TEST_CASE(total_timeout_stops_trickled_body_and_removes_partial_file) { +/// A continuously progressing transfer has no total deadline but remains interruptible. +BOOST_AUTO_TEST_CASE(progressing_body_has_no_total_deadline_and_can_be_cancelled) { scripted_http_server server([](tcp::socket& socket, const std::atomic_bool& stop) { if (!write_bytes(socket, fixed_length_header(1'000))) { return; @@ -433,12 +464,11 @@ BOOST_AUTO_TEST_CASE(total_timeout_stops_trickled_body_and_removes_partial_file) } }); fc::temp_directory temp; - const auto output = temp.path() / "total-timeout.bin"; + const auto output = temp.path() / "progressing-body-cancelled.bin"; auto options = download_options(1'000); - options.idle_read_timeout = fc::milliseconds(short_timeout_ms); - options.total_timeout = fc::milliseconds(250); + delayed_cancellation cancellation; - BOOST_CHECK_THROW(download(server, output, options), fc::exception); + BOOST_CHECK_THROW(download(server, output, options, cancellation.check()), fc::exception); check_download_files_removed(output); } @@ -477,7 +507,6 @@ BOOST_AUTO_TEST_CASE(oversized_chunk_extension_is_bounded_and_removed) { fc::temp_directory temp; const auto output = temp.path() / "oversized-chunk-extension.bin"; auto options = download_options(exact_body_bytes); - options.idle_read_timeout = fc::milliseconds(short_timeout_ms); const auto start = std::chrono::steady_clock::now(); BOOST_CHECK_EXCEPTION(download(server, output, options), fc::exception, @@ -497,11 +526,42 @@ BOOST_AUTO_TEST_CASE(exact_maximum_response_succeeds) { }); fc::temp_directory temp; const auto output = temp.path() / "exact-maximum.bin"; + std::vector statuses; + auto options = download_options(exact_body_bytes); + options.status_callback = [&](const fc::http_file_download_status& status) { statuses.push_back(status); }; - BOOST_CHECK_NO_THROW(download(server, output, download_options(exact_body_bytes))); + BOOST_CHECK_NO_THROW(download(server, output, options)); BOOST_CHECK(std::filesystem::exists(output)); BOOST_CHECK(!std::filesystem::exists(partial_path(output))); BOOST_CHECK_EQUAL(read_file(output), exact_body); + BOOST_REQUIRE(!statuses.empty()); + BOOST_CHECK(std::any_of(statuses.begin(), statuses.end(), [](const auto& status) { + return status.phase == fc::http_file_download_phase::downloading; + })); + const auto& final_status = statuses.back(); + BOOST_CHECK(final_status.phase == fc::http_file_download_phase::complete); + BOOST_CHECK_EQUAL(final_status.downloaded_bytes, exact_body_bytes); + BOOST_REQUIRE(final_status.total_bytes); + BOOST_CHECK_EQUAL(*final_status.total_bytes, exact_body_bytes); +} + +/// Chunked downloads report progress without inventing a total byte count or ETA basis. +BOOST_AUTO_TEST_CASE(chunked_response_reports_unknown_total) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + write_bytes(socket, chunked_header() + "8\r\n" + std::string(exact_body) + "\r\n0\r\n\r\n"); + }); + fc::temp_directory temp; + const auto output = temp.path() / "chunked-progress.bin"; + std::vector statuses; + auto options = download_options(exact_body_bytes); + options.status_callback = [&](const fc::http_file_download_status& status) { statuses.push_back(status); }; + + BOOST_REQUIRE_NO_THROW(download(server, output, options)); + BOOST_REQUIRE(!statuses.empty()); + const auto& final_status = statuses.back(); + BOOST_CHECK(final_status.phase == fc::http_file_download_phase::complete); + BOOST_CHECK_EQUAL(final_status.downloaded_bytes, exact_body_bytes); + BOOST_CHECK(!final_status.total_bytes); } /// A fixed-length response exactly one disk-check budget wide should not require a refill. @@ -514,8 +574,6 @@ BOOST_AUTO_TEST_CASE(exact_disk_space_budget_response_succeeds) { fc::temp_directory temp; const auto output = temp.path() / "exact-disk-budget.bin"; auto options = download_options(disk_space_budget_bytes); - options.idle_read_timeout = fc::milliseconds(30'000); - options.total_timeout = fc::milliseconds(30'000); BOOST_REQUIRE_NO_THROW(download(server, output, options)); BOOST_CHECK_EQUAL(std::filesystem::file_size(output), disk_space_budget_bytes); @@ -532,8 +590,6 @@ BOOST_AUTO_TEST_CASE(chunked_response_refills_disk_space_budget) { fc::temp_directory temp; const auto output = temp.path() / "refilled-disk-budget.bin"; auto options = download_options(response_bytes); - options.idle_read_timeout = fc::milliseconds(30'000); - options.total_timeout = fc::milliseconds(30'000); BOOST_REQUIRE_NO_THROW(download(server, output, options)); BOOST_CHECK_EQUAL(std::filesystem::file_size(output), response_bytes); diff --git a/plugins/chain_plugin/src/chain_plugin.cpp b/plugins/chain_plugin/src/chain_plugin.cpp index 72a46341c5..b88c81aff3 100644 --- a/plugins/chain_plugin/src/chain_plugin.cpp +++ b/plugins/chain_plugin/src/chain_plugin.cpp @@ -37,8 +37,12 @@ #include #include #include +#include + +#include #include #include +#include #include @@ -65,29 +69,10 @@ namespace snapshot_attest { /// provider snapshot interval (_snapshot_provider_block_spacing). constexpr uint32_t snapshot_attestation_grace_blocks = 12500; -constexpr std::string_view snapshot_connect_timeout_option = "snapshot-endpoint-connect-timeout-ms"; -constexpr std::string_view snapshot_header_timeout_option = "snapshot-endpoint-header-timeout-ms"; -constexpr std::string_view snapshot_idle_timeout_option = "snapshot-endpoint-idle-timeout-ms"; -constexpr std::string_view snapshot_total_timeout_option = "snapshot-endpoint-total-timeout-ms"; -constexpr std::string_view snapshot_max_download_size_option = "snapshot-endpoint-max-download-size-mb"; constexpr std::string_view snapshot_min_disk_free_option = "snapshot-endpoint-min-disk-free-mb"; -constexpr uint64_t default_snapshot_connect_timeout_ms = 10'000; -constexpr uint64_t default_snapshot_header_timeout_ms = 30'000; -constexpr uint64_t default_snapshot_idle_timeout_ms = 60'000; -constexpr uint64_t default_snapshot_total_timeout_ms = 24 * 60 * 60 * 1000; constexpr uint64_t bytes_per_mebibyte = 1024 * 1024; -/// Read a positive millisecond option and convert it without overflowing fc::microseconds. -fc::microseconds checked_milliseconds(const boost::program_options::variables_map& options, - std::string_view option_name) { - const auto value = options.at(std::string(option_name)).as(); - constexpr auto max_milliseconds = static_cast(std::numeric_limits::max() / 1000); - SYS_ASSERT(value > 0 && value <= max_milliseconds, sysio::chain::plugin_config_exception, - "{} must be between 1 and {} milliseconds", option_name, max_milliseconds); - return fc::milliseconds(static_cast(value)); -} - /// Convert a positive MiB option value to bytes without overflowing uint64_t. uint64_t checked_mebibytes(uint64_t value, std::string_view option_name) { constexpr auto max_mebibytes = std::numeric_limits::max() / bytes_per_mebibyte; @@ -96,6 +81,87 @@ uint64_t checked_mebibytes(uint64_t value, std::string_view option_name) { return value * bytes_per_mebibyte; } +/** Log attended snapshot-bootstrap phase changes, transfer rate, percentage, and ETA. */ +class snapshot_download_progress_logger { +public: + /** Consume one transport status event. */ + void operator()(const fc::http_file_download_status& status) { + const auto elapsed_seconds = static_cast(status.elapsed.count()) / microseconds_per_second; + if (status.phase != fc::http_file_download_phase::downloading && + status.phase != fc::http_file_download_phase::complete) { + ilog("Snapshot download phase: {} (elapsed {:.1f}s; press Ctrl+C to cancel)", + magic_enum::enum_name(status.phase), elapsed_seconds); + return; + } + + if (!_body_started_at) { + _body_started_at = status.elapsed; + _previous_status_at = status.elapsed; + _previous_downloaded_bytes = status.downloaded_bytes; + } + + const auto body_elapsed = status.elapsed - *_body_started_at; + if (status.phase == fc::http_file_download_phase::complete) { + const auto average_rate = rate_bytes_per_second(status.downloaded_bytes, body_elapsed); + ilog("Snapshot download complete: {} bytes ({:.2f} MiB) in {:.1f}s, average {:.2f} MiB/s", + status.downloaded_bytes, to_mebibytes(status.downloaded_bytes), + static_cast(body_elapsed.count()) / microseconds_per_second, + to_mebibytes(average_rate)); + return; + } + + const auto interval = status.elapsed - _previous_status_at; + const auto interval_bytes = status.downloaded_bytes - _previous_downloaded_bytes; + const auto current_rate = rate_bytes_per_second(interval_bytes, interval); + _previous_status_at = status.elapsed; + _previous_downloaded_bytes = status.downloaded_bytes; + + if (!status.total_bytes) { + ilog("Downloaded {} bytes ({:.2f} MiB) at {:.2f} MiB/s; response size and ETA unavailable", + status.downloaded_bytes, to_mebibytes(status.downloaded_bytes), to_mebibytes(current_rate)); + return; + } + + const auto total_bytes = *status.total_bytes; + const auto percentage = total_bytes == 0 + ? 100.0 + : 100.0 * static_cast(status.downloaded_bytes) / + static_cast(total_bytes); + if (current_rate == 0 || status.downloaded_bytes >= total_bytes) { + ilog("Downloaded {} / {} bytes ({:.1f}%) at {:.2f} MiB/s; ETA unavailable", + status.downloaded_bytes, total_bytes, percentage, to_mebibytes(current_rate)); + return; + } + + const auto remaining_bytes = total_bytes - status.downloaded_bytes; + const auto eta_seconds = static_cast( + std::ceil(static_cast(remaining_bytes) / static_cast(current_rate))); + ilog("Downloaded {} / {} bytes ({:.1f}%) at {:.2f} MiB/s, ETA {}s", + status.downloaded_bytes, total_bytes, percentage, to_mebibytes(current_rate), eta_seconds); + } + +private: + static constexpr double microseconds_per_second = 1'000'000.0; + + /** Convert a byte count to MiB for operator-facing logs. */ + static double to_mebibytes(uint64_t bytes) { + return static_cast(bytes) / static_cast(bytes_per_mebibyte); + } + + /** Calculate whole bytes per second without overflowing intermediate integer arithmetic. */ + static uint64_t rate_bytes_per_second(uint64_t bytes, const fc::microseconds& elapsed) { + if (bytes == 0 || elapsed.count() <= 0) { + return 0; + } + return static_cast( + static_cast(bytes) * microseconds_per_second / static_cast(elapsed.count())); + } + + std::optional _body_started_at; + fc::microseconds _previous_status_at; + uint64_t _previous_downloaded_bytes = 0; +}; + } // anonymous namespace namespace std { @@ -512,20 +578,6 @@ void chain_plugin::set_program_options(options_description& cli, options_descrip " http://host:port - fetches latest snapshot\n" " http://host:port/50000 - fetches snapshot at block 50000\n" "Requires empty database (use --delete-all-blocks to clear existing data).") - (snapshot_connect_timeout_option.data(), - bpo::value()->default_value(default_snapshot_connect_timeout_ms), - "Maximum time in milliseconds to establish the snapshot download connection.") - (snapshot_header_timeout_option.data(), - bpo::value()->default_value(default_snapshot_header_timeout_ms), - "Maximum time in milliseconds for each snapshot request-write and response-header phase.") - (snapshot_idle_timeout_option.data(), - bpo::value()->default_value(default_snapshot_idle_timeout_ms), - "Maximum time in milliseconds without snapshot response-body progress.") - (snapshot_total_timeout_option.data(), - bpo::value()->default_value(default_snapshot_total_timeout_ms), - "Maximum total time in milliseconds for the snapshot file download.") - (snapshot_max_download_size_option.data(), bpo::value(), - "Maximum snapshot response size in MiB. Defaults to chain-state-db-size-mb.") (snapshot_min_disk_free_option.data(), bpo::value(), "Free disk space in MiB that must remain during download. Defaults to chain-state-db-guard-size-mb.") ; @@ -933,13 +985,8 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) { SYS_ASSERT(!options.contains("snapshot"), plugin_config_exception, "--snapshot-endpoint is incompatible with --snapshot; use one or the other"); - const std::string max_download_size_option_name{snapshot_max_download_size_option}; - const bool has_max_download_size = options.contains(max_download_size_option_name); const auto max_download_bytes = checked_mebibytes( - has_max_download_size - ? options.at(max_download_size_option_name).as() - : options.at("chain-state-db-size-mb").as(), - has_max_download_size ? snapshot_max_download_size_option : std::string_view{"chain-state-db-size-mb"}); + options.at("chain-state-db-size-mb").as(), "chain-state-db-size-mb"); const std::string min_disk_free_option_name{snapshot_min_disk_free_option}; const bool has_min_disk_free = options.contains(min_disk_free_option_name); const auto min_free_disk_bytes = checked_mebibytes( @@ -948,15 +995,20 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) { : options.at("chain-state-db-guard-size-mb").as(), has_min_disk_free ? snapshot_min_disk_free_option : std::string_view{"chain-state-db-guard-size-mb"}); const fc::http_file_download_options download_options{ - .connect_timeout = checked_milliseconds(options, snapshot_connect_timeout_option), - .response_header_timeout = checked_milliseconds(options, snapshot_header_timeout_option), - .idle_read_timeout = checked_milliseconds(options, snapshot_idle_timeout_option), - .total_timeout = checked_milliseconds(options, snapshot_total_timeout_option), .max_response_body_bytes = max_download_bytes, .min_free_disk_space_bytes = min_free_disk_bytes, .retry_failed_reused_connection = true, + .status_callback = [logger = snapshot_download_progress_logger{}]( + const fc::http_file_download_status& status) mutable { logger(status); }, }; - fetch_snapshot_from_endpoint(options.at("snapshot-endpoint").as(), download_options); + try { + fetch_snapshot_from_endpoint(options.at("snapshot-endpoint").as(), download_options); + } catch (...) { + if (app().is_quiting()) { + SYS_THROW(chain::interrupt_exception, "Snapshot endpoint bootstrap interrupted"); + } + throw; + } } std::optional chain_id; @@ -1621,19 +1673,16 @@ void chain_plugin_impl::fetch_snapshot_from_endpoint( ilog("Fetching snapshot metadata from endpoint: {}", endpoint_url); fc::http_client http_client; - auto metadata_deadline = fc::time_point::now(); - metadata_deadline.safe_add(download_options.connect_timeout); - metadata_deadline.safe_add(download_options.response_header_timeout); + http_client.set_cancel_check([]() { return app().is_quiting(); }); fc::variant metadata_response; if (request_block_num) { auto url = fc::url(base_url + "/v1/snapshot/by_block"); fc::mutable_variant_object payload; payload("block_num", *request_block_num); - metadata_response = http_client.post_sync(url, fc::variant(payload), metadata_deadline); + metadata_response = http_client.post_sync(url, fc::variant(payload)); } else { auto url = fc::url(base_url + "/v1/snapshot/latest"); - metadata_response = http_client.post_sync( - url, fc::variant(fc::mutable_variant_object()), metadata_deadline); + metadata_response = http_client.post_sync(url, fc::variant(fc::mutable_variant_object())); } auto snap_block_num = metadata_response["block_num"].as(); diff --git a/plugins/chain_plugin/test/plugin_config_test.cpp b/plugins/chain_plugin/test/plugin_config_test.cpp index 801fe872c2..9438181058 100644 --- a/plugins/chain_plugin/test/plugin_config_test.cpp +++ b/plugins/chain_plugin/test/plugin_config_test.cpp @@ -61,7 +61,7 @@ BOOST_AUTO_TEST_CASE(chain_plugin_default_tests) { } -/** Verify that every bounded snapshot-download command-line option is registered and parsed. */ +/** Verify the sole snapshot-specific resource option and removal of endpoint timeout/size knobs. */ BOOST_AUTO_TEST_CASE(chain_plugin_snapshot_endpoint_option_registration) { sysio::chain::application exe({.enable_resource_monitor = false}); sysio::chain_plugin plugin; @@ -74,11 +74,6 @@ BOOST_AUTO_TEST_CASE(chain_plugin_snapshot_endpoint_option_registration) { std::array arguments{ "test_chain_plugin", "--snapshot-endpoint", "http://127.0.0.1:1", - "--snapshot-endpoint-connect-timeout-ms", "11", - "--snapshot-endpoint-header-timeout-ms", "12", - "--snapshot-endpoint-idle-timeout-ms", "13", - "--snapshot-endpoint-total-timeout-ms", "14", - "--snapshot-endpoint-max-download-size-mb", "15", "--snapshot-endpoint-min-disk-free-mb", "16", }; boost::program_options::variables_map variables; @@ -88,25 +83,24 @@ BOOST_AUTO_TEST_CASE(chain_plugin_snapshot_endpoint_option_registration) { boost::program_options::notify(variables); BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint").as(), "http://127.0.0.1:1"); - BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint-connect-timeout-ms").as(), 11); - BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint-header-timeout-ms").as(), 12); - BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint-idle-timeout-ms").as(), 13); - BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint-total-timeout-ms").as(), 14); - BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint-max-download-size-mb").as(), 15); BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint-min-disk-free-mb").as(), 16); + + constexpr std::array removed_options{ + "snapshot-endpoint-connect-timeout-ms", + "snapshot-endpoint-header-timeout-ms", + "snapshot-endpoint-idle-timeout-ms", + "snapshot-endpoint-total-timeout-ms", + "snapshot-endpoint-max-download-size-mb", + }; + for (const auto* option_name : removed_options) { + BOOST_CHECK(options.find_nothrow(option_name, false) == nullptr); + } } -/** Verify that snapshot endpoint limits reject zero and overflow-prone values before connecting. */ +/** Verify that snapshot endpoint resource limits reject zero and overflow-prone values before connecting. */ BOOST_AUTO_TEST_CASE(chain_plugin_snapshot_endpoint_option_validation) { constexpr std::array invalid_options{ - std::pair{"snapshot-endpoint-connect-timeout-ms", "0"}, - std::pair{"snapshot-endpoint-header-timeout-ms", "0"}, - std::pair{"snapshot-endpoint-idle-timeout-ms", "0"}, - std::pair{"snapshot-endpoint-total-timeout-ms", "0"}, - std::pair{"snapshot-endpoint-max-download-size-mb", "0"}, std::pair{"snapshot-endpoint-min-disk-free-mb", "0"}, - std::pair{"snapshot-endpoint-total-timeout-ms", "9223372036854776"}, - std::pair{"snapshot-endpoint-max-download-size-mb", "17592186044416"}, std::pair{"chain-state-db-size-mb", "0"}, std::pair{"chain-state-db-size-mb", "17592186044416"}, std::pair{"chain-state-db-guard-size-mb", "0"}, diff --git a/plugins/snapshot_api_plugin/README.md b/plugins/snapshot_api_plugin/README.md index fcce4a1046..4ca3ff73c5 100644 --- a/plugins/snapshot_api_plugin/README.md +++ b/plugins/snapshot_api_plugin/README.md @@ -247,21 +247,23 @@ The bootstrap process: `--delete-all-blocks` is required when existing chain data is present. The `--snapshot-endpoint` option is incompatible with `--snapshot` (local file). -### Bootstrap download limits +### Bootstrap download status and limits -Snapshot metadata and file transfers use finite deadlines and a streaming byte ceiling. The following CLI-only options -can be tuned for the expected provider latency, snapshot size, and available storage: +Snapshot bootstrap is attended and has no automatic network deadlines. While metadata is fetched, the node identifies +the active operation; the file transfer then reports phase changes and, every five seconds, downloaded bytes, +percentage, transfer rate, and ETA when the response supplies `Content-Length`. Pressing Ctrl+C cancels pending +resolver or socket work and removes the partial file. + +The existing chain database settings supply the download ceiling and default disk reserve. Only the disk reserve has a +snapshot-specific override: | Option | Default | Purpose | |---|---:|---| -| `--snapshot-endpoint-connect-timeout-ms` | `10000` | Maximum time to establish the download connection | -| `--snapshot-endpoint-header-timeout-ms` | `30000` | Maximum time for each request-write and response-header phase | -| `--snapshot-endpoint-idle-timeout-ms` | `60000` | Maximum time without response-body progress | -| `--snapshot-endpoint-total-timeout-ms` | `86400000` | Maximum total snapshot file download time (24 hours) | -| `--snapshot-endpoint-max-download-size-mb` | `chain-state-db-size-mb` | Maximum accepted snapshot response size | +| `--chain-state-db-size-mb` | `1024` | Maximum accepted snapshot response size and chain-state database size | +| `--chain-state-db-guard-size-mb` | build default | Default free-space reserve during snapshot download | | `--snapshot-endpoint-min-disk-free-mb` | `chain-state-db-guard-size-mb` | Free space retained on the snapshots filesystem | -All limits must be positive. A fixed-length response that exceeds the maximum is rejected from its `Content-Length` +Resource limits must be positive. A fixed-length response that exceeds the maximum is rejected from its `Content-Length` before a temporary file is opened. Chunked or lengthless responses are stopped at the same byte ceiling. Available disk space is checked before and throughout the transfer, and a failed transfer removes its `.downloading` file. Long transfers recheck after at most 64 MiB or five seconds of progress and retain an additional 64 MiB safety margin diff --git a/tests/snapshot_api_test.py b/tests/snapshot_api_test.py index 72bb0a21a3..5f184d8c59 100755 --- a/tests/snapshot_api_test.py +++ b/tests/snapshot_api_test.py @@ -453,15 +453,9 @@ def recordPresent(): endpointUrl = node0.endpointHttp Print(f"Restart bootstrap node with --snapshot-endpoint {endpointUrl}") - # Exercise the bounded-download CLI options with a finite cap just above the - # snapshot being bootstrapped and a small deterministic test headroom. - snap2FileSize = os.path.getsize(node0.getLatestSnapshot()) - bytesPerMiB = 1024 * 1024 - downloadLimitMiB = (snap2FileSize + bytesPerMiB - 1) // bytesPerMiB + 1 - downloadLimits = ( - f"--snapshot-endpoint-max-download-size-mb {downloadLimitMiB} " - "--snapshot-endpoint-min-disk-free-mb 1" - ) + # Exercise the snapshot-specific disk-headroom override. Download size is + # bounded by the existing chain-state-db-size-mb setting. + downloadLimits = "--snapshot-endpoint-min-disk-free-mb 1" # Fetches latest snapshot (snap2). The attestation is NOT in the snapshot — # it's in blocks after snap2BlockNum. The bootstrap node syncs forward and From 24f57ecd2f0a60afe11c27b9488ac3106730e386 Mon Sep 17 00:00:00 2001 From: Huang-Ming Huang Date: Wed, 22 Jul 2026 19:49:45 +0000 Subject: [PATCH 4/7] Test truncated snapshot response cleanup --- docs/snapshot-serving-plan.md | 2 +- libraries/libfc/test/network/test_http_client.cpp | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/docs/snapshot-serving-plan.md b/docs/snapshot-serving-plan.md index 52ec0eee38..43870de0d3 100644 --- a/docs/snapshot-serving-plan.md +++ b/docs/snapshot-serving-plan.md @@ -288,7 +288,7 @@ The block number is encoded as a trailing path component of the URL. If the last This works naturally with `--delete-all-blocks` which clears state before snapshot handling. 3. **Fetch metadata:** POST to `/v1/snapshot/latest` or `/v1/snapshot/by_block` depending on URL format. 4. **Download snapshot:** Uses bounded `fc::http_client::post_to_file()` streaming to POST to - `/v1/snapshot/download`, enforce deadlines/size/disk headroom, and atomically save the response to the local + `/v1/snapshot/download`, enforce size and disk-headroom bounds, and atomically save the response to the local snapshots directory. 5. **Root hash verification:** Uses `threaded_snapshot_reader::load_index()` to read the footer and compare the stored root hash against the advertised `root_hash`. This is a fast metadata-only check that catches download corruption. Full integrity verification (re-hashing all sections) happens during snapshot loading, and on-chain attestation verification happens after syncing. 6. **Continue normal loading:** Sets `snapshot_path` to downloaded file and `snapshot_auto_fetched = true`. No `--genesis-json` needed — snapshot contains genesis. diff --git a/libraries/libfc/test/network/test_http_client.cpp b/libraries/libfc/test/network/test_http_client.cpp index d9add38eeb..18c3515b91 100644 --- a/libraries/libfc/test/network/test_http_client.cpp +++ b/libraries/libfc/test/network/test_http_client.cpp @@ -484,6 +484,21 @@ BOOST_AUTO_TEST_CASE(oversized_fixed_length_response_is_rejected_before_write) { check_download_files_removed(output); } +/// A fixed-length response that ends early must fail without retaining either output file. +BOOST_AUTO_TEST_CASE(truncated_fixed_length_response_is_rejected_and_removed) { + scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { + write_bytes(socket, fixed_length_header(exact_body_bytes) + "short"); + boost::system::error_code ec; + socket.shutdown(tcp::socket::shutdown_send, ec); + socket.close(ec); + }); + fc::temp_directory temp; + const auto output = temp.path() / "truncated-fixed.bin"; + + BOOST_CHECK_THROW(download(server, output, download_options(exact_body_bytes)), fc::exception); + check_download_files_removed(output); +} + /// A chunked response that exceeds the configured maximum is aborted and its temp file is removed. BOOST_AUTO_TEST_CASE(oversized_chunked_response_is_rejected_and_removed) { scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { From a1d71152799c91026c5caee2ead6e771f20f7e71 Mon Sep 17 00:00:00 2001 From: Huang-Ming Huang Date: Wed, 22 Jul 2026 20:33:14 +0000 Subject: [PATCH 5/7] Address snapshot download review feedback --- docs/snapshot-serving-plan.md | 6 ++-- .../libfc/src/network/http/http_client.cpp | 28 +++++++++++++---- .../libfc/test/network/test_http_client.cpp | 30 ++++++++++++++++--- plugins/chain_plugin/src/chain_plugin.cpp | 15 ++-------- .../chain_plugin/test/plugin_config_test.cpp | 18 +++++------ plugins/snapshot_api_plugin/README.md | 12 ++++---- tests/snapshot_api_test.py | 6 +--- 7 files changed, 68 insertions(+), 47 deletions(-) diff --git a/docs/snapshot-serving-plan.md b/docs/snapshot-serving-plan.md index 43870de0d3..0500a75a5d 100644 --- a/docs/snapshot-serving-plan.md +++ b/docs/snapshot-serving-plan.md @@ -252,7 +252,7 @@ The peers endpoint is a separate feature and was not implemented in this phase. ### Configuration -The endpoint and its resource limits are CLI-only because bootstrap is a single-use operation: +The endpoint is CLI-only because bootstrap is a single-use operation: ``` --snapshot-endpoint URL Fetch snapshot from URL and bootstrap. @@ -264,8 +264,8 @@ The endpoint and its resource limits are CLI-only because bootstrap is a single- Snapshot bootstrap is an attended operation against an operator-selected endpoint. It reports connection/request phases and, every five seconds during transfer, downloaded bytes, percentage, rate, and ETA when `Content-Length` is available. SIGINT cancels pending resolver or socket work and retains atomic temporary-file cleanup. The existing -`chain-state-db-size-mb` setting bounds both fixed-length and chunked response bodies; the only snapshot-specific -resource override is `--snapshot-endpoint-min-disk-free-mb` for reserved filesystem headroom. +`chain-state-db-size-mb` setting bounds both fixed-length and chunked response bodies. No snapshot-specific resource +options are added. The block number is encoded as a trailing path component of the URL. If the last path segment is a decimal number, it's treated as a specific block request (POST to `/v1/snapshot/by_block`); otherwise POST to `/v1/snapshot/latest`. diff --git a/libraries/libfc/src/network/http/http_client.cpp b/libraries/libfc/src/network/http/http_client.cpp index 6503e2d3ba..a881e4064b 100644 --- a/libraries/libfc/src/network/http/http_client.cpp +++ b/libraries/libfc/src/network/http/http_client.cpp @@ -28,8 +28,10 @@ namespace fc { namespace { -constexpr size_t download_read_buffer_bytes = 1024 * 1024; +// Beast limits an individual parser read to roughly 64 KiB, so a larger body buffer is unused. +constexpr size_t download_read_buffer_bytes = 64 * 1024; constexpr size_t download_parser_buffer_bytes = 64 * 1024; +constexpr size_t max_error_response_body_bytes = 200; constexpr uint64_t disk_space_check_interval_bytes = 64 * 1024 * 1024; constexpr uint64_t disk_space_concurrency_margin_bytes = 64 * 1024 * 1024; constexpr auto disk_space_check_interval = std::chrono::seconds(5); @@ -667,7 +669,24 @@ class http_client_impl { // the final destination would hand the caller a truncated file. const auto status = parser->get().result(); if (status != http::status::ok) { - FC_THROW("HTTP POST failed with status {}", parser->get().result_int()); + // Preserve a useful endpoint diagnostic without buffering an unbounded error response. + // One parser increment is enough for the first 200 decoded bytes and remains cancellable. + std::string error_body(max_error_response_body_bytes, '\0'); + parser->get().body().data = error_body.data(); + parser->get().body().size = error_body.size(); + if (!parser->is_done()) { + ec = std::visit([&](auto& stream) { + return sync_read_parser_some_with_timeout(*stream, *buffer, *parser, no_deadline, monitor); + }, conn_iter->second); + if (ec == http::error::need_buffer) { + ec = {}; + } + } + error_body.resize(error_body.size() - parser->get().body().size); + if (error_body.empty()) { + FC_THROW("HTTP POST failed with status {}", parser->get().result_int()); + } + FC_THROW("HTTP POST failed with status {}: {}", parser->get().result_int(), error_body); } const auto content_length = parser->content_length(); @@ -676,9 +695,6 @@ class http_client_impl { } uint64_t disk_space_write_budget = 0; if (content_length) { - FC_ASSERT(*content_length <= options.max_response_body_bytes, - "HTTP response Content-Length {} exceeds configured maximum of {} bytes", - *content_length, options.max_response_body_bytes); FC_ASSERT(*content_length <= std::numeric_limits::max() - disk_space_concurrency_margin_bytes, "HTTP response Content-Length {} is too large for the {}-byte disk safety margin", *content_length, disk_space_concurrency_margin_bytes); @@ -713,6 +729,8 @@ class http_client_impl { return sync_read_parser_some_with_timeout(*stream, *buffer, *parser, no_deadline, monitor); }, conn_iter->second); if (ec == http::error::need_buffer) { + // buffer_body reports need_buffer when this caller-owned output buffer is filled; + // the decoded bytes are valid and the next loop iteration supplies a fresh buffer. ec = {}; } if (ec == http::error::body_limit) { diff --git a/libraries/libfc/test/network/test_http_client.cpp b/libraries/libfc/test/network/test_http_client.cpp index 18c3515b91..f9501d14be 100644 --- a/libraries/libfc/test/network/test_http_client.cpp +++ b/libraries/libfc/test/network/test_http_client.cpp @@ -139,10 +139,10 @@ bool write_bytes(tcp::socket& socket, std::string_view bytes) { return !ec; } -/** Return a complete 200 response header for a fixed-length body. */ -std::string fixed_length_header(uint64_t body_bytes) { +/** Return a complete fixed-length response header. */ +std::string fixed_length_header(uint64_t body_bytes, std::string_view status = "200 OK") { std::ostringstream response; - response << "HTTP/1.1 200 OK\r\n" + response << "HTTP/1.1 " << status << "\r\n" << "Content-Type: application/octet-stream\r\n" << "Content-Length: " << body_bytes << "\r\n" << "Connection: close\r\n\r\n"; @@ -199,7 +199,7 @@ bool write_chunked_body(tcp::socket& socket, uint64_t body_bytes) { fc::http_file_download_options download_options(uint64_t max_body_bytes) { return fc::http_file_download_options{ .max_response_body_bytes = max_body_bytes, - .min_free_disk_space_bytes = 1, + .min_free_disk_space_bytes = 0, .retry_failed_reused_connection = false, }; } @@ -484,6 +484,28 @@ BOOST_AUTO_TEST_CASE(oversized_fixed_length_response_is_rejected_before_write) { check_download_files_removed(output); } +/// A non-success response includes only a small bounded prefix of its diagnostic body. +BOOST_AUTO_TEST_CASE(error_response_includes_bounded_body_diagnostic) { + const std::string error_prefix = "snapshot not ready: "; + const std::string omitted_suffix = "omitted-tail"; + const std::string error_body = error_prefix + std::string(256, 'x') + omitted_suffix; + scripted_http_server server([&](tcp::socket& socket, const std::atomic_bool&) { + write_bytes(socket, fixed_length_header(error_body.size(), "409 Conflict") + error_body); + }); + fc::temp_directory temp; + const auto output = temp.path() / "error-response.bin"; + + BOOST_CHECK_EXCEPTION( + download(server, output, download_options(error_body.size())), + fc::exception, + [&](const fc::exception& error) { + const auto detail = error.to_detail_string(); + return detail.find("HTTP POST failed with status 409: " + error_prefix) != std::string::npos && + detail.find(omitted_suffix) == std::string::npos; + }); + check_download_files_removed(output); +} + /// A fixed-length response that ends early must fail without retaining either output file. BOOST_AUTO_TEST_CASE(truncated_fixed_length_response_is_rejected_and_removed) { scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { diff --git a/plugins/chain_plugin/src/chain_plugin.cpp b/plugins/chain_plugin/src/chain_plugin.cpp index b88c81aff3..d51fa3db10 100644 --- a/plugins/chain_plugin/src/chain_plugin.cpp +++ b/plugins/chain_plugin/src/chain_plugin.cpp @@ -69,8 +69,6 @@ namespace snapshot_attest { /// provider snapshot interval (_snapshot_provider_block_spacing). constexpr uint32_t snapshot_attestation_grace_blocks = 12500; -constexpr std::string_view snapshot_min_disk_free_option = "snapshot-endpoint-min-disk-free-mb"; - constexpr uint64_t bytes_per_mebibyte = 1024 * 1024; /// Convert a positive MiB option value to bytes without overflowing uint64_t. @@ -578,8 +576,6 @@ void chain_plugin::set_program_options(options_description& cli, options_descrip " http://host:port - fetches latest snapshot\n" " http://host:port/50000 - fetches snapshot at block 50000\n" "Requires empty database (use --delete-all-blocks to clear existing data).") - (snapshot_min_disk_free_option.data(), bpo::value(), - "Free disk space in MiB that must remain during download. Defaults to chain-state-db-guard-size-mb.") ; } @@ -987,16 +983,11 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) { const auto max_download_bytes = checked_mebibytes( options.at("chain-state-db-size-mb").as(), "chain-state-db-size-mb"); - const std::string min_disk_free_option_name{snapshot_min_disk_free_option}; - const bool has_min_disk_free = options.contains(min_disk_free_option_name); - const auto min_free_disk_bytes = checked_mebibytes( - has_min_disk_free - ? options.at(min_disk_free_option_name).as() - : options.at("chain-state-db-guard-size-mb").as(), - has_min_disk_free ? snapshot_min_disk_free_option : std::string_view{"chain-state-db-guard-size-mb"}); const fc::http_file_download_options download_options{ .max_response_body_bytes = max_download_bytes, - .min_free_disk_space_bytes = min_free_disk_bytes, + // Do not add a snapshot-specific reserve option. The HTTP client still checks that + // each bounded write fits on disk with its internal concurrent-consumer margin. + .min_free_disk_space_bytes = 0, .retry_failed_reused_connection = true, .status_callback = [logger = snapshot_download_progress_logger{}]( const fc::http_file_download_status& status) mutable { logger(status); }, diff --git a/plugins/chain_plugin/test/plugin_config_test.cpp b/plugins/chain_plugin/test/plugin_config_test.cpp index 9438181058..09aacf3ca4 100644 --- a/plugins/chain_plugin/test/plugin_config_test.cpp +++ b/plugins/chain_plugin/test/plugin_config_test.cpp @@ -11,9 +11,9 @@ namespace { -/** Initialize chain_plugin with one snapshot-endpoint limit override. */ -sysio::chain::exit_code::exit_code initialize_with_snapshot_limit(std::string_view option_name, - std::string_view option_value) { +/** Initialize chain_plugin with one snapshot response-size limit override. */ +sysio::chain::exit_code::exit_code initialize_with_snapshot_size_limit(std::string_view option_name, + std::string_view option_value) { fc::temp_directory tmp; sysio::chain::application exe({.enable_resource_monitor = false}); @@ -61,7 +61,7 @@ BOOST_AUTO_TEST_CASE(chain_plugin_default_tests) { } -/** Verify the sole snapshot-specific resource option and removal of endpoint timeout/size knobs. */ +/** Verify snapshot endpoint registration and removal of endpoint-specific resource knobs. */ BOOST_AUTO_TEST_CASE(chain_plugin_snapshot_endpoint_option_registration) { sysio::chain::application exe({.enable_resource_monitor = false}); sysio::chain_plugin plugin; @@ -74,7 +74,6 @@ BOOST_AUTO_TEST_CASE(chain_plugin_snapshot_endpoint_option_registration) { std::array arguments{ "test_chain_plugin", "--snapshot-endpoint", "http://127.0.0.1:1", - "--snapshot-endpoint-min-disk-free-mb", "16", }; boost::program_options::variables_map variables; boost::program_options::store( @@ -83,7 +82,6 @@ BOOST_AUTO_TEST_CASE(chain_plugin_snapshot_endpoint_option_registration) { boost::program_options::notify(variables); BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint").as(), "http://127.0.0.1:1"); - BOOST_CHECK_EQUAL(variables.at("snapshot-endpoint-min-disk-free-mb").as(), 16); constexpr std::array removed_options{ "snapshot-endpoint-connect-timeout-ms", @@ -91,25 +89,23 @@ BOOST_AUTO_TEST_CASE(chain_plugin_snapshot_endpoint_option_registration) { "snapshot-endpoint-idle-timeout-ms", "snapshot-endpoint-total-timeout-ms", "snapshot-endpoint-max-download-size-mb", + "snapshot-endpoint-min-disk-free-mb", }; for (const auto* option_name : removed_options) { BOOST_CHECK(options.find_nothrow(option_name, false) == nullptr); } } -/** Verify that snapshot endpoint resource limits reject zero and overflow-prone values before connecting. */ +/** Verify that the snapshot response-size limit rejects zero and overflow before connecting. */ BOOST_AUTO_TEST_CASE(chain_plugin_snapshot_endpoint_option_validation) { constexpr std::array invalid_options{ - std::pair{"snapshot-endpoint-min-disk-free-mb", "0"}, std::pair{"chain-state-db-size-mb", "0"}, std::pair{"chain-state-db-size-mb", "17592186044416"}, - std::pair{"chain-state-db-guard-size-mb", "0"}, - std::pair{"chain-state-db-guard-size-mb", "17592186044416"}, }; for (const auto& [option_name, option_value] : invalid_options) { BOOST_TEST_CONTEXT(option_name << '=' << option_value) { - BOOST_CHECK(initialize_with_snapshot_limit(option_name, option_value) != sysio::chain::exit_code::SUCCESS); + BOOST_CHECK(initialize_with_snapshot_size_limit(option_name, option_value) != sysio::chain::exit_code::SUCCESS); } } } diff --git a/plugins/snapshot_api_plugin/README.md b/plugins/snapshot_api_plugin/README.md index 4ca3ff73c5..e8ea02d846 100644 --- a/plugins/snapshot_api_plugin/README.md +++ b/plugins/snapshot_api_plugin/README.md @@ -254,18 +254,16 @@ the active operation; the file transfer then reports phase changes and, every fi percentage, transfer rate, and ETA when the response supplies `Content-Length`. Pressing Ctrl+C cancels pending resolver or socket work and removes the partial file. -The existing chain database settings supply the download ceiling and default disk reserve. Only the disk reserve has a -snapshot-specific override: +The existing chain database size setting supplies the download ceiling: | Option | Default | Purpose | |---|---:|---| | `--chain-state-db-size-mb` | `1024` | Maximum accepted snapshot response size and chain-state database size | -| `--chain-state-db-guard-size-mb` | build default | Default free-space reserve during snapshot download | -| `--snapshot-endpoint-min-disk-free-mb` | `chain-state-db-guard-size-mb` | Free space retained on the snapshots filesystem | -Resource limits must be positive. A fixed-length response that exceeds the maximum is rejected from its `Content-Length` -before a temporary file is opened. Chunked or lengthless responses are stopped at the same byte ceiling. Available -disk space is checked before and throughout the transfer, and a failed transfer removes its `.downloading` file. +The response-size limit must be positive. A fixed-length response that exceeds the maximum is rejected from its +`Content-Length` before a temporary file is opened. Chunked or lengthless responses are stopped at the same byte +ceiling. Available disk space is checked before and throughout the transfer, and a failed transfer removes its +`.downloading` file. Long transfers recheck after at most 64 MiB or five seconds of progress and retain an additional 64 MiB safety margin to bound interference from concurrent disk consumers between probes. diff --git a/tests/snapshot_api_test.py b/tests/snapshot_api_test.py index 5f184d8c59..8c08b837fc 100755 --- a/tests/snapshot_api_test.py +++ b/tests/snapshot_api_test.py @@ -453,15 +453,11 @@ def recordPresent(): endpointUrl = node0.endpointHttp Print(f"Restart bootstrap node with --snapshot-endpoint {endpointUrl}") - # Exercise the snapshot-specific disk-headroom override. Download size is - # bounded by the existing chain-state-db-size-mb setting. - downloadLimits = "--snapshot-endpoint-min-disk-free-mb 1" - # Fetches latest snapshot (snap2). The attestation is NOT in the snapshot — # it's in blocks after snap2BlockNum. The bootstrap node syncs forward and # finds the attestation record once it reaches those blocks. isRelaunchSuccess = bootstrapNode.relaunch( - chainArg=f"--delete-all-blocks --snapshot-endpoint {endpointUrl} {downloadLimits}") + chainArg=f"--delete-all-blocks --snapshot-endpoint {endpointUrl}") assert isRelaunchSuccess, "Failed to relaunch bootstrap node from snapshot endpoint" # The attestation is in blocks after the snapshot height, so wait for the bootstrap From 2260b339b0bc0b00bb96390662d6702543f81f84 Mon Sep 17 00:00:00 2001 From: Huang-Ming Huang Date: Wed, 22 Jul 2026 20:52:02 +0000 Subject: [PATCH 6/7] Factor streamed HTTP body reads --- .../libfc/src/network/http/http_client.cpp | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/libraries/libfc/src/network/http/http_client.cpp b/libraries/libfc/src/network/http/http_client.cpp index a881e4064b..cf981186c3 100644 --- a/libraries/libfc/src/network/http/http_client.cpp +++ b/libraries/libfc/src/network/http/http_client.cpp @@ -240,6 +240,27 @@ class http_client_impl { }, monitor); } + /** + * Read one decoded response-body increment into caller-owned storage. + * + * @p ec retains transport and parser failures for the caller, while need_buffer is normalized + * because it only means the supplied buffer_body storage was filled successfully. + */ + size_t read_body_increment(connection& conn, boost::beast::flat_buffer& buffer, + http::response_parser& parser, char* sink, size_t sink_size, + const operation_monitor& monitor, error_code& ec) { + parser.get().body().data = sink; + parser.get().body().size = sink_size; + ec = std::visit([&](auto& stream) { + return sync_read_parser_some_with_timeout( + *stream, buffer, parser, deadline_type::max(), monitor); + }, conn); + if (ec == http::error::need_buffer) { + ec = {}; + } + return sink_size - parser.get().body().size; + } + /// Return whether @p ec can indicate that a cached peer connection closed before reuse. static bool is_stale_connection_error(const error_code& ec) { return ec == boost::asio::error::eof || @@ -672,17 +693,12 @@ class http_client_impl { // Preserve a useful endpoint diagnostic without buffering an unbounded error response. // One parser increment is enough for the first 200 decoded bytes and remains cancellable. std::string error_body(max_error_response_body_bytes, '\0'); - parser->get().body().data = error_body.data(); - parser->get().body().size = error_body.size(); if (!parser->is_done()) { - ec = std::visit([&](auto& stream) { - return sync_read_parser_some_with_timeout(*stream, *buffer, *parser, no_deadline, monitor); - }, conn_iter->second); - if (ec == http::error::need_buffer) { - ec = {}; - } + error_body.resize(read_body_increment(conn_iter->second, *buffer, *parser, error_body.data(), + error_body.size(), monitor, ec)); + } else { + error_body.clear(); } - error_body.resize(error_body.size() - parser->get().body().size); if (error_body.empty()) { FC_THROW("HTTP POST failed with status {}", parser->get().result_int()); } @@ -723,22 +739,13 @@ class http_client_impl { std::vector read_buffer(download_read_buffer_bytes); transition_to(http_file_download_phase::downloading); while (!parser->is_done()) { - parser->get().body().data = read_buffer.data(); - parser->get().body().size = read_buffer.size(); - ec = std::visit([&](auto& stream) { - return sync_read_parser_some_with_timeout(*stream, *buffer, *parser, no_deadline, monitor); - }, conn_iter->second); - if (ec == http::error::need_buffer) { - // buffer_body reports need_buffer when this caller-owned output buffer is filled; - // the decoded bytes are valid and the next loop iteration supplies a fresh buffer. - ec = {}; - } + const auto chunk_bytes = read_body_increment(conn_iter->second, *buffer, *parser, read_buffer.data(), + read_buffer.size(), monitor, ec); if (ec == http::error::body_limit) { FC_THROW("HTTP response body exceeds configured maximum of {} bytes", options.max_response_body_bytes); } FC_ASSERT(!ec, "Failed to read response body: {}", ec.message()); - const auto chunk_bytes = read_buffer.size() - parser->get().body().size; FC_ASSERT(chunk_bytes <= options.max_response_body_bytes && downloaded_bytes <= options.max_response_body_bytes - chunk_bytes, "HTTP response body exceeds configured maximum of {} bytes", options.max_response_body_bytes); From c64f6c6f731a52ecb946963adb82a3696f214f72 Mon Sep 17 00:00:00 2001 From: Huang-Ming Huang Date: Wed, 22 Jul 2026 21:00:26 +0000 Subject: [PATCH 7/7] Remove unused HTTP disk reserve option --- .../include/fc/network/http/http_client.hpp | 8 ++- .../libfc/src/network/http/http_client.cpp | 57 +++++++++++-------- .../libfc/test/network/test_http_client.cpp | 48 +++++++++------- plugins/chain_plugin/src/chain_plugin.cpp | 3 - 4 files changed, 69 insertions(+), 47 deletions(-) diff --git a/libraries/libfc/include/fc/network/http/http_client.hpp b/libraries/libfc/include/fc/network/http/http_client.hpp index 5dccf7f3c2..4f62f7825f 100644 --- a/libraries/libfc/include/fc/network/http/http_client.hpp +++ b/libraries/libfc/include/fc/network/http/http_client.hpp @@ -43,8 +43,6 @@ struct http_file_download_status { struct http_file_download_options { /// Maximum accepted response-body size in bytes. uint64_t max_response_body_bytes; - /// Free bytes that must remain on the destination filesystem. - uint64_t min_free_disk_space_bytes; /// Retry a failed request once when it used a cached connection. Only enable for idempotent requests. bool retry_failed_reused_connection = false; /// Receive phase transitions and periodic transfer status. The callback must not throw. @@ -88,6 +86,12 @@ class http_client { void set_verify_peers(bool enabled); private: + friend struct http_client_test_access; + + /// Override the filesystem free-space query for deterministic transport tests. + void set_space_available_provider_for_testing( + std::function provider); + std::unique_ptr _my; }; diff --git a/libraries/libfc/src/network/http/http_client.cpp b/libraries/libfc/src/network/http/http_client.cpp index cf981186c3..bf68791137 100644 --- a/libraries/libfc/src/network/http/http_client.cpp +++ b/libraries/libfc/src/network/http/http_client.cpp @@ -73,6 +73,11 @@ class http_client_impl { _cancel_check = std::move(cancel_check); } + void set_space_available_provider_for_testing( + std::function provider) { + _space_available_provider = std::move(provider); + } + /** Return whether the caller requested cancellation of the active HTTP operation. */ bool is_cancelled() const { return _cancel_check && _cancel_check(); @@ -284,22 +289,25 @@ class http_client_impl { return current; } - /** - * Require enough free space for @p next_write_bytes while retaining @p reserved_bytes. - * - * @return Bytes currently available for writes after retaining the requested reserve. - */ - static uint64_t require_disk_space(const std::filesystem::path& destination, uint64_t next_write_bytes, - uint64_t reserved_bytes) { - const auto query_path = space_query_path(destination); + /// Return available bytes on the destination filesystem, using the test seam when configured. + uint64_t available_disk_space(const std::filesystem::path& query_path) const { + if (_space_available_provider) { + return _space_available_provider(query_path); + } + std::error_code ec; const auto space = std::filesystem::space(query_path, ec); FC_ASSERT(!ec, "Failed to query free disk space at {}: {}", query_path.string(), ec.message()); - FC_ASSERT(space.available >= reserved_bytes && next_write_bytes <= space.available - reserved_bytes, - "Insufficient disk space at {} for HTTP download: {} bytes available, {} bytes required for the " - "next write, and {} bytes reserved as headroom", - query_path.string(), space.available, next_write_bytes, reserved_bytes); - return space.available - reserved_bytes; + return space.available; + } + + /** Require enough free space for @p next_write_bytes. */ + void require_disk_space(const std::filesystem::path& destination, uint64_t next_write_bytes) const { + const auto query_path = space_query_path(destination); + const auto available_bytes = available_disk_space(query_path); + FC_ASSERT(next_write_bytes <= available_bytes, + "Insufficient disk space at {} for HTTP download: {} bytes available and {} bytes required", + query_path.string(), available_bytes, next_write_bytes); } /** @@ -309,16 +317,15 @@ class http_client_impl { * The margin bounds headroom exposure if another process consumes space between probes; * slow transfers also refresh the probe every five seconds. */ - static uint64_t refresh_disk_space_write_budget(const std::filesystem::path& destination, - uint64_t remaining_body_bytes, - uint64_t reserved_bytes) { + uint64_t refresh_disk_space_write_budget(const std::filesystem::path& destination, + uint64_t remaining_body_bytes) const { const auto write_budget = std::min(remaining_body_bytes, disk_space_check_interval_bytes); if (write_budget == 0) { - require_disk_space(destination, 0, reserved_bytes); + require_disk_space(destination, 0); return 0; } - require_disk_space(destination, write_budget + disk_space_concurrency_margin_bytes, reserved_bytes); + require_disk_space(destination, write_budget + disk_space_concurrency_margin_bytes); return write_budget; } @@ -717,11 +724,10 @@ class http_client_impl { const auto preflight_bytes = *content_length == 0 ? 0 : *content_length + disk_space_concurrency_margin_bytes; - require_disk_space(final_dest, preflight_bytes, options.min_free_disk_space_bytes); + require_disk_space(final_dest, preflight_bytes); disk_space_write_budget = std::min(*content_length, disk_space_check_interval_bytes); } else { - disk_space_write_budget = refresh_disk_space_write_budget( - final_dest, options.max_response_body_bytes, options.min_free_disk_space_bytes); + disk_space_write_budget = refresh_disk_space_write_budget(final_dest, options.max_response_body_bytes); } auto next_disk_space_check = std::chrono::steady_clock::now() + disk_space_check_interval; @@ -760,8 +766,7 @@ class http_client_impl { std::chrono::steady_clock::now() >= next_disk_space_check) { const auto response_size_limit = content_length.value_or(options.max_response_body_bytes); const auto remaining_body_bytes = response_size_limit - downloaded_bytes; - disk_space_write_budget = refresh_disk_space_write_budget( - final_dest, remaining_body_bytes, options.min_free_disk_space_bytes); + disk_space_write_budget = refresh_disk_space_write_budget(final_dest, remaining_body_bytes); next_disk_space_check = std::chrono::steady_clock::now() + disk_space_check_interval; } file.write(read_buffer.data(), static_cast(chunk_bytes)); @@ -786,6 +791,7 @@ class http_client_impl { connection_map _connections; unix_url_split_map _unix_url_paths; std::function _cancel_check; + std::function _space_available_provider; }; @@ -804,6 +810,11 @@ void http_client::set_cancel_check(std::function cancel_check) { _my->set_cancel_check(std::move(cancel_check)); } +void http_client::set_space_available_provider_for_testing( + std::function provider) { + _my->set_space_available_provider_for_testing(std::move(provider)); +} + variant http_client::post_sync(const url& dest, const variant& payload, const fc::time_point& deadline) { if(dest.proto() == "unix") return _my->post_sync(_my->get_unix_url(*dest.host()), payload, deadline); diff --git a/libraries/libfc/test/network/test_http_client.cpp b/libraries/libfc/test/network/test_http_client.cpp index f9501d14be..5ab330c878 100644 --- a/libraries/libfc/test/network/test_http_client.cpp +++ b/libraries/libfc/test/network/test_http_client.cpp @@ -16,7 +16,6 @@ #include #include #include -#include #include #include #include @@ -26,6 +25,18 @@ using namespace std::chrono_literals; +namespace fc { + +/** Private free-space-query injection for HTTP download tests. */ +struct http_client_test_access { + static void set_available_disk_space(http_client& client, uint64_t available_bytes) { + client.set_space_available_provider_for_testing( + [available_bytes](const std::filesystem::path&) { return available_bytes; }); + } +}; + +} // namespace fc + namespace { using tcp = boost::asio::ip::tcp; @@ -199,7 +210,6 @@ bool write_chunked_body(tcp::socket& socket, uint64_t body_bytes) { fc::http_file_download_options download_options(uint64_t max_body_bytes) { return fc::http_file_download_options{ .max_response_body_bytes = max_body_bytes, - .min_free_disk_space_bytes = 0, .retry_failed_reused_connection = false, }; } @@ -218,6 +228,16 @@ void download(const scripted_http_server& server, const std::filesystem::path& o client.post_to_file(server_url(server), fc::variant(fc::mutable_variant_object()), output, options); } +/** Invoke a download with a deterministic free-space query result. */ +void download_with_available_disk_space(const scripted_http_server& server, + const std::filesystem::path& output, + const fc::http_file_download_options& options, + uint64_t available_bytes) { + fc::http_client client; + fc::http_client_test_access::set_available_disk_space(client, available_bytes); + client.post_to_file(server_url(server), fc::variant(fc::mutable_variant_object()), output, options); +} + /** Request cancellation after a short deterministic delay. */ class delayed_cancellation { public: @@ -632,17 +652,16 @@ BOOST_AUTO_TEST_CASE(chunked_response_refills_disk_space_budget) { BOOST_CHECK_EQUAL(std::filesystem::file_size(output), response_bytes); } -/// A reserved-headroom requirement larger than the filesystem capacity must fail before writing. -BOOST_AUTO_TEST_CASE(insufficient_disk_headroom_is_rejected_before_write) { +/// A response that cannot fit on the destination filesystem must fail before writing. +BOOST_AUTO_TEST_CASE(insufficient_disk_space_is_rejected_before_write) { scripted_http_server server([](tcp::socket& socket, const std::atomic_bool&) { write_bytes(socket, fixed_length_header(1) + "1"); }); fc::temp_directory temp; const auto output = temp.path() / "disk-headroom.bin"; auto options = download_options(exact_body_bytes); - options.min_free_disk_space_bytes = std::numeric_limits::max(); - BOOST_CHECK_THROW(download(server, output, options), fc::exception); + BOOST_CHECK_THROW(download_with_available_disk_space(server, output, options, 0), fc::exception); check_download_files_removed(output); } @@ -653,14 +672,10 @@ BOOST_AUTO_TEST_CASE(disk_space_budget_requires_concurrency_margin) { }); fc::temp_directory temp; const auto output = temp.path() / "disk-concurrency-margin.bin"; - std::error_code ec; - const auto space = std::filesystem::space(temp.path(), ec); - BOOST_REQUIRE(!ec); - BOOST_REQUIRE_GT(space.available, exact_body_bytes); auto options = download_options(exact_body_bytes); - options.min_free_disk_space_bytes = space.available - exact_body_bytes; - BOOST_CHECK_THROW(download(server, output, options), fc::exception); + BOOST_CHECK_THROW( + download_with_available_disk_space(server, output, options, exact_body_bytes), fc::exception); check_download_files_removed(output); } @@ -673,16 +688,11 @@ BOOST_AUTO_TEST_CASE(fixed_length_preflight_includes_concurrency_margin) { }); fc::temp_directory temp; const auto output = temp.path() / "fixed-preflight-margin.bin"; - std::error_code ec; - const auto space = std::filesystem::space(temp.path(), ec); - BOOST_REQUIRE(!ec); - BOOST_REQUIRE_GT(space.available, response_bytes + headroom_without_full_margin); auto options = download_options(response_bytes); - options.min_free_disk_space_bytes = - space.available - response_bytes - headroom_without_full_margin; BOOST_CHECK_EXCEPTION( - download(server, output, options), + download_with_available_disk_space( + server, output, options, response_bytes + headroom_without_full_margin), fc::exception, [](const fc::exception& error) { return error.to_detail_string().find("Insufficient disk space") != std::string::npos; diff --git a/plugins/chain_plugin/src/chain_plugin.cpp b/plugins/chain_plugin/src/chain_plugin.cpp index d51fa3db10..98b1dcd94e 100644 --- a/plugins/chain_plugin/src/chain_plugin.cpp +++ b/plugins/chain_plugin/src/chain_plugin.cpp @@ -985,9 +985,6 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) { options.at("chain-state-db-size-mb").as(), "chain-state-db-size-mb"); const fc::http_file_download_options download_options{ .max_response_body_bytes = max_download_bytes, - // Do not add a snapshot-specific reserve option. The HTTP client still checks that - // each bounded write fits on disk with its internal concurrent-consumer margin. - .min_free_disk_space_bytes = 0, .retry_failed_reused_connection = true, .status_callback = [logger = snapshot_download_progress_logger{}]( const fc::http_file_download_status& status) mutable { logger(status); },