From 873cf5c10397c66e8c4f54c83784b5969280ab8b Mon Sep 17 00:00:00 2001 From: jiuker Date: Wed, 19 Aug 2026 13:46:17 +0800 Subject: [PATCH 01/19] replace curlapp by httplib replace curlapp by httplib --- cmake/miniocpp-deps.cmake | 4 ++++ src/response.cc | 9 +++++---- vcpkg.json | 1 + 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/cmake/miniocpp-deps.cmake b/cmake/miniocpp-deps.cmake index a0a336e2..9177b5ca 100644 --- a/cmake/miniocpp-deps.cmake +++ b/cmake/miniocpp-deps.cmake @@ -19,6 +19,9 @@ find_package(PkgConfig QUIET) find_package(OpenSSL REQUIRED) find_package(ZLIB REQUIRED) find_package(nlohmann_json CONFIG REQUIRED) +find_package(httplib CONFIG REQUIRED) + +set(MINIO_CPP_HTTPLIB_TARGET httplib::httplib) # curlpp -- pinned master commit, no patches needed: CURLOPT_CLOSEPOLICY is # gone upstream (dropped in curl 8.10) and the build is target-based and @@ -126,6 +129,7 @@ else() endif() set(MINIO_CPP_DEPS_LINK_LIBS + ${MINIO_CPP_HTTPLIB_TARGET} ${MINIO_CPP_CURLPP_TARGET} ${MINIO_CPP_INIH_TARGET} nlohmann_json::nlohmann_json diff --git a/src/response.cc b/src/response.cc index ffdfc64f..3801351b 100644 --- a/src/response.cc +++ b/src/response.cc @@ -17,8 +17,9 @@ #include "miniocpp/response.h" +#include + #include -#include #include #include #include @@ -168,7 +169,7 @@ Result ListObjectsResponse::ParseXML(std::string_view data, const char* raw, std::string_view& target) -> void { if (encoding_type == "url") { - resp.owned_.emplace_back(curlpp::unescape(raw)); + resp.owned_.emplace_back(httplib::decode_uri_component(raw)); target = resp.owned_.back(); } else { target = raw; @@ -260,7 +261,7 @@ Result ListObjectsResponse::ParseXML(std::string_view data, text = content.node().select_node("Key/text()"); raw = text.node().value(); if (resp.encoding_type == "url") { - resp.owned_.emplace_back(curlpp::unescape(raw)); + resp.owned_.emplace_back(httplib::decode_uri_component(raw)); item.name = resp.owned_.back(); } else { item.name = raw; @@ -317,7 +318,7 @@ Result ListObjectsResponse::ParseXML(std::string_view data, text = common_prefix.node().select_node("Prefix/text()"); raw = text.node().value(); if (resp.encoding_type == "url") { - resp.owned_.emplace_back(curlpp::unescape(raw)); + resp.owned_.emplace_back(httplib::decode_uri_component(raw)); item.name = resp.owned_.back(); } else { item.name = raw; diff --git a/vcpkg.json b/vcpkg.json index 4e249e95..3e5a6269 100644 --- a/vcpkg.json +++ b/vcpkg.json @@ -5,6 +5,7 @@ "homepage": "https://github.com/minio/minio-cpp", "license": "Apache-2.0", "dependencies": [ + { "name": "cpp-httplib" }, { "name": "curlpp" }, { "name": "inih", "features": ["cpp"] }, { "name": "nlohmann-json" }, From 012b2b49cbafb6022dae6f2dde5923c848d55227 Mon Sep 17 00:00:00 2001 From: jiuker Date: Wed, 19 Aug 2026 13:49:34 +0800 Subject: [PATCH 02/19] cicd cicd --- cmake/miniocpp-deps.cmake | 41 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/cmake/miniocpp-deps.cmake b/cmake/miniocpp-deps.cmake index 9177b5ca..651ebd69 100644 --- a/cmake/miniocpp-deps.cmake +++ b/cmake/miniocpp-deps.cmake @@ -19,9 +19,46 @@ find_package(PkgConfig QUIET) find_package(OpenSSL REQUIRED) find_package(ZLIB REQUIRED) find_package(nlohmann_json CONFIG REQUIRED) -find_package(httplib CONFIG REQUIRED) -set(MINIO_CPP_HTTPLIB_TARGET httplib::httplib) +# cpp-httplib -- header-only library with optional SSL support. +# Resolution order: vcpkg -> pkg-config -> upstream source (pinned tag). +find_package(httplib CONFIG QUIET) +if (httplib_FOUND) + set(MINIO_CPP_HTTPLIB_TARGET httplib::httplib) +else() + if (PkgConfig_FOUND) + pkg_check_modules(MINIO_CPP_HTTPLIB QUIET IMPORTED_TARGET cpp-httplib) + endif() + if (MINIO_CPP_HTTPLIB_FOUND) + set(MINIO_CPP_HTTPLIB_TARGET PkgConfig::MINIO_CPP_HTTPLIB) + else() + message(STATUS "cpp-httplib: no package found, building from source") + set(MINIO_CPP_HTTPLIB_SRC "${CMAKE_CURRENT_BINARY_DIR}/_deps/cpp-httplib-src") + set(MINIO_CPP_HTTPLIB_PINNED_TAG "v0.18.3") + if (NOT EXISTS "${MINIO_CPP_HTTPLIB_SRC}/CMakeLists.txt") + execute_process(COMMAND git clone --quiet + https://github.com/yhirose/cpp-httplib.git + "${MINIO_CPP_HTTPLIB_SRC}" + RESULT_VARIABLE _httplib_clone) + if (NOT _httplib_clone STREQUAL "0") + message(FATAL_ERROR "cpp-httplib: git clone failed") + endif() + endif() + execute_process(COMMAND git checkout --quiet + ${MINIO_CPP_HTTPLIB_PINNED_TAG} + WORKING_DIRECTORY "${MINIO_CPP_HTTPLIB_SRC}" + RESULT_VARIABLE _httplib_checkout) + if (NOT _httplib_checkout STREQUAL "0") + message(FATAL_ERROR "cpp-httplib: git checkout of pinned tag failed") + endif() + # cpp-httplib is header-only; enable its CMake install target so that + # find_package(httplib) works for downstream consumers after install. + set(HTTPLIB_BUILD_SHARED_LIBS OFF CACHE BOOL "" FORCE) + add_subdirectory("${MINIO_CPP_HTTPLIB_SRC}" + "${CMAKE_CURRENT_BINARY_DIR}/_deps/cpp-httplib-build") + set(MINIO_CPP_HTTPLIB_TARGET httplib::httplib) + endif() +endif() # curlpp -- pinned master commit, no patches needed: CURLOPT_CLOSEPOLICY is # gone upstream (dropped in curl 8.10) and the build is target-based and From 17b1691e796c0d78213ae6c2193c7fcba116844e Mon Sep 17 00:00:00 2001 From: jiuker Date: Wed, 19 Aug 2026 14:53:46 +0800 Subject: [PATCH 03/19] tag --- cmake/miniocpp-deps.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/miniocpp-deps.cmake b/cmake/miniocpp-deps.cmake index 651ebd69..3bf2c924 100644 --- a/cmake/miniocpp-deps.cmake +++ b/cmake/miniocpp-deps.cmake @@ -34,7 +34,7 @@ else() else() message(STATUS "cpp-httplib: no package found, building from source") set(MINIO_CPP_HTTPLIB_SRC "${CMAKE_CURRENT_BINARY_DIR}/_deps/cpp-httplib-src") - set(MINIO_CPP_HTTPLIB_PINNED_TAG "v0.18.3") + set(MINIO_CPP_HTTPLIB_PINNED_TAG "v0.53.1") if (NOT EXISTS "${MINIO_CPP_HTTPLIB_SRC}/CMakeLists.txt") execute_process(COMMAND git clone --quiet https://github.com/yhirose/cpp-httplib.git From c19c2925e93e35e24e5baf6a748f45de2cba2959 Mon Sep 17 00:00:00 2001 From: jiuker Date: Wed, 19 Aug 2026 15:39:16 +0800 Subject: [PATCH 04/19] encode encode --- src/args.cc | 11 +++++++---- src/client.cc | 5 +++-- src/utils.cc | 8 ++++---- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/args.cc b/src/args.cc index e4a89eb8..3af70252 100644 --- a/src/args.cc +++ b/src/args.cc @@ -17,7 +17,8 @@ #include "miniocpp/args.h" -#include +#include + #include #include #include @@ -62,7 +63,8 @@ utils::Multimap ObjectWriteArgs::Headers() const { std::string tagging; for (auto& [key, value] : tags) { - std::string tag = curlpp::escape(key) + "=" + curlpp::escape(value); + std::string tag = httplib::encode_uri_component(key) + "=" + + httplib::encode_uri_component(value); if (!tagging.empty()) { tagging += "&"; } @@ -126,9 +128,10 @@ utils::Multimap ObjectConditionalReadArgs::Headers() const { utils::Multimap ObjectConditionalReadArgs::CopyHeaders() const { utils::Multimap result_headers; - std::string copy_source = curlpp::escape("/" + bucket + "/" + object); + std::string copy_source = + httplib::encode_uri_component("/" + bucket + "/" + object); if (!version_id.empty()) { - copy_source += "?versionId=" + curlpp::escape(version_id); + copy_source += "?versionId=" + httplib::encode_uri_component(version_id); } result_headers.Add("x-amz-copy-source", copy_source); diff --git a/src/client.cc b/src/client.cc index b8474f21..d5582a8a 100644 --- a/src/client.cc +++ b/src/client.cc @@ -24,7 +24,8 @@ #include #endif -#include +#include + #include #include #include @@ -1156,7 +1157,7 @@ Result Client::DownloadObject(DownloadObjectArgs args) { } std::string temp_filename = - args.filename + "." + curlpp::escape(etag) + ".part.minio"; + args.filename + "." + httplib::decode_uri_component(etag) + ".part.minio"; std::ofstream fout(temp_filename, std::ios::trunc | std::ios::out | std::ios::binary); if (!fout.is_open()) { diff --git a/src/utils.cc b/src/utils.cc index 324a9d31..0fdcd903 100644 --- a/src/utils.cc +++ b/src/utils.cc @@ -31,6 +31,7 @@ #include #endif +#include #include #include #include @@ -50,7 +51,6 @@ #include #include #include -#include #include #include #include @@ -218,7 +218,7 @@ std::string EncodePath(const std::string& path) { while (std::getline(str_stream, token, '/')) { if (!token.empty()) { if (!out.empty()) out += "/"; - out += curlpp::escape(token); + out += httplib::encode_uri_component(token); } } @@ -608,9 +608,9 @@ std::string Multimap::GetCanonicalQueryString() const { for (auto& [key, values] : map_) { for (auto& value : values) { if (!query_string.empty()) query_string += "&"; - query_string += curlpp::escape(key); + query_string += httplib::encode_uri_component(key); query_string += '='; - query_string += curlpp::escape(value); + query_string += httplib::encode_uri_component(value); } } return query_string; From 6ada432030d3b98e18baf0d8cf52e1d3c7e64b14 Mon Sep 17 00:00:00 2001 From: jiuker Date: Wed, 19 Aug 2026 15:41:01 +0800 Subject: [PATCH 05/19] Update client.cc --- src/client.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.cc b/src/client.cc index d5582a8a..6d3cb139 100644 --- a/src/client.cc +++ b/src/client.cc @@ -1157,7 +1157,7 @@ Result Client::DownloadObject(DownloadObjectArgs args) { } std::string temp_filename = - args.filename + "." + httplib::decode_uri_component(etag) + ".part.minio"; + args.filename + "." + httplib::encode_uri_component(etag) + ".part.minio"; std::ofstream fout(temp_filename, std::ios::trunc | std::ios::out | std::ios::binary); if (!fout.is_open()) { From b4db317dcf9f09af1629f48338521430ca556bd5 Mon Sep 17 00:00:00 2001 From: jiuker Date: Wed, 19 Aug 2026 17:29:13 +0800 Subject: [PATCH 06/19] refactor refactor --- CMakeLists.txt | 6 +- cmake/miniocpp-deps.cmake | 44 +--- examples/GetBucketTags.cc | 3 +- examples/GetObjectTags.cc | 3 +- include/miniocpp/http.h | 29 +-- include/miniocpp/utils.h | 10 + src/args.cc | 8 +- src/baseclient.cc | 31 ++- src/client.cc | 10 +- src/http.cc | 533 +++++++++++--------------------------- src/response.cc | 6 +- src/types.cc | 24 +- src/utils.cc | 60 ++++- vcpkg.json | 1 - 14 files changed, 280 insertions(+), 488 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9aed78b9..c9b172bf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -172,6 +172,10 @@ target_include_directories(miniocpp PUBLIC $ ) target_link_libraries(miniocpp PUBLIC ${MINIO_CPP_LIBS}) +# cpp-httplib is header-only and its class layout depends on the OpenSSL +# macro; define it once, target-wide, so every translation unit that includes +# httplib.h compiles it identically (see src/http.cc). +target_compile_definitions(miniocpp PRIVATE CPPHTTPLIB_OPENSSL_SUPPORT) if (MINIO_CPP_ENABLE_RDMA) target_compile_definitions(miniocpp PUBLIC MINIO_CPP_RDMA) endif() @@ -315,7 +319,7 @@ install(TARGETS miniocpp INCLUDES DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}") # Source-built deps must be in an export set for install(EXPORT) to resolve -# them (curlpp exports itself; miniocpp_inih does not). +# them (miniocpp_inih does not export itself). if (MINIO_CPP_DEPS_EXPORT_TARGETS) install(TARGETS ${MINIO_CPP_DEPS_EXPORT_TARGETS} EXPORT miniocpp-targets diff --git a/cmake/miniocpp-deps.cmake b/cmake/miniocpp-deps.cmake index 3bf2c924..c2d4d97f 100644 --- a/cmake/miniocpp-deps.cmake +++ b/cmake/miniocpp-deps.cmake @@ -4,7 +4,7 @@ # Resolution order per dependency: vcpkg CONFIG package -> pkg-config -> # upstream source. The source branch is for distros where vcpkg is # impractical (its default setup downloads glibc-linked tools that do not run -# on musl) and no curlpp / C++ INIReader packages exist. It clones at +# on musl) and no C++ INIReader packages exist. It clones at # configure time with plain git, so it works on the CMake 3.13.4 floor (no # FetchContent); vcpkg builds never reach it. # @@ -60,47 +60,6 @@ else() endif() endif() -# curlpp -- pinned master commit, no patches needed: CURLOPT_CLOSEPOLICY is -# gone upstream (dropped in curl 8.10) and the build is target-based and -# self-exporting. Static target keeps BUILD_SHARED_LIBS=OFF builds working. -find_package(unofficial-curlpp CONFIG QUIET) -if (unofficial-curlpp_FOUND) - set(MINIO_CPP_CURLPP_TARGET unofficial::curlpp::curlpp) -else() - if (PkgConfig_FOUND) - pkg_check_modules(MINIO_CPP_CURLPP QUIET IMPORTED_TARGET curlpp) - endif() - if (MINIO_CPP_CURLPP_FOUND) - set(MINIO_CPP_CURLPP_TARGET PkgConfig::MINIO_CPP_CURLPP) - else() - message(STATUS "curlpp: no package found, building from source") - set(MINIO_CPP_CURLPP_SRC "${CMAKE_CURRENT_BINARY_DIR}/_deps/curlpp-src") - if (NOT EXISTS "${MINIO_CPP_CURLPP_SRC}/CMakeLists.txt") - execute_process(COMMAND git clone --quiet - https://github.com/jpbarrette/curlpp.git - "${MINIO_CPP_CURLPP_SRC}" - RESULT_VARIABLE _curlpp_clone) - if (NOT _curlpp_clone STREQUAL "0") - message(FATAL_ERROR "curlpp: git clone failed") - endif() - endif() - # Also reset a cached checkout to the pinned commit, not just a fresh - # clone. - execute_process(COMMAND git checkout --quiet - ec1b66e699557cd9d608d322c013a1ebda16bd08 - WORKING_DIRECTORY "${MINIO_CPP_CURLPP_SRC}" - RESULT_VARIABLE _curlpp_checkout) - if (NOT _curlpp_checkout STREQUAL "0") - message(FATAL_ERROR "curlpp: git checkout of pinned commit failed") - endif() - set(CURLPP_BUILD_SHARED_LIBS OFF CACHE BOOL "Build curlpp shared library" FORCE) - add_subdirectory("${MINIO_CPP_CURLPP_SRC}" - "${CMAKE_CURRENT_BINARY_DIR}/_deps/curlpp-build") - set(MINIO_CPP_CURLPP_TARGET curlpp_static) - set_target_properties(curlpp_static PROPERTIES POSITION_INDEPENDENT_CODE ON) - endif() -endif() - # inih -- Alpine ships only the C library; build the C++ INIReader from source # (inih is meson-only, hence the manual target). An installed # miniocpp::miniocpp_inih (shipped with the miniocpp install) is reused as-is. @@ -167,7 +126,6 @@ endif() set(MINIO_CPP_DEPS_LINK_LIBS ${MINIO_CPP_HTTPLIB_TARGET} - ${MINIO_CPP_CURLPP_TARGET} ${MINIO_CPP_INIH_TARGET} nlohmann_json::nlohmann_json ${MINIO_CPP_PUGIXML_TARGET} diff --git a/examples/GetBucketTags.cc b/examples/GetBucketTags.cc index c73335be..7d6103ab 100644 --- a/examples/GetBucketTags.cc +++ b/examples/GetBucketTags.cc @@ -39,7 +39,8 @@ int main() { if (resp) { std::cout << "Bucket tags: " << std::endl; for (auto& [key, value] : resp->tags) { - std::cout << "Key: " << key << ", " << "Value: " << value << std::endl; + std::cout << "Key: " << key << ", " + << "Value: " << value << std::endl; } } else { std::cout << "unable to get bucket tags; " << resp.error().String() diff --git a/examples/GetObjectTags.cc b/examples/GetObjectTags.cc index 14556adb..c04dcd4b 100644 --- a/examples/GetObjectTags.cc +++ b/examples/GetObjectTags.cc @@ -40,7 +40,8 @@ int main() { if (resp) { std::cout << "Object tags: " << std::endl; for (auto& [key, value] : resp->tags) { - std::cout << "Key: " << key << ", " << "Value: " << value << std::endl; + std::cout << "Key: " << key << ", " + << "Value: " << value << std::endl; } } else { std::cout << "unable to get object tags; " << resp.error().String() diff --git a/include/miniocpp/http.h b/include/miniocpp/http.h index c884087d..91301de2 100644 --- a/include/miniocpp/http.h +++ b/include/miniocpp/http.h @@ -18,8 +18,6 @@ #ifndef MINIO_CPP_HTTP_H_INCLUDED #define MINIO_CPP_HTTP_H_INCLUDED -#include -#include #include #include #include @@ -53,9 +51,9 @@ struct Url { host(std::move(host)), port(port), path(std::move(path)), - query_string(std::move(query_string)) {}; + query_string(std::move(query_string)){}; explicit Url(bool https, std::string host, unsigned int port) - : https(https), host(std::move(host)), port(port) {}; + : https(https), host(std::move(host)), port(port){}; ~Url() = default; @@ -77,16 +75,18 @@ using ProgressFunction = std::function; struct Response; struct DataFunctionArgs { - curlpp::Easy* handle = nullptr; + // Transfer handle is not exposed by the httplib backend; kept as void* for + // API stability (always nullptr). + void* handle = nullptr; Response* response = nullptr; std::string datachunk; void* userdata = nullptr; DataFunctionArgs() = default; - DataFunctionArgs(curlpp::Easy* handle, Response* response, void* userdata) + DataFunctionArgs(void* handle, Response* response, void* userdata) : handle(handle), response(response), userdata(userdata) {} - DataFunctionArgs(curlpp::Easy* handle, Response* response, - std::string datachunk, void* userdata) + DataFunctionArgs(void* handle, Response* response, std::string datachunk, + void* userdata) : handle(handle), response(response), datachunk(std::move(datachunk)), @@ -164,24 +164,11 @@ struct Response { Response() = default; ~Response() = default; - size_t ResponseCallback(curlpp::Multi* const requests, - curlpp::Easy* const request, const char* const buffer, - size_t size, size_t length); - explicit operator bool() const { return error.empty() && status_code >= 200 && status_code <= 299; } error::Error Error() const; - - private: - std::string response_; - bool continue100_ = false; - bool status_code_read_ = false; - bool headers_read_ = false; - - error::Error ReadStatusCode(); - error::Error ReadHeaders(); }; // struct Response } // namespace minio::http diff --git a/include/miniocpp/utils.h b/include/miniocpp/utils.h index f224e5a4..63f619d1 100644 --- a/include/miniocpp/utils.h +++ b/include/miniocpp/utils.h @@ -94,6 +94,16 @@ std::string Join(const std::list& values, std::string Join(const std::vector& values, const std::string& delimiter); +// UriEncode does AWS SigV4 percent-encoding: RFC 3986 unreserved characters +// (A-Z a-z 0-9 - _ . ~) are kept, everything else is percent-encoded with +// uppercase hex. Used for canonical URIs, query strings and signed headers +// (e.g. x-amz-copy-source). +std::string UriEncode(const std::string& value); + +// UriDecode reverses percent-encoding: %XX (either case) is decoded to its +// byte value, anything else is kept verbatim ('+' is not treated as space). +std::string UriDecode(const std::string& value); + // EncodePath does URL encoding of path. It also normalizes multiple slashes. std::string EncodePath(const std::string& path); diff --git a/src/args.cc b/src/args.cc index 3af70252..fc6788ca 100644 --- a/src/args.cc +++ b/src/args.cc @@ -63,8 +63,7 @@ utils::Multimap ObjectWriteArgs::Headers() const { std::string tagging; for (auto& [key, value] : tags) { - std::string tag = httplib::encode_uri_component(key) + "=" + - httplib::encode_uri_component(value); + std::string tag = utils::UriEncode(key) + "=" + utils::UriEncode(value); if (!tagging.empty()) { tagging += "&"; } @@ -128,10 +127,9 @@ utils::Multimap ObjectConditionalReadArgs::Headers() const { utils::Multimap ObjectConditionalReadArgs::CopyHeaders() const { utils::Multimap result_headers; - std::string copy_source = - httplib::encode_uri_component("/" + bucket + "/" + object); + std::string copy_source = utils::UriEncode("/" + bucket + "/" + object); if (!version_id.empty()) { - copy_source += "?versionId=" + httplib::encode_uri_component(version_id); + copy_source += "?versionId=" + utils::UriEncode(version_id); } result_headers.Add("x-amz-copy-source", copy_source); diff --git a/src/baseclient.cc b/src/baseclient.cc index ad2247cf..55deb243 100644 --- a/src/baseclient.cc +++ b/src/baseclient.cc @@ -378,8 +378,11 @@ Result BaseClient::CompleteMultipartUpload( std::stringstream ss; ss << ""; for (auto& part : args.parts) { - ss << "" << "" << part.number << "" - << "" << "\"" << part.etag << "\"" << ""; + ss << "" + << "" << part.number << "" + << "" + << "\"" << part.etag << "\"" + << ""; if (!part.checksum_crc64nvme.empty()) { ss << "" << part.checksum_crc64nvme << ""; @@ -1416,8 +1419,9 @@ Result BaseClient::MakeBucket(MakeBucketArgs args) { std::string body; if (region != "us-east-1") { std::stringstream ss; - ss << "" << "" << region - << "" << ""; + ss << "" + << "" << region << "" + << ""; body = ss.str(); req.body = body; } @@ -1804,8 +1808,10 @@ Result BaseClient::SetBucketTags( if (!args.tags.empty()) { ss << ""; for (auto& [key, value] : args.tags) { - ss << "" << "" << key << "" << "" << value - << "" << ""; + ss << "" + << "" << key << "" + << "" << value << "" + << ""; } ss << ""; } @@ -1934,9 +1940,10 @@ Result BaseClient::SetObjectRetention( } std::stringstream ss; - ss << "" << "" << RetentionModeToString(args.retention_mode) - << "" << "" - << args.retain_until_date.ToISO8601UTC() << "" + ss << "" + << "" << RetentionModeToString(args.retention_mode) << "" + << "" << args.retain_until_date.ToISO8601UTC() + << "" << ""; std::string body = ss.str(); @@ -1978,8 +1985,10 @@ Result BaseClient::SetObjectTags( if (!args.tags.empty()) { ss << ""; for (auto& [key, value] : args.tags) { - ss << "" << "" << key << "" << "" << value - << "" << ""; + ss << "" + << "" << key << "" + << "" << value << "" + << ""; } ss << ""; } diff --git a/src/client.cc b/src/client.cc index 6d3cb139..13e1aba2 100644 --- a/src/client.cc +++ b/src/client.cc @@ -315,7 +315,7 @@ void ListObjectsResult::StartPrefetch() { std::shared_future>>(std::async( std::launch::async, [client = client_, next_args = std::move(next_args)]() mutable - -> std::shared_ptr { + -> std::shared_ptr { try { auto resp = client->GetRegion(next_args.bucket, next_args.region); if (resp) { @@ -711,9 +711,9 @@ Result Client::GetObject(GetObjectArgs args) { base_url_, region}; // RAII, matching the multipart paths below. rdmaGetWithRetry signs and - // sends an HTTP request, and curlpp throws, so a manual Deregister after - // the call is skipped on that path and the buffer stays pinned for the - // life of the process. + // sends an HTTP request, so a manual Deregister after the call is + // skipped on that path and the buffer stays pinned for the life of the + // process. ScopedRDMARegistration reg(&rdma_client, args.buf); ssize_t ret = @@ -1157,7 +1157,7 @@ Result Client::DownloadObject(DownloadObjectArgs args) { } std::string temp_filename = - args.filename + "." + httplib::encode_uri_component(etag) + ".part.minio"; + args.filename + "." + utils::UriEncode(etag) + ".part.minio"; std::ofstream fout(temp_filename, std::ios::trunc | std::ios::out | std::ios::binary); if (!fout.is_open()) { diff --git a/src/http.cc b/src/http.cc index efea6fd3..15fd43fe 100644 --- a/src/http.cc +++ b/src/http.cc @@ -17,28 +17,24 @@ #include "miniocpp/http.h" -#include +// cpp-httplib's OpenSSL backend is enabled once, target-wide, via +// target_compile_definitions(miniocpp PRIVATE CPPHTTPLIB_OPENSSL_SUPPORT): the +// header-only library's class layout depends on that macro, so every +// translation unit must agree on it. +#include #include -#include -#include -#include -#include -#include -#include -#include -#include #include #include #include #include #include -#include +#include +#include #include #include #include #include -#include #include #include "miniocpp/error.h" @@ -59,68 +55,10 @@ namespace { // Abort a transfer that makes no progress for this long. Guards against a // connection that drops mid-transfer without a clean close (TCP never RSTs), -// which would otherwise keep the request alive indefinitely. +// which would otherwise keep the request alive indefinitely. cpp-httplib +// enforces it as a read/write timeout, since it has no low-speed limit. constexpr long kStallTimeoutSecs = 60; -// curl_global_init() is documented as not thread-safe and is expensive -// (OpenSSL init etc). Run it exactly once per process via a function-local -// static (Meyers singleton; C++11 [stmt.dcl]/4 guarantees thread-safe -// initialization), instead of paying the cost — and the race — on every -// request via a stack-local curlpp::Cleanup. -void EnsureGlobalCurlInit() { - static const curlpp::Cleanup kCleanup; - (void)kCleanup; -} - -// Connection, DNS and TLS-session caches, kept per thread. -// -// These were once a single process-wide CURLSH with per-slot mutexes, which -// libcurl's own documentation warns against: a shared connection cache is not -// safe to use from several threads at once, and this crashed reliably under -// concurrent PUTs -- a wild pointer read inside curl_multi_perform, with the -// mutexes held exactly as documented. Bisecting the slots on libcurl 8.5: -// -// none clean CONNECT only crashes -// DNS only clean CONNECT + SSL_SESSION crashes -// DNS + SSL_SESSION crashes -// -// So the sharing itself is the problem, not one slot. Giving each thread its -// own share keeps what the share was for -- a connection and TLS session -// surviving past one Easy handle, so a signed S3 call does not pay a fresh -// handshake every time -- while removing the cross-thread access entirely. -// Nothing is shared between threads, so no lock callbacks are needed. -// -// The handle is destroyed when its thread exits. Every Easy that used it is -// stack-local to Request::execute() and long gone by then. -class ThreadCurlShare { - public: - ThreadCurlShare() : share_(curl_share_init()) { - if (share_ == nullptr) { - std::cerr << "curl_share_init failed" << std::endl; - std::terminate(); - } - curl_share_setopt(share_, CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT); - curl_share_setopt(share_, CURLSHOPT_SHARE, CURL_LOCK_DATA_DNS); - curl_share_setopt(share_, CURLSHOPT_SHARE, CURL_LOCK_DATA_SSL_SESSION); - } - ~ThreadCurlShare() { - if (share_ != nullptr) curl_share_cleanup(share_); - } - ThreadCurlShare(const ThreadCurlShare&) = delete; - ThreadCurlShare& operator=(const ThreadCurlShare&) = delete; - - CURLSH* get() const { return share_; } - - private: - CURLSH* share_; -}; - -CURLSH* CurlShare() { - EnsureGlobalCurlInit(); - static thread_local ThreadCurlShare share; - return share.get(); -} - } // namespace // MethodToString converts http Method enum to string. @@ -239,65 +177,6 @@ Url Url::Parse(std::string value) { std::move(query_string)); } -error::Error Response::ReadStatusCode() { - size_t pos = response_.find("\r\n"); - if (pos == std::string::npos) { - // Not yet received the first line. - return error::SUCCESS; - } - - std::string line = response_.substr(0, pos); - response_.erase(0, pos + 2); - - if (continue100_) { - if (!line.empty()) { - // After '100 Continue', next line must be empty new line. - return error::Error("invalid HTTP response"); - } - - continue100_ = false; - - pos = response_.find("\r\n"); - if (pos == std::string::npos) { - // Not yet received the first line after '100 Continue'. - return error::SUCCESS; - } - - line = response_.substr(0, pos); - response_.erase(0, pos + 2); - } - - // Skip HTTP/1.x. - pos = line.find(" "); - if (pos == std::string::npos) { - // First token must be HTTP/1.x - return error::Error("invalid HTTP response"); - } - line = line.substr(pos + 1); - - // Read status code. - pos = line.find(" "); - if (pos == std::string::npos) { - // The line must contain second token. - return error::Error("invalid HTTP response"); - } - std::string code = line.substr(0, pos); - std::string::size_type st; - status_code = std::stoi(code, &st); - if (st == std::string::npos) { - // Code must be a number. - return error::Error("invalid HTTP response code " + code); - } - - if (status_code == 100) { - continue100_ = true; - } else { - status_code_read_ = true; - } - - return error::SUCCESS; -} - error::Error Response::Error() const { if (!error.empty()) return error::Error(error); if (status_code && (status_code < 200 || status_code > 299)) { @@ -307,99 +186,6 @@ error::Error Response::Error() const { return error::SUCCESS; } -error::Error Response::ReadHeaders() { - size_t pos = response_.find("\r\n\r\n"); - if (pos == std::string::npos) { - // Not yet received the headers. - return error::SUCCESS; - } - - headers_read_ = true; - - std::string lines = response_.substr(0, pos); - response_.erase(0, pos + 4); - - auto add_header = [&headers = headers](std::string line) -> error::Error { - size_t pos = line.find(": "); - if (pos != std::string::npos) { - headers.Add(line.substr(0, pos), line.substr(pos + 2)); - return error::SUCCESS; - } - - return error::Error("invalid HTTP header: " + line); - }; - - while ((pos = lines.find("\r\n")) != std::string::npos) { - std::string line = lines.substr(0, pos); - lines.erase(0, pos + 2); - if (error::Error err = add_header(line)) return err; - } - - if (!lines.empty()) { - if (error::Error err = add_header(lines)) return err; - } - - return error::SUCCESS; -} - -size_t Response::ResponseCallback(curlpp::Multi* const requests, - curlpp::Easy* const request, - const char* const buffer, size_t size, - size_t length) { - size_t realsize = size * length; - - // If error occurred previously, just cancel the request. - if (!error.empty()) { - requests->remove(request); - return realsize; - } - - if (!status_code_read_ || !headers_read_) { - response_.append(buffer, length); - } - - if (!status_code_read_) { - if (error::Error err = ReadStatusCode()) { - error = err.String(); - requests->remove(request); - return realsize; - } - - if (!status_code_read_) return realsize; - } - - if (!headers_read_) { - if (error::Error err = ReadHeaders()) { - error = err.String(); - requests->remove(request); - return realsize; - } - - if (!headers_read_ || response_.empty()) return realsize; - - // If data function is set and the request is successful, send data. - if (datafunc != nullptr && status_code >= 200 && status_code <= 299) { - DataFunctionArgs args(request, this, std::string(this->response_), - userdata); - if (!datafunc(args)) requests->remove(request); - } else { - body = response_; - } - - return realsize; - } - - // If data function is set and the request is successful, send data. - if (datafunc != nullptr && status_code >= 200 && status_code <= 299) { - DataFunctionArgs args(request, this, std::string(buffer, length), userdata); - if (!datafunc(args)) requests->remove(request); - } else { - body.append(buffer, length); - } - - return realsize; -} - Request::Request(Method method, Url url) { this->method = method; this->url = url; @@ -410,183 +196,168 @@ Request::Request(Method method, Url url) { } Response Request::execute() { - EnsureGlobalCurlInit(); - curlpp::Easy request; - curlpp::Multi requests; - - // Attach this thread's share so connections, DNS resolutions and TLS - // sessions survive past this Easy handle's lifetime. Per thread rather than - // per process: see CurlShare() for why sharing these across threads - // corrupts libcurl's state. Also enable TCP keep-alive so the kernel keeps - // pooled sockets healthy across idle gaps between S3 calls. curlpp doesn't - // wrap either option, so set via libcurl. - CURL* const raw_handle = request.getHandle(); - curl_easy_setopt(raw_handle, CURLOPT_SHARE, CurlShare()); - curl_easy_setopt(raw_handle, CURLOPT_TCP_KEEPALIVE, 1L); - - // Fail a stalled transfer instead of hanging forever. Skipped when the caller - // set an explicit total timeout (RDMA control plane) — that already bounds - // it. - if (timeout_secs <= 0) { - curl_easy_setopt(raw_handle, CURLOPT_LOW_SPEED_LIMIT, 1L); - curl_easy_setopt(raw_handle, CURLOPT_LOW_SPEED_TIME, kStallTimeoutSecs); - } + Response response; + response.datafunc = datafunc; + response.userdata = userdata; - // Request settings. - request.setOpt(new curlpp::options::CustomRequest{MethodToString(method)}); - std::string urlstring = url.String(); - request.setOpt(new curlpp::Options::Url(urlstring)); - if (debug) request.setOpt(new curlpp::Options::Verbose(true)); - if (ignore_cert_check) { - request.setOpt(new curlpp::Options::SslVerifyPeer(false)); - request.setOpt(new curlpp::Options::SslVerifyHost(0L)); + // httplib::Client is bound to one endpoint and keeps its connection pool + // (connections, TLS sessions, DNS results) alive across requests; reuse a + // thread-local client per endpoint. httplib::Client is not thread-safe, so + // each thread gets its own, mirroring the per-thread libcurl share it + // replaces. Client certificates are fixed at construction time, hence part + // of the cache key. + std::string endpoint = (url.https ? "https://" : "http://") + url.host; + if (url.port) endpoint += ":" + std::to_string(url.port); + + httplib::Client* cli = nullptr; + auto client = + std::make_unique(endpoint, cert_file, key_file); + if (!client->is_valid()) { + response.error = "unable to create HTTP client for " + endpoint; + return response; } + cli = client.get(); + // Options. httplib defaults to a 5s read/write timeout, far too short for + // S3 transfers; without an explicit timeout keep the 60s stall guard libcurl + // used, aborting only transfers that make no progress for that long. + cli->set_keep_alive(true); + cli->set_follow_location(false); + // Paths are pre-encoded by the caller (EncodePath); never let httplib + // encode them a second time. + cli->set_path_encode(false); + if (connect_timeout_secs > 0) { + cli->set_connection_timeout(connect_timeout_secs, 0); + } + if (timeout_secs > 0) { + cli->set_read_timeout(timeout_secs, 0); + cli->set_write_timeout(timeout_secs, 0); + } else { + cli->set_read_timeout(kStallTimeoutSecs, 0); + cli->set_write_timeout(kStallTimeoutSecs, 0); + } + if (!nic_interface.empty()) cli->set_interface(nic_interface); if (url.https) { - if (!ssl_cert_file.empty()) { - request.setOpt(new curlpp::Options::SslVerifyPeer(true)); - request.setOpt(new curlpp::Options::CaInfo(ssl_cert_file)); - } - if (!key_file.empty()) { - request.setOpt(new curlpp::Options::SslKey(key_file)); - } - if (!cert_file.empty()) { - request.setOpt(new curlpp::Options::SslCert(cert_file)); - } + cli->enable_server_certificate_verification(!ignore_cert_check); + if (!ssl_cert_file.empty()) cli->set_ca_cert_path(ssl_cert_file); } - if (!nic_interface.empty()) { - request.setOpt(new curlpp::Options::Interface(nic_interface)); - } - if (connect_timeout_secs > 0) { - request.setOpt(new curlpp::Options::ConnectTimeout(connect_timeout_secs)); + httplib::Headers request_headers; + for (const auto& key : headers.Keys()) { + for (const auto& value : headers.Get(key)) { + request_headers.insert({key, value}); + } } - if (timeout_secs > 0) { - request.setOpt(new curlpp::Options::Timeout(timeout_secs)); + // httplib sets Host itself from the endpoint when absent; a caller-provided + // Host (the SigV4-signed value) takes precedence and must be sent verbatim. + // Content-Length is derived from the body by httplib. + request_headers.erase("Content-Length"); + std::string content_type = headers.GetFront("Content-Type"); + request_headers.erase("Content-Type"); + + std::string path = url.path; + if (path.empty()) { + path = "/"; + } else if (path.front() != '/') { + path = "/" + path; } + if (!url.query_string.empty()) path += "?" + url.query_string; - utils::CharBuffer charbuf((char*)body.data(), body.size()); - std::istream body_stream(&charbuf); + // httplib reports either upload or download progress per call. + auto download_progress = [this](size_t current, size_t total) -> bool { + if (progressfunc == nullptr) return true; + ProgressFunctionArgs args; + args.download_total_bytes = total; + args.downloaded_bytes = current; + args.userdata = progress_userdata; + return progressfunc(args); + }; + auto upload_progress = [this](size_t current, size_t total) -> bool { + if (progressfunc == nullptr) return true; + ProgressFunctionArgs args; + args.upload_total_bytes = total; + args.uploaded_bytes = current; + args.userdata = progress_userdata; + return progressfunc(args); + }; + + // Streaming receive into the caller's data function, mirroring the old curl + // write callback. A false return aborts the transfer; record that so a + // caller-initiated abort is not reported as an error below (streaming + // consumers such as ListenBucketNotification cancel once they have all the + // records they need). + bool datafunc_canceled = false; + httplib::ContentReceiver content_receiver = + [this, &response, &datafunc_canceled](const char* data, + size_t length) -> bool { + DataFunctionArgs args(nullptr, &response, std::string(data, length), + userdata); + const bool cont = datafunc(args); + if (!cont) datafunc_canceled = true; + return cont; + }; + + // Stream the request body from the caller's buffer instead of copying it: + // single-request PUTs can be multi-GiB (RDMA fallback path). + httplib::ContentProvider content_provider = + [this](size_t offset, size_t length, httplib::DataSink& sink) -> bool { + if (offset >= body.size()) return true; + const size_t n = std::min(length, body.size() - offset); + return sink.write(body.data() + offset, n); + }; + httplib::Result res; + std::string body_str(body.data(), body.size()); + httplib::ResponseHandler response_handler = + [&response](const httplib::Response& res) -> bool { + // Headers (and therefore the status code) are known here, before any body + // is streamed; fill it in so a caller-initiated cancel still yields a + // response with the correct status. + response.status_code = res.status; + return true; + }; switch (method) { - case Method::kDelete: case Method::kGet: + if (datafunc != nullptr) { + res = cli->Get(path, request_headers, response_handler, + content_receiver, download_progress); + } else { + res = cli->Get(path, request_headers, download_progress); + } break; case Method::kHead: - request.setOpt(new curlpp::options::NoBody(true)); + res = cli->Head(path, request_headers); break; - case Method::kPut: case Method::kPost: - if (!headers.Contains("Content-Length")) { - headers.Add("Content-Length", std::to_string(body.size())); - } - request.setOpt(new curlpp::Options::ReadStream(&body_stream)); - // CURLOPT_INFILESIZE_LARGE (curl_off_t), not CURLOPT_INFILESIZE (long): - // the latter is documented to be capped at 2 GiB and silently truncates - // the upload for larger single-request bodies (e.g. a >4 GiB buffer that - // could not be RDMA-registered and falls back to a single PUT). - request.setOpt(new curlpp::Options::InfileSizeLarge( - static_cast(body.size()))); - request.setOpt(new curlpp::Options::Upload(true)); + res = cli->Post(path, request_headers, body_str, content_type, + upload_progress); + break; + case Method::kPut: + res = cli->Put(path, request_headers, body_str, content_type, + upload_progress); + break; + case Method::kDelete: + res = cli->Delete(path, request_headers, download_progress); break; } - std::list headerlist = headers.ToHttpHeaders(); - headerlist.push_back("Expect:"); // Disable 100 continue from server. - request.setOpt(new curlpp::Options::HttpHeader(headerlist)); - - // Response settings. - request.setOpt(new curlpp::options::Header(true)); - - Response response; - response.datafunc = datafunc; - response.userdata = userdata; - - using namespace std::placeholders; - request.setOpt(new curlpp::options::WriteFunction( - std::bind(&Response::ResponseCallback, &response, &requests, &request, _1, - _2, _3))); - - auto progress = - [&progressfunc = progressfunc, &progress_userdata = progress_userdata]( - double dltotal, double dlnow, double ultotal, double ulnow) -> int { - ProgressFunctionArgs args; - args.download_total_bytes = dltotal; - args.downloaded_bytes = dlnow; - args.upload_total_bytes = ultotal; - args.uploaded_bytes = ulnow; - args.userdata = progress_userdata; - if (progressfunc(args)) { - return CURL_PROGRESSFUNC_CONTINUE; - } - return 1; - }; - if (progressfunc != nullptr) { - request.setOpt(new curlpp::options::NoProgress(false)); - request.setOpt(new curlpp::options::ProgressFunction(progress)); - } - - int left = 0; - requests.add(&request); - - // Execute. - while (!requests.perform(&left)) { - } - while (left) { - fd_set fdread{}; - fd_set fdwrite{}; - fd_set fdexcep{}; - int maxfd = -1; - - FD_ZERO(&fdread); - FD_ZERO(&fdwrite); - FD_ZERO(&fdexcep); - - requests.fdset(&fdread, &fdwrite, &fdexcep, &maxfd); - - // Bound the wait so the loop keeps pumping libcurl even when no socket ever - // becomes ready — otherwise a dropped/stalled connection blocks select() - // forever and this (synchronous) call hangs the calling thread. The bounded - // poll lets libcurl enforce its own timeouts (e.g. the low-speed limit set - // above) and abort the dead transfer. - if (maxfd < 0) { - // libcurl has no fd to wait on yet; select() with empty sets errors out - // on Windows, so just poll again shortly. - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - } else { - timeval timeout{}; - timeout.tv_sec = 1; - timeout.tv_usec = 0; - if (select(maxfd + 1, &fdread, &fdwrite, &fdexcep, &timeout) < 0) { -#ifndef _WIN32 - if (errno == EINTR) continue; // interrupted by a signal; retry -#endif - std::cerr << "select() failed; this should not happen" << std::endl; - std::terminate(); - } + if (!res) { + // A false return from the data function cancels the transfer; for + // streaming callers that is a normal completion, not an error. + if (res.error() == httplib::Error::Canceled && datafunc_canceled) { + return response; } - while (!requests.perform(&left)) { - } - } - - // The loop exits once libcurl has no running transfers left. If the transfer - // aborted before delivering a single byte (e.g. the low-speed limit or - // connect timeout fired on a dropped/stalled connection), the write callback - // never ran, so neither status_code nor error was set. Surface a diagnostic - // instead of returning a silently-empty failure. - if (response.error.empty() && response.status_code == 0) { - response.error = - "transfer ended without a response (connection dropped, timed out, or " - "was aborted before any data was received)"; + response.error = httplib::to_string(res.error()); + return response; } - if (progressfunc != nullptr) { - ProgressFunctionArgs args; - args.userdata = progress_userdata; - curlpp::infos::SpeedUpload::get(request, args.upload_speed); - curlpp::infos::SpeedDownload::get(request, args.download_speed); - progressfunc(args); + response.status_code = res->status; + for (const auto& [key, value] : res->headers) { + response.headers.Add(key, value); } + // With a data function the body is streamed to it; otherwise keep the + // buffered response body (including error payloads for non-2xx statuses). + if (datafunc == nullptr) response.body = res->body; return response; } @@ -594,13 +365,9 @@ Response Request::execute() { Response Request::Execute() { try { return execute(); - } catch (curlpp::LogicError& e) { - Response response; - response.error = std::string("curlpp::LogicError: ") + e.what(); - return response; - } catch (curlpp::RuntimeError& e) { + } catch (const std::exception& e) { Response response; - response.error = std::string("curlpp::RuntimeError: ") + e.what(); + response.error = std::string("HTTP error: ") + e.what(); return response; } } diff --git a/src/response.cc b/src/response.cc index 3801351b..851e5e2a 100644 --- a/src/response.cc +++ b/src/response.cc @@ -169,7 +169,7 @@ Result ListObjectsResponse::ParseXML(std::string_view data, const char* raw, std::string_view& target) -> void { if (encoding_type == "url") { - resp.owned_.emplace_back(httplib::decode_uri_component(raw)); + resp.owned_.emplace_back(utils::UriDecode(raw)); target = resp.owned_.back(); } else { target = raw; @@ -261,7 +261,7 @@ Result ListObjectsResponse::ParseXML(std::string_view data, text = content.node().select_node("Key/text()"); raw = text.node().value(); if (resp.encoding_type == "url") { - resp.owned_.emplace_back(httplib::decode_uri_component(raw)); + resp.owned_.emplace_back(utils::UriDecode(raw)); item.name = resp.owned_.back(); } else { item.name = raw; @@ -318,7 +318,7 @@ Result ListObjectsResponse::ParseXML(std::string_view data, text = common_prefix.node().select_node("Prefix/text()"); raw = text.node().value(); if (resp.encoding_type == "url") { - resp.owned_.emplace_back(httplib::decode_uri_component(raw)); + resp.owned_.emplace_back(utils::UriDecode(raw)); item.name = resp.owned_.back(); } else { item.name = raw; diff --git a/src/types.cc b/src/types.cc index b180d6e2..17f4fa1e 100644 --- a/src/types.cc +++ b/src/types.cc @@ -340,8 +340,10 @@ std::string ReplicationConfig::ToXML() const { auto tag_xml = [](std::string key, std::string value) -> std::string { std::stringstream ss; - ss << "" << "" << key << "" << "" << value - << "" << ""; + ss << "" + << "" << key << "" + << "" << value << "" + << ""; return ss.str(); }; @@ -371,7 +373,8 @@ std::string ReplicationConfig::ToXML() const { ss << ""; } if (rule.destination.metrics) { - ss << "" << ""; + ss << "" + << ""; if (rule.destination.metrics.event_threshold_minutes > 0) { ss << minutes_xml(rule.destination.metrics.event_threshold_minutes); } @@ -379,7 +382,8 @@ std::string ReplicationConfig::ToXML() const { << ""; } if (rule.destination.replication_time) { - ss << "" << "